Skip to content

Reject malformed HKDF Salt and Info with XMLEncryptionException - #653

Open
Arpan0995 wants to merge 1 commit into
apache:mainfrom
Arpan0995:hkdf-salt-info-base64-guard
Open

Reject malformed HKDF Salt and Info with XMLEncryptionException#653
Arpan0995 wants to merge 1 commit into
apache:mainfrom
Arpan0995:hkdf-salt-info-base64-guard

Conversation

@Arpan0995

Copy link
Copy Markdown

On the decrypt path of an ECDH-ES (or X25519/X448) key agreement, the HKDF Salt and Info elements of the KeyDerivationMethod are read from the message and base64-decoded in XMLCipherUtil#constructKeyDerivationParameter. Malformed base64 in either element throws the IllegalArgumentException from Base64.Decoder, which escapes the XMLEncryptionException that XMLCipher#decryptKey declares. A caller handling the declared exception type therefore does not catch it, and on a service decrypting untrusted XML the malformed input surfaces as an uncaught error rather than a clean rejection. Reproduced on main with a secp256r1 ECDH-ES round trip and a mutated Salt or Info; the only pre-condition is a message the recipient is willing to decrypt, and the failure happens before the key agreement is attempted.

The change. Wrap the two decodes in a small helper that reports the failure as XMLEncryptionException using the existing KeyDerivation.InvalidParameter message (Key derivation parameter {0} is illegal), naming the offending element. The same strict Base64.getDecoder() is used, so well-formed input is unaffected.

Test. XMLCipherKeyAgreementMalformedHKDFParamsTest: a positive control (the round trip decrypts with well-formed parameters) and a parameterized case for a malformed Salt and a malformed Info, each asserting XMLEncryptionException from the full decrypt path. It runs on the default profile with the JDK's EC provider, no BouncyCastle needed. I confirmed the test is not vacuous by running it against the unchanged code: the positive control passes and both malformed cases fail with the raw IllegalArgumentException: Illegal base64 character.

Verification. mvn test over the new class and XMLCipherTest (which covers the ECDH-ES, X25519 and X448 key-agreement round trips): 40 executions, no regressions.

For context, I found this while looking at the pending ML-KEM key-transport work (#652), which reaches the same code through a new path; the ECDH-ES path is affected on main today, so it seemed right to fix it here independently. The same guard is included in that branch via ffang#6 and can be dropped there on rebase once this lands.

On the decrypt path of an ECDH-ES (or X25519/X448) key agreement, the
HKDF Salt and Info elements of the KeyDerivationMethod are read from the
message and base64-decoded in XMLCipherUtil.constructKeyDerivationParameter.
Malformed base64 there threw the IllegalArgumentException from
Base64.Decoder, escaping the XMLEncryptionException the decrypt API
declares, so a caller handling the declared type would not catch it.

Wrap the decode and report it as XMLEncryptionException using the existing
KeyDerivation.InvalidParameter message. The same strict decoder is used, so
well-formed input is unaffected. Adds a test with a positive control and
the two malformed cases; both fail without this change.
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.

1 participant