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:22 UTC
(cxf) 01/01: Prevent XML Signature wrapping in AbstractXmlSigInHandler
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);
+ }
+}