chore(duvet): modernize duvet

.duvet/.gitignore

@@ -0,0 +1,3 @@
+reports/
+requirements/
+specification/

.duvet/config.toml

@@ -0,0 +1,27 @@
+'$schema' = "https://awslabs.github.io/duvet/config/v0.4.0.json"
+
+[[source]]
+pattern = "src/**/*.java"
+
+# Include required specifications here
+[[specification]]
+source = "specification/s3-encryption/client.md"
+[[specification]]
+source = "specification/s3-encryption/decryption.md"
+[[specification]]
+source = "specification/s3-encryption/encryption.md"
+[[specification]]
+source = "specification/s3-encryption/key-commitment.md"
+[[specification]]
+source = "specification/s3-encryption/key-derivation.md"
+[[specification]]
+source = "specification/s3-encryption/data-format/content-metadata.md"
+[[specification]]
+source = "specification/s3-encryption/data-format/metadata-strategy.md"
+
+[report.html]
+enabled = true
+
+# Enable snapshots to prevent requirement coverage regressions
+[report.snapshot]
+enabled = false

.github/workflows/duvet.yml

@@ -0,0 +1,44 @@
+name: duvet
+
+on:
+  workflow_call:
+    # Optional inputs that can be provided when calling this workflow
+
+jobs:
+  test:
+    runs-on: macos-latest
+    permissions:
+      id-token: write
+      contents: read
+      pages: write
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v5
+        with:
+          submodules: true
+
+      - name: Setup Rust toolchain
+        uses: actions-rust-lang/setup-rust-toolchain@v1
+        with:
+          toolchain: stable
+
+      - name: Clone duvet repository  
+        run: git clone https://github.com/awslabs/duvet.git /tmp/duvet
+
+      - name: Build and install duvet
+        run: |
+          cd /tmp/duvet
+          cargo xtask build
+          cargo install --path ./duvet
+
+      - name: Run duvet
+        run: make duvet
+
+      - name: Upload duvet reports
+        uses: actions/upload-artifact@v4
+        with:
+          name: reports
+          include-hidden-files: true
+          path: .duvet/reports/report.html
+

.gitmodules

@@ -1,3 +1,4 @@
-[submodule "specification"]
+[submodule "private_aws"]
 	path = specification
-	url = git@github.com:awslabs/aws-encryption-sdk-specification.git
+	url = https://github.com/awslabs/aws-encryption-sdk-specification.git
+	branch = tonyknap/s3ec-v3.0.1-candidate

CONTEXT_FOR_DUVET.md

@@ -0,0 +1,56 @@
+# Duvet Annotation Context
+
+## Overview
+Working through incomplete Duvet requirements one at a time to add citations and tests to the codebase.
+
+## Key Insights
+- **Duvet syntax**: Only comments with exactly two slashes (`//`) are read by Duvet
+  - `//=` for specification links
+  - `//#` for requirement descriptions
+  - `///` or `////` are NOT read by Duvet (effectively commented out)
+- **Annotation types**: `type=implication`, `type=exception`, `type=test`
+- **Report location**: `.duvet/reports/report.html`
+- **Total requirements**: 198 (178 complete, 20 incomplete as of last run)
+
+## Completed Requirements
+
+### 1. V1 Format Exclusive Keys
+**Requirement**: "When the object is encrypted using the V1 format, - Mapkeys exclusive to other format versions MUST NOT be present."
+
+**Citation added**: 
+- `MetadataKeyConstants.isV1Format()` at line 73
+- `ContentMetadataDecodingStrategy.isV1InObjectMetadata()` at line 424
+
+**Test**: Already tested via `ContentMetadataStrategyTest.testExclusiveKeysCollision`
+
+### 2. V2 Format Tag Length Requirements
+**Requirements**: 
+- "If the object is encrypted using AES-GCM for content encryption, then the mapkey 'x-amz-tag-len' MUST be present."
+- "If the object is encrypted using AES-CBC for content encryption, then the mapkey 'x-amz-tag-len' MUST NOT be present."
+
+**Bug fixed**: Code was incorrectly writing tag length for CBC (should only be for GCM)
+
+**Citation added**: `ContentMetadataEncodingStrategy.addMetadataToMap()` at line 190
+
+**Implementation**: Check `cipherName().contains("GCM")` to determine whether to write tag length
+
+**Test**: Existing tests validate tag length is written for GCM and read correctly
+
+## Commit History
+- `9068c876`: fix(CBC): Add annotations for V1/V2 format requirements and fix CBC tag length bug
+
+## Files Modified
+- `src/main/java/software/amazon/encryption/s3/internal/MetadataKeyConstants.java`
+- `src/main/java/software/amazon/encryption/s3/internal/ContentMetadataEncodingStrategy.java`
+- `src/main/java/software/amazon/encryption/s3/internal/ContentMetadataDecodingStrategy.java`
+
+## Process for Next Requirements
+1. User provides incomplete requirement text
+2. Find where requirement is implemented in code
+3. Add Duvet annotation at implementation location
+4. Verify test coverage exists
+5. Run tests: `mvn clean compile` then `mvn test -Dtest=ContentMetadataStrategyTest,MetadataKeyConstantsTest,CipherProviderTest,AlgorithmSuiteValidationTest`
+6. Stage and commit changes
+
+## Remaining Work
+18 incomplete requirements remaining (as of last count)

