LDAP search base require JNDI escaping in addition to LDAP escaping for special characters

[NaozumiTaromaru]

Summary

When use <sec:ldap-authentication-provider> with <sec:password-compare>, user's dn lacks JNDI escaping.
Strictly speaking, method that requires JNDI escaping is used when PasswordComparisonAuthenticator is used.
When passing dn to the Context, javax.naming.Name should be used instead of String.

see: https://docs.oracle.com/javase/jndi/tutorial/beyond/names/syntax.html

To avoid any surprises if a name contains special characters that might conflict with the JNDI composite name syntax, you should use the Context methods that accept a Name.

Actual Behavior

Case 1: User can't sign in with the correct username and password.

1. Use user-dn-pattern="uid={0},ou=people" at <sec:ldap-authentication-provider>.
2. Input admin\ as existing username and correct password to login form, and Sign in.
3. (Result) The following message is shown.
   • Bad credentials

• If do any of the following, sign in succeeds:
  • Input admin\\ as username instead of admin\. (But granting no authority.)
  • Use user-search-filter="(uid={0})" and user-search-base="ou=people" instead of user-dn-pattern="uid={0},ou=people". (But, this has another problem Case 2.)
  • Use <sec:ldap-authentication-provider> without <sec:password-compare>. (Also change the password on the LDAP server.)
  • Use <sec:authentication-provider> and <sec:ldap-user-service>(using user-search-filter and user-search-base) instead of <sec:ldap-authentication-provider>. (There is no problem.)

Case 1': Invalid dn is created.

1. Use user-dn-pattern="uid={0},ou=people" at <sec:ldap-authentication-provider>. (same as Case 1)
2. Input admin\y as username and some string as password (username and password do not have to be correct) to login form, and Sign in.
3. (Result) The following message is shown.
   * uid=admin\y,ou=people: [LDAP: error code 34 - invalid DN]; nested exception is javax.naming.InvalidNameException: uid=admin\y,ou=people: [LDAP: error code 34 - invalid DN]; remaining name 'uid=admin\y,ou=people'

Case 2: Invalid dn is created when existing username and wrong password are input only.

1. Use user-search-filter="(uid={0})" and user-search-base="ou=people" at <sec:ldap-authentication-provider>.
2. Input admin\x as existing username and wrong password to login form, and Sign in.
3. (Result) The following message is shown.
   • uid=admin\x,ou=people: [LDAP: error code 34 - invalid DN]; nested exception is javax.naming.InvalidNameException: uid=admin\x,ou=people: [LDAP: error code 34 - invalid DN]; remaining name 'uid=admin\x,ou=people'

Expected Behavior

Result of Case 1:

• Sign in succeeds as admin\.

Result of Case 1':

• The following message is shown.
  • Bad credentials
• If username and password are correct, Sign in succeeds as admin\y.

Result of Case 2:

• The following message is shown.
  • Bad credentials

Configuration

Case 1 and Case 1':

  <sec:authentication-manager>
      <sec:ldap-authentication-provider
          user-dn-pattern="uid={0},ou=people"
          group-search-base="ou=groups">
          <sec:password-compare>
              <sec:password-encoder ref="passwordEncoder"/>
          </sec:password-compare>
      </sec:ldap-authentication-provider>
  </sec:authentication-manager>

Case 2:

  <sec:authentication-manager>
      <sec:ldap-authentication-provider
          user-search-base="ou=people"
          user-search-filter="(uid={0})"
          group-search-base="ou=groups" >
          <sec:password-compare>
              <sec:password-encoder ref="passwordEncoder"/>
          </sec:password-compare>
      </sec:ldap-authentication-provider>
  </sec:authentication-manager>

Common configuration:

  <sec:ldap-server
      url="ldap://xxx:389/dc=xxx,dc=org"
      manager-dn="cn=xxx,dc=xxx,dc=org"
      manager-password="xxx"
  />

Existing User's dn(after LDAP escaping):

• uid=admin\\,ou=people,dc=xxx,dc=org (username is admin\)
• uid=admin\\x,ou=people,dc=xxx,dc=org (username is admin\x)

Version

