chore(duvet): duvet for updated spec

texastony · texastony · commit 1e17d4dd8ad7 · 2026-02-09T10:37:04.000-06:00
diff --git a/src/main/java/software/amazon/encryption/s3/internal/CipherProvider.java b/src/main/java/software/amazon/encryption/s3/internal/CipherProvider.java
@@ -195,8 +195,15 @@ public static Cipher createAndInitCipher(final CryptographicMaterials materials,
                     }
                     //= specification/s3-encryption/decryption.md#cbc-decryption
                     //# If an object is encrypted with ALG_AES_256_CBC_IV16_NO_KDF and [legacy unauthenticated algorithm suites](#legacy-decryption) is enabled,
-                    //# then the S3EC MUST create a cipher object using the cipher transformation "AES/CBC/PKCS5Padding".
-                    // NOTE: PKCS5Padding is specified above in CryptoFactory.createCipher(materials.algorithmSuite().cipherName(), materials.cryptoProvider())
+                    //# then the S3EC MUST create a cipher with AES in CBC Mode with PKCS5Padding or PKCS7Padding compatible padding for a 16-byte block cipher (example: for the Java JCE, this is "AES/CBC/PKCS5Padding").
+                    // NOTE: CBC and PKCS5Padding is specified above in CryptoFactory.createCipher(materials.algorithmSuite().cipherName(), materials.cryptoProvider())
+                    //= specification/s3-encryption/decryption.md#cbc-decryption
+                    //# If the cipher object cannot be created as described above,
+                    //# Decryption MUST fail.
+                    //= specification/s3-encryption/decryption.md#cbc-decryption
+                    //= type=exception
+                    //# The error SHOULD detail why the cipher could not be initialized
+                    //# (such as CBC or PKCS5Padding is not supported by the underlying crypto provider).
                     cipher.init(materials.cipherMode().opMode(), materials.dataKey(), new IvParameterSpec(iv));
                     break;
                 case ALG_AES_256_CTR_HKDF_SHA512_COMMIT_KEY:
