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 aa028e6af Only set trust validation for Signature when we have a
trusted source (#705)
aa028e6af is described below
commit aa028e6afee96c50d38ad047d3e4bca3d8c20c25
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Fri Sep 18 09:53:42 2026 +0100
Only set trust validation for Signature when we have a trusted source (#705)
---
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);