Hi Colm, thanks for your patch. Looks good to me.
What do you think about changing the private methods to protected? This would make it a lot easier to subclass this implementation in case you need to customize something. For example according to the NATO Standard ACP 133, it can be possible to use different LDAP attributes for service certificates (userCertificate;binary) and CA certificates (cACertificate;binary) https://tools.ietf.org/html/draft-dally-acp133-and-ldap-01#section-2.21 Currently I would have to copy all private methods into my subclass. It would be a lot easier if these methods would be protected. BTW. What do you thing about extending the implementation to support different LDAP attributes for “normal” certificates as well as for “CA” certificates as in my example above? We could add a key xkms.ldap.schema.attrCaCrtBinary in addition to xkms.ldap.schema.attrCrtBinary for this purpose. Default value could be “userCertificate” to ensure backward compatibility. WDYT? Viele Grüße Jan From: Colm O hEigeartaigh <[email protected]> Sent: Tuesday, 9 July 2019 12:28 To: Jan Bernhardt <[email protected]> Cc: [email protected]; Andrei Shakirin <[email protected]> Subject: Re: Potential bug in CXF XKMS LDAP implementation Hi Jan, Yes, you're correct - it's a bug. I filed a JIRA here: https://issues.apache.org/jira/browse/CXF-8071<https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.apache.org_jira_browse_CXF-2D8071&d=DwMFaQ&c=2w5q_42kFG40MI2alLPgJw&r=HZeq2aU1KcmwwCZOPf0sQ9nq4vkxyxqiWkZrOI1o4vI&m=lkMbhTZjOKa0SWqLHCuNYQGQAP1Uc72UysSC76k0LmI&s=zs0iyZtxdjt7I4lhx4fnvjErAU_tMR1S8aBtT_lPX2Y&e=> Here is a PR to fix the problem - https://github.com/apache/cxf/pull/565<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_cxf_pull_565&d=DwMFaQ&c=2w5q_42kFG40MI2alLPgJw&r=HZeq2aU1KcmwwCZOPf0sQ9nq4vkxyxqiWkZrOI1o4vI&m=lkMbhTZjOKa0SWqLHCuNYQGQAP1Uc72UysSC76k0LmI&s=rmFOyUMu-VVvrtrjAt2D7BkEuS0s5WV-iQUILVEI5Do&e=> I made a change to the default serviceCertUIDTemplate, as I don't think it makes sense to have it use "cn" by default. Let me know what you think, Colm. On Mon, Jul 8, 2019 at 3:14 PM Jan Bernhardt <[email protected]<mailto:[email protected]>> wrote: Hi CXF developers, I’m trying to understand if there is a bug or a feature that I don’t understand in the LDAP Repository implementation for CXF XKMS. https://github.com/apache/cxf/blob/master/services/xkms/xkms-x509-repo-ldap/src/main/java/org/apache/cxf/xkms/x509/repo/ldap/LdapCertificateRepo.java<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_cxf_blob_master_services_xkms_xkms-2Dx509-2Drepo-2Dldap_src_main_java_org_apache_cxf_xkms_x509_repo_ldap_LdapCertificateRepo.java&d=DwMFaQ&c=2w5q_42kFG40MI2alLPgJw&r=HZeq2aU1KcmwwCZOPf0sQ9nq4vkxyxqiWkZrOI1o4vI&m=lkMbhTZjOKa0SWqLHCuNYQGQAP1Uc72UysSC76k0LmI&s=QfujFyWzEak5z0Wq76ohlq_puKGVo-JbvHaqGJ1aQ5M&e=> Line 206, 207 Here the service LDAP template filter gets applied first (looks fine to me), but then the result is send to the getCertificateForUIDAttr method. Here the UIDAttribute LDAP filter gets applied on top of the other filter, making the first filter useless (or even breaks it). So from my perspective line 207 should look like line 241. Can you confirm? Jan -- Colm O hEigeartaigh Talend Community Coder http://coders.talend.com
