You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2026/09/25 05:20:21 UTC
(cxf) branch coheigea/xmlsig created (now 2c87bbdbe03)
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a change to branch coheigea/xmlsig
in repository https://gitbox.apache.org/repos/asf/cxf.git
at 2c87bbdbe03 Prevent XML Signature wrapping in AbstractXmlSigInHandler
This branch includes the following new commits:
new 2c87bbdbe03 Prevent XML Signature wrapping in AbstractXmlSigInHandler
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(cxf) 01/01: Prevent XML Signature wrapping in AbstractXmlSigInHandler
Posted by co...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch coheigea/xmlsig
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 2c87bbdbe037f2d034edc0de263f16a74096a274
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Sep 25 06:19:59 2026 +0100
Prevent XML Signature wrapping in AbstractXmlSigInHandler
---
.../rs/security/saml/SamlEnvelopedInHandler.java | 14 +-
.../rs/security/xml/AbstractXmlSigInHandler.java | 46 ++--
.../cxf/rs/security/xml/XmlSigInHandlerTest.java | 266 +++++++++++++++++++++
3 files changed, 307 insertions(+), 19 deletions(-)
diff --git a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlEnvelopedInHandler.java b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlEnvelopedInHandler.java
index e22474e2ef8..fca4f4f1716 100644
--- a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlEnvelopedInHandler.java
+++ b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlEnvelopedInHandler.java
@@ -27,6 +27,7 @@ import javax.xml.stream.XMLStreamReader;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
+import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import jakarta.ws.rs.HttpMethod;
@@ -57,6 +58,7 @@ public class SamlEnvelopedInHandler extends AbstractSamlInHandler {
}
Document doc = null;
+ Element signedBody = null;
InputStream is = message.getContent(InputStream.class);
if (is != null) {
try {
@@ -68,6 +70,12 @@ public class SamlEnvelopedInHandler extends AbstractSamlInHandler {
XMLStreamReader reader = message.getContent(XMLStreamReader.class);
if (reader instanceof W3CDOMStreamReader) {
doc = ((W3CDOMStreamReader)reader).getDocument();
+ // A detached XML Signature has already been validated, and the reader
+ // is restricted to the signed element, which is then the body
+ Node node = ((W3CDOMStreamReader)reader).getCurrentNode();
+ if (node instanceof Element && node != doc.getDocumentElement()) {
+ signedBody = (Element)node;
+ }
}
}
if (doc == null) {
@@ -85,7 +93,11 @@ public class SamlEnvelopedInHandler extends AbstractSamlInHandler {
validateToken(message, samlElement);
doc.getDocumentElement().removeChild(samlElement);
- if (bodyIsRoot) {
+ if (signedBody != null) {
+ message.setContent(XMLStreamReader.class,
+ new W3CDOMStreamReader(signedBody));
+ message.setContent(InputStream.class, null);
+ } else if (bodyIsRoot) {
message.setContent(XMLStreamReader.class,
new W3CDOMStreamReader(doc));
message.setContent(InputStream.class, null);
diff --git a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlSigInHandler.java b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlSigInHandler.java
index daa1c2a9d2e..4ea58c9f585 100644
--- a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlSigInHandler.java
+++ b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlSigInHandler.java
@@ -169,20 +169,23 @@ public class AbstractXmlSigInHandler extends AbstractXmlSecInHandler {
if (!valid) {
throwFault("Signature validation failed", null);
}
+ Element signedEl = getSignedElement(root, ref);
+ // Only pass on the signed element. This is the root for an enveloped signature, and
+ // a child of the (unsigned) root for a detached signature. The reader's document
+ // is still the full document, e.g. for SamlEnvelopedInHandler to get the assertion
+ Element body = isEnveloping(root) ? root : signedEl;
if (removeSignature) {
if (!isEnveloping(root)) {
- Element signedEl = getSignedElement(root, ref);
signedEl.removeAttribute("ID");
root.removeChild(signatureElement);
} else {
- Element actualBody = getActualBody(root);
Document newDoc = DOMUtils.createDocument();
- newDoc.adoptNode(actualBody);
- root = actualBody;
+ newDoc.adoptNode(signedEl);
+ body = signedEl;
}
}
message.setContent(XMLStreamReader.class,
- new W3CDOMStreamReader(root));
+ new W3CDOMStreamReader(body));
message.setContent(InputStream.class, null);
}
@@ -196,19 +199,6 @@ public class AbstractXmlSigInHandler extends AbstractXmlSecInHandler {
}
- private Element getActualBody(Element envelopingSigElement) {
- Element objectNode = getNode(envelopingSigElement, Constants.SignatureSpecNS, "Object", 0);
- if (objectNode == null) {
- throwFault("Object envelope is not available", null);
- }
- Element node = DOMUtils.getFirstElement(objectNode);
- if (node == null) {
- throwFault("No signed data is found", null);
- }
- return node;
-
- }
-
private Element getSignatureElement(Element sigParentElement) {
if (isEnveloping(sigParentElement)) {
return sigParentElement;
@@ -276,6 +266,15 @@ public class AbstractXmlSigInHandler extends AbstractXmlSecInHandler {
if (enveloped && !envelopedConfirmed) {
throwFault("Only enveloped signatures are currently supported", null);
}
+ // The Signature is a child of the document root, so an enveloped signature
+ // must reference the root. Otherwise the signed element could be wrapped in
+ // unsigned content which would then be passed on to the application.
+ if (envelopedConfirmed && !enveloped) {
+ throwFault("Enveloped signature must reference the document root", null);
+ }
+ if (isEnveloping(root)) {
+ validateEnvelopingReference(root, signedEl);
+ }
if (c14TransformExpected != null && !c14TransformConfirmed) {
throwFault("Transform Canonicalization is not supported", null);
}
@@ -291,6 +290,17 @@ public class AbstractXmlSigInHandler extends AbstractXmlSecInHandler {
return signedEl;
}
+ // The signed element must be the only element in the only Object of the enveloping
+ // Signature, so that no unsigned content can be passed on to the application
+ private void validateEnvelopingReference(Element root, Element signedEl) {
+ List<Element> objects = DOMUtils.getChildrenWithName(root, Constants.SignatureSpecNS, "Object");
+ if (objects.size() != 1 || signedEl.getParentNode() != objects.get(0)
+ || DOMUtils.getFirstElement(objects.get(0)) != signedEl
+ || DOMUtils.getNextElement(signedEl) != null) {
+ throwFault("Enveloping signature must reference the signed Object content", null);
+ }
+ }
+
private Element getSignedElement(Element root, Reference ref) {
String rootId = root.getAttribute("ID");
String expectedID = ref.getURI().substring(1);
diff --git a/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSigInHandlerTest.java b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSigInHandlerTest.java
new file mode 100644
index 00000000000..d5ca578c25a
--- /dev/null
+++ b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSigInHandlerTest.java
@@ -0,0 +1,266 @@
+/**
+ * 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.cxf.rs.security.xml;
+
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.Properties;
+
+import javax.xml.stream.XMLStreamReader;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
+import jakarta.ws.rs.WebApplicationException;
+import org.apache.cxf.helpers.DOMUtils;
+import org.apache.cxf.message.ExchangeImpl;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.rt.security.SecurityConstants;
+import org.apache.cxf.staxutils.StaxUtils;
+import org.apache.cxf.staxutils.W3CDOMStreamReader;
+import org.apache.wss4j.common.crypto.Crypto;
+import org.apache.wss4j.common.crypto.CryptoFactory;
+import org.apache.wss4j.common.crypto.CryptoType;
+import org.apache.xml.security.algorithms.MessageDigestAlgorithm;
+import org.apache.xml.security.signature.XMLSignature;
+import org.apache.xml.security.transforms.Transforms;
+import org.apache.xml.security.utils.Constants;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+public class XmlSigInHandlerTest {
+
+ private static Crypto crypto;
+
+ @BeforeClass
+ public static void setUpCrypto() throws Exception {
+ org.apache.xml.security.Init.init();
+ Properties props = new Properties();
+ props.put("org.apache.wss4j.crypto.provider", "org.apache.wss4j.common.crypto.Merlin");
+ props.put("org.apache.wss4j.crypto.merlin.keystore.type", "jks");
+ props.put("org.apache.wss4j.crypto.merlin.keystore.password", "password");
+ props.put("org.apache.wss4j.crypto.merlin.keystore.alias", "alice");
+ props.put("org.apache.wss4j.crypto.merlin.keystore.file", "alice.jks");
+ crypto = CryptoFactory.getInstance(props);
+ }
+
+ @Test
+ public void testEnvelopedSignature() throws Exception {
+ Document doc = createSignedDocument();
+
+ assertBody(doc, "CXF");
+ }
+
+ @Test
+ public void testWrappedEnvelopedSignatureIsRejected() throws Exception {
+ Document signedDoc = createSignedDocument();
+ Element signedRoot = signedDoc.getDocumentElement();
+ Element signature =
+ DOMUtils.getFirstChildWithName(signedRoot, Constants.SignatureSpecNS, "Signature");
+
+ // Wrap the signed element in a new unsigned root and move the Signature
+ // to be a direct child of that root
+ Document doc = DOMUtils.createDocument();
+ Element root = createBook(doc, "evil");
+ doc.appendChild(root);
+ signedRoot.removeChild(signature);
+ root.appendChild(doc.importNode(signedRoot, true));
+ root.appendChild(doc.importNode(signature, true));
+
+ assertRejected(doc);
+ }
+
+ @Test
+ public void testEnvelopingSignature() throws Exception {
+ Document doc = createEnvelopingSignedDocument();
+
+ assertBody(doc, "CXF");
+ }
+
+ @Test
+ public void testEnvelopingSignatureWithUnsignedObjectContentIsRejected() throws Exception {
+ Document doc = createEnvelopingSignedDocument();
+ Element object =
+ DOMUtils.getFirstChildWithName(doc.getDocumentElement(), Constants.SignatureSpecNS, "Object");
+
+ // Add an unsigned element before the signed element in the same Object
+ object.insertBefore(createBook(doc, "evil"), object.getFirstChild());
+
+ assertRejected(doc);
+ }
+
+ @Test
+ public void testEnvelopingSignatureWithUnsignedObjectIsRejected() throws Exception {
+ Document doc = createEnvelopingSignedDocument();
+ Element root = doc.getDocumentElement();
+ Element object = DOMUtils.getFirstChildWithName(root, Constants.SignatureSpecNS, "Object");
+
+ // Add an unsigned Object before the signed Object
+ Element evilObject = doc.createElementNS(Constants.SignatureSpecNS, "ds:Object");
+ evilObject.appendChild(createBook(doc, "evil"));
+ root.insertBefore(evilObject, object);
+
+ assertRejected(doc);
+ }
+
+ @Test
+ public void testEnvelopingSignatureWithObjectInKeyInfo() throws Exception {
+ Document doc = createEnvelopingSignedDocument();
+ Element keyInfo =
+ DOMUtils.getFirstChildWithName(doc.getDocumentElement(), Constants.SignatureSpecNS, "KeyInfo");
+
+ // An unsigned Object nested in the KeyInfo must not be passed on as the body
+ Element evilObject = doc.createElementNS(Constants.SignatureSpecNS, "ds:Object");
+ evilObject.appendChild(createBook(doc, "evil"));
+ keyInfo.appendChild(evilObject);
+
+ assertBody(doc, "CXF");
+ }
+
+ @Test
+ public void testDetachedSignature() throws Exception {
+ Document doc = createDetachedSignedDocument();
+
+ // Only the signed element is passed on, not the unsigned wrapper
+ assertBody(doc, "CXF");
+ }
+
+ @Test
+ public void testDetachedSignatureWithUnsignedContent() throws Exception {
+ Document doc = createDetachedSignedDocument();
+
+ // Add unsigned content to the unsigned root, before the signed element
+ Element root = doc.getDocumentElement();
+ root.insertBefore(createBook(doc, "evil"), root.getFirstChild());
+
+ assertBody(doc, "CXF");
+ }
+
+ private static void assertBody(Document doc, String expectedName) throws Exception {
+ Message message = createMessage(doc);
+ new AbstractXmlSigInHandler() { }.checkSignature(message);
+
+ XMLStreamReader reader = message.getContent(XMLStreamReader.class);
+ assertNotNull(reader);
+ Element root = StaxUtils.read(reader).getDocumentElement();
+ assertEquals("Book", root.getLocalName());
+ assertEquals(expectedName, DOMUtils.getFirstElement(root).getTextContent());
+ }
+
+ private static void assertRejected(Document doc) {
+ Message message = createMessage(doc);
+ try {
+ new AbstractXmlSigInHandler() { }.checkSignature(message);
+ fail("Failure expected on a wrapped signature");
+ } catch (WebApplicationException ex) {
+ assertEquals(400, ex.getResponse().getStatus());
+ }
+ }
+
+ private static Message createMessage(Document doc) {
+ Message message = new MessageImpl();
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.HTTP_REQUEST_METHOD, "POST");
+ message.put(SecurityConstants.SIGNATURE_CRYPTO, crypto);
+ message.setContent(XMLStreamReader.class, new W3CDOMStreamReader(doc.getDocumentElement()));
+ return message;
+ }
+
+ private static Document createSignedDocument() throws Exception {
+ Document doc = DOMUtils.createDocument();
+ Element root = createBook(doc, "CXF");
+ doc.appendChild(root);
+
+ String id = "_book";
+ root.setAttributeNS(null, "Id", id);
+ root.setIdAttributeNS(null, "Id", true);
+
+ XMLSignature sig = new XMLSignature(doc, "", XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA256);
+ root.appendChild(sig.getElement());
+ Transforms transforms = new Transforms(doc);
+ transforms.addTransform(Transforms.TRANSFORM_ENVELOPED_SIGNATURE);
+ transforms.addTransform(Transforms.TRANSFORM_C14N_EXCL_OMIT_COMMENTS);
+ sig.addDocument("#" + id, transforms, MessageDigestAlgorithm.ALGO_ID_DIGEST_SHA256);
+ sign(sig);
+ return doc;
+ }
+
+ private static Document createEnvelopingSignedDocument() throws Exception {
+ Document doc = DOMUtils.createDocument();
+ Element book = createBook(doc, "CXF");
+
+ String id = "_book";
+ book.setAttributeNS(null, "Id", id);
+ book.setIdAttributeNS(null, "Id", true);
+
+ XMLSignature sig = new XMLSignature(doc, "", XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA256);
+ doc.appendChild(sig.getElement());
+ Element object = doc.createElementNS(Constants.SignatureSpecNS, "ds:Object");
+ object.appendChild(book);
+ sig.getElement().appendChild(object);
+ Transforms transforms = new Transforms(doc);
+ transforms.addTransform(Transforms.TRANSFORM_C14N_EXCL_OMIT_COMMENTS);
+ sig.addDocument("#" + id, transforms, MessageDigestAlgorithm.ALGO_ID_DIGEST_SHA256);
+ sign(sig);
+ return doc;
+ }
+
+ private static Document createDetachedSignedDocument() throws Exception {
+ Document doc = DOMUtils.createDocument();
+ Element root = doc.createElementNS("http://org.apache.cxf/rs/env", "env:Envelope");
+ doc.appendChild(root);
+ Element book = createBook(doc, "CXF");
+ root.appendChild(book);
+
+ String id = "_book";
+ book.setAttributeNS(null, "Id", id);
+ book.setIdAttributeNS(null, "Id", true);
+
+ XMLSignature sig = new XMLSignature(doc, "", XMLSignature.ALGO_ID_SIGNATURE_RSA_SHA256);
+ root.appendChild(sig.getElement());
+ Transforms transforms = new Transforms(doc);
+ transforms.addTransform(Transforms.TRANSFORM_C14N_EXCL_OMIT_COMMENTS);
+ sig.addDocument("#" + id, transforms, MessageDigestAlgorithm.ALGO_ID_DIGEST_SHA256);
+ sign(sig);
+ return doc;
+ }
+
+ private static Element createBook(Document doc, String bookName) {
+ Element book = doc.createElementNS(null, "Book");
+ Element name = doc.createElementNS(null, "name");
+ name.setTextContent(bookName);
+ book.appendChild(name);
+ return book;
+ }
+
+ private static void sign(XMLSignature sig) throws Exception {
+ CryptoType cryptoType = new CryptoType(CryptoType.TYPE.ALIAS);
+ cryptoType.setAlias("alice");
+ X509Certificate cert = crypto.getX509Certificates(cryptoType)[0];
+ PrivateKey key = crypto.getPrivateKey("alice", "password");
+ sig.addKeyInfo(cert);
+ sig.sign(key);
+ }
+}