Re: Potential bug in CXF XKMS LDAP implementation

[Colm O hEigeartaigh <co...@apache.org>]

Hi Jan,

I've merged a fix to make the private methods in LdapCertificateRepo
protected. I'm fine with adding a separate attribute for CA certs as you
propose - however, how do we know which filter to use in for example
findByServiceName / findByEndpoint?

Colm.

On Mon, Jul 22, 2019 at 2:46 PM Jan Bernhardt <jb...@talend.com> wrote:

> 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 <co...@apache.org>
> *Sent:* Tuesday, 9 July 2019 12:28
> *To:* Jan Bernhardt <jb...@talend.com>
> *Cc:* dev@cxf.apache.org; Andrei Shakirin <as...@talend.com>
> *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 <jb...@talend.com>
> 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
>


-- 
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com