[keycloak-dev] User TLS client certificate authentication - inconsistent DN string representation with LDAP
Hynek Mlnarik
hmlnarik at redhat.com
Tue Feb 12 12:01:04 EST 2019
Note: This thread (or slight alteration thereof) has been resurrected in
parallel thread "Certificate subject DN is provider dependent".
On Tue, Jan 29, 2019 at 12:30 PM Marek Posolda <mposolda at redhat.com> wrote:
> The reason for intoduce LdapDn class instead of LdapName was the
> compatibility issue, but I can't recall the details... Some more below
>
> On 19/01/2019 05:04, Nalyvayko, Peter wrote:
> >> I am not very keen on uppercasing attribute type in the proposed patch
> though it might be the most pragmatic way to go. Is it supported by any DN
> > RFC? In any case, any such a normalization would have to take place
> consistently in both places.
> > An alternative is to modify X500NameRDNExtractor.extractUserIdentity in
> keycloak/services/src/main/java/org/keycloak/authentication/authenticators/x509/UserIdentityExtractor.java
> to return new
> LdapName(IETFUtils.valueToString(cn.getFirst().getValue())).toString()
> instead of IETFUtils.valueToString(cn.getFirst().getValue()) (line 92),
> thus in essence normalizing the user identity from subjectDN/issuerDN to
> the representation conformant to a variant of RFC2253 format which can then
> be safely compared to LDAP_ENTRY_DN attribute.
>
> I think that either this change or the proposed change in LdapDn will
> work. However change in X500NameRDNExtractor has the advantage, that it
> will work immediately for the existing users imported from LDAP.
>
> I mean that if we do the change in the LdapDn class, then the existing
> users in Keycloak DB will still have the LDAP_ENTRY_DN attribute on them
> in the old format like "uid=john,dc=example,dc=com" . So until the
> particular user is re-synced from LDAP to Keycloak DB, it won't be
> successfully lookup when it is searched by authenticator for the
> attribute in new format like "UID=john, DC=example, DC=com" . As the
> change of the LDAP_ENTRY_DN to new format will be triggered by re-sync
> from LDAP.
>
> Marek
>
>
> >
> >
> > ________________________________________
> > From: Hynek Mlnarik [hmlnarik at redhat.com]
> > Sent: Tuesday, January 15, 2019 5:59 AM
> > To: Nalyvayko, Peter
> > Cc: Sebastian Laskawiec; John Dennis; keycloak-dev at lists.jboss.org
> > Subject: Re: [keycloak-dev] User TLS client certificate authentication -
> inconsistent DN string representation with LDAP
> >
> > We should adhere to the standard implementations as much as possible.
> However there likely was a reason for introducing LDAPDn class, possibly
> due to some supported LDAP server having different implementation than
> allowed by Java javax.naming.ldap.LdapName and similar. @Marek, could you
> please comment here?
> >
> > Sidenote: Currently the javax.naming.ldap.LdapName class respects RFC
> 2253 which was obsoleted by RFC 4514. I believe the differences are
> irrelevant in the domain of user ids but comments are welcome.
> >
> > Since a DN is stored as string user attribute value and searched for as
> such, we have to stick to canonical string representation. This can be
> obtained via javax.naming.ldap.LdapName.toString method which should be
> used both in the LDAPDn and the authenticator. It is in the latter, but not
> in the former.
> >
> > I am not very keen on uppercasing attribute type in the proposed patch
> though it might be the most pragmatic way to go. Is it supported by any DN
> RFC? In any case, any such a normalization would have to take place
> consistently in both places.
> >
> > Ideally the comparison would be able to recognize that the following
> inputs are just the same:
> > - DC=example,DC=com
> > - DC=example, DC=com
> > - dc=example,dc=com
> > - dc=ex\61mple,dc=com
> > - 0.9.2342.19200300.100.1.25=example,0.9.2342.19200300.100.1.25=com
> >
> >
> > On Thu, Jan 10, 2019 at 3:21 AM Nalyvayko, Peter <pnalyvayko at agi.com
> <mailto:pnalyvayko at agi.com>> wrote:
> > Hi Sebastian,
> >
> > Short of trying to extend the authenticator itself to handle such cases,
> one possible short term approach can be to implement an attribute mapper
> responsible for importing and translating the LDAP's DN into representation
> that matches the canonical representation of the subject DN returned by
> X509Cert.getSubjectDN().getName() and configuring the x509 authenticator to
> compare the subjectDN against that attribute.
> >
> >
> > ________________________________________
> > From: Sebastian Laskawiec [slaskawi at redhat.com<mailto:
> slaskawi at redhat.com>]
> > Sent: Wednesday, January 9, 2019 10:11 AM
> > To: John Dennis
> > Cc: Nalyvayko, Peter; Peck, Michael A; sblanc at redhat.com<mailto:
> sblanc at redhat.com>; keycloak-dev at lists.jboss.org<mailto:
> keycloak-dev at lists.jboss.org>
> > Subject: Re: [keycloak-dev] User TLS client certificate authentication -
> inconsistent DN string representation with LDAP
> >
> > Hey guys,
> >
> > Answering a few email in a row below.
> >
> > Thanks,
> > Sebastian
> >
> > On Wed, Jan 9, 2019 at 12:19 AM John Dennis <jdennis at redhat.com<mailto:
> jdennis at redhat.com><mailto:jdennis at redhat.com<mailto:jdennis at redhat.com>>>
> wrote:
> > On 1/8/19 5:02 PM, Nalyvayko, Peter wrote:
> >> Hi,
> >>
> >>>> I believe that's a bug. The `X509ClientCertificateAuthenticator`
> should ignore those extra spaces. May I kindly ask you to create a ticket
> for us and assign it either to me or Sebastien?
> >> Sebastian/Michael,
> >>
> >> According to https://tools.ietf.org/html/rfc1779, BNF for
> distinguished name allows for optional space before and after the
> separator. Do you know of any reason why the DN returned by LDAP and the DN
> returned by calling to X509Certificate.getSubjectDN().getName() should or
> expected be identical? It seems to me BNF allows for some discrepancies in
> representation thus comparing two strings verbatim may not be a good idea,
> no?
> > Let me try to reiterate the whole flow to make sure I understand
> everything correctly:
> > - X509ClientCertificateAuthenticator extracts certificates from an
> incoming HTTP request using one of the implementations of
> X509ClientCertificateLookup.
> > - Obtained certificate chain is of a type X509Certificate[].
> > - Then we extract a User identity based on his certificate
> (certs[0].getSubjectDN().getName()).
> > - Once we have a User identity in our hands, we can extract a UserModel,
> our representation of a User in Keycloak. This user might be imported from
> LDAP. Here's where those two pieces play together.
> > - If we find a user and he's valid, we end the flow.
> > So the root problem is that certificate obtained from the request
> doesn't match the one from LDAP, even though they are maintained in a
> single place (somehow, details omitted).
> >
> > So, yes, this situation may happen since in the essence, we do just a DN
> comparison by strings. The easiest workaround in my opinion is to normalize
> those DNs across Keycloak codebase. The simplest solution is to modify
> LDAPDn class as Michel suggested. I believe it should also work if we
> modify a user certificate attribute and put spaces there (", ").
> >
> > At first I though the fix should go X509ClientCertificateAuthenticator
> but obviously, I was wrong. It's just a mismatch problem.
> >
> > RFC 1779 (A String Representation of Distinguished Names) also mentions
> different separators like for example ";", "," or ", " (comma with or
> without a space). In order to support all this usecases, we'd probably need
> to implement a custom DN comparator and modify our database queries that
> extract users based on DN. At this point I'm not 100% convinced we need
> this. Maybe we should just say in the docs that if you synchronize users
> with LDAP, you should use ", " as a separator when specifying user
> certificates?
> >
> > @Michael, @Peter - WDYT about that? Would that solution solve your
> problem?
> >
> > @Hynek, @Marek - maybe you guys have some thoughts around this?
> >
> >
> > On previous projects I did a lot of work with DN handling, I believe the
> > relevant RFC is 4514 (unless it's been superseded). The short answer is
> > it's never correct to compare DN's via string comparison unless the DN
> > was normalized to a consistent representation. The reason is there are
> > multiple valid string representations of the same DN. Aside from issues
> > related to whitespace, encoding, etc. you need to remember that there is
> > such a thing as multi-valued RDN's (e.g. an RDN with an UNORDERED set of
> > AVA's), any ordering of the AVA's in the RDN yield a semantically
> > equivalent RDN.
> >
> > Most libraries that compare DN's do so by parsing the string
> > representation and building a data structure consisting of the
> > individual components which are then iterated over during comparison.
> > Once the DN has been parsed into it's constituent components there is
> > usually a routine to convert it back into a string representation. This
> > can be used to normalize the string representation thus permitting a
> > simple string compare to check for equality.
> >
> >> Kindly,
> >> Peter
> >>
> >>
> >> -----Original Message-----
> >> From: keycloak-dev-bounces at lists.jboss.org<mailto:
> keycloak-dev-bounces at lists.jboss.org><mailto:
> keycloak-dev-bounces at lists.jboss.org<mailto:
> keycloak-dev-bounces at lists.jboss.org>> <
> keycloak-dev-bounces at lists.jboss.org<mailto:
> keycloak-dev-bounces at lists.jboss.org><mailto:
> keycloak-dev-bounces at lists.jboss.org<mailto:
> keycloak-dev-bounces at lists.jboss.org>>> On Behalf Of Sebastian Laskawiec
> >> Sent: Monday, January 7, 2019 7:36 AM
> >> To: Peck, Michael A <mpeck at mitre.org<mailto:mpeck at mitre.org><mailto:
> mpeck at mitre.org<mailto:mpeck at mitre.org>>>; sblanc at redhat.com<mailto:
> sblanc at redhat.com><mailto:sblanc at redhat.com<mailto:sblanc at redhat.com>>
> >> Cc: keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> ><mailto:keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> >>
> >> Subject: Re: [keycloak-dev] User TLS client certificate authentication
> - inconsistent DN string representation with LDAP
> >>
> >> Hey Michael,
> >>
> >> Adding +Sebastien Blanc <sblanc at redhat.com<mailto:sblanc at redhat.com
> ><mailto:sblanc at redhat.com<mailto:sblanc at redhat.com>>> for visibility.
> >>
> >> I believe that's a bug. The `X509ClientCertificateAuthenticator` should
> ignore those extra spaces. May I kindly ask you to create a ticket for us
> and assign it either to me or Sebastien?
> >>
> >> Thanks,
> >> Sebastian
> >>
> >> On Sun, Dec 23, 2018 at 6:49 PM Peck, Michael A <mpeck at mitre.org
> <mailto:mpeck at mitre.org><mailto:mpeck at mitre.org<mailto:mpeck at mitre.org>>>
> wrote:
> >>
> >>> Hello,
> >>>
> >>> I’ve configured Keycloak to authenticate users using TLS client
> >>> certificate authentication.
> >>> I’ve also configured Keycloak to synchronize users with my LDAP server.
> >>>
> >>> I’d like to match the TLS client certificate’s Subject DN to the
> >>> Subject DNs synchronized from my LDAP server (which are stored by
> >>> Keycloak in each user’s LDAP_ENTRY_DN attribute).
> >>>
> >>> I’ve set that up, but am running into an issue that Keycloak appears
> >>> to have inconsistent string representations of DNs between those two
> >>> methods - so the Subject DNs from the TLS client certificate and the
> >>> LDAP server aren’t matching as I was expecting.
> >>>
> >>> The TLS client certificate DNs look like this:
> >>> CN=Peck Michael, OU=People, DC=test, DC=net
> >>>
> >>> While the LDAP_ENTRY_DN attribute is formatted like this:
> >>> cn=Peck Michael,ou=People,dc=test,dc=net
> >>>
> >>> It looks to me that the TLS client certificate DN string
> >>> representation is coming from the standard Java X500Principal class
> >>> used by calls to
> >>> X509Certificate.getSubjectDN().getName() in
> >>> keycloak/services/src/main/java/org/keycloak/authentication/authentica
> >>> tors/x509/X509ClientCertificateAuthenticator.java
> >>> and the LDAP_ENTRY_DN string representation is coming from the
> >>> toString method in
> >>>
> keycloak/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java.
> >>>
> >>> I modified the LDAPDn class’s toString method to follow the same
> >>> format as used in the TLS client certificate DNs, and authentication
> works for me now.
> >>> Would the Keycloak project consider accepting a pull request to change
> >>> the way LDAPDn formats DNs as strings?
> >>> (However I have not checked if this would impact other uses of the
> >>> LDAPDn class within Keycloak or cause problems with upgrading existing
> >>> deployments?)
> >>>
> >>> The suggested change follows:
> >>> diff --git
> >>> a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LD
> >>> APDn.java
> >>> b/federation/ldap/src/main/
> >>> index 39e7d97..2f8c805 100644
> >>> ---
> >>> a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LD
> >>> APDn.java
> >>> +++
> >>> b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LD
> >>> APDn.java @@ -87,9 +87,9 @@ public class LDAPDn {
> >>> if (first) {
> >>> first = false;
> >>> } else {
> >>> - builder.append(",");
> >>> + builder.append(", ");
> >>> }
> >>> -
> >>> builder.append(rdn.attrName).append("=").append(rdn.attrValue);
> >>> +
> >>>
> builder.append(rdn.attrName.toUpperCase()).append("=").append(rdn.attrValue);
> >>> }
> >>>
> >>> return builder.toString();
> >>>
> >>>
> >>>
> >>> Thank you,
> >>> Michael Peck
> >>> The MITRE Corporation
> >>> _______________________________________________
> >>> keycloak-dev mailing list
> >>> keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> ><mailto:keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> >>
> >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >> _______________________________________________
> >> keycloak-dev mailing list
> >> keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> ><mailto:keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> >>
> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>
> >> _______________________________________________
> >> keycloak-dev mailing list
> >> keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> ><mailto:keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org
> >>
> >> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>
> >
> > --
> > John Dennis
> >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev at lists.jboss.org<mailto:keycloak-dev at lists.jboss.org>
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >
> > _______________________________________________
> > keycloak-dev mailing list
> > keycloak-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
More information about the keycloak-dev
mailing list