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
commit a7cd9d2908081c83b0a9367c0b79eab6447a3242 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 52b498418..66b5a34b8 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 @@ -250,16 +250,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)); } } } @@ -434,8 +437,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 @@ -470,9 +473,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" @@ -495,6 +498,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 09e90fd07..24c991666 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 @@ -48,6 +48,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; @@ -100,11 +101,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()); } @@ -141,8 +142,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); } @@ -180,11 +181,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 @@ -213,6 +214,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. */
