This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 702d701a781 Prevent XML Signature wrapping in XmlSecInInterceptor
(#3505)
702d701a781 is described below
commit 702d701a7819ab32cdf8157fb8ae6c9df5b6504f
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Fri Sep 25 14:41:18 2026 +0100
Prevent XML Signature wrapping in XmlSecInInterceptor (#3505)
---
.../cxf/rs/security/xml/XmlSecInInterceptor.java | 43 ++++
.../rs/security/xml/XmlSecInInterceptorTest.java | 256 +++++++++++++++++++++
2 files changed, 299 insertions(+)
diff --git
a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptor.java
b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptor.java
index f1455d6a33f..fda3f5de4fc 100644
---
a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptor.java
+++
b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptor.java
@@ -28,6 +28,7 @@ import java.util.Collection;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
@@ -73,6 +74,7 @@ import
org.apache.xml.security.stax.securityEvent.SecurityEvent;
import org.apache.xml.security.stax.securityEvent.SecurityEventConstants;
import org.apache.xml.security.stax.securityEvent.SecurityEventConstants.Event;
import org.apache.xml.security.stax.securityEvent.SecurityEventListener;
+import org.apache.xml.security.stax.securityEvent.SignedElementSecurityEvent;
import org.apache.xml.security.stax.securityEvent.TokenSecurityEvent;
import org.apache.xml.security.stax.securityToken.SecurityToken;
@@ -83,6 +85,15 @@ public class XmlSecInInterceptor extends
AbstractPhaseInterceptor<Message> imple
private static final Logger LOG =
LogUtils.getL7dLogger(XmlSecInInterceptor.class);
+ private static final Set<String> ALLOWED_TRANSFORMS = Set.of(
+ XMLSecurityConstants.NS_XMLDSIG_ENVELOPED_SIGNATURE,
+ XMLSecurityConstants.NS_C14N_OMIT_COMMENTS,
+ XMLSecurityConstants.NS_C14N_WITH_COMMENTS,
+ XMLSecurityConstants.NS_C14N11_OMIT_COMMENTS,
+ XMLSecurityConstants.NS_C14N11_WITH_COMMENTS,
+ XMLSecurityConstants.NS_C14N_EXCL_OMIT_COMMENTS,
+ XMLSecurityConstants.NS_C14N_EXCL_WITH_COMMENTS);
+
private EncryptionProperties encryptionProperties;
private SignatureProperties sigProps;
private String decryptionAlias;
@@ -243,6 +254,7 @@ public class XmlSecInInterceptor extends
AbstractPhaseInterceptor<Message> imple
@Override
public void registerSecurityEvent(SecurityEvent securityEvent)
throws XMLSecurityException {
if (securityEvent.getSecurityEventType() ==
SecurityEventConstants.AlgorithmSuite) {
+
checkSignatureTransform((AlgorithmSuiteSecurityEvent)securityEvent);
if (encryptionProperties != null) {
checkEncryptionAlgorithms((AlgorithmSuiteSecurityEvent)securityEvent);
}
@@ -262,6 +274,17 @@ public class XmlSecInInterceptor extends
AbstractPhaseInterceptor<Message> imple
return securityEventListener;
}
+ // Only allow transforms which cover the whole of the signed element, so
that
+ // no unsigned content is passed on to the application
+ private void checkSignatureTransform(AlgorithmSuiteSecurityEvent event)
+ throws XMLSecurityException {
+ if (XMLSecurityConstants.SigTransform.equals(event.getAlgorithmUsage())
+ && !ALLOWED_TRANSFORMS.contains(event.getAlgorithmURI())) {
+ throw new XMLSecurityException("empty", new Object[] {"The
signature transformation algorithm "
+ + event.getAlgorithmURI() + " is not allowed"});
+ }
+ }
+
private void checkEncryptionAlgorithms(AlgorithmSuiteSecurityEvent event)
throws XMLSecurityException {
if (XMLSecurityConstants.Enc.equals(event.getAlgorithmUsage())
@@ -503,6 +526,14 @@ public class XmlSecInInterceptor extends
AbstractPhaseInterceptor<Message> imple
new XMLSecurityException("empty", new Object[] {"The
request was not signed"});
throwFault(ex.getMessage(), ex);
}
+ // The signature must cover the document root, otherwise
unsigned content
+ // around the signed element would be passed on to the
application
+ if (!isRootSigned(incomingSecurityEventList)) {
+ LOG.warning("The request root element was not signed");
+ XMLSecurityException ex = new XMLSecurityException("empty",
+ new Object[] {"The request root element was not
signed"});
+ throwFault(ex.getMessage(), ex);
+ }
}
if (encryptionRequired) {
@@ -518,6 +549,18 @@ public class XmlSecInInterceptor extends
AbstractPhaseInterceptor<Message> imple
}
+ private boolean isRootSigned(List<SecurityEvent>
incomingSecurityEventList) {
+ for (SecurityEvent incomingEvent : incomingSecurityEventList) {
+ if (incomingEvent.getSecurityEventType() ==
SecurityEventConstants.SignedElement) {
+ SignedElementSecurityEvent signedEvent =
(SignedElementSecurityEvent)incomingEvent;
+ if (signedEvent.isSigned() &&
signedEvent.getElementPath().size() == 1) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private boolean isEventInResults(Event event, List<SecurityEvent>
incomingSecurityEventList) {
for (SecurityEvent incomingEvent : incomingSecurityEventList) {
if (event == incomingEvent.getSecurityEventType()) {
diff --git
a/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptorTest.java
b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptorTest.java
new file mode 100644
index 00000000000..40fc10401e5
--- /dev/null
+++
b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/xml/XmlSecInInterceptorTest.java
@@ -0,0 +1,256 @@
+/**
+ * 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.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.ListIterator;
+import java.util.Properties;
+
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
+import jakarta.ws.rs.WebApplicationException;
+import org.apache.cxf.bus.managers.PhaseManagerImpl;
+import org.apache.cxf.helpers.DOMUtils;
+import org.apache.cxf.interceptor.Interceptor;
+import org.apache.cxf.message.ExchangeImpl;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.phase.PhaseInterceptorChain;
+import org.apache.cxf.rt.security.SecurityConstants;
+import org.apache.cxf.staxutils.StaxUtils;
+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.assertTrue;
+import static org.junit.Assert.fail;
+
+public class XmlSecInInterceptorTest {
+
+ 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 {
+ Element root = process(createSignedDocument());
+
+ assertEquals("Book", root.getLocalName());
+ assertEquals("CXF", DOMUtils.getFirstElement(root).getTextContent());
+ }
+
+ @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 testDetachedSignatureIsRejected() 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());
+
+ assertRejected(doc);
+ }
+
+ @Test
+ public void testEnvelopingSignatureIsRejected() throws Exception {
+ assertRejected(createEnvelopingSignedDocument());
+ }
+
+ @Test
+ public void testBase64TransformIsRejected() throws Exception {
+ Document doc = DOMUtils.createDocument();
+ Element root = createBook(doc, "Q1hG");
+ doc.appendChild(root);
+ root.setAttributeNS(null, "Id", "_book");
+ 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_BASE64_DECODE);
+ sig.addDocument("#_book", transforms,
MessageDigestAlgorithm.ALGO_ID_DIGEST_SHA256);
+ sign(sig);
+
+ try {
+ process(doc);
+ fail("Failure expected on a Base64 transform");
+ } catch (XMLStreamException ex) {
+ assertTrue(ex.getMessage().contains("is not allowed"));
+ }
+ }
+
+ private static void assertRejected(Document doc) throws Exception {
+ try {
+ process(doc);
+ fail("Failure expected on a wrapped signature");
+ } catch (WebApplicationException ex) {
+ assertEquals(400, ex.getResponse().getStatus());
+ } catch (XMLStreamException ex) {
+ // expected
+ }
+ }
+
+ // Run the message through the XmlSecInInterceptor, read the (verified)
payload, and then run
+ // the interceptor it adds to the chain to check the required actions
+ private static Element process(Document doc) throws Exception {
+ Message message = new MessageImpl();
+ message.setExchange(new ExchangeImpl());
+ message.put(Message.HTTP_REQUEST_METHOD, "POST");
+ message.put(SecurityConstants.SIGNATURE_CRYPTO, crypto);
+ byte[] bytes =
StaxUtils.toString(doc).getBytes(StandardCharsets.UTF_8);
+ message.setContent(InputStream.class, new ByteArrayInputStream(bytes));
+ PhaseInterceptorChain chain = new PhaseInterceptorChain(new
PhaseManagerImpl().getInPhases());
+ message.setInterceptorChain(chain);
+
+ XmlSecInInterceptor interceptor = new XmlSecInInterceptor();
+ interceptor.setRequireSignature(true);
+ interceptor.handleMessage(message);
+
+ Document payload =
StaxUtils.read(message.getContent(XMLStreamReader.class));
+
+ ListIterator<Interceptor<? extends Message>> iterator =
chain.getIterator();
+ while (iterator.hasNext()) {
+ @SuppressWarnings("unchecked")
+ Interceptor<Message> next = (Interceptor<Message>)iterator.next();
+ next.handleMessage(message);
+ }
+ return payload.getDocumentElement();
+ }
+
+ 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);
+ }
+}
