6:38 p.m.
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/authenticators/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/LDAPDn.java
b/federation/ldap/src/main/
index 39e7d97..2f8c805 100644
--- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java
+++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.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
1:36 p.m.
Hey Michael,
Adding +Sebastien Blanc <sblanc(a)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(a)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/authenticators/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/LDAPDn.java
b/federation/ldap/src/main/
index 39e7d97..2f8c805 100644
---
a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java
+++
b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
11:02 p.m.
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?
Kindly,
Peter
-----Original Message-----
From: keycloak-dev-bounces(a)lists.jboss.org <keycloak-dev-bounces(a)lists.jboss.org> On
Behalf Of Sebastian Laskawiec
Sent: Monday, January 7, 2019 7:36 AM
To: Peck, Michael A <mpeck(a)mitre.org>; sblanc(a)redhat.com
Cc: keycloak-dev(a)lists.jboss.org
Subject: Re: [keycloak-dev] User TLS client certificate authentication - inconsistent DN
string representation with LDAP
Hey Michael,
Adding +Sebastien Blanc <sblanc(a)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(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
12:19 a.m.
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?
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(a)lists.jboss.org <keycloak-dev-bounces(a)lists.jboss.org>
On Behalf Of Sebastian Laskawiec
Sent: Monday, January 7, 2019 7:36 AM
To: Peck, Michael A <mpeck(a)mitre.org>; sblanc(a)redhat.com
Cc: keycloak-dev(a)lists.jboss.org
Subject: Re: [keycloak-dev] User TLS client certificate authentication - inconsistent DN
string representation with LDAP
Hey Michael,
Adding +Sebastien Blanc <sblanc(a)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(a)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(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
--
John Dennis
4:11 p.m.
Hey guys,
Answering a few email in a row below.
Thanks,
Sebastian
On Wed, Jan 9, 2019 at 12:19 AM John Dennis <jdennis(a)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(a)lists.jboss.org <
keycloak-dev-bounces(a)lists.jboss.org> On Behalf Of Sebastian Laskawiec
> Sent: Monday, January 7, 2019 7:36 AM
> To: Peck, Michael A <mpeck(a)mitre.org>; sblanc(a)redhat.com
> Cc: keycloak-dev(a)lists.jboss.org
> Subject: Re: [keycloak-dev] User TLS client certificate authentication -
inconsistent DN string representation with LDAP
>
> Hey Michael,
>
> Adding +Sebastien Blanc <sblanc(a)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(a)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(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
--
John Dennis
4:43 p.m.
On 1/9/19 10:11 AM, Sebastian Laskawiec wrote:
RFC 1779 (A String Representation of Distinguished Names) also
mentions
RFC 1779 is obsolete, please do not implement according to deprecated
standards. See https://ldap.com/ldap-related-rfcs/
RFC 4514 is the current standard for the string representation of
distinguished names.
I don't normally code in Java, but I'm sure there are multiple classes
in standard jars which already implement DN parsing and comparison.
--
John Dennis
3:18 a.m.
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(a)redhat.com]
Sent: Wednesday, January 9, 2019 10:11 AM
To: John Dennis
Cc: Nalyvayko, Peter; Peck, Michael A; sblanc(a)redhat.com; keycloak-dev(a)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@redhat.com<mailto:jdennis@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@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>
<keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>>
On Behalf Of Sebastian Laskawiec
Sent: Monday, January 7, 2019 7:36 AM
To: Peck, Michael A <mpeck@mitre.org<mailto:mpeck@mitre.org>>;
sblanc@redhat.com<mailto:sblanc@redhat.com>
Cc: keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
Subject: Re: [keycloak-dev] User TLS client certificate authentication - inconsistent DN
string representation with LDAP
Hey Michael,
Adding +Sebastien Blanc <sblanc@redhat.com<mailto:sblanc@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@mitre.org<mailto:mpeck@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@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
--
John Dennis
11:59 a.m.
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(a)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(a)redhat.com]
Sent: Wednesday, January 9, 2019 10:11 AM
To: John Dennis
Cc: Nalyvayko, Peter; Peck, Michael A; sblanc(a)redhat.com;
keycloak-dev(a)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(a)redhat.com<mailto:
jdennis(a)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(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org> <
keycloak-dev-bounces(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org>> On Behalf Of Sebastian Laskawiec
> Sent: Monday, January 7, 2019 7:36 AM
> To: Peck, Michael A <mpeck@mitre.org<mailto:mpeck@mitre.org>>;
sblanc@redhat.com<mailto:sblanc@redhat.com>
> Cc: keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
> Subject: Re: [keycloak-dev] User TLS client certificate authentication -
inconsistent DN string representation with LDAP
>
> Hey Michael,
>
> Adding +Sebastien Blanc <sblanc@redhat.com<mailto:sblanc@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(a)mitre.org<mailto:
mpeck(a)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@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
--
John Dennis
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
5:04 a.m.
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.
________________________________________
From: Hynek Mlnarik [hmlnarik(a)redhat.com]
Sent: Tuesday, January 15, 2019 5:59 AM
To: Nalyvayko, Peter
Cc: Sebastian Laskawiec; John Dennis; keycloak-dev(a)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@agi.com<mailto:pnalyvayko@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@redhat.com<mailto:slaskawi@redhat.com>]
Sent: Wednesday, January 9, 2019 10:11 AM
To: John Dennis
Cc: Nalyvayko, Peter; Peck, Michael A; sblanc@redhat.com<mailto:sblanc@redhat.com>;
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@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@redhat.com<mailto:jdennis@redhat.com><mailto:jdennis@redhat.com<mailto:jdennis@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@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org><mailto:keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>>
<keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org><mailto:keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>>>
On Behalf Of Sebastian Laskawiec
Sent: Monday, January 7, 2019 7:36 AM
To: Peck, Michael A
<mpeck@mitre.org<mailto:mpeck@mitre.org><mailto:mpeck@mitre.org<mailto:mpeck@mitre.org>>>;
sblanc@redhat.com<mailto:sblanc@redhat.com><mailto:sblanc@redhat.com<mailto:sblanc@redhat.com>>
Cc:
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
Subject: Re: [keycloak-dev] User TLS client certificate authentication - inconsistent DN
string representation with LDAP
Hey Michael,
Adding +Sebastien Blanc
<sblanc@redhat.com<mailto:sblanc@redhat.com><mailto:sblanc@redhat.com<mailto:sblanc@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@mitre.org<mailto:mpeck@mitre.org><mailto:mpeck@mitre.org<mailto:mpeck@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@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
--
John Dennis
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
12:30 p.m.
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(a)redhat.com]
Sent: Tuesday, January 15, 2019 5:59 AM
To: Nalyvayko, Peter
Cc: Sebastian Laskawiec; John Dennis; keycloak-dev(a)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@agi.com<mailto:pnalyvayko@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@redhat.com<mailto:slaskawi@redhat.com>]
Sent: Wednesday, January 9, 2019 10:11 AM
To: John Dennis
Cc: Nalyvayko, Peter; Peck, Michael A; sblanc@redhat.com<mailto:sblanc@redhat.com>;
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@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@redhat.com<mailto:jdennis@redhat.com><mailto:jdennis@redhat.com<mailto:jdennis@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@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org><mailto:keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>>
<keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org><mailto:keycloak-dev-bounces@lists.jboss.org<mailto:keycloak-dev-bounces@lists.jboss.org>>>
On Behalf Of Sebastian Laskawiec
> Sent: Monday, January 7, 2019 7:36 AM
> To: Peck, Michael A
<mpeck@mitre.org<mailto:mpeck@mitre.org><mailto:mpeck@mitre.org<mailto:mpeck@mitre.org>>>;
sblanc@redhat.com<mailto:sblanc@redhat.com><mailto:sblanc@redhat.com<mailto:sblanc@redhat.com>>
> Cc:
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
> Subject: Re: [keycloak-dev] User TLS client certificate authentication - inconsistent
DN string representation with LDAP
>
> Hey Michael,
>
> Adding +Sebastien Blanc
<sblanc@redhat.com<mailto:sblanc@redhat.com><mailto:sblanc@redhat.com<mailto:sblanc@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@mitre.org<mailto:mpeck@mitre.org><mailto:mpeck@mitre.org<mailto:mpeck@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@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> _______________________________________________
> keycloak-dev mailing list
>
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
> _______________________________________________
> keycloak-dev mailing list
>
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
--
John Dennis
_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev
6:01 p.m.
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(a)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(a)redhat.com]
> Sent: Tuesday, January 15, 2019 5:59 AM
> To: Nalyvayko, Peter
> Cc: Sebastian Laskawiec; John Dennis; keycloak-dev(a)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(a)agi.com
<mailto:pnalyvayko@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(a)redhat.com<mailto:
slaskawi(a)redhat.com>]
> Sent: Wednesday, January 9, 2019 10:11 AM
> To: John Dennis
> Cc: Nalyvayko, Peter; Peck, Michael A; sblanc(a)redhat.com<mailto:
sblanc(a)redhat.com>; keycloak-dev(a)lists.jboss.org<mailto:
keycloak-dev(a)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(a)redhat.com<mailto:
jdennis@redhat.com><mailto:jdennis@redhat.com<mailto:jdennis@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(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org><mailto:
keycloak-dev-bounces(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org>> <
keycloak-dev-bounces(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org><mailto:
keycloak-dev-bounces(a)lists.jboss.org<mailto:
keycloak-dev-bounces(a)lists.jboss.org>>> On Behalf Of Sebastian Laskawiec
>> Sent: Monday, January 7, 2019 7:36 AM
>> To: Peck, Michael A
<mpeck@mitre.org<mailto:mpeck@mitre.org><mailto:
mpeck@mitre.org<mailto:mpeck@mitre.org>>>; sblanc(a)redhat.com<mailto:
sblanc@redhat.com><mailto:sblanc@redhat.com<mailto:sblanc@redhat.com>>
>> Cc: keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
>>
>> Subject: Re: [keycloak-dev] User TLS client certificate authentication
- inconsistent DN string representation with LDAP
>>
>> Hey Michael,
>>
>> Adding +Sebastien Blanc <sblanc@redhat.com<mailto:sblanc@redhat.com
><mailto:sblanc@redhat.com<mailto:sblanc@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(a)mitre.org
<mailto:mpeck@mitre.org><mailto:mpeck@mitre.org<mailto:mpeck@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@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
>>
>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
>>
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
><mailto:keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org
>>
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>
> --
> John Dennis
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev@lists.jboss.org<mailto:keycloak-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
1893
days inactive
1944
days old
10 comments
6 participants
participants (6)
-
Hynek Mlnarik -
John Dennis -
Marek Posolda -
Nalyvayko, Peter -
Peck, Michael A -
Sebastian Laskawiec