This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/saml-holder-of-key in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 7cf538efab3fd7445d305a038e79a0d870714f5f Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Mon Jul 20 11:59:34 2026 +0100 For SAML, require that the TLS client certificate subject matches that of the SAML Assertion --- 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 d417395050e..7c1d9793499 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; + } +}
