Skip to content

Prevent XML Signature wrapping in AbstractXmlSigInHandler #3504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
coheigea merged 3 commits into from
Sep 25, 2026
+332 −25
Merged

Prevent XML Signature wrapping in AbstractXmlSigInHandler #3504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
300efd4
Prevent XML Signature wrapping in AbstractXmlSigInHandler
coheigea Sep 25, 2026
a1bdd32
Apply suggestion from @reta
coheigea Sep 25, 2026
d267147
Fixup
coheigea Sep 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlEnvelopedInHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

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;
Expand Down Expand Up @@ -57,6 +58,7 @@ public void filter(ContainerRequestContext context) {
}

Document doc = null;
Element signedBody = null;
InputStream is = message.getContent(InputStream.class);
if (is != null) {
try {
Expand All @@ -67,7 +69,15 @@ public void filter(ContainerRequestContext context) {
} else {
XMLStreamReader reader = message.getContent(XMLStreamReader.class);
if (reader instanceof W3CDOMStreamReader) {
doc = ((W3CDOMStreamReader)reader).getDocument();
W3CDOMStreamReader w3cReader = (W3CDOMStreamReader)reader;
doc = w3cReader.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 = w3cReader.getCurrentNode();
if (node instanceof Element && node != doc.getDocumentElement()) {
signedBody = (Element)node;
}
}
}
if (doc == null) {
Expand All @@ -85,7 +95,11 @@ public void filter(ContainerRequestContext context) {
validateToken(message, samlElement);

doc.getDocumentElement().removeChild(samlElement);
if (bodyIsRoot) {
if (signedBody != null) {
message.setContent(XMLStreamReader.class,
Comment thread
reta marked this conversation as resolved.
new W3CDOMStreamReader(signedBody));
message.setContent(InputStream.class, null);
} else if (bodyIsRoot) {
message.setContent(XMLStreamReader.class,
new W3CDOMStreamReader(doc));
message.setContent(InputStream.class, null);
Expand Down
73 changes: 50 additions & 23 deletions rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlSigInHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

Expand Down Expand Up @@ -55,6 +56,15 @@

public class AbstractXmlSigInHandler extends AbstractXmlSecInHandler {

private static final Set<String> ALLOWED_TRANSFORMS = Set.of(
Transforms.TRANSFORM_ENVELOPED_SIGNATURE,
Transforms.TRANSFORM_C14N_OMIT_COMMENTS,
Transforms.TRANSFORM_C14N_WITH_COMMENTS,
Transforms.TRANSFORM_C14N11_OMIT_COMMENTS,
Transforms.TRANSFORM_C14N11_WITH_COMMENTS,
Transforms.TRANSFORM_C14N_EXCL_OMIT_COMMENTS,
Transforms.TRANSFORM_C14N_EXCL_WITH_COMMENTS);

private boolean removeSignature = true;
private boolean persistSignature = true;
private boolean keyInfoMustBeAvailable = true;
Expand Down Expand Up @@ -169,20 +179,23 @@ protected void checkSignature(Message message) {
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);

}
Expand All @@ -196,19 +209,6 @@ protected String getUserName(Crypto crypto, Message message) {

}

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;
Expand Down Expand Up @@ -262,20 +262,36 @@ protected Element validateReference(Element root, Reference ref) {
String c14TransformExpected = sigProps != null ? sigProps.getSignatureC14nTransform() : null;
boolean envelopedConfirmed = false;
for (int i = 0; i < transforms.getLength(); i++) {
String transformURI = null;
try {
Transform tr = transforms.item(i);
if (Transforms.TRANSFORM_ENVELOPED_SIGNATURE.equals(tr.getURI())) {
envelopedConfirmed = true;
} else if (c14TransformExpected != null && c14TransformExpected.equals(tr.getURI())) {
c14TransformConfirmed = true;
}
transformURI = tr.getURI();
} catch (Exception ex) {
throwFault("Problem accessing Transform instance", ex);
}
// Only allow transforms which cover the whole of the signed element, so that
// no unsigned content (e.g. excluded via XPath) is passed on to the application
if (!ALLOWED_TRANSFORMS.contains(transformURI)) {
throwFault("Signature Transform is not supported", null);
}
if (Transforms.TRANSFORM_ENVELOPED_SIGNATURE.equals(transformURI)) {
envelopedConfirmed = true;
} else if (c14TransformExpected != null && c14TransformExpected.equals(transformURI)) {
c14TransformConfirmed = true;
}
}
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);
}
Expand All @@ -291,6 +307,17 @@ protected Element validateReference(Element root, Reference ref) {
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);
Expand Down
Loading
Loading