Skip to content

Commit 7fc97c6

Browse files
committed
Select Most Specific RDN in X500 Principal Extractor
SubjectX500PrincipalExtractor now returns the left-most, most-specific matching RDN value, matching both convention and SubjectDnX509PrincipalExtractor. Since LdapName#getRdns lists RDNs most-significant first, the extractor reads them in reverse so that a subject with more than one matching attribute (e.g. two CNs) resolves to the most specific value. Closes gh-19254 Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
1 parent 3f1a794 commit 7fc97c6

4 files changed

Lines changed: 55 additions & 1 deletion

File tree

web/src/main/java/org/springframework/security/web/authentication/preauth/x509/SubjectX500PrincipalExtractor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package org.springframework.security.web.authentication.preauth.x509;
1818

1919
import java.security.cert.X509Certificate;
20+
import java.util.ArrayList;
21+
import java.util.Collections;
2022
import java.util.List;
2123

2224
import javax.naming.InvalidNameException;
@@ -72,7 +74,10 @@ public Object extractPrincipal(X509Certificate clientCert) {
7274

7375
private List<Rdn> getDns(String subjectDn) {
7476
try {
75-
return new LdapName(subjectDn).getRdns();
77+
// read most-specific first, see gh-19254
78+
List<Rdn> rdns = new ArrayList<>(new LdapName(subjectDn).getRdns());
79+
Collections.reverse(rdns);
80+
return rdns;
7681
}
7782
catch (InvalidNameException ex) {
7883
throw new BadCredentialsException("Failed to parse client certificate", ex);

web/src/test/java/org/springframework/security/web/authentication/preauth/x509/SubjectDnX509PrincipalExtractorTests.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ public void defaultCNPatternReturnsPrincipalAtEndOfDNString() throws Exception {
7474
assertThat(principal).isEqualTo("Duke");
7575
}
7676

77+
// gh-19254
78+
@Test
79+
public void defaultCNPatternReturnsMostSpecificPrincipalWhenMultipleCns() throws Exception {
80+
Object principal = this.extractor.extractPrincipal(X509TestUtils.buildTestCertificateWithMultipleCns());
81+
assertThat(principal).isEqualTo("alice");
82+
}
83+
7784
@Test
7885
public void setMessageSourceWhenNullThenThrowsException() {
7986
assertThatIllegalArgumentException().isThrownBy(() -> this.extractor.setMessageSource(null));

web/src/test/java/org/springframework/security/web/authentication/preauth/x509/SubjectX500PrincipalExtractorTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ void extractWhenDnEmbeddedInCnThenExtractsPrincipalName() throws Exception {
6060
assertThat(principal).isEqualTo("luke");
6161
}
6262

63+
// gh-19254
64+
@Test
65+
void extractWhenMultipleCnsThenExtractsMostSpecificCn() throws Exception {
66+
Object principal = this.extractor.extractPrincipal(X509TestUtils.buildTestCertificateWithMultipleCns());
67+
68+
assertThat(principal).isEqualTo("alice");
69+
}
70+
6371
@Test
6472
void extractWhenEmailDnEmbeddedInCnThenExtractsEmail() throws Exception {
6573
this.extractor.setExtractPrincipalNameFromEmail(true);

web/src/test/java/org/springframework/security/web/authentication/preauth/x509/X509TestUtils.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,4 +185,38 @@ public static X509Certificate buildTestCertficateWithEmbeddedEmailDn() throws Ex
185185
return (X509Certificate) cf.generateCertificate(in);
186186
}
187187

188+
/**
189+
* Builds an X.509 certificate whose subject contains more than one CN. The subject DN
190+
* is:
191+
*
192+
* <pre>
193+
* CN=alice, CN=bob, O=Example Corp, C=US
194+
* </pre>
195+
*
196+
* where {@code CN=alice} is the most specific (left-most) RDN.
197+
*/
198+
public static X509Certificate buildTestCertificateWithMultipleCns() throws Exception {
199+
String cert = "-----BEGIN CERTIFICATE-----\n"
200+
+ "MIIDJzCCAg+gAwIBAgIIclOz1VulWC8wDQYJKoZIhvcNAQEMBQAwQjELMAkGA1UE\n"
201+
+ "BhMCVVMxFTATBgNVBAoTDEV4YW1wbGUgQ29ycDEMMAoGA1UEAxMDYm9iMQ4wDAYD\n"
202+
+ "VQQDEwVhbGljZTAeFw0yNjA2MDExOTQzMTVaFw0zNjA1MjkxOTQzMTVaMEIxCzAJ\n"
203+
+ "BgNVBAYTAlVTMRUwEwYDVQQKEwxFeGFtcGxlIENvcnAxDDAKBgNVBAMTA2JvYjEO\n"
204+
+ "MAwGA1UEAxMFYWxpY2UwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCX\n"
205+
+ "q8hZrTRHJEN7+D6yK65OKeCTVU+WccI6awz6g4T6O3aoC+1IiUljsEYn+xuDfx5L\n"
206+
+ "L/O1kejjmbYt+vzRmiILoJ9xKfW3ERcB4+gEar959Dkj6wmpsgOKRjmOvcOFOkEe\n"
207+
+ "gU1F7t04JHOou3DaAkHMNMQV+3jsWSh9Rry7XZBDkcT8XbbagoCgSIIef03Qtw31\n"
208+
+ "zOBwqmJmd6CQhFJva5cl8cCE2xZinOIiz/2j7VprZTjhkud2M4bRnK3T0WExyOCg\n"
209+
+ "jvjSf5ZoOKwC2Z9Q/Oyf8WKpren1+GfZhAKKmn7QZ2foHhdPPRtNjRGE6SqQyW2u\n"
210+
+ "8+1tOXl+aRCF19rR7gIpAgMBAAGjITAfMB0GA1UdDgQWBBRfLtnK5WKU0q3zxIrr\n"
211+
+ "y3GD+lYplzANBgkqhkiG9w0BAQwFAAOCAQEAe+/FHqErVPsF/sHrVHny8mIsn3ux\n"
212+
+ "qE9P24KNF0oIfmBrAqqge6hoVQ8PS+JialyqFf//osuDjiuYaBEKBw7GCoA6I8mr\n"
213+
+ "FA7wyFaGosfq7An5vxkJl7lap2u5oSVv3dCy13Bs0ziYmNlTkfHDLy9yh7jpH1wg\n"
214+
+ "TvylH9O4Vc7y9rzzpIjMCuQJ/wJ4MuJ2mSarYZsx3UQHIRfpKtR/9jbFMX1Rbv/A\n"
215+
+ "N0XD+NrtFjiikp71y3aCO1EHnGG7qPKCWh3PzaNoWFpyZKDBvud8ymW7RMiLPgv9\n"
216+
+ "eTa82KGgnCwtKCNzGkszIn7fza/6xnCuzvh4y9tYE5BWP2mcMRtRmDD07w==\n" + "-----END CERTIFICATE-----";
217+
ByteArrayInputStream in = new ByteArrayInputStream(cert.getBytes());
218+
CertificateFactory cf = CertificateFactory.getInstance("X.509");
219+
return (X509Certificate) cf.generateCertificate(in);
220+
}
221+
188222
}

0 commit comments

Comments
 (0)