From aab6a37d80e7bcfaad252bede3a2d75f2f3be68b Mon Sep 17 00:00:00 2001 From: Sean Mullan Date: Mon, 6 Jul 2026 12:43:29 -0400 Subject: [PATCH 1/2] Fix bit masking issue when checking. Check for invalid key packets when unmarshalling PGPData from XML. --- .../jcp/xml/dsig/internal/dom/DOMPGPData.java | 5 +- .../xml/crypto/dsig/keyinfo/PGPDataTest.java | 79 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMPGPData.java b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMPGPData.java index 51df5b783..ccbb5a663 100644 --- a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMPGPData.java +++ b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMPGPData.java @@ -157,6 +157,7 @@ public DOMPGPData(Element pdElem) throws MarshalException { } else if ("PGPKeyPacket".equals(localName) && XMLSignature.XMLNS.equals(namespace)) { String content = XMLUtils.getFullTextChildrenFromNode(childElem); pgpKeyPacket = XMLUtils.decode(content); + checkKeyPacket(pgpKeyPacket); } else { other.add (new javax.xml.crypto.dom.DOMStructure(childElem)); @@ -250,8 +251,8 @@ private void checkKeyPacket(byte[] keyPacket) { } // tag value must be 6, 14, 5 or 7 - if ((tag & 6) != 6 && (tag & 14) != 14 && - (tag & 5) != 5 && (tag & 7) != 7) { + tag = tag & 0x3f; + if (tag != 5 && tag != 6 && tag != 7 && tag != 14) { throw new IllegalArgumentException("keypacket tag is invalid: " + "must be 6, 14, 5, or 7"); } diff --git a/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java b/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java index 743c826c3..64b530612 100644 --- a/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java +++ b/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java @@ -25,15 +25,28 @@ import java.util.ArrayList; import java.util.List; +import javax.xml.crypto.MarshalException; import javax.xml.crypto.XMLStructure; +import javax.xml.crypto.dsig.XMLSignature; +import javax.xml.crypto.dom.DOMStructure; +import javax.xml.crypto.dsig.keyinfo.KeyInfo; import javax.xml.crypto.dsig.keyinfo.KeyInfoFactory; import javax.xml.crypto.dsig.keyinfo.PGPData; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; + +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Text; + +import org.apache.xml.security.utils.XMLUtils; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; @@ -44,6 +57,7 @@ class PGPDataTest { private final KeyInfoFactory fac; + private final DocumentBuilder db; private final byte[][] values = { { 0x01, 0x02, 0x03, 0x04, @@ -54,9 +68,35 @@ class PGPDataTest { } }; + private final byte[][] invalidKeyPackets = { + { + // key packet too short + 0x01, 0x02 + }, + { + // invalid key packet header (bit 7 not set to 1) + 0x66, 0x01, 0x00 + }, + { + // invalid old key packet format (bit 6 not set to 1) + (byte)0x86, 0x01, 0x00 + }, + { + // invalid key packet tag (must be 6, 14, 5, or 7) + (byte)0xc3, 0x01, 0x00 + }, + { + // invalid key packet tag (must be 6, 14, 5, or 7) + (byte)0xd6, 0x01, 0x00 + } + }; + public PGPDataTest() throws Exception { fac = KeyInfoFactory.getInstance ("DOM", new org.apache.jcp.xml.dsig.internal.dom.XMLDSigRI()); + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + db = dbf.newDocumentBuilder(); } @SuppressWarnings("rawtypes") @@ -123,6 +163,45 @@ void testConstructor() { // test newPGPData(byte[], List) pd = fac.newPGPData(values[1], null); assertArrayEquals(values[1], pd.getKeyPacket()); + + // test that newPGPData throws IllegalArgumentExc on invalid key packets + for (byte[] keyPacket : invalidKeyPackets) { + testInvalidKeyPacket(keyPacket); + } + } + + private void testInvalidKeyPacket(byte[] keyPacket) { + assertThrows(IllegalArgumentException.class, () -> + fac.newPGPData(keyPacket, null)); + } + + @Test + void testUnmarshal() throws Exception { + Document doc = db.newDocument(); + Element kiElem = doc.createElementNS(XMLSignature.XMLNS, "KeyInfo"); + Element pdElem = doc.createElementNS(XMLSignature.XMLNS, "PGPData"); + Element pkpElem = doc.createElementNS(XMLSignature.XMLNS, "PGPKeyPacket"); + Element pkiElem = doc.createElementNS(XMLSignature.XMLNS, "PGPKeyID"); + kiElem.appendChild(pdElem); + + // test unmarshalling key id and packets + Text pkiText = doc.createTextNode(XMLUtils.encodeToString(values[0])); + pkiElem.appendChild(pkiText); + Text pkpText = doc.createTextNode(XMLUtils.encodeToString(values[1])); + pkpElem.appendChild(pkpText); + pdElem.appendChild(pkiElem); + pdElem.appendChild(pkpElem); + KeyInfo ki = fac.unmarshalKeyInfo(new DOMStructure(kiElem)); + PGPData pd = (PGPData)ki.getContent().get(0); + assertArrayEquals(values[0], pd.getKeyId()); + assertArrayEquals(values[1], pd.getKeyPacket()); + + // test that unmarshalKeyInfo throws MarshalExc on invalid key packets + for (byte[] keyPacket : invalidKeyPackets) { + pkpText.setNodeValue(XMLUtils.encodeToString(keyPacket)); + assertThrows(MarshalException.class, () -> + fac.unmarshalKeyInfo(new DOMStructure(kiElem))); + } } @Test From 3f4cbbec05941b2fc8bd2519b5a3c1717caca75c Mon Sep 17 00:00:00 2001 From: Sean Mullan Date: Wed, 8 Jul 2026 11:18:15 -0400 Subject: [PATCH 2/2] Check that cause is instanceof IAE. --- .../test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java b/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java index 64b530612..a1096e85d 100644 --- a/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java +++ b/src/test/java/org/apache/xml/security/test/javax/xml/crypto/dsig/keyinfo/PGPDataTest.java @@ -47,6 +47,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -199,8 +200,9 @@ void testUnmarshal() throws Exception { // test that unmarshalKeyInfo throws MarshalExc on invalid key packets for (byte[] keyPacket : invalidKeyPackets) { pkpText.setNodeValue(XMLUtils.encodeToString(keyPacket)); - assertThrows(MarshalException.class, () -> + MarshalException me = assertThrows(MarshalException.class, () -> fac.unmarshalKeyInfo(new DOMStructure(kiElem))); + assertTrue(me.getCause() instanceof IllegalArgumentException); } }