This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 4.1.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/4.1.x-fixes by this push:
new 01de4dcd7bf For SAML, require that the TLS client certificate subject
matches that of the SAML Assertion (#3322)
01de4dcd7bf is described below
commit 01de4dcd7bf5cd10769c4600008232f19b22cdc8
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Jul 21 09:32:06 2026 +0100
For SAML, require that the TLS client certificate subject matches that of
the SAML Assertion (#3322)
(cherry picked from commit 0008935708d0ede779164f276b0241ebe50ba125)
---
rt/rs/security/oauth-parent/oauth2-saml/pom.xml | 6 +
.../grants/saml/Saml2BearerGrantHandler.java | 113 +++++++++++
.../grants/saml/Saml2BearerGrantHandlerTest.java | 218 +++++++++++++++++++++
3 files changed, 337 insertions(+)
diff --git a/rt/rs/security/oauth-parent/oauth2-saml/pom.xml
b/rt/rs/security/oauth-parent/oauth2-saml/pom.xml
index 77243f11a22..7922e8687d6 100644
--- a/rt/rs/security/oauth-parent/oauth2-saml/pom.xml
+++ b/rt/rs/security/oauth-parent/oauth2-saml/pom.xml
@@ -52,5 +52,11 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>${cxf.mockito.version}</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git
a/rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandler.java
b/rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandler.java
index 8da8e1551e5..f7a18c94287 100644
---
a/rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandler.java
+++
b/rt/rs/security/oauth-parent/oauth2-saml/src/main/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandler.java
@@ -25,10 +25,13 @@ import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.security.Principal;
import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -75,6 +78,7 @@ import org.opensaml.xmlsec.signature.Signature;
*/
public class Saml2BearerGrantHandler extends AbstractGrantHandler {
private static final String ENCODED_SAML2_BEARER_GRANT;
+ private static final Pattern CN_RDN_PATTERN =
Pattern.compile("\\s*CN\\s*=\\s*([^,]+?)\\s*(?:,|$)");
static {
WSSConfig.init();
// AccessTokenService may be configured with the form provider
@@ -210,6 +214,9 @@ public class Saml2BearerGrantHandler extends
AbstractGrantHandler {
);
} else if (getTLSCertificates(message) == null) {
throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ } else {
+ // Unsigned assertion + mTLS present: require Holder-of-Key
binding
+ validateUnsignedAssertionBinding(message, assertion);
}
if (samlValidator != null) {
@@ -228,6 +235,112 @@ public class Saml2BearerGrantHandler extends
AbstractGrantHandler {
return tlsInfo != null ? tlsInfo.getPeerCertificates() : null;
}
+ /**
+ * Validates that an unsigned SAML assertion is bound to the mTLS client
certificate
+ * (Holder-of-Key binding). This prevents subject impersonation attacks
where an attacker
+ * with a valid mTLS certificate could submit an unsigned assertion
claiming a different
+ * subject.
+ *
+ * @param message The CXF message containing TLS session info
+ * @param assertion The unsigned SAML assertion
+ * @throws OAuthServiceException If validation fails
+ */
+ protected void validateUnsignedAssertionBinding(Message message,
SamlAssertionWrapper assertion) {
+ Certificate[] tlsCerts = getTLSCertificates(message);
+ if (tlsCerts == null || tlsCerts.length == 0) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
+
+ // Extract subject identifier from mTLS certificate
+ String certSubjectId =
extractCertificateSubjectIdentifier(tlsCerts[0]);
+ if (certSubjectId == null || certSubjectId.isEmpty()) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
+
+ // Extract subject from SAML assertion
+ String assertionSubject = extractAssertionSubject(assertion);
+ if (assertionSubject == null || assertionSubject.isEmpty()) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
+
+ // Verify Holder-of-Key binding: subject must match
+ if (!certSubjectId.equals(assertionSubject)) {
+ throw new OAuthServiceException(OAuthConstants.INVALID_GRANT);
+ }
+ }
+
+ /**
+ * Extracts the subject identifier from an X.509 certificate.
+ * Attempts to extract the Common Name (CN) from the subject DN first;
+ * falls back to the full DN if CN is not found.
+ *
+ * @param cert The certificate to extract from
+ * @return The subject identifier, or null if extraction fails
+ */
+ protected String extractCertificateSubjectIdentifier(Certificate cert) {
+ if (!(cert instanceof X509Certificate)) {
+ return null;
+ }
+
+ X509Certificate x509 = (X509Certificate) cert;
+ String dn = x509.getSubjectX500Principal().getName();
+ if (dn == null || dn.isEmpty()) {
+ return null;
+ }
+
+ // Try to extract CN (Common Name) first
+ String cn = extractCNValue(dn);
+ if (cn != null && !cn.isEmpty()) {
+ return cn;
+ }
+
+ // Fall back to full DN if CN not found
+ return dn;
+ }
+
+ /**
+ * Extracts the SAML assertion subject from the SAML2 NameID.
+ *
+ * @param assertion The SAML assertion
+ * @return The subject identifier from the NameID, or null if not found
+ */
+ protected String extractAssertionSubject(SamlAssertionWrapper assertion) {
+ if (assertion == null) {
+ return null;
+ }
+
+ if (assertion.getSaml2() != null && assertion.getSaml2().getSubject()
!= null) {
+ org.opensaml.saml.saml2.core.NameID nameID =
assertion.getSaml2().getSubject().getNameID();
+ if (nameID != null) {
+ return nameID.getValue();
+ }
+ }
+
+ return null;
+ }
+
+ /**
+ * Extracts the CN value from an X.500 DN.
+ * For example, extractCNValue("[email protected],O=Company,C=US")
returns "[email protected]"
+ * Also handles DNs with spaces like "CN = [email protected] , O = Company"
+ *
+ * @param dn The distinguished name string
+ * @return The CN value, or null if not found
+ */
+ protected String extractCNValue(String dn) {
+ if (dn == null || dn.isEmpty()) {
+ return null;
+ }
+
+ Matcher m = CN_RDN_PATTERN.matcher(dn);
+
+ if (m.find()) {
+ return m.group(1).trim();
+ }
+
+ return null;
+ }
+
protected void setSecurityContext(Message message, SamlAssertionWrapper
wrapper) {
if (scProvider != null) {
SecurityContext sc = scProvider.getSecurityContext(message,
wrapper);
diff --git
a/rt/rs/security/oauth-parent/oauth2-saml/src/test/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandlerTest.java
b/rt/rs/security/oauth-parent/oauth2-saml/src/test/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandlerTest.java
new file mode 100644
index 00000000000..5f11acc0991
--- /dev/null
+++
b/rt/rs/security/oauth-parent/oauth2-saml/src/test/java/org/apache/cxf/rs/security/oauth2/grants/saml/Saml2BearerGrantHandlerTest.java
@@ -0,0 +1,218 @@
+/**
+ * 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.cxf.rs.security.oauth2.grants.saml;
+
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
+
+import javax.security.auth.x500.X500Principal;
+
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException;
+import org.apache.cxf.security.transport.TLSSessionInfo;
+import org.apache.wss4j.common.saml.SamlAssertionWrapper;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link Saml2BearerGrantHandler} Holder-of-Key binding
validation.
+ * Tests the extraction and validation logic for X.509 certificate subjects
and SAML assertion subjects.
+ */
+public class Saml2BearerGrantHandlerTest {
+ private final Saml2BearerGrantHandler handler = new
Saml2BearerGrantHandler();
+
+ /**
+ * Test extracting CN from a standard X.500 DN
+ */
+ @Test
+ public void testExtractCNValueStandard() {
+ String dn = "[email protected],O=Acme,C=US";
+ String cn = handler.extractCNValue(dn);
+ assertEquals("[email protected]", cn);
+ }
+
+ /**
+ * Test extracting CN when it's the only component
+ */
+ @Test
+ public void testExtractCNValueOnly() {
+ String dn = "[email protected]";
+ String cn = handler.extractCNValue(dn);
+ assertEquals("[email protected]", cn);
+ }
+
+ /**
+ * Test extracting CN when DN has spaces around values
+ */
+ @Test
+ public void testExtractCNValueWithSpaces() {
+ String dn = "CN = [email protected] , O = Acme , C = US";
+ String cn = handler.extractCNValue(dn);
+ assertEquals("[email protected]", cn);
+ }
+
+ /**
+ * Test extracting CN when CN doesn't exist in DN
+ */
+ @Test
+ public void testExtractCNValueNotFound() {
+ String cn = handler.extractCNValue("O=Acme,C=US");
+ assertNull("CN should not be found in DN without CN component", cn);
+ }
+
+ /**
+ * Test extracting CN from null DN
+ */
+ @Test
+ public void testExtractCNValueNullDN() {
+ String result = handler.extractCNValue(null);
+ assertNull("Should return null for null DN", result);
+ }
+
+ /**
+ * Test extracting CN with empty DN
+ */
+ @Test
+ public void testExtractCNValueEmptyDN() {
+ String result = handler.extractCNValue("");
+ assertNull("Should return null for empty DN", result);
+ }
+
+ /**
+ * Test extracting CN with complex DN that has multiple levels
+ */
+ @Test
+ public void testExtractCNValueComplexDN() {
+ String dn = "[email protected],OU=Engineering,O=Acme,C=US";
+ String cn = handler.extractCNValue(dn);
+ assertEquals("[email protected]", cn);
+ }
+
+ /**
+ * Test extracting from DN with equals sign in value (edge case)
+ */
+ @Test
+ public void testExtractCNValueWithEqualsInValue() {
+ // This is an edge case - the regex should extract up to the first
comma
+ String dn = "[email protected],O=Acme,C=US";
+ String cn = handler.extractCNValue(dn);
+ // The regex will match up to first comma
+ assertEquals("[email protected]", cn);
+ }
+
+ /**
+ * Test extracting assertion subject from null wrapper
+ */
+ @Test
+ public void testExtractAssertionSubjectNullWrapper() {
+ String subject = handler.extractAssertionSubject(null);
+ assertNull("Should return null for null assertion", subject);
+ }
+
+ /**
+ * Test extracting certificate subject identifier from null certificate
+ */
+ @Test
+ public void testExtractCertificateSubjectIdentifierNullCertificate() {
+ String subject = handler.extractCertificateSubjectIdentifier(null);
+ assertNull("Should return null for null certificate", subject);
+ }
+
+ /**
+ * Test extracting certificate subject identifier with non-X509 certificate
+ */
+ @Test
+ public void testExtractCertificateSubjectIdentifierNonX509Certificate() {
+ Certificate mockCert = new Certificate("MockType") {
+ @Override
+ public byte[] getEncoded() {
+ return new byte[0];
+ }
+
+ @Override
+ public void verify(java.security.PublicKey key) {
+ }
+
+ @Override
+ public void verify(java.security.PublicKey key, String
sigProvider) {
+ }
+
+ @Override
+ public String toString() {
+ return "MockCertificate";
+ }
+
+ @Override
+ public java.security.PublicKey getPublicKey() {
+ return null;
+ }
+ };
+
+ String subject = handler.extractCertificateSubjectIdentifier(mockCert);
+ assertNull("Should return null for non-X509 certificate", subject);
+ }
+
+ @Test
+ public void testValidateUnsignedAssertionBindingMatchesTLSCertificate() {
+ Saml2BearerGrantHandler bindingHandler =
createHandlerWithAssertionSubject("[email protected]");
+ Message message =
createMessageWithPeerCertificate("[email protected],O=Acme,C=US");
+ SamlAssertionWrapper assertion = mock(SamlAssertionWrapper.class);
+
+ bindingHandler.validateUnsignedAssertionBinding(message, assertion);
+ }
+
+ @Test
+ public void testValidateUnsignedAssertionBindingRejectsSubjectMismatch() {
+ Saml2BearerGrantHandler bindingHandler =
createHandlerWithAssertionSubject("[email protected]");
+ Message message =
createMessageWithPeerCertificate("[email protected],O=Acme,C=US");
+ SamlAssertionWrapper assertion = mock(SamlAssertionWrapper.class);
+
+ try {
+ bindingHandler.validateUnsignedAssertionBinding(message,
assertion);
+ fail("Expected OAuthServiceException for mismatched SAML subject
and TLS certificate CN");
+ } catch (OAuthServiceException ex) {
+ // expected
+ }
+ }
+
+ private Saml2BearerGrantHandler createHandlerWithAssertionSubject(String
assertionSubject) {
+ return new Saml2BearerGrantHandler() {
+ @Override
+ protected String extractAssertionSubject(SamlAssertionWrapper
assertion) {
+ return assertionSubject;
+ }
+ };
+ }
+
+ private Message createMessageWithPeerCertificate(String subjectDn) {
+ Message message = new MessageImpl();
+ X509Certificate cert = mock(X509Certificate.class);
+ when(cert.getSubjectX500Principal()).thenReturn(new
X500Principal(subjectDn));
+ TLSSessionInfo tlsInfo = new TLSSessionInfo("TLS_FAKE", null, new
Certificate[] {cert});
+ message.put(TLSSessionInfo.class, tlsInfo);
+ return message;
+ }
+}