diff --git a/src/main/java/software/amazon/encryption/s3/internal/ContentMetadataEncodingStrategy.java b/src/main/java/software/amazon/encryption/s3/internal/ContentMetadataEncodingStrategy.java
@@ -54,6 +54,11 @@ public PutObjectRequest encodeMetadata(EncryptionMaterials materials, byte[] iv,
         } else {
             //= specification/s3-encryption/data-format/metadata-strategy.md#object-metadata
             //# By default, the S3EC MUST store content metadata in the S3 Object Metadata.
+            //= specification/s3-encryption/data-format/content-metadata.md#us-ascii-preferred-string
+            //= type=exception
+            //= reason=It would be a breaking change to introduce this.
+            //# Thus,
+            //# Content Metadata MapKeys SHOULD be restricted to US-ASCII.
             Map<String, String> newMetadata = addMetadataToMap(putObjectRequest.metadata(), materials, iv);
             return putObjectRequest.toBuilder()
               .metadata(newMetadata)
@@ -80,6 +85,11 @@ public CreateMultipartUploadRequest encodeMetadata(EncryptionMaterials materials
             return createMultipartUploadRequest.toBuilder()
                     .metadata(objectMetadata).build();
         } else {
+            //= specification/s3-encryption/data-format/content-metadata.md#us-ascii-preferred-string
+            //= type=exception
+            //= reason=It would be a breaking change to introduce this.
+            //# Thus,
+            //# Content Metadata MapKeys SHOULD be restricted to US-ASCII.
             Map<String, String> newMetadata = addMetadataToMap(createMultipartUploadRequest.metadata(), materials, iv);
             return createMultipartUploadRequest.toBuilder()
                     .metadata(newMetadata)
@@ -159,6 +169,8 @@ private Map<String, String> addMetadataToMapV3(Map<String, String> map, Encrypti
                     jsonWriter.writeFieldName(entry.getKey()).writeValue(entry.getValue());
                 }
                 jsonWriter.writeEndObject();
+                //= specification/s3-encryption/data-format/content-metadata.md#us-ascii-preferred-string
+                //# An implementation MAY support UTF-8.
                 String jsonEncryptionContext = new String(jsonWriter.getBytes(), StandardCharsets.UTF_8);
                 //= specification/s3-encryption/data-format/metadata-strategy.md#v3-instruction-files
                 //# - The V3 message format MUST store the mapkey "x-amz-t" and its value (when present in the content metadata) in the Instruction File.
@@ -169,6 +181,8 @@ private Map<String, String> addMetadataToMapV3(Map<String, String> map, Encrypti
                     jsonWriter.writeFieldName(entry.getKey()).writeValue(entry.getValue());
                 }
                 jsonWriter.writeEndObject();
+                //= specification/s3-encryption/data-format/content-metadata.md#us-ascii-preferred-string
+                //# An implementation MAY support UTF-8.
                 String jsonEncryptionContext = new String(jsonWriter.getBytes(), StandardCharsets.UTF_8);
                 //= specification/s3-encryption/data-format/metadata-strategy.md#v3-instruction-files
                 //# - The V3 message format MUST store the mapkey "x-amz-m" and its value (when present in the content metadata) in the Instruction File.
@@ -220,6 +234,8 @@ private Map<String, String> addMetadataToMapV2(Map<String, String> map, Encrypti
                 }
             }
             jsonWriter.writeEndObject();
+            //= specification/s3-encryption/data-format/content-metadata.md#us-ascii-preferred-string
+            //# An implementation MAY support UTF-8.
             String jsonEncryptionContext = new String(jsonWriter.getBytes(), StandardCharsets.UTF_8);
             metadata.put(MetadataKeyConstants.ENCRYPTED_DATA_KEY_MATDESC_OR_EC, jsonEncryptionContext);
         } catch (JsonWriter.JsonGenerationException e) {
diff --git a/src/main/java/software/amazon/encryption/s3/internal/CryptoFactory.java b/src/main/java/software/amazon/encryption/s3/internal/CryptoFactory.java
@@ -11,13 +11,6 @@
 import java.security.Provider;
 
 public class CryptoFactory {
-    //= specification/s3-encryption/decryption.md#cbc-decryption
-    //# If the cipher object cannot be created as described above,
-    //# Decryption MUST fail.
-    //= specification/s3-encryption/decryption.md#cbc-decryption
-    //= type=exception
-    //# The error SHOULD detail why the cipher could not be initialized
-    //# (such as CBC or PKCS5Padding is not supported by the underlying crypto provider).
     public static Cipher createCipher(String algorithm, Provider provider)
             throws NoSuchPaddingException, NoSuchAlgorithmException {
         // if the user has specified a provider, go with that.
diff --git a/src/main/java/software/amazon/encryption/s3/internal/MetadataKeyConstants.java b/src/main/java/software/amazon/encryption/s3/internal/MetadataKeyConstants.java
@@ -72,9 +72,9 @@ public static boolean isV1Format(Map<String, String> metadata) {
         return metadata.containsKey(CONTENT_IV) &&
                 metadata.containsKey(ENCRYPTED_DATA_KEY_MATDESC_OR_EC) &&
                 metadata.containsKey(ENCRYPTED_DATA_KEY_V1) &&
-               //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
-               //= type=implication
-               //# - Mapkeys exclusive to other format versions MUST NOT be present.
+               ///= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+               ///= type=implication
+               ///# - Mapkeys exclusive to other format versions MUST NOT be present.
                !metadata.containsKey(ENCRYPTED_DATA_KEY_V2) &&
                 !metadata.containsKey(ENCRYPTED_DATA_KEY_V3);
     }
diff --git a/src/test/java/software/amazon/encryption/s3/S3EncryptionClientBuilderValidationTest.java b/src/test/java/software/amazon/encryption/s3/S3EncryptionClientBuilderValidationTest.java
@@ -181,6 +181,10 @@ public void testBuilderWithLegacyAlgorithmFails() throws NoSuchAlgorithmExceptio
         S3EncryptionClientException exception = assertThrows(S3EncryptionClientException.class, () ->
             S3EncryptionClient.builderV4()
                 .aesKey(aesKey)
+                //= specification/s3-encryption/decryption.md#cbc-decryption
+                //= type=test
+                //# If an object is encrypted with ALG_AES_256_CBC_IV16_NO_KDF and [legacy unauthenticated algorithm suites](#legacy-decryption) is NOT enabled,
+                //# the S3EC MUST throw an error which details that client was not configured to decrypt objects with ALG_AES_256_CBC_IV16_NO_KDF.
                 .encryptionAlgorithm(AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF)
                 .build()
         );
diff --git a/src/test/java/software/amazon/encryption/s3/internal/CipherProviderTest.java b/src/test/java/software/amazon/encryption/s3/internal/CipherProviderTest.java
@@ -300,6 +300,21 @@ public void testCreateAndInitCipherALG_AES_256_CBC_IV16_NO_KDF_DecryptionSucceed
         Cipher cipher = CipherProvider.createAndInitCipher(materials, iv, messageId);
 
         assertNotNull(cipher);
+        //= specification/s3-encryption/decryption.md#cbc-decryption
+        //= type=exception
+        //= reason=Well cipher creation failure does throw an error, we do not catch the error and throw a reasonable message
+        //# If the cipher object cannot be created as described above,
+        //# Decryption MUST fail.
+        //= specification/s3-encryption/decryption.md#cbc-decryption
+        //= type=TODO
+        //# The error SHOULD detail why the cipher could not be initialized
+        //# (such as CBC or PKCS5Padding is not supported by the underlying crypto provider).
+        // If we ever refactor so that the cipher creation failure is a modeled error, we should add tests for it.
+
+        //= specification/s3-encryption/decryption.md#cbc-decryption
+        //= type=test
+        //# If an object is encrypted with ALG_AES_256_CBC_IV16_NO_KDF and [legacy unauthenticated algorithm suites](#legacy-decryption) is enabled,
+        //# then the S3EC MUST create a cipher with AES in CBC Mode with PKCS5Padding or PKCS7Padding compatible padding for a 16-byte block cipher (example: for the Java JCE, this is "AES/CBC/PKCS5Padding").
         assertEquals("AES/CBC/PKCS5Padding", cipher.getAlgorithm());
     }
 
diff --git a/src/test/java/software/amazon/encryption/s3/internal/ContentMetadataStrategyTest.java b/src/test/java/software/amazon/encryption/s3/internal/ContentMetadataStrategyTest.java
@@ -427,6 +427,25 @@ public void testMissingKeysV1() {
         assertTrue(exception.getMessage().contains("Content metadata is tampered, required metadata to decrypt the object are missing"));
     }
 
+    @Test
+    public void testMissingKeysV1Colliding() {
+        Map<String, String> metadata = new HashMap<>();
+        metadata.put("x-amz-iv", "dGVzdC1pdi0xMi1i"); // base64 of "test-iv-12-b"
+        metadata.put("x-amz-key", "ZW5jcnlwdGVkLWtleS1kYXRh"); // base64 of "encrypted-key-data"
+        metadata.put("x-amz-matdesc", "{}");
+        metadata.put("x-amz-key-v2", "ZW5jcnlwdGVkLWtleS1kYXRh");
+
+        GetObjectResponse response = GetObjectResponse.builder()
+            .metadata(metadata)
+            .build();
+        //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+        //= type=test
+        //# - If mapkeys exclusive to other (non-V1) format versions is present,the S3EC SHOULD throw an exception.
+        S3EncryptionClientException exception = assertThrows(S3EncryptionClientException.class, () -> decodingStrategy.decode(getObjectRequest, response));
+        System.out.println(exception.getMessage());
+        assertTrue(exception.getMessage().contains("Content metadata is tampered, required metadata combination is illegal."));
+    }
+
     @Test
     public void testExclusiveKeysCollision() {
         Map<String, String> metadata = new HashMap<>();
