Repository navigation
Conversation
…crypt The KMS+Context and RSA-OAEP decrypt paths selected the content algorithm suite from the object's content metadata without checking it against the algorithm this data key was bound to. A mismatch (for example a GCM-sealed object whose content-algorithm metadata reads CBC) was accepted, so the object could be processed under a different suite than it was written with. KmsKeyring now rejects a stored aws:x-amz-cek-alg that does not match the selected suite before calling KMS Decrypt. RsaKeyring now accepts only GCM-family selected suites for the RSA-OAEP pseudo-data-key binding. Both preserve the GCM->CTR equivalence used by ranged gets.
RSA-OAEP data keys record AES/GCM/NoPadding for every non-committing suite, including CBC objects rewrapped by reEncryptInstructionFile, so the wrapped value cannot identify the content algorithm. Revert the RsaKeyring check and its tests; keep the KMS+Context check, where the stored aws:x-amz-cek-alg records the actual content algorithm.
kessplas
changed the title
| if (storedContentAlgorithm != null | ||
| && !storedContentAlgorithm.equals(expectedContentAlgorithm(materials.algorithmSuite()))) { | ||
| throw new S3EncryptionClientException("The content encryption algorithm in the KMS-authenticated " | ||
| + "encryption context does not match the algorithm suite selected for decryption."); |
There was a problem hiding this comment.
We should include both values (content encryption algorithm and selected algorithm suite for decryption) in the error message.
| throw new S3EncryptionClientException("Provided encryption context does not match information retrieved from S3"); | ||
| } | ||
|
|
||
| // The content encryption algorithm is part of the KMS-authenticated encryption context, while the |
There was a problem hiding this comment.
I feel like we can simplify this comment, possibly something along the lines of "The stored content-encryption algorithm (KMS-authenticated) MUST match the algorithm suite selected from content metadata (unauthenticated).
| return decryptDataKeyStrategies; | ||
| } | ||
|
|
||
| // The canonical value the content-encryption algorithm (aws:x-amz-cek-alg) holds in the stored encryption |
There was a problem hiding this comment.
Can we simplify this to "Return content-encryption algorithm for a given AlgorithmSuite"
| // context for a given selected algorithm suite. The committing GCM/CTR pair share the committing suite ID, | ||
| // and the non-committing GCM/CTR pair share GCM's cipher name, because a ranged get selects the CTR variant | ||
| // of an object that was sealed under its GCM counterpart. | ||
| private static String expectedContentAlgorithm(AlgorithmSuite algorithmSuite) { |
There was a problem hiding this comment.
Should we remove this private helper function and instead add contentAlgorithm (String) as a field in the AlgorithmSuite?
|
Fix looks right for |
Issue #, if available: n/a
Description of changes:
The KMS+Context and RSA-OAEP decrypt paths selected the content algorithm suite from the object's content metadata without checking it against the algorithm this data key was bound to.
KmsKeyring now rejects a stored aws:x-amz-cek-alg that does not match the selected suite before calling KMS Decrypt. RsaKeyring now accepts only GCM-family selected suites for the RSA-OAEP pseudo-data-key binding. Both preserve the GCM->CTR equivalence used by ranged gets.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: