This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/more-xkms-ldap-changs in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 640cc9a8ae9ff22c8e119837ea94a4b432bcbd5b Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Tue May 12 06:31:09 2026 +0100 Some other DN injection issues found in the LdapCertificateRepo --- .../xkms/x509/repo/ldap/LdapCertificateRepo.java | 40 +++++++++++++++++++--- .../systest/ldap/xkms/LDAPCertificateRepoTest.java | 36 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/services/xkms/xkms-x509-repo-ldap/src/main/java/org/apache/cxf/xkms/x509/repo/ldap/LdapCertificateRepo.java b/services/xkms/xkms-x509-repo-ldap/src/main/java/org/apache/cxf/xkms/x509/repo/ldap/LdapCertificateRepo.java index a586d8d153e..3369c5c62cc 100644 --- a/services/xkms/xkms-x509-repo-ldap/src/main/java/org/apache/cxf/xkms/x509/repo/ldap/LdapCertificateRepo.java +++ b/services/xkms/xkms-x509-repo-ldap/src/main/java/org/apache/cxf/xkms/x509/repo/ldap/LdapCertificateRepo.java @@ -38,6 +38,7 @@ import javax.naming.directory.Attributes; import javax.naming.directory.BasicAttribute; import javax.naming.directory.BasicAttributes; import javax.naming.directory.SearchResult; +import javax.naming.ldap.LdapName; import org.apache.cxf.common.logging.LogUtils; import org.apache.cxf.xkms.handlers.Applications; @@ -168,11 +169,10 @@ public class LdapCertificateRepo implements CertificateRepo { public X509Certificate findBySubjectDn(String id) { X509Certificate cert = null; try { - String dn = id; - if (rootDN != null && !rootDN.isEmpty()) { - dn = dn + "," + rootDN; + String dn = toPkixLookupDn(id); + if (dn != null) { + cert = getCertificateForDn(dn); } - cert = getCertificateForDn(dn); } catch (NamingException e) { // Not found } @@ -356,7 +356,7 @@ public class LdapCertificateRepo implements CertificateRepo { final String dn; Map<String, String> attrs = new HashMap<>(); if (application == Applications.PKIX) { - dn = key.getIdentifier() + "," + rootDN; + dn = toPkixRegistrationDn(key.getIdentifier()); } else if (application == Applications.SERVICE_NAME) { dn = getDnForIdentifier(key.getIdentifier()); } else if (application == Applications.SERVICE_ENDPOINT) { @@ -368,4 +368,34 @@ public class LdapCertificateRepo implements CertificateRepo { saveCertificate(cert, dn, attrs); } + private String toPkixLookupDn(String identifier) { + if (identifier == null || identifier.indexOf('=') < 0 || identifier.indexOf('/') >= 0) { + return null; + } + try { + String normalized = new LdapName(identifier).toString(); + if (rootDN != null && !rootDN.isEmpty()) { + normalized = normalized + "," + rootDN; + } + return new LdapName(normalized).toString(); + } catch (NamingException ex) { + return null; + } + } + + private String toPkixRegistrationDn(String identifier) { + if (identifier == null || identifier.indexOf('/') >= 0) { + throw new IllegalArgumentException("Invalid PKIX DN identifier"); + } + try { + String normalized = new LdapName(identifier).toString(); + if (rootDN != null && !rootDN.isEmpty()) { + normalized = normalized + "," + rootDN; + } + return new LdapName(normalized).toString(); + } catch (NamingException ex) { + throw new IllegalArgumentException("Invalid PKIX DN identifier", ex); + } + } + } diff --git a/systests/ldap/src/test/java/org/apache/cxf/systest/ldap/xkms/LDAPCertificateRepoTest.java b/systests/ldap/src/test/java/org/apache/cxf/systest/ldap/xkms/LDAPCertificateRepoTest.java index a38085c000d..5d1e5865507 100644 --- a/systests/ldap/src/test/java/org/apache/cxf/systest/ldap/xkms/LDAPCertificateRepoTest.java +++ b/systests/ldap/src/test/java/org/apache/cxf/systest/ldap/xkms/LDAPCertificateRepoTest.java @@ -165,6 +165,18 @@ public class LDAPCertificateRepoTest { assertNull(result); } + @Test + public void testX509LocatorFindBySubjectDnUsesEscapedDn() throws Exception { + CapturingFindLdapCertificateRepo persistenceManager = new CapturingFindLdapCertificateRepo(); + X509Locator locator = new X509Locator(persistenceManager); + + LocateRequestType request = createLocateRequest(Applications.PKIX, "cn=bad,ou=admins"); + UnverifiedKeyBindingType result = locator.locate(request); + + assertNull(result); + org.junit.Assert.assertEquals("cn=bad,ou=admins,dc=example,dc=com", persistenceManager.getLookedUpDn()); + } + @Test public void testX509LocatorReturnsNullForUnknownSubjectDn() throws Exception { CertificateRepo persistenceManager = createLdapCertificateRepo(); @@ -358,4 +370,28 @@ public class LDAPCertificateRepoTest { return new LdapCertificateRepo(ldapSearch, ldapSchemaConfig, ROOT_DN); } + private static class CapturingFindLdapCertificateRepo extends LdapCertificateRepo { + private String lookedUpDn; + + CapturingFindLdapCertificateRepo() { + super(new LdapSearch("ldap://localhost:389", "UID=admin,DC=example,DC=com", "ldap_su", 1), + new LdapSchemaConfig(), ROOT_DN); + } + + @Override + protected X509Certificate getCertificateForDn(String dn) { + this.lookedUpDn = dn; + return null; + } + + @Override + protected X509Certificate getCertificateForUIDAttr(String uid) { + return null; + } + + String getLookedUpDn() { + return lookedUpDn; + } + } + }
