Re: [PR] Add ML-DSA (FIPS 204) post-quantum signature su pport [santuario-xml-security-java]

ffang commented on PR #651:
URL: https://github.com/apache/santuario-xml-security-java/pull/651#issuecomment-5375209024

   > While adding inbound-path coverage after the rebase, I found that StAX verification of an ML-DSA signature is not exercised by the current tests, and driving it revealed a round-trip issue with the KeyValue key identifier. Everything below was reproduced on this branch.
   > 
   > **The issue.** With `setSignatureKeyIdentifier(SecurityTokenConstants.KeyIdentifier_KeyValue)`, which is the identifier `StaxMLDSASignatureTest` itself configures, outbound signing with an ML-DSA key succeeds but emits an empty key element:
   > 
   > ```
   > <dsig:KeyInfo xmlns:dsig="http://www.w3.org/2000/09/xmldsig#" Id="..."><dsig:KeyValue/></dsig:KeyInfo>
   > ```
   > 
   > Feeding that document back through the library's own inbound side (`InboundXMLSec#processInMessage`) then fails schema validation and never reaches signature verification, even when the signature is valid:
   > 
   > ```
   > XMLSecurityException: cvc-complex-type.2.4.b: The content of element 'dsig:KeyValue' is not complete.
   > One of '{...DSAKeyValue, ...RSAKeyValue, WC[##other:"http://www.w3.org/2000/09/xmldsig#"]}' is expected.
   > ```
   > 
   > **Root cause.** `XMLSecurityUtils#createKeyValueTokenStructure(AbstractOutputProcessor, OutputProcessorChain, PublicKey)` opens the `dsig:KeyValue` element, then dispatches on `publicKey.getAlgorithm()` with branches for `"RSA"`, `"DSA"` and `"EC"` only. For any other key type it falls through all branches and closes the element, so an ML-DSA key produces `<dsig:KeyValue/>` with no content. (The writer predates this PR and behaves this way for any key type outside those three; this PR is what makes the fall-through reachable, in the very configuration its own test uses.)
   > 
   > **The inbound side of this PR is otherwise sound.** Repeating the same round trip with `KeyIdentifier_NoKeyInfo` and a verification key supplied via `setSignatureVerificationKey`, the inbound path behaves exactly as it should for ML-DSA-65: the valid document verifies, a tampered `SignatureValue` is rejected ("INVALID signature -- core validation failed"), and verification against a wrong public key is rejected the same way. So the gap is confined to the KeyValue serialization, not to the new signature wiring.
   > 
   > **Why the tests do not catch it:** `StaxMLDSASignatureTest` signs through the outbound path but verifies through the DOM engine (`verifyUsingDOM` / `XMLSignature#checkSignatureValue`), so the empty `KeyValue` is never consumed by the inbound processor.
   > 
   > Two possible fixes, in order of preference:
   > 
   >     1. Emit `dsig11:DEREncodedKeyValue` (the SubjectPublicKeyInfo encoding) for key types that have no structured KeyValue form. XML Signature 1.1 defines it as a direct `KeyInfo` child, and it would equally be schema-valid inside `dsig:KeyValue` through the same `##other` wildcard this writer already relies on for `dsig11:ECKeyValue`. Carrying an arbitrary public key is exactly what that element exists for, and this PR already adds the ML-DSA names to the DOM `DEREncodedKeyValue` class, so the resolving side has a basis.
   > 
   Hi @Arpan0995 ,
   
   Agree that "Emit DEREncodedKeyValue " is better and please go this way.
   
   >     2. Failing that, throw at signing time when `KeyIdentifier_KeyValue` is requested for an unsupported key type, rather than emitting XML the library itself will not accept back.
   > 
   > 
   > Happy to contribute the inbound-path tests either way (valid round trip, tampered signature, wrong key, all through `processInMessage`, parameterized across the three parameter sets); they would have caught this and would pin the inbound behavior against future refactors.
   Thanks a lot!
    Related note on the encryption split's inbound coverage is on #652.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]