• 5.1.3.RELEASE (I use this version.)
• 5.2.1.RELEASE (latest) (It's reproduced also this version.)

Environment

• Redhat OpenJDK 1.8.0.212-3.b04
• Open LDAP

Sample

Use default Login Form of Spring Security(<sec:form-login/>) and the above configuration.

Analysis

Case 1 (and Case 1'):
At org.springframework.security.ldap.authentication.PasswordComparisonAuthenticator#authenticate(Authentication),

user = ldapTemplate.retrieveEntry(userDn, getUserAttributes());

This userDn is String that value is uid=admin\\,ou=people. (after LDAP escaping)
(When Case 1', the value is uid=admin\\y,ou=people.)

This ldapTemplate is org.springframework.security.ldap.SpringSecurityLdapTemplate.

At org.springframework.security.ldap.SpringSecurityLdapTemplate#retrieveEntry(String, String[]),

Attributes attrs = ctx.getAttributes(dn, attributesToRetrieve);

This dn is String that value is uid=admin\\,ou=people. (after LDAP escaping)
(When Case 1', the value is uid=admin\\y,ou=people.)

This ctx is javax.naming.ldap.InitialLdapContext.

At javax.naming.ldap.InitialLdapContext#getAttributes(String, String[])

return getURLOrDefaultInitDirCtx(name).getAttributes(name, attrIds);

This getURLOrDefaultInitDirCtx(name) returns com.sun.jndi.ldap.LdapCtx object.
(LdapCtx is created by com.sun.jndi.ldap.LdapCtxFactory.)

As a result, at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext#getAttributes(String, String[])

return getAttributes(new CompositeName(name), attrIds);

This name is String that value is uid=admin\\,ou=people. (after LDAP escaping)
(When Case 1', the value is uid=admin\\y,ou=people.)

The constructor of CompositeName JNDI unescape the argument.
(Therefore, in Case 1, \\, of uid=admin\\,ou=people becomes \, meaning LDAP escaped ,. And in Case 1', \\y, of uid=admin\\y,ou=people becomes an invalid sequence \y,.)
This is an extra unescaping.
(If new CompositeName().add(name) is used instead of new CompositeName(name),
this extra unescaping is not performed.)

If LdapCtxFactory creates com.sun.jndi.ldap.LdapReferralContext instead of LdapCtx,
LdapReferralContext#getAttributes(String, String[]) is executed,
and it execute new CompositeName().add(name) instead of new CompositeName(name).
(However, I have never seen LdapReferralContext created.)

SpringSecurityLdapTemplate should also have a method that accepts dn as javax.naming.Name instead of String.
And new method SpringSecurityLdapTemplate#retrieveEntry(Name, String[]) should use javax.naming.ldap.InitialLdapContext#getAttributes(Name, String[]).
PasswordComparisonAuthenticator#authenticate(Authentication) should use
new method SpringSecurityLdapTemplate#retrieveEntry(Name, String[]) instead of SpringSecurityLdapTemplate#retrieveEntry(String, String[]),
and Name should be created as one of the following:

• new CompositeName().add(name)
• new LdapName(name) (new CompositeName().add(name) is called by class of JDK.)

see: https://docs.oracle.com/javase/jndi/tutorial/beyond/names/syntax.html

Case 2:

When existing username and wrong password are input,
org.springframework.security.ldap.authentication.PasswordComparisonAuthenticator#isLdapPasswordCompare(DirContextOperations, SpringSecurityLdapTemplate, String)
is executed.
(This method is not executed, when correct password.)

At org.springframework.security.ldap.authentication.PasswordComparisonAuthenticator#isLdapPasswordCompare(DirContextOperations, SpringSecurityLdapTemplate, String),

return ldapTemplate.compare(user.getDn().toString(), passwordAttributeName,
passwordBytes);

This user.getDn().toString() returns uid=admin\\x,ou=people. (after LDAP escaping)

This ldapTemplate is org.springframework.security.ldap.SpringSecurityLdapTemplate.

At org.springframework.security.ldap.SpringSecurityLdapTemplate#compare(String, String, Object)

NamingEnumeration results = ctx.search(dn,
comparisonFilter, new Object[] { value }, ctls);

This dn is String that value is uid=admin\\x,ou=people. (after LDAP escaping)

As a result, at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext#search(String, String, Object[], SearchControls)

return search(new CompositeName(name), filterExpr, filterArgs, cons);

This name is String that value is uid=admin\\x,ou=people. (after LDAP escaping)

The constructor of CompositeName JNDI unescape the argument.
This is an extra unescaping.

Again, like Case 1, PasswordComparisonAuthenticator#isLdapPasswordCompare and SpringSecurityLdapTemplate#compare should pass dn as javax.naming.Name instead of String.

see: https://docs.oracle.com/javase/jndi/tutorial/beyond/names/syntax.html

[fhanik]

Thank you for a very detailed report @NaozumiTaromaru . The double escaping between JNDI and LDAP is indeed confusing and can create regressions if we don't tread carefully, as there may be implementations that have added escaping on top of the existing code base.

If you wish to collaborate on this let me know. Regression is something we wish to avoid.

[NaozumiTaromaru]

The double escaping between JNDI and LDAP is indeed confusing and can create regressions if we don't tread carefully, as there may be implementations that have added escaping on top of the existing code base.

If you pay attention to the following three, I think that regressions will not occur.

• String dn is according to RFC2253. In other words, the value of dn is after LDAP escaping according to RFC2253.
  • I think this is already done in most places.
• When passing dn to each method of InitialLdapContext as seach base, pass it as Name. (Do not write JNDI escape logic.)
  • The common thing about the problematic part is that dn is passed as a String instead of a Name.
  • When creating a Name, use one of the following:
    • new DistinguishedName(dn)
    • new LdapName(dn) (LdapUtils.newLdapName(dn))
    • new new CompositeName().add(dn)
• If the implementation class of Name cannot be specified, do not write processing that depends on Name#toString().
  • This is because, for example, CompositeName#toString() does JNDI escaping, but LdapName#toString() does not do JNDI escaping.
  • return ldapTemplate.compare(user.getDn().toString(), passwordAttributeName, passwordBytes);
    of PasswordComparisonAuthenticator#isLdapPasswordCompare.
    is a bad example.

As reference information on the impact on existing applications, I confirmed what escaped values should be set for user-dn-pattern, user-search-base, and group-search-base to work in AS IS.

• user-dn-pattern of <sec:ldap-authentication-provider>

  • with <sec:password-compare>:
    Due to the implementation of org.springframework.security.ldap.SpringSecurityLdapTemplate#retrieveEntry(String, String[]), there is no correct escaping method.
    user-dn-pattern is passed to InitialLdapContext#getAttributes as String dn.
    However, it is also passed to the constructor of DistinguishedName.
    For example, if the value of ou is people\a\\b+,
    (dn is uid={0},ou=people\\a\\\\b\+ (after LDAP escaping according to RFC2253))
    • user-dn-pattern="uid={0},ou=people\\a\\\\b\+" ... Invalid sequence as dn after JNDI upescaping.
      • It is correct value when passing to DistinguishedName, but it is invalid sequence when passing to InitialLdapContext#getAttributes as String dn.
    • user-dn-pattern="uid={0},ou=people\\\\a\\\\\\\\b\\+" (adding JNDI escaping) ... Invalid sequence as dn.
      • It is correct value of one when passing to InitialLdapContext#getAttributes as String dn, but it is invalid sequence when passing to DistinguishedName.
    • user-dn-pattern="uid={0},ou=people\\\a\\\\\\\b\+" (adding JNDI escaping) ... Invalid sequence as dn.
      • It is correct value of one when passing to InitialLdapContext#getAttributes as String dn, but it is invalid sequence when passing to DistinguishedName.
    • user-dn-pattern="uid={0},ou=people\\\\a\\\\\\\\b\+" (adding JNDI escaping) ... I can sign in. But granting no authority.
      • It is correct value of one when passing to InitialLdapContext#getAttributes as String dn, and it is valid sequence when passing to DistinguishedName, but it is wrong value when passing to DistinguishedName.
  • without <sec:password-compare>:
    LDAP escaping according to RFC2253 only.
    For example, if the value of ou is people\a\\b+,
    • user-dn-pattern="uid={0},ou=people\\a\\\\b\+"

• user-search-base of <sec:ldap-authentication-provider> and <sec:ldap-user-service>
  LDAP escaping according to RFC2253 only.
  For example, if the value of ou is people\a\\b+,

  • user-dn-pattern="uid={0},ou=people\\a\\\\b\+"

• group-search-base of <sec:ldap-authentication-provider> and <sec:ldap-user-service>
  LDAP escaping according to RFC2253 and JNDI escaping.
  For example, if the value of ou is groups\a\\b+,
  (dn is ou=groups\\a\\\\b\+ (after LDAP escaping according to RFC2253))

  • group-search-base="ou=groups\\\\a\\\\\\\\b\\+"
  • group-search-base="ou=groups\\\a\\\\\\\b\+"

The problem of user-dn-pattern of <sec:ldap-authentication-provider> with <sec:password-compare>, I think it's OK to fix it right away.
If this is fixed, Case 1 and Case 1' of Actual Behavior will be fixed.
In particular, if Case 1 and Case 1' are not fixed, LDAP escaping are unescaped unexpectedly, which is not good.

The behavior of group-search-base of <sec:ldap-authentication-provider> and <sec:ldap-user-service> does not need to be changed immediately.
It's hard to use, but it doesn't mean it can't be used. In addition, the application may be affected by the modification.
To make it easy to use like other attributes, I think that there is no problem in a later version.

The cause of Case2 of Actual Behavior is just a bug.
As long as user.getDn()
of return ldapTemplate.compare(user.getDn().toString(), passwordAttributeName, passwordBytes);
of PasswordComparisonAuthenticator#isLdapPasswordCompare
returns correct LdapName instance, special characters will not work properly.
Here, to avoid escape leakage and double escape, it is better to use the Name returned by user.getDn() without using toString().

If the user has added a JNDI escape as in user-dn-pattern="uid={0},ou=people\\\\a\\\\\\\\b\+", the value uid=XXX,ou=people\\\\a\\\\\\\\b\+ with the extra JNDI escape applied only at ou is set in the LdapName instance returned by user.getDn().
By passing the result of toString() of LdapName with an incorrect value to the method of InitialLdapContext, there is a case what incorrect (extra JNDI escaping) value is JNDI unescaped and it become correct dn.
(Because the uid part - that is not JNDI escaped - is JNDI unescaped, there is a case what it become wrong dn like Case 1 or Case 1 '.)

In other words, Case2 must be resolved simultaneously with Case 1 and Case 1'.