This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 2_4_x-fixes
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/2_4_x-fixes by this push:
new b2e3639df Stricter policy enforcement (#715)
b2e3639df is described below
commit b2e3639dfc51e486f5049a53044bcb982b5e0d41
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon Sep 21 08:02:31 2026 +0100
Stricter policy enforcement (#715)
---
.../wss4j/policy/stax/enforcer/PolicyEnforcer.java | 48 +++++++-
.../stax/enforcer/PolicyEnforcerFactory.java | 82 ++++++++++++-
.../policy/stax/test/VulnerabliltyVectorsTest.java | 129 +++++++++++++++++++++
.../test/resources/testdata/wsdl/rpcOperation.wsdl | 64 ++++++++++
4 files changed, 318 insertions(+), 5 deletions(-)
diff --git
a/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcer.java
b/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcer.java
index c95fde087..5e48f7281 100644
---
a/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcer.java
+++
b/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcer.java
@@ -204,8 +204,7 @@ public class PolicyEnforcer implements
SecurityEventListener {
if (operationName != null) {
if (soapOperationName.equals(operationName)) {
return operationPolicy;
- } else if ((operationName.getNamespaceURI() == null ||
operationName.getNamespaceURI().length() == 0)
- &&
soapOperationName.getLocalPart().equals(operationName.getLocalPart())) {
+ } else if (matchesOnLocalPartAlone(operationName,
soapOperationName)) {
noNamespaceOperation = operationPolicy;
}
}
@@ -221,8 +220,49 @@ public class PolicyEnforcer implements
SecurityEventListener {
if (policyOperationName.equals(soapOperationName)) {
return true;
}
- return (policyOperationName.getNamespaceURI() == null ||
policyOperationName.getNamespaceURI().length() == 0)
- &&
soapOperationName.getLocalPart().equals(policyOperationName.getLocalPart());
+ return matchesOnLocalPartAlone(policyOperationName, soapOperationName);
+ }
+
+ /**
+ * A policy operation name that carries no namespace can only be compared
against the local
+ * part of the Body element - the namespace it should have is simply not
known. That
+ * comparison is a last resort for operations whose QName could not be
determined, and it
+ * accepts a Body element in ANY namespace.
+ *
+ * It is only defensible while that local part still identifies one
operation. As soon as a
+ * second operation shares it, matching on the local part alone no longer
says which of the
+ * two the SOAP stack will dispatch to, and an attacker can pick whichever
of the two
+ * policies is the weaker by varying nothing but the namespace of the Body
element - or, on
+ * the SOAPAction path, by naming the weaker operation in the SOAPAction
header while the
+ * Body dispatches to the stronger one. Refuse the loose match in that
case, so that the
+ * caller fails closed instead of enforcing a policy that belongs to
another operation.
+ */
+ private boolean matchesOnLocalPartAlone(QName policyOperationName, QName
soapOperationName) {
+ String policyNamespace = policyOperationName.getNamespaceURI();
+ if (policyNamespace != null && policyNamespace.length() > 0) {
+ return false;
+ }
+ String localPart = policyOperationName.getLocalPart();
+ if (!soapOperationName.getLocalPart().equals(localPart)) {
+ return false;
+ }
+ if (countOperationsNamed(localPart) > 1) {
+ LOG.warn("More than one operation is named {}; refusing to match
the policy for it "
+ + "against {} on the local name alone", localPart,
soapOperationName);
+ return false;
+ }
+ return true;
+ }
+
+ private int countOperationsNamed(String localPart) {
+ int count = 0;
+ for (OperationPolicy operationPolicy : operationPolicies) {
+ QName operationName = operationPolicy.getOperationName();
+ if (operationName != null &&
localPart.equals(operationName.getLocalPart())) {
+ count++;
+ }
+ }
+ return count;
}
/**
diff --git
a/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcerFactory.java
b/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcerFactory.java
index d25bbfde3..7f32cfcae 100644
---
a/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcerFactory.java
+++
b/ws-security-policy-stax/src/main/java/org/apache/wss4j/policy/stax/enforcer/PolicyEnforcerFactory.java
@@ -26,6 +26,7 @@ import java.util.List;
import java.util.Map;
import javax.wsdl.Binding;
+import javax.wsdl.BindingInput;
import javax.wsdl.BindingOperation;
import javax.wsdl.Definition;
import javax.wsdl.Message;
@@ -38,7 +39,11 @@ import javax.wsdl.WSDLElement;
import javax.wsdl.WSDLException;
import javax.wsdl.extensions.ExtensibilityElement;
import javax.wsdl.extensions.UnknownExtensibilityElement;
+import javax.wsdl.extensions.soap.SOAPBinding;
+import javax.wsdl.extensions.soap.SOAPBody;
import javax.wsdl.extensions.soap.SOAPOperation;
+import javax.wsdl.extensions.soap12.SOAP12Binding;
+import javax.wsdl.extensions.soap12.SOAP12Body;
import javax.wsdl.extensions.soap12.SOAP12Operation;
import javax.wsdl.factory.WSDLFactory;
import javax.wsdl.xml.WSDLReader;
@@ -258,7 +263,15 @@ public class PolicyEnforcerFactory {
}
}
if (operationName == null) {
- operationName = new QName(null, operation.getName());
+ //For rpc style the Body child element is the
operation name, qualified
+ //with the namespace given on the soap:body of the
binding input, so the
+ //QName that will be reported in the
OperationSecurityEvent is knowable
+ //here too. Registering it keeps the operation out of
the local-name-only
+ //fallback in PolicyEnforcer, which matches a Body
element in ANY
+ //namespace. Where the namespace is not given we are
no worse off than
+ //before and still register the bare operation name.
+ operationName = new
QName(rpcStyleBodyNamespace(binding, bindingOperation),
+ operation.getName());
}
OperationPolicy operationPolicy = new
OperationPolicy(operationName);
operationPolicyList.add(operationPolicy);
@@ -288,6 +301,73 @@ public class PolicyEnforcerFactory {
return operationPolicyList;
}
+ /**
+ * Returns the namespace that the SOAP Body child element carries for an
rpc style
+ * operation - the namespace attribute of its soap:body - or null when the
operation is
+ * not rpc style or when no namespace is given.
+ */
+ private String rpcStyleBodyNamespace(Binding binding, BindingOperation
bindingOperation) {
+ if (!isRpcStyle(binding, bindingOperation)) {
+ return null;
+ }
+ BindingInput bindingInput = bindingOperation.getBindingInput();
+ if (bindingInput == null) {
+ return null;
+ }
+ List<?> extensibilityElements =
bindingInput.getExtensibilityElements();
+ if (extensibilityElements == null) {
+ return null;
+ }
+ for (int i = 0; i < extensibilityElements.size(); i++) {
+ Object extensibilityElement = extensibilityElements.get(i);
+ String namespace = null;
+ if (extensibilityElement instanceof SOAPBody) {
+ namespace = ((SOAPBody)
extensibilityElement).getNamespaceURI();
+ } else if (extensibilityElement instanceof SOAP12Body) {
+ namespace = ((SOAP12Body)
extensibilityElement).getNamespaceURI();
+ }
+ if (namespace != null && namespace.length() > 0) {
+ return namespace;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * The style is given either on the binding operation itself or, failing
that, on the
+ * binding. The default in WSDL 1.1 is "document".
+ */
+ private boolean isRpcStyle(Binding binding, BindingOperation
bindingOperation) {
+ String style = findStyle(bindingOperation.getExtensibilityElements());
+ if (style == null) {
+ style = findStyle(binding.getExtensibilityElements());
+ }
+ return "rpc".equals(style);
+ }
+
+ private String findStyle(List<?> extensibilityElements) {
+ if (extensibilityElements == null) {
+ return null;
+ }
+ for (int i = 0; i < extensibilityElements.size(); i++) {
+ Object extensibilityElement = extensibilityElements.get(i);
+ String style = null;
+ if (extensibilityElement instanceof SOAPOperation) {
+ style = ((SOAPOperation) extensibilityElement).getStyle();
+ } else if (extensibilityElement instanceof SOAP12Operation) {
+ style = ((SOAP12Operation) extensibilityElement).getStyle();
+ } else if (extensibilityElement instanceof SOAPBinding) {
+ style = ((SOAPBinding) extensibilityElement).getStyle();
+ } else if (extensibilityElement instanceof SOAP12Binding) {
+ style = ((SOAP12Binding) extensibilityElement).getStyle();
+ }
+ if (style != null && style.length() > 0) {
+ return style;
+ }
+ }
+ return null;
+ }
+
private Policy getPolicy(Service service, Port port, Binding binding,
BindingOperation bindingOperation, Operation
operation) throws WSSPolicyException {
List<Policy> policies = new ArrayList<>();
diff --git
a/ws-security-policy-stax/src/test/java/org/apache/wss4j/policy/stax/test/VulnerabliltyVectorsTest.java
b/ws-security-policy-stax/src/test/java/org/apache/wss4j/policy/stax/test/VulnerabliltyVectorsTest.java
index 8f83f24d6..7a6ba07b8 100644
---
a/ws-security-policy-stax/src/test/java/org/apache/wss4j/policy/stax/test/VulnerabliltyVectorsTest.java
+++
b/ws-security-policy-stax/src/test/java/org/apache/wss4j/policy/stax/test/VulnerabliltyVectorsTest.java
@@ -32,8 +32,10 @@ import javax.xml.transform.stream.StreamResult;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
+import org.apache.neethi.Policy;
import org.apache.wss4j.common.ext.WSSecurityException;
import org.apache.wss4j.dom.handler.WSHandlerConstants;
+import org.apache.wss4j.policy.stax.OperationPolicy;
import org.apache.wss4j.policy.stax.enforcer.PolicyEnforcer;
import org.apache.wss4j.policy.stax.enforcer.PolicyEnforcerFactory;
import org.apache.wss4j.policy.stax.enforcer.PolicyInputProcessor;
@@ -124,6 +126,133 @@ public class VulnerabliltyVectorsTest extends
AbstractTestBase {
assertEquals(WSSecurityException.INVALID_SECURITY, ex.getFaultCode());
}
+ /**
+ * An operation whose policy is registered without a namespace is matched
on its local name
+ * alone, which accepts a Body element in any namespace. That is only
tenable while the
+ * local name identifies one operation: once a second operation shares it,
the local name no
+ * longer says which of the two the SOAP stack will dispatch to, and the
policy of the one
+ * could be enforced for a message that invokes the other. Fail closed
instead.
+ */
+ @Test
+ public void
testAmbiguousNoNamespaceOperationNameDoesNotMatchAnyNamespace() throws
Exception {
+ List<OperationPolicy> operationPolicies =
+ List.of(operationPolicy(new QName(null, "getBalance"), null),
+ operationPolicy(new
QName("http://www.example.net/secure";, "getBalance"), null));
+
+ PolicyEnforcer policyEnforcer =
+ new PolicyEnforcer(operationPolicies, "", false, null, 0,
null, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://www.example.net/other";, "getBalance"));
+
+ WSSecurityException ex = assertThrows(WSSecurityException.class,
+ () ->
policyEnforcer.registerSecurityEvent(operationSecurityEvent));
+
+ assertEquals(WSSecurityException.INVALID_SECURITY, ex.getFaultCode());
+ }
+
+ /**
+ * The same ambiguity on the SOAPAction path: the SOAPAction header
selects the policy of the
+ * operation registered without a namespace, while the Body element
dispatches to the other
+ * operation of that name. The cross-check added for SOAPAction spoofing
must reject this.
+ */
+ @Test
+ public void
testAmbiguousNoNamespaceOperationNameDoesNotSatisfyTheSOAPActionCheck() throws
Exception {
+ List<OperationPolicy> operationPolicies =
+ List.of(operationPolicy(new QName(null, "getBalance"),
"urn:getBalanceUnsecured"),
+ operationPolicy(new
QName("http://www.example.net/secure";, "getBalance"), "urn:getBalance"));
+
+ PolicyEnforcer policyEnforcer =
+ new PolicyEnforcer(operationPolicies,
"urn:getBalanceUnsecured", false, null, 0, null, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://www.example.net/secure";, "getBalance"));
+
+ WSSecurityException ex = assertThrows(WSSecurityException.class,
+ () ->
policyEnforcer.registerSecurityEvent(operationSecurityEvent));
+
+ assertEquals("SOAPAction (urn:getBalanceUnsecured) does not match with
the current Operation: "
+ + "{http://www.example.net/secure}getBalance";,
+ ex.getCause().getMessage());
+ assertEquals(WSSecurityException.INVALID_SECURITY, ex.getFaultCode());
+ }
+
+ /**
+ * An operation name that carries a namespace is matched exactly, so the
policy of the
+ * namespace-less operation of the same name is not selected for it.
+ */
+ @Test
+ public void testNamespacedOperationNameSelectsItsOwnPolicy() throws
Exception {
+ List<OperationPolicy> operationPolicies =
+ List.of(operationPolicy(new QName(null, "getBalance"), null),
+ operationPolicy(new
QName("http://www.example.net/secure";, "getBalance"), null));
+
+ PolicyEnforcer policyEnforcer =
+ new PolicyEnforcer(operationPolicies, "", false, null, 0,
null, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://www.example.net/secure";, "getBalance"));
+
+ policyEnforcer.registerSecurityEvent(operationSecurityEvent);
+ }
+
+ /**
+ * Where the local name is unambiguous the fallback is kept, so an
integrator that registers
+ * its operations without a namespace keeps working.
+ */
+ @Test
+ public void testUnambiguousNoNamespaceOperationNameStillMatches() throws
Exception {
+ List<OperationPolicy> operationPolicies =
+ List.of(operationPolicy(new QName(null, "getBalance"), null));
+
+ PolicyEnforcer policyEnforcer =
+ new PolicyEnforcer(operationPolicies, "", false, null, 0,
null, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://www.example.net/other";, "getBalance"));
+
+ policyEnforcer.registerSecurityEvent(operationSecurityEvent);
+ }
+
+ /**
+ * An rpc style operation is registered under the QName that will actually
appear as the Body
+ * child element - the operation name qualified with the namespace from
soap:body - and so is
+ * matched exactly rather than on its local name alone.
+ */
+ @Test
+ public void testRpcOperationIsBoundToItsSOAPBodyNamespace() throws
Exception {
+ PolicyEnforcerFactory policyEnforcerFactory =
PolicyEnforcerFactory.newInstance(
+
this.getClass().getClassLoader().getResource("testdata/wsdl/rpcOperation.wsdl"));
+ PolicyEnforcer policyEnforcer =
policyEnforcerFactory.newPolicyEnforcer("", false, null, 0, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://www.example.net/rpc";, "getBalance"));
+
+ policyEnforcer.registerSecurityEvent(operationSecurityEvent);
+ }
+
+ @Test
+ public void testRpcOperationInAnotherNamespaceIsRejected() throws
Exception {
+ PolicyEnforcerFactory policyEnforcerFactory =
PolicyEnforcerFactory.newInstance(
+
this.getClass().getClassLoader().getResource("testdata/wsdl/rpcOperation.wsdl"));
+ PolicyEnforcer policyEnforcer =
policyEnforcerFactory.newPolicyEnforcer("", false, null, 0, false);
+
+ OperationSecurityEvent operationSecurityEvent = new
OperationSecurityEvent();
+ operationSecurityEvent.setOperation(new
QName("http://example.com/evil";, "getBalance"));
+
+ WSSecurityException ex = assertThrows(WSSecurityException.class,
+ () ->
policyEnforcer.registerSecurityEvent(operationSecurityEvent));
+
+ assertEquals(WSSecurityException.INVALID_SECURITY, ex.getFaultCode());
+ }
+
+ private static OperationPolicy operationPolicy(QName operationName, String
soapAction) {
+ OperationPolicy operationPolicy = new OperationPolicy(operationName);
+ operationPolicy.setPolicy(new Policy().normalize(true));
+ operationPolicy.setOperationAction(soapAction);
+ return operationPolicy;
+ }
+
@Test
public void testSignedBodyRelocationToHeader() throws Exception {
InputStream sourceDocument =
this.getClass().getClassLoader().getResourceAsStream("testdata/plain-soap-1.1.xml");
diff --git
a/ws-security-policy-stax/src/test/resources/testdata/wsdl/rpcOperation.wsdl
b/ws-security-policy-stax/src/test/resources/testdata/wsdl/rpcOperation.wsdl
new file mode 100644
index 000000000..d8f178a52
--- /dev/null
+++ b/ws-security-policy-stax/src/test/resources/testdata/wsdl/rpcOperation.wsdl
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<wsdl:definitions
+ name="RpcService"
+ targetNamespace="http://www.example.net/RpcService";
+ xmlns:tns="http://www.example.net/RpcService";
+ xmlns:xs="http://www.w3.org/2001/XMLSchema";
+ xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/";
+ xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/";
+ >
+
+ <!-- An rpc style operation: the Body child element is the operation name
qualified
+ with the namespace given below on soap:body, not a schema element. -->
+
+ <wsdl:message name="getBalanceRequest">
+ <wsdl:part name="account" type="xs:string"/>
+ </wsdl:message>
+ <wsdl:message name="getBalanceResponse">
+ <wsdl:part name="balance" type="xs:string"/>
+ </wsdl:message>
+
+ <wsdl:portType name="RpcPort">
+ <wsdl:operation name="getBalance">
+ <wsdl:input message="tns:getBalanceRequest"/>
+ <wsdl:output message="tns:getBalanceResponse"/>
+ </wsdl:operation>
+ </wsdl:portType>
+
+ <wsdl:binding name="RpcSOAPBinding" type="tns:RpcPort">
+ <soap:binding transport="http://schemas.xmlsoap.org/soap/http";
style="rpc"/>
+ <wsdl:operation name="getBalance">
+ <soap:operation soapAction="getBalance" style="rpc"/>
+ <wsdl:input>
+ <soap:body use="literal"
namespace="http://www.example.net/rpc"/>
+ </wsdl:input>
+ <wsdl:output>
+ <soap:body use="literal"
namespace="http://www.example.net/rpc"/>
+ </wsdl:output>
+ </wsdl:operation>
+ </wsdl:binding>
+
+ <wsdl:service name="RpcService">
+ <wsdl:port name="Rpc" binding="tns:RpcSOAPBinding">
+ <soap:address location="http://localhost:8080/RpcService"/>
+ </wsdl:port>
+ </wsdl:service>
+</wsdl:definitions>
