-
Notifications
You must be signed in to change notification settings - Fork 17
feat: reEncryptInstructionFile Implementation #475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* I created the MaterialsDescription class to help identify AES + RSA keys for reEncryptInstructionFile feature Co-authored-by: Anirav Kareddy <[email protected]>
--------- Co-authored-by: Anirav Kareddy <[email protected]>
Implemented a stronger level of validation for reEncryptInstructionFile where, when enabled, the client will attempt to decrypt the re-encrypted instruction file with the old key material and throw an exception when decryption succeeds. This is a stronger level of validation that the wrapping key has been rotated than the standard assertion that the materials descriptions are different. --------- Co-authored-by: Anirav Kareddy <[email protected]>
if (newKeyring == null) { | ||
throw new S3EncryptionClientException("New keyring must be provided!"); | ||
} | ||
if (newKeyring instanceof AesKeyring) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change this to check !(newKeyring instanceof RsaKeyring)
and the error message to say "is only applicable for RSA keyring!" so the validation is more robust and fails closed for new raw keyrings.
} | ||
|
||
//Create or update instruction file with the re-encrypted metadata while preserving IV | ||
ContentMetadataEncodingStrategy encodeStrategy = new ContentMetadataEncodingStrategy(_instructionFileConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to validate the instruction file config here to make sure that Inst file is enabled, otherwise the new key will get lost.
First, can you write a test which reproduces this bug?
Then, can you add validation to make sure that isInstructionFilePutEnabled()
is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This reverts commit ff66e72.
## [3.4.0](v3.3.5...v3.4.0) (2025-07-30) ### Features * put object with instruction file configured ([#466](#466)) ([99077dc](99077dc)) * reEncryptInstructionFile Implementation ([#475](#475)) ([ff66e72](ff66e72)) * reEncryptInstructionFile Implementation ([#478](#478)) ([f7e6fa5](f7e6fa5)) ### Fixes * Revert "feat: reEncryptInstructionFile Implementation ([#475](#475))" ([#477](#477)) ([6d45ec5](6d45ec5)) ### Maintenance * guard against properties conflicts ([#479](#479)) ([793c73b](793c73b)) * **pom:** fix scm url ([#469](#469)) ([1bc2ca3](1bc2ca3)) * **release:** Migrate release to Central Portal ([#468](#468)) ([da71231](da71231)) * validate against legacy wrapping on client but customer passes keyring with no legacy wrapping ([#473](#473)) ([bb898d1](bb898d1))
Issue #, if available:
Description of changes:
Implemented the following feature: re-encrypting an instruction file with a new keyring while preserving the original encrypted object in S3. This enables:
1. Key rotation by updating instruction file metadata without re-encrypting object content
2. Sharing encrypted objects with partners by creating new instruction files with a custom suffix using their public keys
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: