From 8f26458b8c0873fc585eba5f7cd08a26bad225d6 Mon Sep 17 00:00:00 2001 From: overtrue Date: Sat, 5 Sep 2026 16:11:47 +0800 Subject: [PATCH] fix(ecstore): refuse an empty azure account key at client build --- .../src/bucket/on_demand_migration/azure.rs | 52 +++++++++++++++++-- docs/operations/on-demand-migration.md | 4 +- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/ecstore/src/bucket/on_demand_migration/azure.rs b/crates/ecstore/src/bucket/on_demand_migration/azure.rs index f2a6b7f60..08e687797 100644 --- a/crates/ecstore/src/bucket/on_demand_migration/azure.rs +++ b/crates/ecstore/src/bucket/on_demand_migration/azure.rs @@ -91,11 +91,17 @@ impl AzureSourceBackend { ca_cert_pem: Option<&str>, ) -> Result { let credential = match &spec.auth { - AzureAuth::SharedKey(key) => Credential::SharedKey( - base64_simd::STANDARD + AzureAuth::SharedKey(key) => { + let key = base64_simd::STANDARD .decode_to_vec(key.as_bytes()) - .map_err(|_| RemoteS3ClientError::Credentials("azure account key is not base64"))?, - ), + .map_err(|_| RemoteS3ClientError::Credentials("azure account key is not base64"))?; + // HMAC accepts a zero-length key, so an absent one would sign + // every request with nothing rather than fail here. + if key.is_empty() { + return Err(RemoteS3ClientError::Credentials("azure account key is empty")); + } + Credential::SharedKey(key) + } AzureAuth::Sas(sas) => { let pairs: Vec<(String, String)> = url::form_urlencoded::parse(sas.trim_start_matches('?').as_bytes()) .into_owned() @@ -705,6 +711,44 @@ mod tests { ] } + #[test] + fn an_absent_or_malformed_account_key_is_refused_before_any_request() { + for key in ["", "not base64!"] { + let spec = AzureSourceSpec { + account: "acct".to_string(), + auth: AzureAuth::SharedKey(key.to_string()), + }; + let built = AzureSourceBackend::new( + "https://acct.blob.core.windows.net", + "legacy", + &spec, + SourceTimeouts::default(), + false, + None, + ); + assert!( + matches!(built, Err(RemoteS3ClientError::Credentials(_))), + "{key:?} must not build a client" + ); + } + let spec = AzureSourceSpec { + account: "acct".to_string(), + auth: AzureAuth::Sas(String::new()), + }; + assert!( + AzureSourceBackend::new( + "https://acct.blob.core.windows.net", + "legacy", + &spec, + SourceTimeouts::default(), + false, + None + ) + .is_err(), + "an empty SAS token carries no parameters" + ); + } + #[tokio::test] async fn head_signs_the_request_and_maps_azure_metadata() { let (endpoint, recorded) = scripted_server(vec![ScriptedResponse::new(200, blob_headers(), String::new())]).await; diff --git a/docs/operations/on-demand-migration.md b/docs/operations/on-demand-migration.md index b353bba42..8baebd3b7 100644 --- a/docs/operations/on-demand-migration.md +++ b/docs/operations/on-demand-migration.md @@ -92,11 +92,11 @@ The persisted blob is `on-demand-migration.json` in the bucket's metadata. Unkno | `version` | integer | `1` | Must be `1` | | `enabled` | bool | `true` | `false` keeps the config but stops all source traffic | | `source.provider` | `s3` \| `aws` \| `minio` \| `rustfs` \| `r2` \| `gcs` \| `azure` \| `gcs_native` | — (required) | Drives endpoint and addressing defaults, and which backend the client builds: every value but `azure` and `gcs_native` speaks S3 | -| `source.endpoint` | string \| null | — | `http(s)://host[:port]`, no path, query, fragment or userinfo. Required except for `aws` (derived from `region`), `azure` (derived as `https://.blob.core.windows.net`) and `gcs_native` (`https://storage.googleapis.com`). Set it explicitly to point at Azurite or fake-gcs-server | +| `source.endpoint` | string \| null | — | `http(s)://host[:port]`, no path, query, fragment or userinfo. Required except for `aws` (derived from `region`), `azure` (derived as `https://.blob.core.windows.net`) and `gcs_native` (`https://storage.googleapis.com`). Set it explicitly to point at Azurite or fake-gcs-server, subject to the same outbound policy as any other source endpoint | | `source.region` | string | — (required) | Non-empty. `auto` is accepted for `r2`, `minio`, `rustfs` and for the native providers, and is signed as `us-east-1`. `azure` and `gcs_native` never sign with a region, so `auto` is the honest value there | | `source.bucket` | string | — (required) | Non-empty, no `/` and no whitespace. For `azure` this is the container name, for `gcs_native` the bucket name; the provider block never repeats it | | `source.path_style` | `auto` \| `path` \| `virtual` | `auto` | `auto` resolves to path-style for IP-literal or `localhost` endpoints and for `s3`/`minio`/`rustfs`; virtual-host for `aws`/`gcs`/`r2` | -| `source.credentials` | object \| null | `null` | `null` means anonymous, which the client builder does not support yet: the admin `PUT` refuses it with `InvalidArgument`, and a config that reached the metadata another way resolves as unavailable. `access_key` and `secret_key` must be non-empty; `session_token` is optional but must be non-empty when present | +| `source.credentials` | object \| null | `null` | Read only by the S3 providers; `azure` and `gcs_native` must leave it `null` and carry their credentials in their own block. `null` means anonymous, which the client builder does not support yet: the admin `PUT` refuses it with `InvalidArgument`, and a config that reached the metadata another way resolves as unavailable. `access_key` and `secret_key` must be non-empty; `session_token` is optional but must be non-empty when present | | `source.tls.skip_verify` | bool | `false` | Disables certificate verification for the source connection | | `source.tls.ca_cert_pem` | string \| null | `null` | Must contain `-----BEGIN CERTIFICATE-----` | | `source.azure` | object \| null | `null` | Required for `provider = "azure"` and rejected for every other provider |