This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/master by this push:
new 2d4a0e7da Switch to use BASE-64 encoding rather than 32 bit hash
values for signature confirmation (#725)
2d4a0e7da is described below
commit 2d4a0e7da15be3e97d83b20b739886cea724b5b4
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Mon Sep 21 15:46:41 2026 +0100
Switch to use BASE-64 encoding rather than 32 bit hash values for signature
confirmation (#725)
---
.../org/apache/wss4j/dom/handler/WSHandler.java | 29 ++++--
.../wss4j/dom/handler/WSHandlerConstants.java | 6 ++
.../dom/handler/SignatureConfirmationTest.java | 109 +++++++++++++++++++--
3 files changed, 126 insertions(+), 18 deletions(-)
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java
index 242803038..116bc2cf3 100644
--- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java
+++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandler.java
@@ -251,16 +251,19 @@ public abstract class WSHandler {
if (reqData.isEnableSignatureConfirmation()
&& isRequest && !reqData.getSignatureValues().isEmpty()) {
@SuppressWarnings("unchecked")
- Set<Integer> savedSignatures =
- (Set<Integer>)getProperty(reqData.getMsgContext(),
WSHandlerConstants.SEND_SIGV);
+ Set<String> savedSignatures =
+ (Set<String>)getProperty(reqData.getMsgContext(),
WSHandlerConstants.SEND_SIGV);
if (savedSignatures == null) {
savedSignatures = new HashSet<>();
setProperty(
reqData.getMsgContext(), WSHandlerConstants.SEND_SIGV,
savedSignatures
);
}
+ // The full signature value is stored, rather than its 32-bit
Arrays.hashCode, which
+ // is trivially collidable: a responder that did not process the
request could
+ // otherwise satisfy the SignatureConfirmation check with a value
it made up.
for (byte[] signatureValue : reqData.getSignatureValues()) {
- savedSignatures.add(Arrays.hashCode(signatureValue));
+ savedSignatures.add(encodeSignatureValue(signatureValue));
}
}
}
@@ -427,8 +430,8 @@ public abstract class WSHandler {
//
// First get all Signature values stored during sending the request
//
- Set<Integer> savedSignatures =
- (Set<Integer>) getProperty(reqData.getMsgContext(),
WSHandlerConstants.SEND_SIGV);
+ Set<String> savedSignatures =
+ (Set<String>) getProperty(reqData.getMsgContext(),
WSHandlerConstants.SEND_SIGV);
//
// Now get all results that hold a SignatureConfirmation element from
// the current run of receiver (we can have more than one run: if we
@@ -463,9 +466,9 @@ public abstract class WSHandler {
);
}
} else {
- Integer hash = Arrays.hashCode(sc.getSignatureValue());
- if (savedSignatures.contains(hash)) {
- savedSignatures.remove(hash);
+ String encodedValue =
encodeSignatureValue(sc.getSignatureValue());
+ if (savedSignatures.contains(encodedValue)) {
+ savedSignatures.remove(encodedValue);
} else {
throw new
WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "empty",
new Object[] {"Received a
SignatureConfirmation element, but there are no matching"
@@ -488,6 +491,16 @@ public abstract class WSHandler {
}
}
+ /**
+ * Encode a signature value for storage in, and lookup against, the set of
signature values
+ * saved for SignatureConfirmation. The whole value is retained: a hash
truncated to an int
+ * (as Arrays.hashCode produces) is trivially collidable, so matching on
one would let any
+ * peer satisfy the confirmation with a value of its own choosing.
+ */
+ private static String encodeSignatureValue(byte[] signatureValue) {
+ return Base64.getEncoder().encodeToString(signatureValue);
+ }
+
protected void decodeUTParameter(RequestData reqData)
throws WSSecurityException {
Object mc = reqData.getMsgContext();
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandlerConstants.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandlerConstants.java
index ce0191ed9..75b9058b7 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandlerConstants.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/handler/WSHandlerConstants.java
@@ -72,6 +72,12 @@ public final class WSHandlerConstants extends
ConfigurationConstants {
/**
* internally used property names to store values inside the message
context
* that must have the same lifetime as a message (request/response model).
+ *
+ * The value stored under this key is a {@code Set<String>} holding the
Base64 encoding of
+ * each outbound Signature value, which an inbound SignatureConfirmation
is matched against.
+ * Before WSS4J 4.0.2 it held a {@code Set<Integer>} of {@code
Arrays.hashCode} values; those
+ * are 32 bits wide and trivially collidable, so the confirmation they
backed was not a
+ * binding to the request's signature.
*/
public static final String SEND_SIGV = "_sendSignatureValues_";
diff --git
a/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/SignatureConfirmationTest.java
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/SignatureConfirmationTest.java
index 36704d6c2..aa3f61166 100644
---
a/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/SignatureConfirmationTest.java
+++
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/handler/SignatureConfirmationTest.java
@@ -44,6 +44,7 @@ import org.junit.jupiter.api.Test;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -96,11 +97,11 @@ public class SignatureConfirmationTest {
}
msgContext = (java.util.Map<String, Object>)reqData.getMsgContext();
- Set<Integer> savedSignatures =
- (Set<Integer>)msgContext.get(WSHandlerConstants.SEND_SIGV);
+ Set<String> savedSignatures =
+ (Set<String>)msgContext.get(WSHandlerConstants.SEND_SIGV);
assertTrue(savedSignatures != null && savedSignatures.size() == 1);
- Integer signatureValue = savedSignatures.iterator().next();
- assertTrue(signatureValue != null && signatureValue != 0);
+ String signatureValue = savedSignatures.iterator().next();
+ assertTrue(signatureValue != null && !signatureValue.isEmpty());
}
@@ -137,8 +138,8 @@ public class SignatureConfirmationTest {
}
msgContext = (java.util.Map<String, Object>)reqData.getMsgContext();
- Set<Integer> savedSignatures =
- (Set<Integer>)msgContext.get(WSHandlerConstants.SEND_SIGV);
+ Set<String> savedSignatures =
+ (Set<String>)msgContext.get(WSHandlerConstants.SEND_SIGV);
assertNull(savedSignatures);
}
@@ -176,11 +177,11 @@ public class SignatureConfirmationTest {
}
msgContext = (java.util.Map<String, Object>)reqData.getMsgContext();
- Set<Integer> savedSignatures =
- (Set<Integer>)msgContext.get(WSHandlerConstants.SEND_SIGV);
+ Set<String> savedSignatures =
+ (Set<String>)msgContext.get(WSHandlerConstants.SEND_SIGV);
assertTrue(savedSignatures != null && savedSignatures.size() == 1);
- Integer signatureValue = savedSignatures.iterator().next();
- assertTrue(signatureValue != null && signatureValue != 0);
+ String signatureValue = savedSignatures.iterator().next();
+ assertTrue(signatureValue != null && !signatureValue.isEmpty());
//
// Verify the inbound request, and create a response with a Signature
Confirmation
@@ -209,6 +210,94 @@ public class SignatureConfirmationTest {
}
+ /**
+ * A SignatureConfirmation whose Value is not the signature value of the
request, but merely
+ * collides with it under Arrays.hashCode, must be rejected. Such a
collision is trivial to
+ * construct - the hash is 32 bits wide and its recurrence is linear - so
matching on it would
+ * let any responder satisfy the confirmation with a value it never
computed.
+ */
+ @SuppressWarnings("unchecked")
+ @Test
+ public void
+ testSignatureConfirmationHashCollisionRejected() throws Exception {
+ final RequestData reqData = new RequestData();
+ java.util.Map<String, Object> msgContext = new java.util.TreeMap<>();
+ msgContext.put(WSHandlerConstants.ENABLE_SIGNATURE_CONFIRMATION,
"true");
+ msgContext.put(WSHandlerConstants.SIG_PROP_FILE, "crypto.properties");
+ msgContext.put("password", "security");
+ reqData.setMsgContext(msgContext);
+ reqData.setUsername("16c73ab6-b892-458f-abf5-2f875f74882e");
+
+ Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG);
+ CustomHandler handler = new CustomHandler();
+ HandlerAction action = new HandlerAction(WSConstants.SIGN);
+ handler.send(doc, reqData, Collections.singletonList(action), true);
+
+ //
+ // Verify the inbound request, and create a response with a Signature
Confirmation
+ //
+ WSHandlerResult results = verify(doc);
+ doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG);
+ msgContext = (java.util.Map<String, Object>)reqData.getMsgContext();
+ List<WSHandlerResult> receivedResults = new ArrayList<>();
+ receivedResults.add(results);
+ msgContext.put(WSHandlerConstants.RECV_RESULTS, receivedResults);
+ handler.send(
+ doc, reqData, Collections.singletonList(new
HandlerAction(WSConstants.NO_SECURITY)),
+ false
+ );
+
+ //
+ // Replace the SignatureConfirmation Value with a different byte
sequence that has the
+ // same Arrays.hashCode, as a hostile responder would.
+ //
+ Element sigConfElement =
+ XMLUtils.findElement(
+ doc.getDocumentElement(),
WSConstants.SIGNATURE_CONFIRMATION_LN, WSConstants.WSSE11_NS
+ );
+ assertNotNull(sigConfElement);
+ byte[] genuineValue =
+ org.apache.xml.security.utils.XMLUtils.decode(
+ sigConfElement.getAttributeNS(null,
SignatureConfirmation.SC_VALUE_ATTR));
+ byte[] collidingValue = hashCodeCollision(genuineValue);
+ assertFalse(Arrays.equals(genuineValue, collidingValue));
+ assertEquals(Arrays.hashCode(genuineValue),
Arrays.hashCode(collidingValue));
+ sigConfElement.setAttributeNS(
+ null, SignatureConfirmation.SC_VALUE_ATTR,
+
org.apache.xml.security.utils.XMLUtils.encodeToString(collidingValue));
+
+ results = verify(doc);
+ WSSecurityEngineResult scResult =
+ results.getActionResults().get(WSConstants.SC).get(0);
+ assertNotNull(scResult);
+
assertNotNull(scResult.get(WSSecurityEngineResult.TAG_SIGNATURE_CONFIRMATION));
+
+ try {
+ handler.signatureConfirmation(reqData, results);
+ fail("Failure expected on a SignatureConfirmation that only
collides with the "
+ + "signature value");
+ } catch (WSSecurityException ex) {
+ assertEquals(WSSecurityException.ErrorCode.FAILURE,
ex.getErrorCode());
+ }
+ }
+
+ /**
+ * Return a byte array that differs from the argument but has the same
Arrays.hashCode.
+ * Arrays.hashCode is h = 31*h + b, so adding one to a byte and
subtracting 31 from the byte
+ * after it leaves the result unchanged.
+ */
+ private static byte[] hashCodeCollision(byte[] value) {
+ for (int i = 0; i < value.length - 1; i++) {
+ if (value[i] < Byte.MAX_VALUE && value[i + 1] >= Byte.MIN_VALUE +
31) {
+ byte[] collision = value.clone();
+ collision[i]++;
+ collision[i + 1] -= 31;
+ return collision;
+ }
+ }
+ throw new IllegalStateException("No collision could be constructed for
this value");
+ }
+
/**
* Test to see that a signature confirmation response is correctly
processed.
*/