diff --git a/crates/kms/src/backends/contract_tests.rs b/crates/kms/src/backends/contract_tests.rs index 7e118121e..ed1342f1f 100644 --- a/crates/kms/src/backends/contract_tests.rs +++ b/crates/kms/src/backends/contract_tests.rs @@ -183,6 +183,7 @@ async fn assert_state_machine_contract(backend: &dyn KmsBackend, key_id: &str) { .await .expect("decrypt with a disabled key must keep working"); assert_eq!(decrypted.plaintext, data_key.plaintext_key, "decrypt must recover the original data key"); + assert_eq!(decrypted.key_id, key_id, "decrypt must report the master key that opened the envelope"); // ...disable stays idempotent, cancel has nothing to cancel, and enable recovers. backend.disable_key(key_id).await.expect("disable must be idempotent"); expect_invalid_key_state(backend.cancel_key_deletion(cancel_request(key_id)).await, "not pending deletion"); diff --git a/crates/kms/src/backends/local.rs b/crates/kms/src/backends/local.rs index 239966a34..836c6d979 100644 --- a/crates/kms/src/backends/local.rs +++ b/crates/kms/src/backends/local.rs @@ -1529,14 +1529,7 @@ impl LocalKmsClient { ensure_key_status_permits(&request.master_key_id, &key_info.status, StateGatedOperation::GenerateDataKey)?; // Generate random data key material - let key_length = match request.key_spec.as_str() { - "AES_256" => 32, - "AES_128" => 16, - _ => return Err(KmsError::unsupported_algorithm(&request.key_spec)), - }; - - let mut plaintext_key = vec![0u8; key_length]; - rand::rng().fill(&mut plaintext_key[..]); + let plaintext_key = generate_key_material(&request.key_spec)?; // Encrypt the data key with the master key let (encrypted_key, nonce) = self.encrypt_with_master_key(&request.master_key_id, &plaintext_key).await?; @@ -1596,11 +1589,19 @@ impl LocalKmsClient { }) } - pub(crate) async fn decrypt(&self, request: &DecryptRequest, _context: Option<&OperationContext>) -> Result> { + /// Open a data-key envelope, returning the plaintext and the master key + /// that wrapped it. + pub(crate) async fn decrypt( + &self, + request: &DecryptRequest, + _context: Option<&OperationContext>, + ) -> Result<(Vec, String)> { debug!("Decrypting data"); - // Parse the data key envelope from ciphertext - let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext)?; + // Parse the data key envelope from ciphertext. Mapped to the same + // error class the other backends report for unparseable ciphertext. + let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext) + .map_err(|error| KmsError::cryptographic_error("parse", format!("Failed to parse data key envelope: {error}")))?; // NOTE: this comparison is an authorization check, not a cryptographic // binding. `DekCrypto` seals only the plaintext, so `encryption_context` @@ -1634,7 +1635,7 @@ impl LocalKmsClient { .await?; debug!("Local KMS data decrypted"); - Ok(plaintext) + Ok((plaintext, envelope.master_key_id)) } /// Test-only lifecycle driver: the product path goes through [`KmsBackend`]. @@ -1994,16 +1995,11 @@ impl KmsBackend for LocalKmsBackend { } async fn decrypt(&self, request: DecryptRequest) -> Result { - let plaintext = self.client.decrypt(&request, None).await?; - - // The envelope that was just opened names the master key that opened it. - // Reporting "unknown" left every caller unable to tell which key was - // actually used, which is what audit and key-rotation checks read. - let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext)?; + let (plaintext, key_id) = self.client.decrypt(&request, None).await?; Ok(DecryptResponse { plaintext, - key_id: envelope.master_key_id, + key_id, encryption_algorithm: Some("AES-256-GCM".to_string()), }) } @@ -2017,12 +2013,19 @@ impl KmsBackend for LocalKmsBackend { grant_tokens: Vec::new(), }; - let data_key = self.client.generate_data_key(&generate_request, None).await?; + let mut data_key = self.client.generate_data_key(&generate_request, None).await?; + // Fields are taken, not destructured or cloned: `DataKeyInfo` has a + // `Drop` impl, and a clone would leave a second un-zeroized plaintext + // DEK on the heap. + let plaintext_key = data_key + .plaintext + .take() + .ok_or_else(|| KmsError::internal_error("Generated data key is missing plaintext"))?; Ok(GenerateDataKeyResponse { key_id: request.key_id, - plaintext_key: data_key.plaintext.clone().unwrap_or_default(), - ciphertext_blob: data_key.ciphertext.clone(), + plaintext_key, + ciphertext_blob: std::mem::take(&mut data_key.ciphertext), }) } @@ -2423,8 +2426,9 @@ mod tests { let decrypt_request = DecryptRequest::new(data_key.ciphertext.clone()).with_context("bucket".to_string(), "test-bucket".to_string()); - let decrypted = client.decrypt(&decrypt_request, None).await.expect("Failed to decrypt"); + let (decrypted, opened_by) = client.decrypt(&decrypt_request, None).await.expect("Failed to decrypt"); assert_eq!(decrypted, data_key.plaintext.clone().expect("No plaintext")); + assert_eq!(opened_by, key_id, "decrypt must report the master key that opened the envelope"); } #[tokio::test] @@ -2455,7 +2459,7 @@ mod tests { // Pre-fix, each of those regenerated the master key, so this unwrap fails with an AEAD // error. Post-fix, the original material is preserved and the DEK still decrypts. let decrypt_request = DecryptRequest::new(ciphertext).with_context("bucket".to_string(), "b".to_string()); - let decrypted = client + let (decrypted, _opened_by) = client .decrypt(&decrypt_request, None) .await .expect("DEK must still decrypt after status transitions"); @@ -2796,7 +2800,7 @@ mod tests { assert!(matches!(error, KmsError::InvalidOperation { .. })); for (index, (ciphertext, plaintext)) in batch.iter().enumerate() { - let decrypted = client + let (decrypted, _opened_by) = client .decrypt(&DecryptRequest::new(ciphertext.clone()), None) .await .unwrap_or_else(|error| panic!("batch member {index} must decrypt: {error}")); diff --git a/crates/kms/src/backends/static_kms.rs b/crates/kms/src/backends/static_kms.rs index 02d29bc36..3c82fb345 100644 --- a/crates/kms/src/backends/static_kms.rs +++ b/crates/kms/src/backends/static_kms.rs @@ -26,7 +26,7 @@ use crate::backends::{BackendCapabilities, KmsBackend, empty_key_page, list_keys_page_size}; use crate::config::{BackendConfig, KmsConfig}; -use crate::encryption::{DataKeyEnvelope, context_aad}; +use crate::encryption::{DataKeyEnvelope, context_aad, generate_key_material}; use crate::error::{KmsError, Result}; use crate::types::*; use aes_gcm::{ @@ -124,16 +124,7 @@ impl StaticKmsBackend { // The requested spec decides the DEK length; a caller that asked for // AES_128 and silently got 256 bits would build objects whose recorded // spec does not match their key material. - // Lengths track `KeySpec::key_size`; the request carries the spec as a - // string, so the mapping is repeated here rather than shared. - let key_length = match request.key_spec.as_str() { - "AES_256" | "ChaCha20" => 32, - "AES_128" => 16, - _ => return Err(KmsError::unsupported_algorithm(&request.key_spec)), - }; - - let mut plaintext = vec![0u8; key_length]; - rand::rng().fill(&mut plaintext[..]); + let plaintext = generate_key_material(&request.key_spec)?; // Encrypt DEK with AES-256-GCM using the static key directly let key = Key::::from(*self.key); @@ -167,7 +158,7 @@ impl StaticKmsBackend { Ok(DataKeyInfo::new( self.key_id.clone(), 0, - Some(plaintext.to_vec()), + Some(plaintext), ciphertext, request.key_spec.clone(), )) @@ -360,17 +351,20 @@ impl KmsBackend for StaticKmsBackend { encryption_context: request.encryption_context, grant_tokens: Vec::new(), }; - let data_key = self.generate_data_key_envelope(&gen_req)?; + let mut data_key = self.generate_data_key_envelope(&gen_req)?; + // Fields are taken, not destructured or cloned: `DataKeyInfo` has a + // `Drop` impl, and a clone would leave a second un-zeroized plaintext + // DEK on the heap. let plaintext_key = data_key .plaintext - .clone() + .take() .ok_or_else(|| KmsError::internal_error("Generated data key is missing plaintext"))?; Ok(GenerateDataKeyResponse { - key_id: data_key.key_id.clone(), + key_id: std::mem::take(&mut data_key.key_id), plaintext_key, - ciphertext_blob: data_key.ciphertext.clone(), + ciphertext_blob: std::mem::take(&mut data_key.ciphertext), }) } diff --git a/crates/kms/src/backends/vault.rs b/crates/kms/src/backends/vault.rs index 2cca931b8..188f2fac8 100644 --- a/crates/kms/src/backends/vault.rs +++ b/crates/kms/src/backends/vault.rs @@ -875,7 +875,13 @@ impl VaultKmsClient { }) } - pub(crate) async fn decrypt(&self, request: &DecryptRequest, _context: Option<&OperationContext>) -> Result> { + /// Open a data-key envelope, returning the plaintext and the master key + /// that wrapped it. + pub(crate) async fn decrypt( + &self, + request: &DecryptRequest, + _context: Option<&OperationContext>, + ) -> Result<(Vec, String)> { debug!("Decrypting data"); // Parse the data key envelope from ciphertext @@ -929,7 +935,7 @@ impl VaultKmsClient { }; debug!("Vault KMS data decrypted"); - Ok(plaintext) + Ok((plaintext, envelope.master_key_id)) } /// Report which master key version wraps an envelope, and whether that is @@ -1626,16 +1632,11 @@ impl KmsBackend for VaultKmsBackend { } async fn decrypt(&self, request: DecryptRequest) -> Result { - let plaintext = self.client.decrypt(&request, None).await?; - - // The envelope that was just opened names the master key that opened it. - // Reporting "unknown" left every caller unable to tell which key was - // actually used, which is what audit and key-rotation checks read. - let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext)?; + let (plaintext, key_id) = self.client.decrypt(&request, None).await?; Ok(DecryptResponse { plaintext, - key_id: envelope.master_key_id, + key_id, encryption_algorithm: Some("AES-256-GCM".to_string()), }) } @@ -1660,12 +1661,19 @@ impl KmsBackend for VaultKmsBackend { grant_tokens: Vec::new(), }; - let data_key = self.client.generate_data_key(&generate_request, None).await?; + let mut data_key = self.client.generate_data_key(&generate_request, None).await?; + // Fields are taken, not destructured or cloned: `DataKeyInfo` has a + // `Drop` impl, and a clone would leave a second un-zeroized plaintext + // DEK on the heap. + let plaintext_key = data_key + .plaintext + .take() + .ok_or_else(|| KmsError::internal_error("Generated data key is missing plaintext"))?; Ok(GenerateDataKeyResponse { key_id: request.key_id, - plaintext_key: data_key.plaintext.clone().unwrap_or_default(), - ciphertext_blob: data_key.ciphertext.clone(), + plaintext_key, + ciphertext_blob: std::mem::take(&mut data_key.ciphertext), }) } @@ -2525,7 +2533,7 @@ mod tests { // A mixed batch of envelopes from every historical version must decrypt. for (data_key, label) in [(&dk_v1, "v1"), (&dk_v3, "v3"), (&dk_v2, "v2"), (&dk_v1, "v1 again")] { - let plaintext = client + let (plaintext, _opened_by) = client .decrypt(&integration_decrypt_request(data_key.ciphertext.clone()), None) .await .unwrap_or_else(|error| panic!("envelope wrapped under {label} must stay decryptable: {error}")); @@ -2561,7 +2569,7 @@ mod tests { // The baseline rule must route the legacy envelope to the frozen version 1 // material even though the current version has moved on. - let plaintext = client + let (plaintext, _opened_by) = client .decrypt(&integration_decrypt_request(legacy_ciphertext), None) .await .expect("legacy envelope must stay decryptable after rotation"); @@ -2605,7 +2613,7 @@ mod tests { ); // The untampered envelope still decrypts through its recorded version. - let plaintext = client + let (plaintext, _opened_by) = client .decrypt(&integration_decrypt_request(data_key.ciphertext.clone()), None) .await .expect("untampered envelope must still decrypt"); @@ -2870,7 +2878,7 @@ mod tests { assert_eq!(envelope.master_key_id, "wired-key"); assert_eq!(envelope.master_key_version, Some(1)); - let decrypted = client + let (decrypted, opened_by) = client .decrypt( &DecryptRequest { ciphertext: encrypted.ciphertext.clone(), @@ -2882,6 +2890,7 @@ mod tests { .await .expect("decrypt must round-trip the envelope"); assert_eq!(decrypted, b"kv2-direct-encrypt".to_vec()); + assert_eq!(opened_by, "wired-key", "decrypt must report the master key that opened the envelope"); // A different object context must not decrypt (checked before any // Vault read, so no scripted response is consumed). @@ -4181,7 +4190,7 @@ mod tests { let (vault, client) = scripted_client(vec![ScriptedResponse::ok(kv2_read_data(&key_data))]).await; - let plaintext = client + let (plaintext, _opened_by) = client .decrypt( &DecryptRequest { ciphertext, @@ -4315,7 +4324,7 @@ mod tests { } let (vault, client) = scripted_client(responses).await; - let plaintext = client + let (plaintext, _opened_by) = client .decrypt( &DecryptRequest { ciphertext: ciphertext.to_vec(), diff --git a/crates/kms/src/backends/vault_transit.rs b/crates/kms/src/backends/vault_transit.rs index 5894bd561..3e29541a1 100644 --- a/crates/kms/src/backends/vault_transit.rs +++ b/crates/kms/src/backends/vault_transit.rs @@ -817,7 +817,13 @@ impl VaultTransitKmsClient { }) } - pub(crate) async fn decrypt(&self, request: &DecryptRequest, _context: Option<&OperationContext>) -> Result> { + /// Open a data-key envelope, returning the plaintext and the master key + /// that wrapped it. + pub(crate) async fn decrypt( + &self, + request: &DecryptRequest, + _context: Option<&OperationContext>, + ) -> Result<(Vec, String)> { let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext) .map_err(|e| KmsError::cryptographic_error("parse", format!("Failed to parse data key envelope: {e}")))?; @@ -839,7 +845,7 @@ impl VaultTransitKmsClient { .transit_decrypt(&envelope.master_key_id, encrypted_key, &envelope.encryption_context) .await { - Ok(plaintext) => Ok(plaintext), + Ok(plaintext) => Ok((plaintext, envelope.master_key_id)), Err(error) => { self.invalidate_metadata_on_state_error(&envelope.master_key_id, &error).await; Err(error) @@ -1384,11 +1390,10 @@ impl KmsBackend for VaultTransitKmsBackend { } async fn decrypt(&self, request: DecryptRequest) -> Result { - let envelope: DataKeyEnvelope = serde_json::from_slice(&request.ciphertext)?; - let plaintext = self.client.decrypt(&request, None).await?; + let (plaintext, key_id) = self.client.decrypt(&request, None).await?; Ok(DecryptResponse { plaintext, - key_id: envelope.master_key_id, + key_id, encryption_algorithm: Some("vault-transit".to_string()), }) } @@ -1413,13 +1418,19 @@ impl KmsBackend for VaultTransitKmsBackend { grant_tokens: Vec::new(), }; - let data_key = self.client.generate_data_key(&generate_request, None).await?; - let plaintext_key = data_key.plaintext.clone().unwrap_or_default(); - let ciphertext_blob = data_key.ciphertext.clone(); + let mut data_key = self.client.generate_data_key(&generate_request, None).await?; + + // Fields are taken, not destructured or cloned: `DataKeyInfo` has a + // `Drop` impl, and a clone would leave a second un-zeroized plaintext + // DEK on the heap. + let plaintext_key = data_key + .plaintext + .take() + .ok_or_else(|| KmsError::internal_error("Generated data key is missing plaintext"))?; Ok(GenerateDataKeyResponse { key_id: request.key_id, plaintext_key, - ciphertext_blob, + ciphertext_blob: std::mem::take(&mut data_key.ciphertext), }) } @@ -2089,7 +2100,7 @@ mod tests { // Historical ciphertext keeps decrypting per Vault's version semantics, // interleaved with post-rotation ciphertext. for (data_key, label) in [(&dk_v1, "v1"), (&dk_v2, "v2"), (&dk_v1, "v1 again")] { - let plaintext = client + let (plaintext, _opened_by) = client .decrypt( &DecryptRequest { ciphertext: data_key.ciphertext.clone(), @@ -2659,7 +2670,7 @@ mod tests { let rotated = client.rotate_key("wired-key", None).await.expect("rotation must commit"); assert_eq!(rotated.version, 2, "the rotation must record the version bump"); - let plaintext = client + let (plaintext, opened_by) = client .decrypt( &DecryptRequest { ciphertext: data_key.ciphertext.clone(), @@ -2675,6 +2686,7 @@ mod tests { RECOVERED_DEK.to_vec(), "the decrypt must hand back the recovered material, not merely avoid an error" ); + assert_eq!(opened_by, "wired-key", "decrypt must report the master key that opened the envelope"); let requests = vault.requests(); assert_eq!(requests.len(), 7, "{requests:?}"); diff --git a/crates/kms/src/encryption/dek.rs b/crates/kms/src/encryption/dek.rs index ca3f2658d..f8f2ae275 100644 --- a/crates/kms/src/encryption/dek.rs +++ b/crates/kms/src/encryption/dek.rs @@ -389,16 +389,18 @@ impl Default for AesDekCrypto { } } -/// Generate random key material for the given algorithm +/// Generate random key material for the given algorithm. +/// +/// The lengths must track [`crate::types::KeySpec::key_size`]. /// /// # Arguments -/// * `algorithm` - The key algorithm (e.g., "AES_256", "AES_128") +/// * `algorithm` - The key algorithm (e.g., "AES_256", "AES_128", "ChaCha20") /// /// # Returns /// A vector containing the generated key material pub fn generate_key_material(algorithm: &str) -> Result> { let key_size = match algorithm { - "AES_256" => 32, + "AES_256" | "ChaCha20" => 32, "AES_128" => 16, _ => return Err(KmsError::unsupported_algorithm(algorithm)), }; diff --git a/crates/kms/tests/behavior_crypto.rs b/crates/kms/tests/behavior_crypto.rs index 30dc6670c..a2606522d 100644 --- a/crates/kms/tests/behavior_crypto.rs +++ b/crates/kms/tests/behavior_crypto.rs @@ -33,7 +33,7 @@ mod common; -use common::{BackendCase, BackendKind, TestKms, assert_context_mismatch, ctx, flip_middle_bit, for_each_backend, payload}; +use common::{BackendCase, TestKms, assert_context_mismatch, ctx, flip_middle_bit, for_each_backend, payload}; use rustfs_kms::{ DecryptRequest, EncryptRequest, GenerateDataKeyRequest, KeySpec, KmsError, ObjectEncryptionContext, is_data_key_envelope, }; @@ -259,55 +259,51 @@ async fn data_key_spec_controls_the_length_of_the_generated_key() { // A backend that accepts a `key_spec` must honour it. Silently // returning a different size means the caller builds a cipher from // material it did not ask for, and the envelope records a spec its - // payload does not match. - // ChaCha20 material is 32 random bytes, exactly like AES_256, so a - // backend that mints DEKs itself has no technical reason to refuse it. - // Static accepts it; Local and both Vault backends route through - // `generate_key_material`, which only knows the two AES specs. That - // split is pinned per backend rather than tolerated on both sides: a - // blanket "honoured or refused" contract would accept a backend - // regressing from working into refusing, which is exactly how a - // silently dropped spec would ship. + // payload does not match. Every backend in this matrix mints DEKs via + // the shared `generate_key_material`, so all three specs must be + // honoured; tolerating a refusal would accept a backend regressing + // out of that shared mapping. for spec in [KeySpec::Aes256, KeySpec::Aes128, KeySpec::ChaCha20] { - let must_be_honoured = spec != KeySpec::ChaCha20 || case.kind() == BackendKind::Static; - match manager + let generated = manager .generate_data_key(GenerateDataKeyRequest { key_id: case.key_id.clone(), key_spec: spec.clone(), encryption_context: context(), }) .await - { - Ok(generated) => assert_eq!( - generated.plaintext_key.len(), - spec.key_size(), - "[{label}] {spec:?} must yield a {}-byte data key", - spec.key_size() - ), - Err(KmsError::UnsupportedAlgorithm { .. }) if !must_be_honoured => {} - Err(error) => panic!("[{label}] {spec:?} must yield a {}-byte data key: {error:?}", spec.key_size()), - } + .unwrap_or_else(|error| panic!("[{label}] {spec:?} must yield a data key: {error:?}")); + assert_eq!( + generated.plaintext_key.len(), + spec.key_size(), + "[{label}] {spec:?} must yield a {}-byte data key", + spec.key_size() + ); + assert_eq!( + generated.key_id, case.key_id, + "[{label}] {spec:?} must name the master key that wrapped the DEK" + ); + + // The envelope must record the spec it was minted under, or a + // reader can no longer tell what the wrapped material is. + let envelope: serde_json::Value = + serde_json::from_slice(&generated.ciphertext_blob).expect("ciphertext must be a KMS envelope"); + assert_eq!( + envelope.get("key_spec").and_then(|value| value.as_str()), + Some(spec.as_str()), + "[{label}] the envelope must record the requested spec" + ); + + // Whatever the length, the blob still round-trips. + let decrypted = manager + .decrypt(DecryptRequest { + ciphertext: generated.ciphertext_blob, + encryption_context: context(), + grant_tokens: Vec::new(), + }) + .await + .unwrap_or_else(|error| panic!("[{label}] {spec:?} blob should decrypt: {error:?}")); + assert_eq!(decrypted.plaintext, generated.plaintext_key); } - - let aes128 = manager - .generate_data_key(GenerateDataKeyRequest { - key_id: case.key_id.clone(), - key_spec: KeySpec::Aes128, - encryption_context: context(), - }) - .await - .unwrap_or_else(|error| panic!("[{label}] AES-128 generate should succeed: {error:?}")); - - // Whatever the length, the blob still round-trips. - let decrypted = manager - .decrypt(DecryptRequest { - ciphertext: aes128.ciphertext_blob, - encryption_context: context(), - grant_tokens: Vec::new(), - }) - .await - .unwrap_or_else(|error| panic!("[{label}] AES-128 blob should decrypt: {error:?}")); - assert_eq!(decrypted.plaintext, aes128.plaintext_key); }) .await; } @@ -344,7 +340,8 @@ async fn corrupt_ciphertext_fails_cleanly() { let tampered = flip_middle_bit(&dek.ciphertext_blob); assert!(decrypt(tampered).await.is_err(), "[{label}] a bit-flipped envelope must not decrypt"); - // Truncation, emptiness, and non-envelope bytes are all typed errors. + // Truncation, emptiness, and non-envelope bytes are unparseable + // ciphertext, and every backend reports that as the same error class. for (name, input) in [ ("truncated", dek.ciphertext_blob[..dek.ciphertext_blob.len() / 2].to_vec()), ("empty", Vec::new()), @@ -355,8 +352,8 @@ async fn corrupt_ciphertext_fails_cleanly() { .await .expect_err(&format!("[{label}] {name} input must be rejected")); assert!( - !matches!(error, KmsError::InternalError { .. }), - "[{label}] {name} input must map to a specific error, not InternalError: {error:?}" + matches!(error, KmsError::CryptographicError { .. }), + "[{label}] {name} input must be rejected as unparseable ciphertext, got: {error:?}" ); } diff --git a/crates/kms/tests/behavior_objects.rs b/crates/kms/tests/behavior_objects.rs index b154a6e6d..979cf0bb8 100644 --- a/crates/kms/tests/behavior_objects.rs +++ b/crates/kms/tests/behavior_objects.rs @@ -925,3 +925,181 @@ async fn a_rewritten_context_header_fails_authentication() { "a context the object was not sealed under must not open it" ); } + +/// The SSE-C flank of the tamper check above: the customer-key path prefers +/// the stored AAD bytes through the same branch, so a reverted preference — +/// re-deriving canonical bytes from the parsed map — would open a tampered +/// object here too, and only an SSE-C probe would notice. +#[tokio::test] +async fn a_rewritten_sse_c_context_header_fails_authentication() { + let (_kms, service) = service_with_key("sse-c-tampered-context").await; + let object_key = "tampered-sse-c.bin"; + let customer_key = [0x55u8; 32]; + let data = payload(256); + + let encrypted = service + .encrypt_object_with_customer_key(BUCKET, object_key, data.as_slice(), &customer_key, None) + .await + .expect("SSE-C encrypt should succeed"); + + let mut headers = service.metadata_to_headers(&encrypted.metadata); + // Same pairs, different serialization: a pure ordering rewrite, so the + // rejection can only come from the AAD bytes and not from a changed map. + let rewritten = non_canonical_context_json(&encrypted.metadata.encryption_context); + assert_ne!( + Some(rewritten.as_str()), + headers.get("x-rustfs-encryption-context").map(String::as_str), + "the rewrite must actually change the stored bytes, or this proves nothing" + ); + headers.insert("x-rustfs-encryption-context".to_string(), rewritten); + + let tampered = service.headers_to_metadata(&headers).expect("tampered headers still parse"); + assert!( + discard( + service + .decrypt_object_with_customer_key(BUCKET, object_key, encrypted.ciphertext.clone(), &tampered, &customer_key) + .await + ) + .is_err(), + "a context the SSE-C object was not sealed under must not open it, even with the right key" + ); +} + +/// An SSE-KMS object written before the internal `x-rustfs-` headers existed +/// must still open, and must rebuild into a record that names its real cipher. +/// +/// Back then `x-amz-server-side-encryption: aws:kms` plus the S3 key-id header +/// was the whole record, and AES-256-GCM was the only cipher in use — which is +/// exactly the assumption the `aws:kms` fallback in `headers_to_metadata` +/// encodes. The fallback is a normalization: `aws:kms` also parses as a cipher +/// alias for AES-256-GCM, so the object opens either way, but only the +/// normalized record re-projects the cipher header a modern read expects. The +/// legacy header shape is reconstructed here by rewriting the SSE mode to +/// `aws:kms`, adding the S3 key-id header, and dropping both internal headers. +#[tokio::test] +async fn a_legacy_aws_kms_object_without_the_cipher_header_still_opens() { + let (_kms, service) = service_with_key("sse-legacy-mode").await; + let object_key = "legacy-aws-kms.bin"; + let data = payload(512); + + let encrypted = service + .encrypt_object(BUCKET, object_key, data.as_slice(), &EncryptionAlgorithm::Aes256, None, None) + .await + .expect("encrypt should succeed"); + + let mut headers = service.metadata_to_headers(&encrypted.metadata); + headers.insert("x-amz-server-side-encryption".to_string(), "aws:kms".to_string()); + headers.insert( + "x-amz-server-side-encryption-aws-kms-key-id".to_string(), + encrypted.metadata.key_id.clone(), + ); + for internal in ["x-rustfs-encryption-algorithm", "x-rustfs-encryption-key-id"] { + headers + .remove(internal) + .unwrap_or_else(|| panic!("the modern projection must write the {internal} header this test deletes")); + } + + let rebuilt = service + .headers_to_metadata(&headers) + .expect("a pre-internal-header record must still parse"); + assert_eq!(rebuilt.key_id, encrypted.metadata.key_id, "the S3 key-id header must resolve the key"); + assert_eq!( + rebuilt.algorithm, + EncryptionAlgorithm::Aes256.as_str(), + "aws:kms with no cipher header must normalize to the only cipher that era wrote" + ); + + // The normalization is what a re-projection stores: the upgraded record + // writes the modern cipher header instead of perpetuating the gap. + let reprojected = service.metadata_to_headers(&rebuilt); + assert_eq!( + reprojected.get("x-rustfs-encryption-algorithm").map(String::as_str), + Some(EncryptionAlgorithm::Aes256.as_str()), + "re-projecting the rebuilt record must write the cipher header" + ); + + let decrypted = read_all( + service + .decrypt_object(BUCKET, object_key, encrypted.ciphertext.clone(), &rebuilt, None) + .await + .expect("a legacy aws:kms object must still open"), + ) + .await; + assert_eq!(decrypted, data, "the rebuilt record must recover the full plaintext"); +} + +/// Metadata persisted before `context_aad` existed deserializes with `None` +/// there, and decrypt must then re-derive the AAD from the parsed context. +/// That derived path only opens the object because the seal side canonicalizes +/// the very same way — this is the independent probe of that pairing, for both +/// the KMS and the customer-key flavours. +#[tokio::test] +async fn metadata_without_stored_context_bytes_still_opens() { + let (_kms, service) = service_with_key("sse-derived-aad").await; + let data = payload(512); + // Several entries, so canonicalization has an ordering to actually decide. + let context = ctx(&[("zeta", "26"), ("alpha", "1"), ("mu", "13")]); + + // Byte-equality of the derived and stored AAD is what keeps the `None` + // path working, so pin the seal side of that pairing directly: the sealed + // record must carry exactly the canonical serialization of its context. + let canonical_aad = |context: &HashMap| { + let canonical: std::collections::BTreeMap<&str, &str> = + context.iter().map(|(key, value)| (key.as_str(), value.as_str())).collect(); + serde_json::to_vec(&canonical).expect("context serializes") + }; + + let encrypted = service + .encrypt_object( + BUCKET, + "derived-aad.bin", + data.as_slice(), + &EncryptionAlgorithm::Aes256, + None, + Some(&context), + ) + .await + .expect("encrypt should succeed"); + assert_eq!( + encrypted.metadata.context_aad.as_deref(), + Some(canonical_aad(&encrypted.metadata.encryption_context).as_slice()), + "the seal must pin the exact canonical AAD bytes it fed the AEAD" + ); + let stripped = EncryptionMetadata { + context_aad: None, + ..encrypted.metadata.clone() + }; + let decrypted = read_all( + service + .decrypt_object(BUCKET, "derived-aad.bin", encrypted.ciphertext.clone(), &stripped, None) + .await + .expect("metadata with no stored AAD bytes must open through the derived path"), + ) + .await; + assert_eq!(decrypted, data, "the derived AAD must match the bytes the object was sealed under"); + + // The SSE-C record carries the same optional field through the same serde + // default, so its derived path needs its own proof. + let customer_key = [0x66u8; 32]; + let sse_c = service + .encrypt_object_with_customer_key(BUCKET, "derived-aad-c.bin", data.as_slice(), &customer_key, None) + .await + .expect("SSE-C encrypt should succeed"); + assert_eq!( + sse_c.metadata.context_aad.as_deref(), + Some(canonical_aad(&sse_c.metadata.encryption_context).as_slice()), + "the SSE-C seal must pin the exact canonical AAD bytes it fed the AEAD" + ); + let stripped = EncryptionMetadata { + context_aad: None, + ..sse_c.metadata.clone() + }; + let decrypted = read_all( + service + .decrypt_object_with_customer_key(BUCKET, "derived-aad-c.bin", sse_c.ciphertext.clone(), &stripped, &customer_key) + .await + .expect("SSE-C metadata with no stored AAD bytes must open through the derived path"), + ) + .await; + assert_eq!(decrypted, data, "the SSE-C derived AAD must match the bytes the object was sealed under"); +} diff --git a/crates/kms/tests/common/mod.rs b/crates/kms/tests/common/mod.rs index f8123d803..9d2b8d7ec 100644 --- a/crates/kms/tests/common/mod.rs +++ b/crates/kms/tests/common/mod.rs @@ -445,22 +445,6 @@ pub fn assert_configuration_error(result: Result, message_fragment: } } -#[track_caller] -pub fn assert_validation_error(result: Result) { - match result { - Err(KmsError::ValidationError { .. }) => {} - other => panic!("expected ValidationError, got {other:?}"), - } -} - -#[track_caller] -pub fn assert_cryptographic_error(result: Result) { - match result { - Err(KmsError::CryptographicError { .. }) => {} - other => panic!("expected CryptographicError, got {other:?}"), - } -} - #[track_caller] pub fn assert_invalid_key_size(result: Result, expected: usize, actual: usize) { match result {