From 5c99417073fdca14f9387f6e507828d738e4e488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Sat, 3 Oct 2026 09:31:08 +0800 Subject: [PATCH] fix(sse): guard copy-source SSE-C keys over TLS and reject bucket-keyed KMS context (#8296) * fix(sse): count copy-source SSE-C headers in the TLS transport guard The SSE-C transport guard only looked at the object's own customer-key headers. A CopyObject or UploadPartCopy that read an SSE-C source into a non-SSE-C destination carried the source key only in the x-amz-copy-source-server-side-encryption-customer-* headers, so it was accepted on a plaintext transport with RUSTFS_SSE_C_REQUIRE_TLS=true and was missing from rustfs_ssec_plaintext_requests_total. Treat the copy-source triple as SSE-C headers too. * fix(sse): reject a KMS context key that replaces the location entry Managed SSE wraps each data key under an encryption context that carries {bucket: bucket/key}, added with or_insert, while only the client context is persisted and the entry is rebuilt on read. A client context entry keyed by the bucket name therefore replaced the location entry on write and on every read, so the data key was no longer tied to the object's location. Reject such a context with 400 InvalidArgument at the single managed-SSE write entry, before the KMS is called. The read side is unchanged, so objects stored with such an entry stay readable, and other keys, including other bucket names, are still accepted. --------- Co-authored-by: Hauser --- CHANGELOG.md | 2 + docs/operations/kms-backend-security.md | 8 + rustfs/src/server/ssec_transport.rs | 49 +++++- rustfs/src/storage/sse.rs | 193 +++++++++++++++++++++++- 4 files changed, 236 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db20aff54..00c2b33b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Presigned URLs honour only signed headers** (GHSA-g8w9-qw9q-fghr): a SigV4 presigned request that carries an `x-amz-*` request header not listed in `X-Amz-SignedHeaders` is now rejected with `403 AccessDenied` ("There were headers present in the request which were not signed"), matching AWS S3. Previously the holder of a presigned `PutObject` URL could add unsigned `x-amz-tagging`, `x-amz-storage-class`, `x-amz-website-redirect-location`, ACL, metadata, Object Lock or SSE headers and have them applied. Presigners that intend a property must set it before signing so the SDK lists the header in `SignedHeaders`; `x-amz-cf-id` (CloudFront) remains tolerated unsigned. Header-signed SigV4 and SigV2 requests are unchanged. ### Fixed +- **SSE-C TLS requirement now covers copy-source keys**: with `RUSTFS_SSE_C_REQUIRE_TLS=true`, a `CopyObject` or `UploadPartCopy` that carried only the `x-amz-copy-source-server-side-encryption-customer-*` headers (an SSE-C source copied to a destination that is not SSE-C) was accepted on a plaintext transport and was not counted in `rustfs_ssec_plaintext_requests_total`, although those headers carry the source object's customer key. They now count as SSE-C headers: such requests are counted, and refused with `400 InvalidRequest` when the switch is on. Requests over TLS are unaffected. +- **SSE-KMS encryption context could replace the location entry**: data keys are wrapped under an encryption-context entry, keyed by the bucket name, that ties them to the object's location. A client `x-amz-server-side-encryption-context` entry under the same key replaced it, and because the client context is stored with the object, the replacement also applied on every read. SSE-KMS writes now refuse such a context with `400 InvalidArgument`; other keys, including the names of other buckets, are unaffected. Objects stored with such an entry by an earlier release remain readable. See `docs/operations/kms-backend-security.md`. - **Two admin handlers reported a missing policy or user as `500 InternalError`**: `GET /rustfs/admin/v3/info-canned-policy` and `DELETE /rustfs/admin/v3/remove-user` wrapped every IAM error as an internal fault instead of classifying it, so a lookup for something that does not exist answered `500 InternalError` where the rest of the admin API answers `404 NoSuchResource` — `GET /rustfs/admin/v3/user-info` already did, and `iam_error_to_s3_error` already mapped `NoSuchPolicy`, `NoSuchUser`, `NoSuchServiceAccount` and `NoSuchTempAccount` to `NoSuchResource`. A client could not tell "does not exist" from a real server failure, and a controller that checks existence before creating (a Kubernetes operator reconciling policies or users) could never create the object: the missing-policy lookup failed with a `500` and it retried forever. Both handlers now map their IAM errors through the shared `iam_error_to_s3_error` helper, so a missing policy or user is `404 NoSuchResource` and the `error` is preserved as the S3 error's source; the three IAM calls in the removal path (temporary-user lookup, service-account lookup, delete) are all reclassified, and every other IAM error stays `500 InternalError`. Fixes #8117. - **Vault backends accepted key names that escape the key prefix**: only the Local backend refused a key identifier containing a path separator, so on Vault KV2 `POST /rustfs/admin/v3/kms/keys` with a name such as `bad/name` succeeded and created a nested KV2 path that the key listing then reported as a directory rather than a key, and an identifier containing `..` addressed a record outside the configured key prefix once the HTTP client normalised the URL. Both Vault backends now apply the Local backend's containment rule at the point where the identifier becomes a path or a Transit key name: an empty identifier, one containing `/`, `\\` or NUL, or the dot segments `.` and `..` is refused with `400` (`InvalidKey`) before any request reaches Vault. The AWS backend is unaffected, since it addresses keys by ARN and alias. A Vault deployment that created such a key on an earlier build must re-create it under a plain name; objects encrypted under the old name stay readable only until that key is refused, so migrate them first. Refs rustfs/backlog#2474. - **Fresh multi-pool bootstrap with distinct format creators**: a new deployment whose pools have their first endpoint on different nodes (for example two single-node pools) could never publish its initial `pool.bin`: each node held fresh-bootstrap proof only for the pool it formatted, the deployment-wide proof collapsed to none, and every node died with `pool metadata recovery required: no durable bootstrap identity or pool.bin replica is available` after the startup retry budget. The first pool's creator now mints the pending cluster identity on its own pool, every other creator copies that nonce-bound identity onto the pool it formatted first-hand, and the elected writer publishes `pool.bin` once every pool replica carries the same pending identity. Corrupt or disagreeing replicas, pools that merely have a format, expansion pools joining an initialized deployment, and restarts without first-hand proof still fail closed. Non-elected nodes that start before `pool.bin` exists, and the elected writer while it waits for the other creators, no longer latch their pool-metadata write gate for the life of the process. Refs rustfs/backlog#2338, rustfs/backlog#2375. diff --git a/docs/operations/kms-backend-security.md b/docs/operations/kms-backend-security.md index 3ef65348d..a39be9d03 100644 --- a/docs/operations/kms-backend-security.md +++ b/docs/operations/kms-backend-security.md @@ -137,6 +137,8 @@ This release reports rather than refuses, because flipping straight to a rejecti - `RUSTFS_SSE_C_REQUIRE_TLS=true` (default `false`) refuses those requests now, with the same `400 InvalidRequest` wording AWS uses. Confirm the counter reads zero before enabling it. - The default is expected to flip in a later release. +The guard covers both the object's own SSE-C headers and the `x-amz-copy-source-server-side-encryption-customer-*` headers that `CopyObject` and `UploadPartCopy` use to read an SSE-C source: either set carries a customer key. + The verdict is per connection: a listener that terminates TLS satisfies it, and so does an `https` protocol forwarded by a proxy the trusted-proxy configuration accepts. A direct plaintext client asserts nothing, and a forwarded protocol from an untrusted peer is not consulted. ## Object ciphertext format: what the v1 frame layout does and does not authenticate @@ -179,6 +181,12 @@ Historically the KV2 and Local backends sealed only the DEK plaintext; the `encr Rollout constraint: reading bound envelopes needs no switch, but **a node that predates the field cannot open them** — its unwrap runs without the additional data and fails authentication. The switch defaults off (`ENV_KMS_ENVELOPE_AAD` in `crates/kms/src/config.rs`); enable it only after every node runs a release that understands `context_binding`, mirroring the `RUSTFS_ENCRYPTION_FRAME_V2` rollout. With the switch on, a rewrap sweep upgrades unbound envelopes to the bound format (converging to zero writes on re-run); a bound envelope never regresses to the unbound shape, and an envelope carrying an unrecognized `context_binding` value is refused rather than decrypted without its binding. +### Location entry in the encryption context + +Every SSE-S3 and SSE-KMS data key is wrapped under an encryption context that includes the entry `{"": "/"}`. Only the client-supplied part of the context (`x-amz-server-side-encryption-context`) is stored with the object; the location entry is rebuilt from the object's current location on every read, so a data key does not open at another location. How strongly that is enforced depends on the backend, as described above. + +A client context entry whose key equals the bucket name would replace the location entry, so SSE-KMS writes refuse it with `400 InvalidArgument`. Other keys, including the names of other buckets, are accepted. Objects that an earlier release stored with such an entry remain readable. Builds with the `rio-v2` feature additionally bind each object key to its location when sealing it. + ### Guarantees that hold only once every node is upgraded These are properties of builds from `1.0.0-rc.1` onward; a single older node removes them for the whole cluster. diff --git a/rustfs/src/server/ssec_transport.rs b/rustfs/src/server/ssec_transport.rs index cb65b258a..bcda8f08a 100644 --- a/rustfs/src/server/ssec_transport.rs +++ b/rustfs/src/server/ssec_transport.rs @@ -40,8 +40,9 @@ use http_body_util::{BodyExt, Full}; use metrics::counter; use rustfs_trusted_proxies::ClientInfo; use rustfs_utils::http::headers::{ - AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY, - AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_ALGORITHM, AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY_MD5, AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, + AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY, AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5, }; use std::sync::Once; use std::task::{Context, Poll}; @@ -62,15 +63,26 @@ pub(crate) const METRIC_SSEC_PLAINTEXT_REQUESTS_TOTAL: &str = "rustfs_ssec_plain type BoxError = Box; type BoxBody = http_body_util::combinators::UnsyncBoxBody; +/// Every header that carries, or announces, a customer-provided key: the +/// object's own triple and the copy-source triple that CopyObject and +/// UploadPartCopy use to read an SSE-C source. +const SSEC_HEADERS: [&str; 6] = [ + AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM, + AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY, + AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_ALGORITHM, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY_MD5, +]; + /// Whether the request carries any SSE-C header. /// -/// Any one of the three is enough: an incomplete triple is still an attempt to -/// use SSE-C, and it is rejected later for being incomplete — but the key may -/// already have crossed the wire. +/// Any one is enough: an incomplete triple is still an attempt to use SSE-C, +/// and it is rejected later for being incomplete — but the key may already +/// have crossed the wire. The copy-source triple counts the same as the +/// object's own: it is the source object's key, sent in the clear just the same. fn carries_ssec_headers(headers: &HeaderMap) -> bool { - headers.contains_key(AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM) - || headers.contains_key(AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY) - || headers.contains_key(AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_KEY_MD5) + SSEC_HEADERS.iter().any(|name| headers.contains_key(*name)) } /// Whether this request reached the server over TLS. @@ -204,6 +216,7 @@ where #[cfg(test)] mod tests { use super::*; + use rustfs_utils::http::headers::AMZ_COPY_SOURCE; use std::net::{IpAddr, SocketAddr}; fn ssec_headers() -> HeaderMap { @@ -244,6 +257,26 @@ mod tests { assert!(carries_ssec_headers(&only_md5)); } + #[test] + fn copy_source_ssec_headers_count_as_an_ssec_request() { + // CopyObject / UploadPartCopy of an SSE-C source send the source key in + // the copy-source triple; each one alone must trip the guard. + for name in [ + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_ALGORITHM, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY, + AMZ_SERVER_SIDE_ENCRYPTION_COPY_CUSTOMER_KEY_MD5, + ] { + let mut headers = HeaderMap::new(); + headers.insert(name, HeaderValue::from_static("dmFsdWU=")); + assert!(carries_ssec_headers(&headers), "{name} must count as an SSE-C header"); + } + + // Other copy-source headers are not key material. + let mut copy_only = HeaderMap::new(); + copy_only.insert(AMZ_COPY_SOURCE, HeaderValue::from_static("bucket/object")); + assert!(!carries_ssec_headers(©_only)); + } + #[test] fn transport_is_secure_only_on_tls_or_a_resolved_https_proxy() { assert!(is_secure_transport(true, None), "a TLS listener needs no header to prove it"); diff --git a/rustfs/src/storage/sse.rs b/rustfs/src/storage/sse.rs index 7c472ccc2..005bd4678 100644 --- a/rustfs/src/storage/sse.rs +++ b/rustfs/src/storage/sse.rs @@ -1610,6 +1610,24 @@ fn build_kms_request_context( context } +/// Rejects a client encryption context that would replace the location binding. +/// +/// [`build_kms_request_context`] binds the data key to `{bucket: bucket/key}` +/// with `or_insert`, and only the client context is persisted. A client entry +/// keyed by the bucket name therefore travels with the object and replaces the +/// binding on every read, so the envelope would no longer be tied to the +/// object's location. Only the write side rejects it: objects stored with such +/// an entry stay readable because the read side rebuilds the context unchanged. +fn reject_kms_context_location_override(bucket: &str, ssekms_context: Option<&HashMap>) -> Result<(), ApiError> { + if ssekms_context.is_some_and(|context| context.contains_key(bucket)) { + return Err(sse_invalid_argument(&format!( + "The x-amz-server-side-encryption-context header contains the key \"{bucket}\", which equals the bucket name; \ + that key is reserved for binding the data key to the object's location." + ))); + } + Ok(()) +} + fn build_object_encryption_context( bucket: &str, key: &str, @@ -2697,6 +2715,10 @@ async fn apply_managed_encryption_material( content_size: i64, principal: Option<&SseKmsPrincipal>, ) -> Result { + // Request validation, not a KMS operation: reject before the KMS is called + // and before an audit outcome is recorded. + reject_kms_context_location_override(bucket, ssekms_context.as_ref())?; + let requested_sse_type = managed_sse_type(server_side_encryption.as_str()); let requested_key_id = kms_key_id.clone(); let result = apply_managed_encryption_material_inner( @@ -4358,14 +4380,15 @@ mod tests { MINIO_INTERNAL_ENCRYPTION_S3_SEALED_KEY_HEADER, MINIO_INTERNAL_ENCRYPTION_SSEC_SEALED_KEY_HEADER, ObjectDekRewrapOutcome, ObjectEncryptionResolver, PrepareEncryptionRequest, ReadEncryptionMode, ReadEncryptionRequest, SSEC_ORIGINAL_SIZE_HEADER, SSEType, SseDekProvider, SseKmsPrincipal, SseObjectEncryptionResolver, SsecParams, StorageError, TestSseDekProvider, - apply_managed_decryption_material, apply_managed_encryption_material, authorize_sse_kms_object_read, - build_kms_request_context, classify_sse_read_response, encode_minio_kms_context, encryption_material_to_metadata, - extract_server_side_encryption_from_headers, extract_ssec_params_from_headers, extract_ssekms_context_from_headers, - generate_ssec_nonce, is_managed_sse, kms_operation_error, map_get_object_reader_error, mark_encrypted_multipart_metadata, - md5_base64, normalize_managed_metadata, project_sse_read_response_headers, recode_minio_kms_context, - reset_sse_dek_provider, resolve_effective_kms_key_id, resolve_stored_kms_key_id, rewrap_object_encryption_metadata, - sse_decryption, sse_encryption, sse_prepare_encryption, strip_managed_encryption_metadata, validate_sse_headers_for_read, - validate_sse_headers_for_write, validate_ssec_for_read, validate_ssec_params, verify_ssec_key_match, + apply_managed_decryption_material, apply_managed_encryption_material, apply_managed_encryption_material_inner, + authorize_sse_kms_object_read, build_kms_request_context, classify_sse_read_response, encode_minio_kms_context, + encryption_material_to_metadata, extract_server_side_encryption_from_headers, extract_ssec_params_from_headers, + extract_ssekms_context_from_headers, generate_ssec_nonce, is_managed_sse, kms_operation_error, + map_get_object_reader_error, mark_encrypted_multipart_metadata, md5_base64, normalize_managed_metadata, + project_sse_read_response_headers, recode_minio_kms_context, reset_sse_dek_provider, resolve_effective_kms_key_id, + resolve_stored_kms_key_id, rewrap_object_encryption_metadata, sse_decryption, sse_encryption, sse_prepare_encryption, + strip_managed_encryption_metadata, validate_sse_headers_for_read, validate_sse_headers_for_write, validate_ssec_for_read, + validate_ssec_params, verify_ssec_key_match, }; #[cfg(feature = "rio-v2")] use super::{ @@ -8229,6 +8252,160 @@ mod tests { assert_eq!(audit_tag(&read_tags, "kmsOutcome").as_deref(), Some("success")); } + async fn install_test_kms_provider(key_name: &str) { + use rustfs_kms::types::{CreateKeyRequest, KeyUsage}; + + reset_sse_dek_provider(); + let manager = configure_test_global_local_kms().await; + manager + .get_encryption_service() + .await + .expect("encryption service should exist") + .create_key(CreateKeyRequest { + key_name: Some(key_name.to_string()), + key_usage: KeyUsage::EncryptDecrypt, + description: None, + policy: None, + tags: HashMap::new(), + origin: None, + }) + .await + .expect("kms test key should be created"); + let provider = KmsSseDekProvider::new_with_service_manager(manager) + .await + .expect("kms provider should initialize from the configured test manager"); + super::set_sse_dek_provider_for_test(Arc::new(provider)); + } + + fn sse_kms_request<'a>(bucket: &'a str, key: &'a str, context: HashMap) -> EncryptionRequest<'a> { + EncryptionRequest { + bucket, + key, + server_side_encryption: Some(ServerSideEncryption::from_static(ServerSideEncryption::AWS_KMS)), + ssekms_key_id: Some("binding-key".to_string()), + ssekms_context: Some(context), + sse_customer_algorithm: None, + sse_customer_key: None, + sse_customer_key_md5: None, + content_size: 64, + principal: None, + } + } + + #[tokio::test] + async fn sse_kms_write_rejects_a_context_key_equal_to_the_bucket_name() { + let _guard = lock_sse_test_state().await; + install_test_kms_provider("binding-key").await; + let context = HashMap::from([("finance".to_string(), "finance/elsewhere".to_string())]); + + let err = sse_encryption(sse_kms_request("finance", "ledger.csv", context.clone())) + .await + .expect_err("a context entry under the bucket name must be rejected"); + assert_eq!(err.code, S3ErrorCode::InvalidArgument); + assert!( + err.message.contains("\"finance\""), + "the message must name the offending key: {}", + err.message + ); + + // CreateMultipartUpload generates the session data key through the + // prepare path; it must refuse the same context. + let err = sse_prepare_encryption(PrepareEncryptionRequest { + bucket: "finance", + key: "ledger.csv", + server_side_encryption: Some(ServerSideEncryption::from_static(ServerSideEncryption::AWS_KMS)), + ssekms_key_id: Some("binding-key".to_string()), + ssekms_context: Some(context), + sse_customer_algorithm: None, + sse_customer_key: None, + sse_customer_key_md5: None, + principal: None, + }) + .await + .expect_err("the multipart prepare path must reject the same context"); + assert_eq!(err.code, S3ErrorCode::InvalidArgument); + + reset_sse_dek_provider(); + } + + #[tokio::test] + async fn sse_kms_write_keeps_the_location_binding_when_the_context_names_another_bucket() { + let _guard = lock_sse_test_state().await; + install_test_kms_provider("binding-key").await; + let context = HashMap::from([ + ("archive".to_string(), "archive/ledger.csv".to_string()), + ("tenant".to_string(), "acct-4711".to_string()), + ]); + + let material = sse_encryption(sse_kms_request("finance", "ledger.csv", context)) + .await + .expect("a key naming a different bucket is an ordinary context entry") + .expect("managed sse-kms material"); + let metadata = encryption_material_to_metadata(&material).expect("kms metadata should serialize"); + + let read = |key: &'static str| { + let metadata = metadata.clone(); + async move { + sse_decryption(DecryptionRequest { + bucket: "finance", + key, + metadata: &metadata, + sse_customer_key: None, + sse_customer_key_md5: None, + principal: None, + }) + .await + } + }; + let decrypted = read("ledger.csv") + .await + .expect("the object decrypts at its own location") + .expect("managed sse-kms material"); + assert_eq!(decrypted.key_bytes, material.key_bytes); + read("relocated.csv") + .await + .expect_err("the data key stays bound to the object's location"); + + reset_sse_dek_provider(); + } + + #[tokio::test] + async fn sse_kms_objects_stored_with_a_bucket_keyed_context_stay_readable() { + let _guard = lock_sse_test_state().await; + install_test_kms_provider("binding-key").await; + let context = HashMap::from([("finance".to_string(), "finance/elsewhere".to_string())]); + + // Objects written before the write-side rejection reached the inner + // routine without validation; reproduce that stored shape directly. + let material = apply_managed_encryption_material_inner( + "finance", + "ledger.csv", + ServerSideEncryption::from_static(ServerSideEncryption::AWS_KMS), + Some("binding-key".to_string()), + Some(context), + 64, + None, + ) + .await + .expect("the inner routine does not validate the client context"); + let metadata = encryption_material_to_metadata(&material).expect("kms metadata should serialize"); + + let decrypted = sse_decryption(DecryptionRequest { + bucket: "finance", + key: "ledger.csv", + metadata: &metadata, + sse_customer_key: None, + sse_customer_key_md5: None, + principal: None, + }) + .await + .expect("an existing object must stay readable after the write-side rejection") + .expect("managed sse-kms material"); + assert_eq!(decrypted.key_bytes, material.key_bytes); + + reset_sse_dek_provider(); + } + #[tokio::test] async fn sse_c_requests_produce_no_kms_audit_summary() { let key = [0x21u8; 32];