Makefile

@@ -1,15 +1,12 @@
 # Used for misc supporting functions like Duvet and prettier. Builds, tests, etc. should use the usual Java/Maven tooling.
 
-duvet: | duvet_extract duvet_report
-
-duvet_extract:
-	rm -rf compliance
-	$(foreach file, $(shell find specification/s3-encryption -name '*.md'), duvet extract -o compliance -f MARKDOWN $(file);)
+duvet: | duvet_clean duvet_report
 
 duvet_report:
-	duvet \
-		report \
-		--spec-pattern "compliance/**/*.toml" \
-		--source-pattern "src/**/*.java" \
-		--source-pattern "compliance_exceptions/*.txt" \
-		--html specification_compliance_report.html
+	duvet report
+
+duvet-view-report-mac:
+	open .duvet/reports/report.html
+
+duvet_clean:
+	rm -rf .duvet/reports/ .duvet/requirements/

src/main/java/software/amazon/encryption/s3/internal/CipherProvider.java

@@ -193,6 +193,17 @@ public static Cipher createAndInitCipher(final CryptographicMaterials materials,
                     if (materials.cipherMode().opMode() == Cipher.ENCRYPT_MODE) {
                         throw new S3EncryptionClientException("Encryption is not supported for algorithm: " + materials.algorithmSuite().cipherName());
                     }
+                    //= 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 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:

src/main/java/software/amazon/encryption/s3/internal/ContentMetadataDecodingStrategy.java

@@ -92,6 +92,8 @@ private static ContentMetadata readFromV3FormatMap(Map<String, String> metadata,
         if (!MetadataKeyConstants.isV3Format(metadata)) {
             //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
             //# In general, if there is any deviation from the above format, with the exception of additional unrelated mapkeys, then the S3EC SHOULD throw an exception.
+            //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+            //# - If a mapkey exclusive to other (non-V3) format versions is present, the S3EC SHOULD throw an exception.
             throw new S3EncryptionClientException("Content metadata is tampered, required metadata to decrypt the object are missing");
         }
 
@@ -227,9 +229,6 @@ private static ContentMetadata readFromMapV1V2(Map<String, String> metadata, Get
         //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
         //= type=exception
         //# - The mapkey "x-amz-unencrypted-content-length" SHOULD be present for V1 format objects.
-        //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
-
-        //# - The mapkey "x-amz-cek-alg" MUST be present for V2 format objects.
         if (contentEncryptionAlgorithm == null
                 || contentEncryptionAlgorithm.equals(AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF.cipherName())) {
             algorithmSuite = AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF;
@@ -257,14 +256,27 @@ private static ContentMetadata readFromMapV1V2(Map<String, String> metadata, Get
                 //= specification/s3-encryption/data-format/content-metadata.md#algorithm-suite-and-message-format-version-compatibility
                 //# Objects encrypted with ALG_AES_256_CBC_IV16_NO_KDF MAY use either the V1 or V2 message format version.
                 if (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)) {
+                    //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+                    //# - If mapkeys exclusive to other (non-V1) format versions is present,the S3EC SHOULD throw an exception.
+                    if (isV2InObjectMetadata(metadata) || isV3InObjectMetadata(metadata)) {
+                        throw new S3EncryptionClientException("Object metadata is tampered, conflicting keys are present.");
+                    }
                     //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
                     //# - The mapkey "x-amz-key" MUST be present for V1 format objects.
                     edkCiphertext = DECODER.decode(metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1));
                 } else if (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2)) {
+                    //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+                    //# - If a mapkey exclusive to other (non-V2) format versions is present, the S3EC SHOULD throw an exception.
+                    if (isV1InObjectMetadata(metadata) || isV3InObjectMetadata(metadata)) {
+                        throw new S3EncryptionClientException("Object metadata is tampered, conflicting keys are present.");
+                    }
                     //= specification/s3-encryption/data-format/content-metadata.md#algorithm-suite-and-message-format-version-compatibility
                     //# Objects encrypted with ALG_AES_256_CBC_IV16_NO_KDF MAY use either the V1 or V2 message format version.
                     //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
                     //# - The mapkey "x-amz-key-v2" MUST be present for V2 format objects.
+                    //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
+                    //= type=exception
+                    //# - The mapkey "x-amz-unencrypted-content-length" SHOULD be present for V2 format objects.
                     edkCiphertext = DECODER.decode(metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2));
                 } else {
                     // this shouldn't happen under normal circumstances- only if out-of-band modification
@@ -303,8 +315,6 @@ private static ContentMetadata readFromMapV1V2(Map<String, String> metadata, Get
                 break;
             case ALG_AES_256_GCM_IV12_TAG16_NO_KDF:
             case ALG_AES_256_CTR_IV16_TAG16_NO_KDF:
-                //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
-                //# - The mapkey "x-amz-tag-len" MUST be present for V2 format objects.
                 final int tagLength = Integer.parseInt(metadata.get(MetadataKeyConstants.CONTENT_CIPHER_TAG_LENGTH));
                 if (tagLength != algorithmSuite.cipherTagLengthBits()) {
                     throw new S3EncryptionClientException("Expected tag length (bits) of: "
@@ -419,17 +429,27 @@ public Map<String, String> loadInstructionFileMetadata(GetObjectRequest request)
     }
 
     /**
-     * Determines if V1/V2 format is present in object metadata.
-     * All V1/V2 keys must be present in object metadata.
+     * Determines if V1 format is present in object metadata.
      */
-    public static boolean isV1V2InObjectMetadata(Map<String, String> objectMetadata) {
+    public static boolean isV1InObjectMetadata(Map<String, String> objectMetadata) {
         //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-        //# - If the metadata contains "x-amz-iv" and "x-amz-key" then the object MUST be considered as an S3EC-encrypted object using the V1 format.
+        //# - If the metadata contains "x-amz-iv" and "x-amz-key" but no other version exclusive keys then the object MUST be considered as an S3EC-encrypted object using the V1 format.
+        return objectMetadata.containsKey(MetadataKeyConstants.CONTENT_IV)
+                && objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)
+               && !objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2)
+               && !objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V3);
+    }
+
+    /**
+     * Determines if V2 format is present in object metadata.
+     */
+    public static boolean isV2InObjectMetadata(Map<String, String> objectMetadata) {
         //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-        //# - If the metadata contains "x-amz-iv" and "x-amz-metadata-x-amz-key-v2" then the object MUST be considered as an S3EC-encrypted object using the V2 format.
+        //# - If the metadata contains "x-amz-iv" and "x-amz-key-v2" but no other version exclusive keys then the object MUST be considered as an S3EC-encrypted object using the V2 format.
         return objectMetadata.containsKey(MetadataKeyConstants.CONTENT_IV)
-                && (objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)
-                || objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2));
+                && objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2)
+                && !objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)
+                && !objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V3);
     }
 
     /**
@@ -438,7 +458,7 @@ public static boolean isV1V2InObjectMetadata(Map<String, String> objectMetadata)
      */
     public static boolean isV3InObjectMetadata(Map<String, String> objectMetadata) {
         //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-        //# - If the metadata contains "x-amz-3" and "x-amz-d" and "x-amz-i" then the object MUST be considered an S3EC-encrypted object using the V3 format.
+        //# - If the metadata contains "x-amz-3" and "x-amz-d" and "x-amz-i" but no other version exclusive keys then the object MUST be considered an S3EC-encrypted object using the V3 format.
         return objectMetadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V3)
                 && objectMetadata.containsKey(MetadataKeyConstants.KEY_COMMITMENT_V3)
                 && objectMetadata.containsKey(MetadataKeyConstants.MESSAGE_ID_V3)
@@ -511,24 +531,25 @@ public ContentMetadata decodeV3FromInstructionFile(GetObjectRequest request, Get
 
     public ContentMetadata decode(GetObjectRequest request, GetObjectResponse response) {
         Map<String, String> objectMetadata = response.metadata();
-
-        //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-        //= type=exception
-        //# If there are multiple mapkeys which are meant to be exclusive, such as "x-amz-key", "x-amz-key-v2", and "x-amz-3" then the S3EC SHOULD throw an exception.
-
         if (objectMetadata != null) {
+            //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
+            //# If there are multiple mapkeys which are meant to be exclusive to different versions, such as "x-amz-key", "x-amz-key-v2", and "x-amz-3" then the S3EC SHOULD throw an exception.
+            if (MetadataKeyConstants.hasExclusiveKeyCollision(objectMetadata)) {
+                throw new S3EncryptionClientException("Content metadata is tampered, required metadata combination is illegal.");
+            }
+
             // V1/V2 in Object Metadata - All V1/V2 keys present in object metadata
             //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-            //# - If the metadata contains "x-amz-iv" and "x-amz-key" then the object MUST be considered as an S3EC-encrypted object using the V1 format.
+            //# - If the metadata contains "x-amz-iv" and "x-amz-key" but no other version exclusive keys then the object MUST be considered as an S3EC-encrypted object using the V1 format.
             //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-            //# - If the metadata contains "x-amz-iv" and "x-amz-metadata-x-amz-key-v2" then the object MUST be considered as an S3EC-encrypted object using the V2 format.
-            if (isV1V2InObjectMetadata(objectMetadata)) {
+            //# - If the metadata contains "x-amz-iv" and "x-amz-key-v2" but no other version exclusive keys then the object MUST be considered as an S3EC-encrypted object using the V2 format.
+            if (isV1InObjectMetadata(objectMetadata) || isV2InObjectMetadata(objectMetadata)) {
                 return readFromMapV1V2(objectMetadata, response);
             }
 
             // V3 in Object Metadata - c/d/i always in object metadata, x-amz-3 also in object metadata
             //= specification/s3-encryption/data-format/content-metadata.md#determining-s3ec-object-status
-            //# - If the metadata contains "x-amz-3" and "x-amz-d" and "x-amz-i" then the object MUST be considered an S3EC-encrypted object using the V3 format.
+            //# - If the metadata contains "x-amz-3" and "x-amz-d" and "x-amz-i" but no other version exclusive keys then the object MUST be considered an S3EC-encrypted object using the V3 format.
             //= specification/s3-encryption/data-format/content-metadata.md#content-metadata-mapkeys
             //# In the V3 format, the mapkeys "x-amz-c", "x-amz-d", and "x-amz-i" MUST be stored exclusively in the Object Metadata.
             else if (isV3InObjectMetadata(objectMetadata)) {