This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 3_0_x-fixes
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/3_0_x-fixes by this push:
new 0fa3438ff Fix sender-vouches signature referencing (#685)
0fa3438ff is described below
commit 0fa3438ffb3a757a3854e0f5bcac43c037fca08f
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Thu Sep 10 14:12:19 2026 +0100
Fix sender-vouches signature referencing (#685)
---
.../apache/wss4j/dom/str/SignatureSTRParser.java | 25 +++-
.../saml/SamlSenderVouchesSecretKeyBypassTest.java | 136 +++++++++++++++++++++
2 files changed, 155 insertions(+), 6 deletions(-)
diff --git
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java
index 478556fac..aae89bfbe 100644
---
a/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java
+++
b/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java
@@ -101,19 +101,32 @@ public class SignatureSTRParser implements STRParser {
/**
* A method to create a Principal from a SAML Assertion
* @param samlAssertion An SamlAssertionWrapper object
+ * @param secretKey the raw secret key (if any) taken from the assertion's
Subject KeyInfo,
+ * which is only a meaningful proof-of-possession
credential for the
+ * holder-of-key confirmation method
* @return A principal
*/
private Principal createPrincipalFromSAML(
- SamlAssertionWrapper samlAssertion, STRParserResult parserResult
- ) {
+ SamlAssertionWrapper samlAssertion, STRParserResult parserResult,
byte[] secretKey
+ ) throws WSSecurityException {
SAMLTokenPrincipalImpl samlPrincipal = new
SAMLTokenPrincipalImpl(samlAssertion);
String confirmMethod = null;
List<String> methods = samlAssertion.getConfirmationMethods();
if (methods != null && !methods.isEmpty()) {
confirmMethod = methods.get(0);
}
- if (OpenSAMLUtil.isMethodHolderOfKey(confirmMethod) &&
samlAssertion.isSigned()) {
+ boolean holderOfKey = OpenSAMLUtil.isMethodHolderOfKey(confirmMethod);
+ if (holderOfKey && samlAssertion.isSigned()) {
parserResult.setTrustedCredential(true);
+ } else if (!holderOfKey && secretKey != null && secretKey.length > 0) {
+ // A raw secret key from the Subject KeyInfo of a
non-holder-of-key assertion (e.g.
+ // sender-vouches) has no defined proof-of-possession semantics,
and no trust anchor is
+ // ever checked for it (SignatureTrustValidator only validates
certs / public keys). Using
+ // such a key to "verify" a signature is a self-referential no-op
that an attacker can
+ // trivially satisfy by embedding their own key in an unsigned
assertion, so reject it.
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.FAILED_CHECK,
"invalidSAMLsecurity"
+ );
}
return samlPrincipal;
}
@@ -145,7 +158,7 @@ public class SignatureSTRParser implements STRParser {
}
secretKey = samlKi.getSecret();
parserResult.setPublicKey(samlKi.getPublicKey());
- parserResult.setPrincipal(createPrincipalFromSAML(samlAssertion,
parserResult));
+ parserResult.setPrincipal(createPrincipalFromSAML(samlAssertion,
parserResult, secretKey));
}
parserResult.setSecretKey(secretKey);
}
@@ -313,7 +326,7 @@ public class SignatureSTRParser implements STRParser {
}
parserResult.setSecretKey(keyInfo.getSecret());
parserResult.setPublicKey(keyInfo.getPublicKey());
- parserResult.setPrincipal(createPrincipalFromSAML(samlAssertion,
parserResult));
+ parserResult.setPrincipal(createPrincipalFromSAML(samlAssertion,
parserResult, keyInfo.getSecret()));
}
REFERENCE_TYPE referenceType = getReferenceType(secRef);
@@ -391,7 +404,7 @@ public class SignatureSTRParser implements STRParser {
parserResult.setCerts(new
X509Certificate[]{foundCerts[0]});
}
secretKey = keyInfo.getSecret();
- principal = createPrincipalFromSAML(samlAssertion,
parserResult);
+ principal = createPrincipalFromSAML(samlAssertion,
parserResult, secretKey);
} else if (el.equals(WSConstants.ENCRYPTED_KEY)) {
STRParserUtil.checkEncryptedKeyBSPCompliance(secRef,
data.getBSPEnforcer());
Processor proc =
data.getWssConfig().getProcessor(WSConstants.ENCRYPTED_KEY);
diff --git
a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlSenderVouchesSecretKeyBypassTest.java
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlSenderVouchesSecretKeyBypassTest.java
new file mode 100644
index 000000000..a9f4f211c
--- /dev/null
+++
b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlSenderVouchesSecretKeyBypassTest.java
@@ -0,0 +1,136 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.wss4j.dom.saml;
+
+import java.util.Base64;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.wss4j.common.saml.SAMLCallback;
+import org.apache.wss4j.common.saml.SAMLUtil;
+import org.apache.wss4j.common.saml.SamlAssertionWrapper;
+import org.apache.wss4j.common.saml.bean.KeyInfoBean;
+import org.apache.wss4j.common.saml.builder.SAML1Constants;
+import org.apache.wss4j.common.token.SecurityTokenReference;
+import org.apache.wss4j.common.util.SOAPUtil;
+import org.apache.wss4j.dom.WSConstants;
+import org.apache.wss4j.dom.WSDocInfo;
+import org.apache.wss4j.dom.common.SAML1CallbackHandler;
+import org.apache.wss4j.dom.engine.WSSConfig;
+import org.apache.wss4j.dom.engine.WSSecurityEngineResult;
+import org.apache.wss4j.dom.handler.RequestData;
+import org.apache.wss4j.dom.str.STRParser;
+import org.apache.wss4j.dom.str.STRParserParameters;
+import org.apache.wss4j.dom.str.STRParserResult;
+import org.apache.wss4j.dom.str.SignatureSTRParser;
+import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Regression test for a sender-vouches trust bypass: an unsigned SAML
assertion carries a raw,
+ * attacker-chosen secret key in its Subject KeyInfo
(SubjectConfirmationData/ds:KeyInfo/wst:BinarySecret).
+ * That confirmation method is not holder-of-key, so the secret has no defined
proof-of-possession
+ * semantics and no trust anchor is ever consulted for it -
SignatureTrustValidator only validates
+ * certs/public keys, so a raw secret credential used to "verify" a signature
is a self-referential
+ * no-op that any attacker can satisfy. SignatureSTRParser must reject such a
credential instead of
+ * silently handing back the attacker's own key.
+ */
+public class SamlSenderVouchesSecretKeyBypassTest {
+
+ private static final String WST_NS =
"http://schemas.xmlsoap.org/ws/2005/02/trust";;
+
+ public SamlSenderVouchesSecretKeyBypassTest() throws Exception {
+ WSSConfig.init();
+ }
+
+ @Test
+ public void testUnsignedSenderVouchesSubjectSecretKeyIsRejected() throws
Exception {
+ // An unsigned, sender-vouches SAML assertion (the default for
SAML1CallbackHandler) whose
+ // Subject KeyInfo embeds a raw secret of the attacker's own choosing.
+ byte[] attackerSecret =
"attacker-controlled-shared-secret".getBytes("UTF-8");
+
+ SAML1CallbackHandler callbackHandler = new SAML1CallbackHandler();
+ callbackHandler.setStatement(SAML1CallbackHandler.Statement.AUTHN);
+
callbackHandler.setConfirmationMethod(SAML1Constants.CONF_SENDER_VOUCHES);
+ callbackHandler.setIssuer("attacker.example.com");
+
+ SAMLCallback samlCallback = new SAMLCallback();
+ SAMLUtil.doSAMLCallback(callbackHandler, samlCallback);
+
samlCallback.getSubject().setKeyInfo(createBinarySecretKeyInfo(attackerSecret));
+
+ SamlAssertionWrapper samlAssertion = new
SamlAssertionWrapper(samlCallback);
+ Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG);
+ samlAssertion.toDOM(doc);
+ samlAssertion.parseSubject(new WSSSAMLKeyInfoProcessor(new
RequestData()), null);
+
+ assertThrows(WSSecurityException.class, () ->
parseSubjectKeyIdentifier(doc, samlAssertion));
+ }
+
+ // Directly exercises SignatureSTRParser, i.e. what a ds:Signature's
KeyInfo/SecurityTokenReference
+ // resolves to when it points at the SAML assertion above - this is the
exact sink that a forged
+ // "self-vouching" HMAC signature over the SOAP Body would rely on.
+ private STRParserResult parseSubjectKeyIdentifier(
+ Document doc, SamlAssertionWrapper samlAssertion
+ ) throws WSSecurityException {
+ WSDocInfo wsDocInfo = new WSDocInfo(doc);
+ WSSecurityEngineResult samlResult =
+ new WSSecurityEngineResult(WSConstants.ST_UNSIGNED, samlAssertion);
+ samlResult.put(WSSecurityEngineResult.TAG_ID, samlAssertion.getId());
+ wsDocInfo.addResult(samlResult);
+
+ RequestData requestData = new RequestData();
+ requestData.setWsDocInfo(wsDocInfo);
+
+ SecurityTokenReference secRef = new SecurityTokenReference(doc);
+ secRef.addTokenType(WSConstants.WSS_SAML_TOKEN_TYPE);
+ secRef.setKeyIdentifier(WSConstants.WSS_SAML_KI_VALUE_TYPE,
samlAssertion.getId());
+
+ STRParserParameters parameters = new STRParserParameters();
+ parameters.setData(requestData);
+ parameters.setStrElement(secRef.getElement());
+
+ STRParser strParser = new SignatureSTRParser();
+ return strParser.parseSecurityTokenReference(parameters);
+ }
+
+ private KeyInfoBean createBinarySecretKeyInfo(byte[] secret) throws
Exception {
+ DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+ dbf.setNamespaceAware(true);
+ DocumentBuilder docBuilder = dbf.newDocumentBuilder();
+ Document keyInfoDoc = docBuilder.newDocument();
+
+ Element keyInfoElement =
keyInfoDoc.createElementNS(WSConstants.SIG_NS, "ds:KeyInfo");
+ keyInfoElement.setAttributeNS("http://www.w3.org/2000/xmlns/";,
"xmlns:ds", WSConstants.SIG_NS);
+ Element binarySecretElement = keyInfoDoc.createElementNS(WST_NS,
"wst:BinarySecret");
+ binarySecretElement.setAttributeNS("http://www.w3.org/2000/xmlns/";,
"xmlns:wst", WST_NS);
+
binarySecretElement.setTextContent(Base64.getEncoder().encodeToString(secret));
+ keyInfoElement.appendChild(binarySecretElement);
+ keyInfoDoc.appendChild(keyInfoElement);
+
+ KeyInfoBean keyInfo = new KeyInfoBean();
+ keyInfo.setElement(keyInfoElement);
+ return keyInfo;
+ }
+}
