Emit dsig11:DEREncodedKeyValue for keys without a structured KeyValue form - #4
Merged
Merged
Conversation
… form StAX signing with the KeyValue key identifier dispatched on the public key algorithm with branches for RSA, DSA and EC only. For any other key type (ML-DSA, EdDSA) it fell through and emitted an empty <dsig:KeyValue/>, which the inbound processor then rejects at schema validation before reaching signature verification. Emit a dsig11:DEREncodedKeyValue holding the DER SubjectPublicKeyInfo for those key types, which is schema-valid inside dsig:KeyValue via its ##other wildcard and mirrors the DOM DEREncodedKeyValue support. Adds a parameterized test across ML-DSA-44/65/87 asserting the KeyValue carries a DEREncodedKeyValue that round-trips to the signer's public key and verifies the signature. Inbound extraction of a public key from DEREncodedKeyValue is not added here; the StAX inbound path has no DEREncodedKeyValue support for any key type yet.
ffang
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed on apache#651: this fixes the empty-
KeyValueissue for ML-DSA StAX signing, taking theDEREncodedKeyValueroute you preferred.The problem.
XMLSecurityUtils#createKeyValueTokenStructuredispatched on the public key algorithm with branches for RSA, DSA and EC only. For an ML-DSA key it fell through and emitted an empty<dsig:KeyValue/>, which is not schema-valid and which the StAX inbound processor rejects at schema validation (cvc-complex-type.2.4.b) before signature verification is ever reached.The fix. For key types with no structured KeyValue form, emit a
dsig11:DEREncodedKeyValueholding the DER SubjectPublicKeyInfo (PublicKey.getEncoded()). That is schema-valid insidedsig:KeyValuethrough the same##otherwildcard the existingdsig11:ECKeyValuebranch relies on, and it mirrors the DOM-sideDEREncodedKeyValuesupport this PR already extends for ML-DSA. A key whose encoding is unavailable is rejected with a clearXMLSecurityExceptionrather than an empty element.Test. Adds
StaxMLDSAKeyValueTest, parameterized across ML-DSA-44/65/87, asserting theKeyValueis non-empty and carries aDEREncodedKeyValue, that its DER decodes back to the signer's public key, and that the recovered key verifies the signature. Verified withmvn test -P bouncycastle(3/3); without the profile it compiles and skips via the existingassumeTrueguard.Scope. This corrects the malformed output so it is schema-valid and the embedded key is recoverable and usable. Full StAX-inbound round-trip additionally needs the inbound path to extract a public key from a
DEREncodedKeyValue, which it does not do for any key type today; with this change the inbound path now gets past schema validation and fails later at key resolution ("No or unsupported key in KeyValue") rather than at the schema. Adding inboundDEREncodedKeyValueextraction is a larger, separate piece and is intentionally not in this change; I am happy to follow up with it if you would like.