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];