This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/signature-trusted in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit 427759f6da0891a651e67c6260c2c8d60e8385dd Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Sep 18 09:46:47 2026 +0100 Only set trust validation for Signature when we have a trusted source --- THREAT-MODEL.md | 42 +++++++++++++++ .../wss4j/dom/engine/WSSecurityEngineResult.java | 9 ++++ .../wss4j/dom/processor/SignatureProcessor.java | 20 +++++-- .../apache/wss4j/dom/message/SignatureTest.java | 4 ++ .../wss4j/dom/message/SymmetricSignatureTest.java | 61 +++++++++++++++++++++- .../wss4j/dom/saml/SignedSamlTokenHOKTest.java | 5 ++ 6 files changed, 135 insertions(+), 6 deletions(-) diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 5111954d2..957498a9d 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -564,6 +564,21 @@ matching disclaimer. - **`enableSignatureConfirmation` does not authenticate the *responder* to the original requester** beyond the signature it confirms. It is a replay-style defense, not an extra factor. +- **A verified XML Signature does not authenticate the sender when the + verification key is a *symmetric* key.** Under a symmetric binding the + signing key typically reaches the recipient as an `xenc:EncryptedKey` + addressed to the recipient's certificate — and that certificate is + public, so any unauthenticated sender can provision such a key and sign + with it. The resulting `SIGN` result establishes the integrity of the + signed parts (P2) and nothing whatsoever about who sent them; it does + not carry P1. Authentication has to come from a token that carries an + identity — a UsernameToken, a SAML assertion, an X.509 signature — which + is exactly what WS-SecurityPolicy's supporting-token requirements + express. A result of this shape carries `TAG_SECRET` with no + `TAG_X509_CERTIFICATES` / `TAG_PUBLIC_KEY`, and its + `TAG_VALIDATED_TOKEN` is `FALSE` *(documented: + `WSSecurityEngineResult.TAG_VALIDATED_TOKEN` javadoc; + `SignatureTrustValidator`)*. - **A successful X.509 trust chain validation does not authenticate the *holder of the private key* to be the *expected* principal unless `SIG_SUBJECT_CERT_CONSTRAINTS` (or equivalent) is set.** Any cert from @@ -696,6 +711,14 @@ The embedding SOAP stack / application **must**: - **Mixing the action-based and WS-SecurityPolicy approaches in the same handler chain.** The behavior across both is documented but rarely tested. +- **Reading a `SIGN` result as proof of sender identity without looking + at what keyed it.** With the action-based approach, `action="Signature + Encrypt"` is satisfied by a signature keyed from an inbound + `EncryptedKey` — `checkReceiverResultsAnyOrder` deliberately skips a + bare `ENCR` result that protects no data. A caller that needs the + sender authenticated must require a token that authenticates them, or + check that the signature was keyed by a certificate / public key rather + than by `TAG_SECRET`. See the symmetric-key false-friend entry in §9. ## §11a Known non-findings (recurring false positives) @@ -758,6 +781,25 @@ model, the section that licenses the call. stack does. → `OUT-OF-MODEL: trusted-input` per §3 item 2. - **"`InputStream.close()` not in finally."** Code-quality finding, not a security one. → `OUT-OF-MODEL: out-of-layer`. +- **"An unauthenticated sender can wrap a symmetric key under the + service's published certificate (`xenc:EncryptedKey`), sign the SOAP + body with that key, and WSS4J reports a valid signature without + invoking `SignatureTrustValidator`."** Accurate as a description, but + it is the WS-SecurityPolicy symmetric binding working as specified: + the signature carries integrity (P2), authentication comes from a + supporting token, and a raw symmetric key has no trust anchor for a + `Validator` to check in the first place. The DOM engine reports what + happened; deciding whether an authenticating token was *also* required + belongs to the policy layer — Apache CXF's + `AsymmetricBindingPolicyValidator.checkInitiatorTokens` rejects such a + signature wherever the policy names an `X509Token`, and + `AbstractSupportingTokenPolicyValidator.checkSignatureOrEncryptionResult` + binds a supporting token to the signature by comparing the actual key + material. → `BY-DESIGN: property-disclaimed` per §9, with the + false-friend entry in §9 as the statement of what is and is not + claimed. (The `TAG_VALIDATED_TOKEN` flag on such a result is a separate, + now-fixed defect: it used to read `TRUE` merely because a `Validator` + was registered for the Signature action, even when no validation ran.) ## §12 Conditions that would change this model diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/engine/WSSecurityEngineResult.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/engine/WSSecurityEngineResult.java index 885f92610..5d397e433 100644 --- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/engine/WSSecurityEngineResult.java +++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/engine/WSSecurityEngineResult.java @@ -110,6 +110,15 @@ public class WSSecurityEngineResult extends java.util.HashMap<String, Object> { * and so this is not set. Note that this is set for the NoOpValidator if it is * configured. * + * For a Signature result this tag is only set when a trust decision was actually taken + * on the signing credential: either a Validator was invoked on it, or the credential + * carries its own trust (for example the key of a signed holder-of-key SAML assertion, + * or a previously validated BinarySecurityToken). It is deliberately NOT set when the + * signature was verified with a symmetric key, because a symmetric key carries no + * identity that a Validator could check. Such a signature establishes the integrity of + * the signed parts, not the identity of the sender - see the "False-friend properties" + * section of THREAT-MODEL.md. + * * The value under this tag is a Boolean instance. */ public static final String TAG_VALIDATED_TOKEN = "validated-token"; 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 b09a15c7e..9a2a43778 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 @@ -136,6 +136,11 @@ public class SignatureProcessor implements Processor { REFERENCE_TYPE referenceType = null; Credential credential = new Credential(); + // Whether trust in the signing credential was actually established: either a + // Validator ran on it, or the credential carries its own trust (a signed + // holder-of-key SAML assertion, a previously validated BST). Only then may the + // result advertise TAG_VALIDATED_TOKEN. + boolean trustEstablished = false; Validator validator = data.getValidator(WSConstants.SIGNATURE); if (keyInfoElement == null) { certs = getDefaultCerts(data.getSigVerCrypto()); @@ -165,6 +170,7 @@ public class SignatureProcessor implements Processor { principal = new PublicKeyPrincipalImpl(publicKey); credential.setPrincipal(principal); credential = validator.validate(credential, data); + trustEstablished = true; } } else { STRParserParameters parameters = new STRParserParameters(); @@ -182,15 +188,17 @@ public class SignatureProcessor implements Processor { secretKey = parserResult.getSecretKey(); referenceType = parserResult.getCertificatesReferenceType(); - boolean trusted = parserResult.isTrustedCredential(); - if (trusted) { + trustEstablished = parserResult.isTrustedCredential(); + if (trustEstablished) { LOG.debug("Direct Trust for SAML/BST credential"); } - if (!trusted && (publicKey != null || (certs != null && certs.length > 0)) && validator != null) { + if (!trustEstablished && (publicKey != null || (certs != null && certs.length > 0)) + && validator != null) { credential.setPublicKey(publicKey); credential.setCertificates(certs); credential.setPrincipal(principal); credential = validator.validate(credential, data); + trustEstablished = true; } } } @@ -261,7 +269,11 @@ public class SignatureProcessor implements Processor { result.put(WSSecurityEngineResult.TAG_PUBLIC_KEY, publicKey); result.put(WSSecurityEngineResult.TAG_X509_REFERENCE_TYPE, referenceType); result.put(WSSecurityEngineResult.TAG_TOKEN_ELEMENT, elem); - if (validator != null) { + // The mere presence of a registered Validator establishes nothing: stamp the result + // as validated only when a trust decision was actually taken on this credential. + // In particular a signature verified with a symmetric key is not stamped, as a + // symmetric key carries no identity for a Validator to check. + if (validator != null && trustEstablished) { result.put(WSSecurityEngineResult.TAG_VALIDATED_TOKEN, Boolean.TRUE); if (credential != null) { result.put(WSSecurityEngineResult.TAG_SUBJECT, credential.getSubject()); diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SignatureTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SignatureTest.java index 128f13db5..3b276ea0e 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SignatureTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SignatureTest.java @@ -121,6 +121,10 @@ public class SignatureTest { REFERENCE_TYPE referenceType = (REFERENCE_TYPE)actionResult.get(WSSecurityEngineResult.TAG_X509_REFERENCE_TYPE); assertTrue(referenceType == REFERENCE_TYPE.ISSUER_SERIAL); + + // Trust in the signing certificate was established by the Signature Validator + assertTrue((Boolean)actionResult.get(WSSecurityEngineResult.TAG_VALIDATED_TOKEN), + "An X.509 signature that passed trust validation must be reported as validated"); } @Test diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SymmetricSignatureTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SymmetricSignatureTest.java index 84819491d..bf4bbeccd 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SymmetricSignatureTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/message/SymmetricSignatureTest.java @@ -37,6 +37,7 @@ import org.apache.wss4j.dom.common.SecretKeyCallbackHandler; import org.apache.wss4j.dom.engine.WSSConfig; import org.apache.wss4j.dom.engine.WSSecurityEngine; +import org.apache.wss4j.dom.engine.WSSecurityEngineResult; import org.apache.wss4j.common.crypto.Crypto; import org.apache.wss4j.common.crypto.CryptoFactory; import org.apache.wss4j.common.ext.WSPasswordCallback; @@ -45,11 +46,16 @@ import org.apache.wss4j.common.util.XMLUtils; import org.apache.wss4j.dom.handler.HandlerAction; import org.apache.wss4j.dom.handler.RequestData; import org.apache.wss4j.dom.handler.WSHandlerConstants; +import org.apache.wss4j.dom.handler.WSHandlerResult; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.w3c.dom.Document; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + /** * Test symmetric key signature created using an encrypted key @@ -250,6 +256,55 @@ public class SymmetricSignatureTest implements CallbackHandler { } + /** + * A signature verified with a symmetric key that arrived in an EncryptedKey establishes + * the integrity of the signed parts, but it does not authenticate the sender: anybody + * holding this service's certificate can wrap a key of their choosing and sign with it. + * No trust decision is taken on such a credential - a symmetric key carries no identity + * for a Validator to check - so the result must not advertise itself as a validated + * token, even though a Validator is registered for the Signature action by default. + */ + @Test + public void testSymmetricSignatureIsNotReportedAsValidated() throws Exception { + Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG); + + WSSecHeader secHeader = new WSSecHeader(doc); + secHeader.insertSecurityHeader(); + + WSSecEncryptedKey encrKey = new WSSecEncryptedKey(secHeader); + encrKey.setKeyIdentifierType(WSConstants.ISSUER_SERIAL); + encrKey.setUserInfo("wss40", "security"); + + KeyGenerator keyGen = KeyUtils.getKeyGenerator(WSConstants.AES_192); + SecretKey symmetricKey = keyGen.generateKey(); + encrKey.prepare(crypto, symmetricKey); + + WSSecSignature sign = new WSSecSignature(secHeader); + sign.setKeyIdentifierType(WSConstants.CUSTOM_SYMM_SIGNING); + sign.setCustomTokenId(encrKey.getId()); + sign.setSecretKey(symmetricKey.getEncoded()); + sign.setSignatureAlgorithm(SignatureMethod.HMAC_SHA1); + sign.setCustomTokenValueType(WSConstants.WSS_ENC_KEY_VALUE_TYPE); + + Document signedDoc = sign.build(crypto); + encrKey.prependToHeader(); + + WSHandlerResult results = verify(signedDoc); + + WSSecurityEngineResult actionResult = + results.getActionResults().get(WSConstants.SIGN).get(0); + assertNotNull(actionResult); + + // The signature was keyed by the symmetric key, not by a certificate or public key + assertNotNull(actionResult.get(WSSecurityEngineResult.TAG_SECRET)); + assertNull(actionResult.get(WSSecurityEngineResult.TAG_X509_CERTIFICATES)); + assertNull(actionResult.get(WSSecurityEngineResult.TAG_PUBLIC_KEY)); + + assertFalse((Boolean)actionResult.get(WSSecurityEngineResult.TAG_VALIDATED_TOKEN), + "A signature keyed by an EncryptedKey secret must not be reported as a " + + "validated token: no trust decision was taken on the key"); + } + /** * Verifies the soap envelope * <p/> @@ -257,14 +312,16 @@ public class SymmetricSignatureTest implements CallbackHandler { * @param doc * @throws Exception Thrown when there is a problem in verification */ - private void verify(Document doc) throws Exception { - secEngine.processSecurityHeader(doc, null, callbackHandler, null, crypto); + private WSHandlerResult verify(Document doc) throws Exception { + WSHandlerResult results = + secEngine.processSecurityHeader(doc, null, callbackHandler, null, crypto); if (LOG.isDebugEnabled()) { LOG.debug("Verfied and decrypted message:"); String outputString = XMLUtils.prettyDocumentToString(doc); LOG.debug(outputString); } + return results; } public void handle(Callback[] callbacks) diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SignedSamlTokenHOKTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SignedSamlTokenHOKTest.java index c4a55243c..fb5470cd1 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SignedSamlTokenHOKTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SignedSamlTokenHOKTest.java @@ -178,6 +178,11 @@ public class SignedSamlTokenHOKTest { refs = (List<WSDataRef>) actionResult.get(WSSecurityEngineResult.TAG_DATA_REF_URIS); assertTrue(refs.size() == 1); + // The key comes from a signed holder-of-key assertion, so the credential carries its + // own trust and is reported as validated even though no separate Validator call ran + assertTrue((Boolean)actionResult.get(WSSecurityEngineResult.TAG_VALIDATED_TOKEN), + "A signature keyed by a signed holder-of-key assertion must be reported as validated"); + wsDataRef = refs.get(0); xpath = wsDataRef.getXpath(); assertEquals("/SOAP-ENV:Envelope/SOAP-ENV:Body", xpath);
