From 5898249237b100c6ba9629ced36b9dd1cb3e373d Mon Sep 17 00:00:00 2001 From: arpansharma Date: Sat, 22 Aug 2026 13:33:12 -0500 Subject: [PATCH 1/3] Resolve dsig11:DEREncodedKeyValue on the StAX inbound path The StAX inbound processor resolved a KeyValue only through its RSA, DSA and EC forms, so a dsig11:DEREncodedKeyValue (the KeyValue form for key types without a structured element, such as ML-DSA) failed at key resolution with "No or unsupported key in KeyValue" even though the outbound side now emits it. Add DEREncodedKeyValueSecurityToken, which rebuilds the public key from the DER SubjectPublicKeyInfo by trying the same key types as the DOM DEREncodedKeyValue, and resolve it from both placements: nested inside ds:KeyValue (as emitted) and as a direct ds:KeyInfo child (the XML Signature 1.1 placement). With this a StAX-signed ML-DSA document verifies through the StAX inbound path with no out-of-band key. Adds StaxMLDSAKeyValueInboundTest covering both placements (asserting the resolved key is the signer's) and tampered signature rejection, parameterized across ML-DSA-44/65/87; all nine cases fail without the factory changes. --- .../DEREncodedKeyValueSecurityToken.java | 95 +++++++ .../SecurityTokenFactoryImpl.java | 20 ++ .../StaxMLDSAKeyValueInboundTest.java | 264 ++++++++++++++++++ 3 files changed, 379 insertions(+) create mode 100644 src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java create mode 100644 src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java diff --git a/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java b/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java new file mode 100644 index 000000000..221b75cc2 --- /dev/null +++ b/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.xml.security.stax.impl.securityToken; + +import java.security.KeyFactory; +import java.security.NoSuchAlgorithmException; +import java.security.PublicKey; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.X509EncodedKeySpec; + +import org.apache.xml.security.binding.xmldsig11.DEREncodedKeyValueType; +import org.apache.xml.security.exceptions.XMLSecurityException; +import org.apache.xml.security.stax.ext.InboundSecurityContext; +import org.apache.xml.security.stax.impl.util.IDGenerator; +import org.apache.xml.security.stax.securityToken.SecurityTokenConstants; + +/** + * Inbound security token for a {@code dsig11:DEREncodedKeyValue}: the DER-encoded + * SubjectPublicKeyInfo of a public key, which is the KeyValue form for key types that have no + * structured KeyValue element (ML-DSA, EdDSA, ...). The StAX counterpart of the DOM + * {@code DEREncodedKeyValue} support; the public key is rebuilt lazily from the encoding by + * trying each supported key type's {@link KeyFactory}. + */ +public class DEREncodedKeyValueSecurityToken extends AbstractInboundSecurityToken { + + // Same key types as the DOM DEREncodedKeyValue.supportedKeyTypes + private static final String[] SUPPORTED_KEY_TYPES = { "RSA", "DSA", "EC", + "DiffieHellman", "DH", "XDH", "X25519", "X448", + "EdDSA", "Ed25519", "Ed448", + "ML-DSA-44", "ML-DSA-65", "ML-DSA-87", + "RSASSA-PSS"}; + + private final byte[] encodedKey; + + public DEREncodedKeyValueSecurityToken(DEREncodedKeyValueType derEncodedKeyValueType, + InboundSecurityContext inboundSecurityContext) + throws XMLSecurityException { + super(inboundSecurityContext, IDGenerator.generateID(null), SecurityTokenConstants.KeyIdentifier_KeyValue, true); + + byte[] value = derEncodedKeyValueType.getValue(); + if (value == null || value.length == 0) { + throw new XMLSecurityException("stax.unsupportedKeyValue"); + } + this.encodedKey = value.clone(); + } + + private PublicKey buildPublicKey() throws XMLSecurityException { + X509EncodedKeySpec keySpec = new X509EncodedKeySpec(encodedKey); + for (String keyType : SUPPORTED_KEY_TYPES) { + try { + PublicKey publicKey = KeyFactory.getInstance(keyType).generatePublic(keySpec); + if (publicKey != null) { + return publicKey; + } + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { //NOPMD + // Not this key type; try the next one + } + } + throw new XMLSecurityException("stax.unsupportedKeyValue"); + } + + @Override + public PublicKey getPublicKey() throws XMLSecurityException { + if (super.getPublicKey() == null) { + setPublicKey(buildPublicKey()); + } + return super.getPublicKey(); + } + + @Override + public boolean isAsymmetric() { + return true; + } + + @Override + public SecurityTokenConstants.TokenType getTokenType() { + return SecurityTokenConstants.KeyValueToken; + } +} diff --git a/src/main/java/org/apache/xml/security/stax/impl/securityToken/SecurityTokenFactoryImpl.java b/src/main/java/org/apache/xml/security/stax/impl/securityToken/SecurityTokenFactoryImpl.java index 5378e1975..d5d76b299 100644 --- a/src/main/java/org/apache/xml/security/stax/impl/securityToken/SecurityTokenFactoryImpl.java +++ b/src/main/java/org/apache/xml/security/stax/impl/securityToken/SecurityTokenFactoryImpl.java @@ -33,6 +33,7 @@ import org.apache.xml.security.binding.xmldsig.RSAKeyValueType; import org.apache.xml.security.binding.xmldsig.X509DataType; import org.apache.xml.security.binding.xmldsig.X509IssuerSerialType; +import org.apache.xml.security.binding.xmldsig11.DEREncodedKeyValueType; import org.apache.xml.security.binding.xmldsig11.ECKeyValueType; import org.apache.xml.security.exceptions.XMLSecurityException; import org.apache.xml.security.stax.ext.InboundSecurityContext; @@ -76,6 +77,17 @@ public InboundSecurityToken getSecurityToken(KeyInfoType keyInfoType, return getSecurityToken(keyValueType, securityProperties, inboundSecurityContext, keyUsage); } + // DEREncodedKeyValue as a direct KeyInfo child, the XML Signature 1.1 placement + // (the nested-in-KeyValue placement is handled in the KeyValue branch above) + final DEREncodedKeyValueType derEncodedKeyValueType = XMLSecurityUtils.getQNameType( + keyInfoType.getContent(), XMLSecurityConstants.TAG_dsig11_DEREncodedKeyValue); + if (derEncodedKeyValueType != null) { + DEREncodedKeyValueSecurityToken token = + new DEREncodedKeyValueSecurityToken(derEncodedKeyValueType, inboundSecurityContext); + setTokenKey(securityProperties, keyUsage, token); + return token; + } + // KeyName final String keyName = XMLSecurityUtils.getQNameType(keyInfoType.getContent(), XMLSecurityConstants.TAG_dsig_KeyName); @@ -165,6 +177,14 @@ private static InboundSecurityToken getSecurityToken(KeyValueType keyValueType, setTokenKey(securityProperties, keyUsage, token); return token; } + final DEREncodedKeyValueType derEncodedKeyValueType = + XMLSecurityUtils.getQNameType(keyValueType.getContent(), XMLSecurityConstants.TAG_dsig11_DEREncodedKeyValue); + if (derEncodedKeyValueType != null) { + DEREncodedKeyValueSecurityToken token = + new DEREncodedKeyValueSecurityToken(derEncodedKeyValueType, inboundSecurityContext); + setTokenKey(securityProperties, keyUsage, token); + return token; + } throw new XMLSecurityException("stax.unsupportedKeyValue"); } diff --git a/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java b/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java new file mode 100644 index 000000000..9abe7116b --- /dev/null +++ b/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java @@ -0,0 +1,264 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.xml.security.test.stax.signature; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.Base64; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.xml.namespace.QName; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; + +import org.apache.xml.security.stax.ext.InboundXMLSec; +import org.apache.xml.security.stax.ext.SecurePart; +import org.apache.xml.security.stax.ext.XMLSec; +import org.apache.xml.security.stax.ext.XMLSecurityConstants; +import org.apache.xml.security.stax.ext.XMLSecurityProperties; +import org.apache.xml.security.stax.securityEvent.KeyValueTokenSecurityEvent; +import org.apache.xml.security.stax.securityEvent.SecurityEvent; +import org.apache.xml.security.stax.securityToken.SecurityTokenConstants; +import org.apache.xml.security.test.stax.utils.StAX2DOM; +import org.apache.xml.security.test.stax.utils.XMLSecEventAllocator; +import org.apache.xml.security.utils.Constants; +import org.apache.xml.security.utils.XMLUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; +import org.w3c.dom.Text; + +/** + * StAX inbound verification of ML-DSA signatures whose KeyInfo carries the public key as a + * {@code dsig11:DEREncodedKeyValue} (the KeyValue form emitted for key types without a + * structured KeyValue element). No verification key is supplied out of band: the inbound + * processor must resolve the key from the document itself, then verify with it. + */ +class StaxMLDSAKeyValueInboundTest extends AbstractSignatureCreationTest { + + private static final Map keyPairs = new HashMap<>(); + + @BeforeAll + static void generateKeys() throws Exception { + if (!isBcInstalled()) { + return; + } + try { + for (String alg : new String[]{"ML-DSA-44", "ML-DSA-65", "ML-DSA-87"}) { + KeyPairGenerator kpg = KeyPairGenerator.getInstance(alg, "BC"); + keyPairs.put(alg, kpg.generateKeyPair()); + } + } catch (Exception e) { + // ML-DSA not available with this BC version + } + } + + @ParameterizedTest + @CsvSource({ + "http://www.w3.org/tbd#ml-dsa-44,ML-DSA-44", + "http://www.w3.org/tbd#ml-dsa-65,ML-DSA-65", + "http://www.w3.org/tbd#ml-dsa-87,ML-DSA-87" + }) + void testInboundVerifiesWithKeyFromDerEncodedKeyValue(String sigAlgorithm, String jcaAlgorithm) throws Exception { + Assumptions.assumeTrue(isBcInstalled() && keyPairs.containsKey(jcaAlgorithm), + "ML-DSA requires BouncyCastle 1.81+"); + + KeyPair kp = keyPairs.get(jcaAlgorithm); + byte[] signed = signWithKeyValue(sigAlgorithm, kp); + + List events = new ArrayList<>(); + Document verified = verifyInbound(signed, events); + Assertions.assertNotNull(verified.getDocumentElement(), "Inbound processing must yield a document"); + + // The key the inbound processor verified with must be the signer's, recovered from the + // document's DEREncodedKeyValue (no key was supplied out of band). + PublicKey resolved = null; + for (SecurityEvent event : events) { + if (event instanceof KeyValueTokenSecurityEvent) { + resolved = ((KeyValueTokenSecurityEvent) event).getSecurityToken().getPublicKey(); + } + } + Assertions.assertNotNull(resolved, "Expected a KeyValueTokenSecurityEvent carrying the resolved key"); + Assertions.assertArrayEquals(kp.getPublic().getEncoded(), resolved.getEncoded(), + "Key resolved from DEREncodedKeyValue must be the signer's public key"); + } + + @ParameterizedTest + @CsvSource({ + "http://www.w3.org/tbd#ml-dsa-44,ML-DSA-44", + "http://www.w3.org/tbd#ml-dsa-65,ML-DSA-65", + "http://www.w3.org/tbd#ml-dsa-87,ML-DSA-87" + }) + void testInboundTamperedSignatureRejected(String sigAlgorithm, String jcaAlgorithm) throws Exception { + Assumptions.assumeTrue(isBcInstalled() && keyPairs.containsKey(jcaAlgorithm), + "ML-DSA requires BouncyCastle 1.81+"); + + KeyPair kp = keyPairs.get(jcaAlgorithm); + byte[] tampered = tamperSignatureValue(signWithKeyValue(sigAlgorithm, kp)); + + // The key still resolves from the document; the rejection must come from signature + // validation itself, not from a failure to resolve the key. + XMLStreamException ex = Assertions.assertThrows(XMLStreamException.class, + () -> verifyInbound(tampered, new ArrayList<>())); + String chain = messageChain(ex); + Assertions.assertTrue(chain.contains("INVALID signature"), + "Expected a core-validation failure, got: " + chain); + } + + private static String messageChain(Throwable t) { + StringBuilder sb = new StringBuilder(); + for (Throwable c = t; c != null; c = c.getCause()) { + sb.append(c.getClass().getSimpleName()).append(": ").append(c.getMessage()).append(" | "); + } + return sb.toString(); + } + + /** + * XML Signature 1.1 defines {@code dsig11:DEREncodedKeyValue} as a direct child of + * {@code ds:KeyInfo}; the nested-in-KeyValue placement is what this library emits. A document + * from another implementation may use the canonical placement, so the inbound side must + * resolve the key from there as well. + */ + @ParameterizedTest + @CsvSource({ + "http://www.w3.org/tbd#ml-dsa-44,ML-DSA-44", + "http://www.w3.org/tbd#ml-dsa-65,ML-DSA-65", + "http://www.w3.org/tbd#ml-dsa-87,ML-DSA-87" + }) + void testInboundVerifiesWithDerEncodedKeyValueAsKeyInfoChild(String sigAlgorithm, String jcaAlgorithm) + throws Exception { + Assumptions.assumeTrue(isBcInstalled() && keyPairs.containsKey(jcaAlgorithm), + "ML-DSA requires BouncyCastle 1.81+"); + + KeyPair kp = keyPairs.get(jcaAlgorithm); + byte[] signed = moveDerEncodedKeyValueToKeyInfo(signWithKeyValue(sigAlgorithm, kp)); + + List events = new ArrayList<>(); + Document verified = verifyInbound(signed, events); + Assertions.assertNotNull(verified.getDocumentElement(), "Inbound processing must yield a document"); + + PublicKey resolved = null; + for (SecurityEvent event : events) { + if (event instanceof KeyValueTokenSecurityEvent) { + resolved = ((KeyValueTokenSecurityEvent) event).getSecurityToken().getPublicKey(); + } + } + Assertions.assertNotNull(resolved, "Expected a KeyValueTokenSecurityEvent carrying the resolved key"); + Assertions.assertArrayEquals(kp.getPublic().getEncoded(), resolved.getEncoded(), + "Key resolved from a KeyInfo-level DEREncodedKeyValue must be the signer's public key"); + } + + /** + * Re-parents the emitted {@code dsig11:DEREncodedKeyValue} from inside {@code ds:KeyValue} to + * be a direct child of {@code ds:KeyInfo} (removing the now-empty KeyValue), giving the + * canonical XML Signature 1.1 layout. KeyInfo is outside the signed content, so the + * signature stays valid. + */ + private byte[] moveDerEncodedKeyValueToKeyInfo(byte[] signed) throws Exception { + Document document; + try (InputStream is = new ByteArrayInputStream(signed)) { + document = XMLUtils.read(is, false); + } + Element keyInfo = (Element) document.getElementsByTagNameNS(Constants.SignatureSpecNS, "KeyInfo").item(0); + Element keyValue = (Element) keyInfo.getElementsByTagNameNS(Constants.SignatureSpecNS, "KeyValue").item(0); + Element der = (Element) keyValue.getElementsByTagNameNS( + "http://www.w3.org/2009/xmldsig11#", "DEREncodedKeyValue").item(0); + Assertions.assertNotNull(der, "Expected a DEREncodedKeyValue inside KeyValue to re-parent"); + keyValue.removeChild(der); + keyInfo.replaceChild(der, keyValue); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + javax.xml.transform.TransformerFactory.newInstance().newTransformer().transform( + new javax.xml.transform.dom.DOMSource(document), + new javax.xml.transform.stream.StreamResult(bos)); + return bos.toByteArray(); + } + + private Document verifyInbound(byte[] signed, List events) throws Exception { + XMLSecurityProperties properties = new XMLSecurityProperties(); + // deliberately no setSignatureVerificationKey(...) + InboundXMLSec inboundXMLSec = XMLSec.getInboundWSSec(properties); + XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance(); + xmlInputFactory.setEventAllocator(new XMLSecEventAllocator()); + XMLStreamReader xmlStreamReader = xmlInputFactory.createXMLStreamReader(new ByteArrayInputStream(signed)); + XMLStreamReader securityStreamReader = + inboundXMLSec.processInMessage(xmlStreamReader, null, events::add); + return StAX2DOM.readDoc(securityStreamReader); + } + + private byte[] signWithKeyValue(String sigAlgorithm, KeyPair kp) throws Exception { + XMLSecurityProperties properties = new XMLSecurityProperties(); + List actions = new ArrayList<>(); + actions.add(XMLSecurityConstants.SIGNATURE); + properties.setActions(actions); + properties.setSignatureKeyIdentifier(SecurityTokenConstants.KeyIdentifier_KeyValue); + properties.setSignatureAlgorithm(sigAlgorithm); + properties.setSignatureKey(kp.getPrivate()); + properties.setSignatureVerificationKey(kp.getPublic()); + + SecurePart securePart = new SecurePart( + new QName("urn:example:po", "PaymentInfo"), + SecurePart.Modifier.Content, + new String[]{"http://www.w3.org/2001/10/xml-exc-c14n#"}, + "http://www.w3.org/2001/04/xmlenc#sha256"); + properties.addSignaturePart(securePart); + + return process("ie/baltimore/merlin-examples/merlin-xmlenc-five/plaintext.xml", properties, null); + } + + /** Flips one byte of the SignatureValue and re-serializes. */ + private byte[] tamperSignatureValue(byte[] signed) throws Exception { + Document document; + try (InputStream is = new ByteArrayInputStream(signed)) { + document = XMLUtils.read(is, false); + } + NodeList sigValues = document.getElementsByTagNameNS(Constants.SignatureSpecNS, "SignatureValue"); + Assertions.assertEquals(1, sigValues.getLength(), "Expected exactly one SignatureValue element"); + Element sigValueElement = (Element) sigValues.item(0); + + byte[] sigBytes = Base64.getMimeDecoder().decode(sigValueElement.getTextContent()); + sigBytes[sigBytes.length / 2] ^= (byte) 0xFF; + + NodeList children = sigValueElement.getChildNodes(); + for (int i = children.getLength() - 1; i >= 0; i--) { + sigValueElement.removeChild(children.item(i)); + } + Text newText = document.createTextNode(Base64.getEncoder().encodeToString(sigBytes)); + sigValueElement.appendChild(newText); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + javax.xml.transform.TransformerFactory.newInstance().newTransformer().transform( + new javax.xml.transform.dom.DOMSource(document), + new javax.xml.transform.stream.StreamResult(bos)); + return bos.toByteArray(); + } +} From dcd65cdb49d2e139bae23578aa6542dadf61b655 Mon Sep 17 00:00:00 2001 From: arpansharma Date: Tue, 25 Aug 2026 14:26:55 -0500 Subject: [PATCH 2/3] Reject malformed DEREncodedKeyValue content cleanly buildPublicKey() caught only NoSuchAlgorithmException and InvalidKeySpecException, but some providers (BouncyCastle's XDH/EdDSA KeyFactorySpi) throw an unchecked ArrayIndexOutOfBoundsException for malformed or short input rather than InvalidKeySpecException. Because the DEREncodedKeyValue content is untrusted, attacker-controlled inbound data, that exception propagated out of processInMessage instead of being rejected cleanly. Also catch RuntimeException in the key-type loop so a malformed encoding falls through to a clean stax.unsupportedKeyValue rejection, and add a garbage-content case to StaxMLDSAKeyValueInboundTest (parameterized across ML-DSA-44/65/87) that fails without this change. --- .../DEREncodedKeyValueSecurityToken.java | 8 ++- .../StaxMLDSAKeyValueInboundTest.java | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java b/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java index 221b75cc2..4e8042c05 100644 --- a/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java +++ b/src/main/java/org/apache/xml/security/stax/impl/securityToken/DEREncodedKeyValueSecurityToken.java @@ -68,8 +68,12 @@ private PublicKey buildPublicKey() throws XMLSecurityException { if (publicKey != null) { return publicKey; } - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { //NOPMD - // Not this key type; try the next one + } catch (NoSuchAlgorithmException | InvalidKeySpecException | RuntimeException e) { //NOPMD + // Not this key type; try the next one. Some providers (e.g. BouncyCastle's + // XDH/EdDSA KeyFactorySpi) throw an unchecked exception such as + // ArrayIndexOutOfBoundsException instead of InvalidKeySpecException for + // malformed or short input, which must not propagate since encodedKey here is + // untrusted, attacker-controlled inbound content. } } throw new XMLSecurityException("stax.unsupportedKeyValue"); diff --git a/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java b/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java index 9abe7116b..5b93e604f 100644 --- a/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java +++ b/src/test/java/org/apache/xml/security/test/stax/signature/StaxMLDSAKeyValueInboundTest.java @@ -203,6 +203,55 @@ private byte[] moveDerEncodedKeyValueToKeyInfo(byte[] signed) throws Exception { return bos.toByteArray(); } + /** + * A DEREncodedKeyValue whose content is garbage (decodes to no valid SubjectPublicKeyInfo) + * must be rejected cleanly at key resolution, not crash the pipeline with an uncaught + * RuntimeException. Some providers throw an unchecked exception (e.g. BouncyCastle's + * XDH/EdDSA KeyFactorySpi throws ArrayIndexOutOfBoundsException) for malformed input, and + * inbound KeyInfo content is attacker-controlled. + */ + @ParameterizedTest + @CsvSource({ + "http://www.w3.org/tbd#ml-dsa-44,ML-DSA-44", + "http://www.w3.org/tbd#ml-dsa-65,ML-DSA-65", + "http://www.w3.org/tbd#ml-dsa-87,ML-DSA-87" + }) + void testInboundGarbageDerContentRejectedCleanly(String sigAlgorithm, String jcaAlgorithm) throws Exception { + Assumptions.assumeTrue(isBcInstalled() && keyPairs.containsKey(jcaAlgorithm), + "ML-DSA requires BouncyCastle 1.81+"); + + KeyPair kp = keyPairs.get(jcaAlgorithm); + byte[] corrupted = corruptDerEncodedKeyValue(signWithKeyValue(sigAlgorithm, kp)); + + XMLStreamException ex = Assertions.assertThrows(XMLStreamException.class, + () -> verifyInbound(corrupted, new ArrayList<>())); + String chain = messageChain(ex); + Assertions.assertFalse(chain.contains("ArrayIndexOutOfBoundsException"), + "Malformed DEREncodedKeyValue content must not surface as an uncaught RuntimeException: " + chain); + } + + /** Replaces the DEREncodedKeyValue's base64 content with bytes that decode to no known SubjectPublicKeyInfo. */ + private byte[] corruptDerEncodedKeyValue(byte[] signed) throws Exception { + Document document; + try (InputStream is = new ByteArrayInputStream(signed)) { + document = XMLUtils.read(is, false); + } + Element der = (Element) document.getElementsByTagNameNS( + "http://www.w3.org/2009/xmldsig11#", "DEREncodedKeyValue").item(0); + byte[] garbage = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; + NodeList children = der.getChildNodes(); + for (int i = children.getLength() - 1; i >= 0; i--) { + der.removeChild(children.item(i)); + } + der.appendChild(document.createTextNode(Base64.getEncoder().encodeToString(garbage))); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + javax.xml.transform.TransformerFactory.newInstance().newTransformer().transform( + new javax.xml.transform.dom.DOMSource(document), + new javax.xml.transform.stream.StreamResult(bos)); + return bos.toByteArray(); + } + private Document verifyInbound(byte[] signed, List events) throws Exception { XMLSecurityProperties properties = new XMLSecurityProperties(); // deliberately no setSignatureVerificationKey(...) From 2859acc5aadfffc58c69d5dd5ab4c1a4632c0824 Mon Sep 17 00:00:00 2001 From: arpansharma Date: Tue, 25 Aug 2026 14:56:00 -0500 Subject: [PATCH 3/3] Reject malformed DEREncodedKeyValue content cleanly on the DOM path The DOM DEREncodedKeyValue#getPublicKey() has the same narrow exception handling as the StAX token fixed in the previous commit: it caught only NoSuchAlgorithmException and InvalidKeySpecException while iterating the supported key types, so an unchecked exception from a KeyFactorySpi (BouncyCastle 1.85's XDH/EdDSA throw ArrayIndexOutOfBoundsException for malformed or short input) propagated out instead of a clean rejection. Because DEREncodedKeyValueResolver is a default KeyResolver and a DEREncodedKeyValue in an inbound document is untrusted, attacker-controlled content, this could crash key resolution reached via KeyInfo#getPublicKey() with an uncontrolled runtime exception. Catch RuntimeException in the loop so a malformed encoding falls through to the declared XMLSecurityException. Adds a test that fails without the fix (BouncyCastle at first provider position, skipped otherwise). --- .../keys/content/DEREncodedKeyValue.java | 7 +- ...EREncodedKeyValueMalformedContentTest.java | 80 +++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/apache/xml/security/test/dom/keys/DEREncodedKeyValueMalformedContentTest.java diff --git a/src/main/java/org/apache/xml/security/keys/content/DEREncodedKeyValue.java b/src/main/java/org/apache/xml/security/keys/content/DEREncodedKeyValue.java index 85be2f938..6cbd66b5a 100644 --- a/src/main/java/org/apache/xml/security/keys/content/DEREncodedKeyValue.java +++ b/src/main/java/org/apache/xml/security/keys/content/DEREncodedKeyValue.java @@ -121,8 +121,11 @@ public PublicKey getPublicKey() throws XMLSecurityException { if (publicKey != null) { return publicKey; } - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { //NOPMD - // Do nothing, try the next type + } catch (NoSuchAlgorithmException | InvalidKeySpecException | RuntimeException e) { //NOPMD + // Do nothing, try the next type. Some providers (e.g. BouncyCastle's XDH/EdDSA + // KeyFactorySpi) throw an unchecked exception such as ArrayIndexOutOfBoundsException + // instead of InvalidKeySpecException for malformed or short input, which must not + // propagate since the encoded key here is untrusted, attacker-controlled content. } } throw new XMLSecurityException("DEREncodedKeyValue.UnsupportedEncodedKey"); diff --git a/src/test/java/org/apache/xml/security/test/dom/keys/DEREncodedKeyValueMalformedContentTest.java b/src/test/java/org/apache/xml/security/test/dom/keys/DEREncodedKeyValueMalformedContentTest.java new file mode 100644 index 000000000..65eb7ece1 --- /dev/null +++ b/src/test/java/org/apache/xml/security/test/dom/keys/DEREncodedKeyValueMalformedContentTest.java @@ -0,0 +1,80 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.xml.security.test.dom.keys; + +import java.security.Provider; +import java.security.Security; + +import org.apache.xml.security.exceptions.XMLSecurityException; +import org.apache.xml.security.keys.content.DEREncodedKeyValue; +import org.apache.xml.security.test.dom.TestUtils; +import org.junit.jupiter.api.Test; +import org.w3c.dom.Document; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * {@link DEREncodedKeyValue#getPublicKey()} resolves the key by trying each supported key type's + * {@code KeyFactory}. Some providers throw an unchecked exception for malformed or short input + * instead of {@code InvalidKeySpecException} - notably BouncyCastle 1.85's XDH/EdDSA + * KeyFactorySpi throws {@code ArrayIndexOutOfBoundsException}. Since a DEREncodedKeyValue read + * from an inbound document (via {@code DEREncodedKeyValueResolver}, a default KeyResolver) is + * untrusted, attacker-controlled content, such an exception must not propagate out of key + * resolution. + * + *

The unchecked exception is only observed when BouncyCastle is the provider selected for + * XDH/EdDSA, i.e. registered ahead of the JDK's own providers (a common BouncyCastle-primary + * deployment). With the JDK providers taking precedence they reject the same input cleanly with + * {@code InvalidKeySpecException}, so this test inserts BouncyCastle at the first position (as + * {@code XMLCipherTest} does for its BouncyCastle-specific case) and is skipped when BouncyCastle + * is unavailable. + */ +class DEREncodedKeyValueMalformedContentTest { + + @Test + void testMalformedDerContentRejectedCleanly() throws Exception { + boolean bcAtFirstPosition = false; + if (Security.getProvider("BC") == null) { + try { + Class bcClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider"); + Provider bc = (Provider) bcClass.getConstructor().newInstance(); + Security.insertProviderAt(bc, 1); + bcAtFirstPosition = true; + } catch (ReflectiveOperationException e) { + // BouncyCastle not installed, ignore + } + } + assumeTrue(bcAtFirstPosition, "requires BouncyCastle at first provider position"); + + try { + Document doc = TestUtils.newDocument(); + // Bytes that decode to no valid SubjectPublicKeyInfo; short enough that BouncyCastle + // 1.85's XDH/EdDSA KeyFactory reads past the end (ArrayIndexOutOfBoundsException). + DEREncodedKeyValue derEncodedKeyValue = + new DEREncodedKeyValue(doc, new byte[]{0, 1, 2, 3, 4, 5, 6, 7}); + + // Must fail cleanly with the declared XMLSecurityException, not an uncaught + // RuntimeException such as ArrayIndexOutOfBoundsException. + assertThrows(XMLSecurityException.class, derEncodedKeyValue::getPublicKey); + } finally { + Security.removeProvider("BC"); + } + } +}