This is an automated email from the ASF dual-hosted git repository. robertlazarski pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/axis-axis2-java-rampart.git
commit 9d0abc1ba487aabd5978ff32f909e1f9ee24434e Author: Robert Lazarski <[email protected]> AuthorDate: Tue Jun 9 16:04:30 2026 -1000 RAMPART-428: validate the signed SOAP Body by identity, not by name The signed-parts check confirmed only that *some* element named "Body" was covered by the signature (actuallySigned.contains({soap}Body)). Because that test is name-based, a message that relocates the signed Body into a wrapper and places a different, unsigned Body in the Body position would still pass validation while the application consumed the unsigned Body (signature wrapping). validateSignedPartsHeaders now resolves the actual SOAP Body element that will be consumed (WSSecurityUtil.findBodyElement) and requires that exact element, by node identity, to be among the elements the signature covered. The signed elements are now tracked as DOM elements alongside their QNames for this check. Adds PolicyBasedResultsValidatorTest covering the identity-based decision (relocated same-name body rejected, actual signed body accepted). Verified with a full clean -Papache-release verify across all modules, including the signed-body integration scenarios and the nine policy samples, on JDK 25. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --- .../rampart/PolicyBasedResultsValidator.java | 46 ++++++++-- .../rampart/PolicyBasedResultsValidatorTest.java | 99 ++++++++++++++++++++++ 2 files changed, 136 insertions(+), 9 deletions(-) diff --git a/modules/rampart-core/src/main/java/org/apache/rampart/PolicyBasedResultsValidator.java b/modules/rampart-core/src/main/java/org/apache/rampart/PolicyBasedResultsValidator.java index a06a0ac8..250d7a52 100644 --- a/modules/rampart-core/src/main/java/org/apache/rampart/PolicyBasedResultsValidator.java +++ b/modules/rampart-core/src/main/java/org/apache/rampart/PolicyBasedResultsValidator.java @@ -590,6 +590,25 @@ public class PolicyBasedResultsValidator implements ExtendedPolicyValidatorCallb } + /** + * Returns true only if {@code target} is, by node identity, one of the elements + * actually covered by the signature. This is the core of the signature-wrapping + * defence (RAMPART-428): an attacker can place an element with the same QName as + * a signed element elsewhere in the message, so a name-based check is not enough - + * the consumed element itself must be the signed one. + */ + static boolean isElementSigned(Element target, List<Element> signedElements) { + if (target == null || signedElements == null) { + return false; + } + for (Element signedElement : signedElements) { + if (signedElement != null && signedElement.isSameNode(target)) { + return true; + } + } + return false; + } + protected void validateSignedPartsHeaders(ValidatorData data, List<WSEncryptionPart> signatureParts, List<WSSecurityEngineResult> results) throws RampartException { @@ -599,8 +618,12 @@ public class PolicyBasedResultsValidator implements ExtendedPolicyValidatorCallb WSSecurityEngineResult[] actionResults = fetchActionResults(results, WSConstants.SIGN); - // Find elements that are signed + // Find elements that are signed. We keep both the QNames (used for header + // checks) and the actual signed DOM elements. The latter lets us verify the + // signed element by identity rather than just by name, which is required to + // defeat signature-wrapping attacks (RAMPART-428). List<QName> actuallySigned = new ArrayList<QName>(); + List<Element> actuallySignedElements = new ArrayList<Element>(); if (actionResults != null) { for (WSSecurityEngineResult actionResult : actionResults) { @@ -620,6 +643,7 @@ public class PolicyBasedResultsValidator implements ExtendedPolicyValidatorCallb String ns = (nodeList.item(x)).getNamespaceURI(); String ln = (nodeList.item(x)).getLocalName(); actuallySigned.add(new QName(ns, ln)); + actuallySignedElements.add((Element) nodeList.item(x)); break; } } @@ -627,6 +651,7 @@ public class PolicyBasedResultsValidator implements ExtendedPolicyValidatorCallb String ns = protectedElement.getNamespaceURI(); String ln = protectedElement.getLocalName(); actuallySigned.add(new QName(ns, ln)); + actuallySignedElements.add(protectedElement); } } @@ -636,16 +661,19 @@ public class PolicyBasedResultsValidator implements ExtendedPolicyValidatorCallb for (WSEncryptionPart wsep : signatureParts) { if (wsep.getName().equals(WSConstants.ELEM_BODY)) { - QName bodyQName; + // RAMPART-428: verify the actual SOAP Body element that the + // application will consume is the very element that was signed, + // not merely that *some* element named "Body" was signed. A + // signature-wrapping attack relocates the signed body into a + // wrapper and inserts a new, unsigned body in the Body position; + // a QName-only check would pass that message. Comparing by node + // identity rejects it. + Element actualBody = WSSecurityUtil.findBodyElement(rmd.getDocument()); - if (WSConstants.URI_SOAP11_ENV.equals(envelope.getNamespaceURI())) { - bodyQName = new SOAP11Constants().getBodyQName(); - } else { - bodyQName = new SOAP12Constants().getBodyQName(); - } + boolean bodySigned = isElementSigned(actualBody, actuallySignedElements); - if (!actuallySigned.contains(bodyQName) && !rmd.getPolicyData().isSignBodyOptional()) { - // soap body is not signed + if (!bodySigned && !rmd.getPolicyData().isSignBodyOptional()) { + // soap body is not signed (or a different element was signed) throw new RampartException("bodyNotSigned"); } diff --git a/modules/rampart-core/src/test/java/org/apache/rampart/PolicyBasedResultsValidatorTest.java b/modules/rampart-core/src/test/java/org/apache/rampart/PolicyBasedResultsValidatorTest.java new file mode 100644 index 00000000..47e3a26f --- /dev/null +++ b/modules/rampart-core/src/test/java/org/apache/rampart/PolicyBasedResultsValidatorTest.java @@ -0,0 +1,99 @@ +/* + * 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.rampart; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; + +import junit.framework.TestCase; + +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; + +/** + * Tests the signature-wrapping defence in {@link PolicyBasedResultsValidator} + * (RAMPART-428): the SOAP Body the application consumes must be the very element + * that was signed, identified by node identity rather than by element name. + */ +public class PolicyBasedResultsValidatorTest extends TestCase { + + private static final String SOAP_NS = "http://schemas.xmlsoap.org/soap/envelope/";; + + // A signature-wrapping layout: the original (signed) Body is relocated into a + // <wrapper> inside the header, and a new arbitrary Body sits in the Body position. + private static final String WRAPPED = + "<env:Envelope xmlns:env='" + SOAP_NS + "'>" + + " <env:Header>" + + " <wrapper xmlns=''>" + + " <env:Body env:id='signed'><echo>original</echo></env:Body>" + + " </wrapper>" + + " </env:Header>" + + " <env:Body><echo>arbitrary</echo></env:Body>" + + "</env:Envelope>"; + + private Document doc; + private Element realBody; // the Body the application consumes (Body position) + private Element wrappedBody; // the relocated, signed Body + + protected void setUp() throws Exception { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + DocumentBuilder db = dbf.newDocumentBuilder(); + doc = db.parse(new ByteArrayInputStream(WRAPPED.getBytes(StandardCharsets.UTF_8))); + + NodeList bodies = doc.getElementsByTagNameNS(SOAP_NS, "Body"); + assertEquals("test fixture should contain two Body elements", 2, bodies.getLength()); + // Document order: the wrapped (signed) Body comes first (inside the header), + // the real consumed Body is the direct child of Envelope. + wrappedBody = (Element) bodies.item(0); + realBody = (Element) bodies.item(1); + assertEquals("Envelope", realBody.getParentNode().getLocalName()); + assertEquals("wrapper", wrappedBody.getParentNode().getLocalName()); + } + + /** Wrapping attack: only the relocated copy is signed, so the real body must be rejected. */ + public void testWrappedBodyIsNotConsideredSigned() { + assertFalse("a relocated same-name Body must not count as the signed body", + PolicyBasedResultsValidator.isElementSigned(realBody, Collections.singletonList(wrappedBody))); + } + + /** Legitimate case: the real consumed body is itself the signed element. */ + public void testActualSignedBodyIsAccepted() { + assertTrue("the actual signed body must be recognised", + PolicyBasedResultsValidator.isElementSigned(realBody, Collections.singletonList(realBody))); + } + + /** Mixed set still matches by identity. */ + public void testAcceptedWhenRealBodyAmongSignedElements() { + assertTrue(PolicyBasedResultsValidator.isElementSigned( + realBody, Arrays.asList(wrappedBody, realBody))); + } + + public void testNullInputsAreNotSigned() { + assertFalse(PolicyBasedResultsValidator.isElementSigned(null, Collections.singletonList(realBody))); + assertFalse(PolicyBasedResultsValidator.isElementSigned(realBody, null)); + assertFalse(PolicyBasedResultsValidator.isElementSigned(realBody, Collections.<Element>emptyList())); + } +}
