This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/signature-replay in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit 361615f632d23be43158278cb5f7a897038c954f Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Thu Sep 17 06:42:15 2026 +0100 Fix signature replay caching --- .../wss4j/dom/processor/SignatureProcessor.java | 91 ++++++++++++++++++---- .../org/apache/wss4j/dom/message/ReplayTest.java | 72 +++++++++++++++++ 2 files changed, 150 insertions(+), 13 deletions(-) diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/SignatureProcessor.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/SignatureProcessor.java index 2f2a55490..941a3270a 100644 --- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/SignatureProcessor.java +++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/processor/SignatureProcessor.java @@ -19,15 +19,19 @@ package org.apache.wss4j.dom.processor; +import java.nio.charset.StandardCharsets; import java.security.Key; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.Principal; import java.security.Provider; import java.security.PublicKey; import java.security.cert.X509Certificate; import java.security.spec.AlgorithmParameterSpec; +import java.time.Instant; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Base64; import java.util.List; import java.util.Map; @@ -374,12 +378,21 @@ public class SignatureProcessor implements Processor { } // Test for replay attacks - testMessageReplay(elem, xmlSignature.getSignatureValue().getValue(), key, data, wsDocInfo); + ReplayCacheEntry replayCacheEntry = + testMessageReplay(elem, xmlSignature.getSignatureValue().getValue(), key, data, wsDocInfo); setElementsOnContext(xmlSignature, (DOMValidateContext)context, data, wsDocInfo); boolean signatureOk = xmlSignature.validate(context); if (signatureOk) { + // Only now that the signature has actually been validated may the identifier be + // added to the replay cache. Adding it beforehand lets an attacker poison the + // cache with the identifier of a message whose signature does not verify - by + // replaying a genuine message with a tampered payload, for example - so that the + // genuine message is subsequently rejected as a replay. + if (replayCacheEntry != null) { + replayCacheEntry.add(); + } return xmlSignature; } // @@ -632,9 +645,11 @@ public class SignatureProcessor implements Processor { * @param key * @param requestData * @param wsDocInfo + * @return a pending ReplayCacheEntry, to be added to the cache once the signature has been + * validated, or null if no replay checking was performed * @throws WSSecurityException */ - private void testMessageReplay( + private ReplayCacheEntry testMessageReplay( Element signatureElement, byte[] signatureValue, Key key, @@ -643,7 +658,7 @@ public class SignatureProcessor implements Processor { ) throws WSSecurityException { ReplayCache replayCache = requestData.getTimestampReplayCache(); //NOPMD if (replayCache == null) { - return; + return null; } // Find the Timestamp @@ -665,12 +680,13 @@ public class SignatureProcessor implements Processor { timeStamp = (Timestamp)foundResults.get(0).get(WSSecurityEngineResult.TAG_TIMESTAMP); } if (timeStamp == null) { - return; + return null; } - // Test for replay attacks - String identifier = timeStamp.getCreatedString() + "" + Arrays.hashCode(signatureValue) - + "" + Arrays.hashCode(key.getEncoded()); + // Test for replay attacks. The identifier is a digest over the full Timestamp Created + // String, signature value and encoded signing key, rather than over the 32-bit hashCodes + // of the latter two, which are trivially collidable. + String identifier = createIdentifier(timeStamp.getCreatedString(), signatureValue, key); if (replayCache.contains(identifier)) { throw new WSSecurityException( @@ -679,11 +695,60 @@ public class SignatureProcessor implements Processor { new Object[] {"A replay attack has been detected"}); } - // Store the Timestamp/SignatureValue/Key combination in the cache - if (timeStamp.getExpires() != null) { - replayCache.add(identifier, timeStamp.getExpires()); - } else { - replayCache.add(identifier); + // Return the Timestamp/SignatureValue/Key combination so that it can be stored in the + // cache once the signature has been validated + return new ReplayCacheEntry(replayCache, identifier, timeStamp.getExpires()); + } + + /** + * Create the replay cache identifier for the given Timestamp Created String, signature value + * and signing key. The values are separated by a '|', which can appear in neither a Created + * String nor Base64, so that they cannot run together into an ambiguous identifier. + */ + private static String createIdentifier( + String createdString, byte[] signatureValue, Key key + ) throws WSSecurityException { + // getEncoded() returns null for a key whose material cannot be extracted, for example one + // held in a hardware token. The signature value alone still identifies the message then. + byte[] keyBytes = key.getEncoded(); + String identifier = createdString + + "|" + encode(signatureValue) + + "|" + encode(keyBytes); + + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return encode(digest.digest(identifier.getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException ex) { + throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, ex); + } + } + + private static String encode(byte[] value) { + return value == null ? "" : Base64.getEncoder().encodeToString(value); + } + + /** + * A pending replay cache addition: the Timestamp/SignatureValue/Key identifier of the current + * message, to be added to the ReplayCache only once the signature it was derived from has + * been successfully validated. + */ + private static final class ReplayCacheEntry { + private final ReplayCache replayCache; + private final String identifier; + private final Instant expires; + + ReplayCacheEntry(ReplayCache replayCache, String identifier, Instant expires) { + this.replayCache = replayCache; + this.identifier = identifier; + this.expires = expires; + } + + void add() { + if (expires != null) { + replayCache.add(identifier, expires); + } else { + replayCache.add(identifier); + } } } diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/ReplayTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/ReplayTest.java index 2cdaa24ad..3edebe460 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/ReplayTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/ReplayTest.java @@ -55,6 +55,8 @@ import org.junit.jupiter.api.io.TempDir; 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.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -130,6 +132,76 @@ public class ReplayTest { } } + /** + * A message whose Signature fails to validate must not leave its identifier in the replay + * cache. Otherwise an attacker can take a genuine message, tamper with a signed part of it so + * that the Signature no longer validates, and send that first: the tampered message is + * rejected, but it has claimed the genuine message's replay cache identifier - the Timestamp, + * SignatureValue and signing key are untouched - so the genuine message is then rejected as a + * replay when it arrives. + */ + @Test + public void testReplayCacheNotPoisonedByInvalidSignature() throws Exception { + + Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG); + WSSecHeader secHeader = new WSSecHeader(doc); + secHeader.insertSecurityHeader(); + + WSSecTimestamp timestamp = new WSSecTimestamp(secHeader); + timestamp.setTimeToLive(300); + Document createdDoc = timestamp.build(); + + WSSecSignature builder = new WSSecSignature(secHeader); + builder.setUserInfo("16c73ab6-b892-458f-abf5-2f875f74882e", "security"); + builder.setKeyIdentifierType(WSConstants.ISSUER_SERIAL); + + // Sign the Timestamp and the SOAP Body. The Body is what gets tampered with below; the + // Timestamp, SignatureValue and signing key - the values the replay cache identifier is + // derived from - are left exactly as they are in the genuine message. + builder.getParts().add(new WSEncryptionPart("Timestamp", WSConstants.WSU_NS, "")); + builder.getParts().add(WSSecurityUtil.getDefaultEncryptionPart(createdDoc)); + + builder.prepare(crypto); + + List<javax.xml.crypto.dsig.Reference> referenceList = + builder.addReferencesToSign(builder.getParts()); + + builder.computeSignature(referenceList, false, null); + + String genuineMessage = XMLUtils.prettyDocumentToString(createdDoc); + if (LOG.isDebugEnabled()) { + LOG.debug(genuineMessage); + } + + String tamperedMessage = genuineMessage.replace(">15<", ">16<"); + assertNotEquals(genuineMessage, tamperedMessage, "The signed SOAP Body was not modified"); + + WSSConfig wssConfig = WSSConfig.getNewInstance(); + RequestData data = new RequestData(); + data.setWssConfig(wssConfig); + data.setCallbackHandler(callbackHandler); + data.setTimestampReplayCache(new MemoryReplayCache()); + + // The tampered message must be rejected, as its Signature no longer validates + try { + verify(SOAPUtil.toSOAPPart(tamperedMessage), wssConfig, data); + fail("Expected failure on a tampered message"); + } catch (WSSecurityException ex) { + assertEquals(WSSecurityException.ErrorCode.FAILED_CHECK, ex.getErrorCode()); + } + + // ...and it must not have cached the genuine message's identifier on its way out + verify(SOAPUtil.toSOAPPart(genuineMessage), wssConfig, data); + + // Replay detection must still work for the genuine message itself + try { + verify(SOAPUtil.toSOAPPart(genuineMessage), wssConfig, data); + fail("Expected failure on a replay attack"); + } catch (WSSecurityException ex) { + assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY, ex.getErrorCode()); + } + } + @Test public void testEhCacheReplayedTimestamp() throws Exception {
