Skip to content

fix(kms-keyring): validate content algorithm matches selected suite on decrypt - #527

Merged
kessplas merged 4 commits into
mainfrom
kessplas/bind-content-algorithm-to-suite
Nov 2, 2026
Merged

kessplas merged 4 commits into
mainfrom
kessplas/bind-content-algorithm-to-suite

Conversation

@kessplas

Copy link
Copy Markdown
Contributor

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:

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

…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 kessplas changed the title fix(keyring): validate content algorithm matches selected suite on decrypt fix(kms-keyring): validate content algorithm matches selected suite on decrypt Oct 26, 2026
@kessplas
kessplas marked this pull request as ready for review October 26, 2026 22:53
@kessplas
kessplas requested a review from a team as a code owner October 26, 2026 22:53
@sharmabikram

sharmabikram commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Fix looks right for KmsKeyring, but KmsDiscoveryKeyring's kms+context strategy has the same decrypt path (strips aws:x-amz-cek-alg, never compares it to the selected suite), so the downgrade still works with a discovery keyring. I reproduced it with the same mock setup as kmsContextDecryptFailsWhenAuthenticatedCekAlgDoesNotMatchSelectedSuite. Suggest hoisting the check + expectedContentAlgorithm somewhere both keyrings share.

Move the Kms+Context content algorithm check into KmsContentAlgorithm and
call it from both KmsKeyring and KmsDiscoveryKeyring. Include the stored
content algorithm and the selected suite in the error message.

@sharmabikram sharmabikram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@akareddy04

Copy link
Copy Markdown
Contributor

LGTM

@kessplas
kessplas merged commit 96b1c2d into main Nov 2, 2026
35 of 36 checks passed
@kessplas
kessplas deleted the kessplas/bind-content-algorithm-to-suite branch November 2, 2026 21:07
aws-crypto-tools-ci-bot pushed a commit that referenced this pull request Nov 4, 2026
## [4.0.4](v4.0.3...v4.0.4) (2026-11-04)

### Fixes

* **kms-keyring:** validate content algorithm matches selected suite on decrypt ([#527](#527)) ([96b1c2d](96b1c2d))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants