MaterialsDescription Support for Keyrings #467

akareddy04 · 2025-06-17T21:04:50Z

Issue #, if available:

Description of changes:
Added configuration of MaterialsDescription for Keyrings

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

…eys for reEncryptInstructionFile feature. I also included simple test cases to verify that the builder for the MaterialsDescription class is working as well as validation checks to ensure that MaterialsDescription is provided in the case that reEncryptInstructionFile is true

… keyrings

kessplas

Either in this PR, or in a follow up, we need to add this to the ContentMetadata such that it gets written to object metadata/instruction file on PutObject and also returned on GetObject. We need to this before implementing the reEncrypt API as it can't work without the MatDesc actually being present in the metadata.

kessplas · 2025-06-18T19:53:19Z

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

+ * This will be useful during re-encryption of instruction file.
+ * The stored Materials Description are immutable once created.
+ */
+


Suggested change

kessplas · 2025-06-18T19:53:32Z

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

+import java.util.HashMap;
+import java.util.Map;
+/**
+ * This class is used to store and manage key-value pairs that describe keyring,specifically for AES and RSA Keyring.


Suggested change

* This class is used to store and manage key-value pairs that describe keyring,specifically for AES and RSA Keyring.

* This class is used to provide key-value pairs that describe key material used with the Keyring, specifically for AES and RSA Keyrings.

kessplas · 2025-06-18T19:54:50Z

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

+    public static Builder builder() {
+        return new Builder();
+    }
+    public Map<String, String> getDescription() {


Suggested change

public Map<String, String> getDescription() {

public Map<String, String> getMaterialsDescription() {

kessplas · 2025-06-18T20:00:37Z

src/main/java/software/amazon/encryption/s3/materials/AesKeyring.java

            _wrappingKey = wrappingKey;
            return builder();
        }
+        public Builder reEncryptInstructionFile(boolean reEncryptInstructionFile) {


Discussed offline: since the "from" keyring can have a null matdesc, we can't enforce this check until the reEncrypt API is called, so we'll need to remove this.

kessplas · 2025-06-18T20:02:47Z

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

+    assertEquals(1, materialsDescription.getDescription().size());
+    try {
+      materialsDescription.getDescription().put("version", "2.0");
+      throw new RuntimeException("Expected UnsupportedOperationException");


Suggested change

throw new RuntimeException("Expected UnsupportedOperationException");

fail("Expected UnsupportedOperationException");

kessplas · 2025-06-18T20:12:00Z

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

+ * The stored Materials Description are immutable once created.
+ */
+
+public class MaterialsDescription {


We can have this be a Map<String, String> extension, meaning:

implements Map<String, String>

We'll need to implement a bunch of other methods, but they can all be deferred to the "inner" map pretty easily. This is a nicer customer experience as customers can use all of the usual Map API and aren't limited to the things we chose to implement.

…RSA Keyrings will extend from and I also moved the warnIfEncryptionContextIsPresent() method from S3Keyring to RawKeyring since the method really only targets AES + RSA keyrings which will extend this class

…extending the abstract builder from RawKeyring

…bstract builder from the RawKeyring

… transferred it over to RawKeyring class which AES + RSA extend from (note: this method is only for AES + RSA Keyrings)

…Keyring

…ty between sub-classes AES + RSA Keyrings

…ill already be inherited from RawKeyring class

…nt MaterialsDescription (note: MaterialsDescription and Encryption Context are stored in the same field, but you will never have both at the same time (one of them will always be empty

…context and materials description will be treated as different

…DataKeyContext to _encryptionContextOrMatDesc bc in V2 both encryption context and materials description were treated the same (backwards compatability)

…tion Materials in putObject by first creating a helper method in the RawKeyring class that both AES + RSA keyrings will call in the override modifyMaterials method to return materials description instead of empty encryption materials

…ject's metadata correctly

…account null case

…extOrMatDesc

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

kessplas · 2025-06-23T17:20:50Z

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

            jsonWriter.writeStartObject();
-            for (Map.Entry<String, String> entry : materials.encryptionContext().entrySet()) {
-                jsonWriter.writeFieldName(entry.getKey()).writeValue(entry.getValue());
+            if (!materials.encryptionContext().isEmpty() && materials.materialsDescription().isEmpty()) {


We should add a test which passes both a MatDesc on keyring initialization and an EC on PutObject for AES and RSA keyrings. While this is not a good idea (the EC will be dropped and a warning will be logged) we should add a test to make sure that we store the MatDesc and not the EC.

src/main/java/software/amazon/encryption/s3/materials/EncryptionMaterials.java

src/main/java/software/amazon/encryption/s3/materials/MaterialsDescription.java

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

kessplas · 2025-06-23T19:58:59Z

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

+    AesKeyring aesKeyring = AesKeyring.builder()
+      .wrappingKey(AES_KEY)
+      .reEncryptInstructionFile(true)
+      .secureRandom(new SecureRandom())


nit: this is superfluous

kessplas · 2025-06-23T19:59:26Z

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

+      .keyring(aesKeyring)
+      .build();
+    final String input = "Testing Materials Description in Object Metadata!";
+    final String objectKey = "test-aes-materials-description-in-object-metadata";


Suggested change

final String objectKey = "test-aes-materials-description-in-object-metadata";

final String objectKey = appendTestSuffix("test-aes-materials-description-in-object-metadata");

kessplas · 2025-06-23T20:00:54Z

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

+      .key(objectKey)
+      .build());
+    assertEquals(input, responseBytes.asUtf8String());
+    assertEquals("{\"admin\":\"yes\",\"version\":\"1.0\"}", responseBytes.response().metadata().get("x-amz-matdesc"));


This is fine for one key, but I'd prefer using a parser here, as I'm not sure if the order is guaranteed.

kessplas · 2025-06-23T20:01:26Z

src/test/java/software/amazon/encryption/s3/materials/MaterialsDescriptionTest.java

+      .build();
+
+    final String input = "Testing Materials Description in Instruction File!";
+    final String objectKey = "test-rsa-materials-description-in-instruction-file";


ditto appendTestSuffix (here and everywhere else)

…ionality

kessplas

See comment about Encryption Context

kessplas · 2025-06-24T23:15:04Z

src/test/java/software/amazon/encryption/s3/S3EncryptionClientMatDescTest.java

+      .serverSideEncryption(ServerSideEncryption.AWS_KMS)
+      .ssekmsEncryptionContext(Base64.getEncoder().encodeToString(encryptionContext.getBytes(StandardCharsets.UTF_8)))


This is not how EncryptionContext is passed to S3EC.

Refer to e.g. KmsContextV2toV3 in the compatibility tests for an example of S3EC EncryptionContext.

kessplas · 2025-06-24T23:15:19Z

src/test/java/software/amazon/encryption/s3/S3EncryptionClientMatDescTest.java

+      .serverSideEncryption(ServerSideEncryption.AWS_KMS)
+      .ssekmsEncryptionContext(Base64.getEncoder().encodeToString(encryptionContext.getBytes(StandardCharsets.UTF_8)))


kessplas · 2025-06-24T23:15:58Z

src/test/java/software/amazon/encryption/s3/S3EncryptionClientMatDescTest.java

+  }
+
+  @Test
+  public void testMaterialsDescriptionRsaKeyring() {


This test and the one above it should be moved to the unit test class (MaterialsDescriptionTest)

kessplas

LGTM!

Anirav Kareddy added 4 commits June 17, 2025 11:24

Added S3Keyring class

8e438d3

Added Materials Description class

c11f832

Added class description for MaterialsDescription

1e2553a

akareddy04 requested a review from a team as a code owner June 17, 2025 21:04

added _reEncryptInstructionFile boolean field to builder in AES + RSA…

575627e

… keyrings

kessplas requested changes Jun 18, 2025

View reviewed changes

Anirav Kareddy added 23 commits June 18, 2025 14:41

Fixed changes from first PR review

239d286

removed unused import

f182706

removed unused import

3bd8556

Created an abstract class with an abstract builder method that AES + …

20ed743

…RSA Keyrings will extend from and I also moved the warnIfEncryptionContextIsPresent() method from S3Keyring to RawKeyring since the method really only targets AES + RSA keyrings which will extend this class

AESKeyring class is now extending RawKeyring and the builder is also …

18f21e0

…extending the abstract builder from RawKeyring

RsaKeyring class is now extending RawKeyring and also extending the a…

dfa7b4f

…bstract builder from the RawKeyring

removed warnIfEncryptionContextIsPresent() method from this class and…

cd3ac2c

… transferred it over to RawKeyring class which AES + RSA extend from (note: this method is only for AES + RSA Keyrings)

removed getMaterialsDescription() since it will be inherited from Raw…

3ae4b9c

…Keyring

removed things that aren't neccessary due to inheritance

7db7d6b

added modifyMaterials() method to RawKeyring since common functionali…

0bd11ab

…ty between sub-classes AES + RSA Keyrings

removed the modifyMaterials method from AES + RSA Keyrings since it w…

20a8364

…ill already be inherited from RawKeyring class

re-named getter functions

cfe9e33

Modified the ContentMetadataEncodingStrategy class to take into accou…

674053d

…nt MaterialsDescription (note: MaterialsDescription and Encryption Context are stored in the same field, but you will never have both at the same time (one of them will always be empty

Added MaterialsDescription to Encryption Materials (note: encryption …

81215b1

…context and materials description will be treated as different

removed materials description from this class and renamed _encryption…

edc1cac

…DataKeyContext to _encryptionContextOrMatDesc bc in V2 both encryption context and materials description were treated the same (backwards compatability)

rennamed variables + moving things around

3ee7571

Added test cases for testing if material description was placed in ob…

97ff17a

…ject's metadata correctly

modified the materialsDescription builder setter method to take into …

241889d

…account null case

modified test cases to now include the new field name: encryptionCont…

db3b701

…extOrMatDesc

added test cases for materials description in instruction file

09c734d

fixed naming of variable

f62afd6

added class-level definition for abstract class RawKeyring

5847f8d

removed unused imports

c4fc099

kessplas requested changes Jun 23, 2025

View reviewed changes

Anirav Kareddy added 2 commits June 24, 2025 13:16

Fixed second round of PR comments

ae7e20d

modified object_key + input values to match description of test funct…

5b0cbce

…ionality

kessplas requested changes Jun 24, 2025

View reviewed changes

correctly using encryption context via overrideConfiguration

5cd05db

kessplas approved these changes Jun 25, 2025

View reviewed changes

akareddy04 merged commit b643bc8 into aniravk/ReEncrypt-feature Jun 25, 2025
30 of 35 checks passed

	* This class is used to store and manage key-value pairs that describe keyring,specifically for AES and RSA Keyring.
	* This class is used to provide key-value pairs that describe key material used with the Keyring, specifically for AES and RSA Keyrings.

	public Map<String, String> getDescription() {
	public Map<String, String> getMaterialsDescription() {

	throw new RuntimeException("Expected UnsupportedOperationException");
	fail("Expected UnsupportedOperationException");

	final String objectKey = "test-aes-materials-description-in-object-metadata";
	final String objectKey = appendTestSuffix("test-aes-materials-description-in-object-metadata");

		.serverSideEncryption(ServerSideEncryption.AWS_KMS)
		.ssekmsEncryptionContext(Base64.getEncoder().encodeToString(encryptionContext.getBytes(StandardCharsets.UTF_8)))

MaterialsDescription Support for Keyrings #467

MaterialsDescription Support for Keyrings #467

Uh oh!

Conversation

akareddy04 commented Jun 17, 2025

Check any applicable:

Uh oh!

kessplas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kessplas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kessplas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants