Update: LDAP_Injection_Prevention_Cheat_Sheet

[einhirn]

What is missing or needs to be updated?

After a casual reading, the Cheat Sheet seems to suggests that even passwords should always be escaped. When this is taken at face value, it can cause all kinds of issues, mainly stuff not working as expected any more (see https://github.com/dexidp/dex/issues/3433).

The Safe Java Escaping Example wrongly assumes that a password check against LDAP is done via LDAP filter and escapes the password, when actually password checking against an ldap server is (or at least should be) exclusively done via BIND request which safely accepts the password in unfiltered plaintext.

How should this be resolved?

The usual methods for password authentication against LDAP should be mentioned:

• Single-Step Auth:
  • BIND-Request with DN constructed from template like uid=${USERID},ou=people,dc=example,dc=com. Whatever is inserted in that template should be escaped as needed. Password from user input is given to BIND call as is.
• Three-Step-Auth:
  • BIND with credentials limited to searching - these would be configured in the app.
  • SEARCH LDAP for users DN like (&(uid=${USERID})(deactivated=0)) or something. Again, anything inserted into that query should be escaped as needed
  • BIND with DN from search result above, if any. Password from user input is given to BIND call as is.

These two both assume the LDAP server to require (non-anonymous) BIND before allowing access to user data, which I think is (or should be) default or at least best practice. So Enabling Bind Authentication should be moved up in the document's focus, i.e. "To prevent LDAP Injection, first the LDAP server needs to be configured properly...". Even if it's out of scope of this document, proper safety precautions should be mentioned - "Simple BIND transmits plain text passwords to the server, so it should only be used with an encrypted connection" or "If you really need to use unencrypted connection, make sure to only use SASL BIND with challenge protocols etc. to prevent password sniffing."

The Safe Java Escaping Example should be updated to reflect either one of the above methods and no longer use the password in a search filter.

[jmanico]

This cheatsheet can use a lot of changes. I like your ideas so far. Would you care to submit PR's?

[einhirn]

I'll look into it and see when I might find some time to do that... I tend to overthink what I write on a word for word level, so it takes a lot of time to write something, especially something public in the field of security 😬

[Jeymz]

What is missing or needs to be updated?

After a casual reading, the Cheat Sheet seems to suggests that even passwords should always be escaped. When this is taken at face value, it can cause all kinds of issues, mainly stuff not working as expected any more (see dexidp/dex#3433).

The Safe Java Escaping Example wrongly assumes that a password check against LDAP is done via LDAP filter and escapes the password, when actually password checking against an ldap server is (or at least should be) exclusively done via BIND request which safely accepts the password in unfiltered plaintext.

How should this be resolved?

The usual methods for password authentication against LDAP should be mentioned:

• Single-Step Auth:

  • BIND-Request with DN constructed from template like uid=${USERID},ou=people,dc=example,dc=com. Whatever is inserted in that template should be escaped as needed. Password from user input is given to BIND call as is.

• Three-Step-Auth:

  • BIND with credentials limited to searching - these would be configured in the app.
  • SEARCH LDAP for users DN like (&(uid=${USERID})(deactivated=0)) or something. Again, anything inserted into that query should be escaped as needed
  • BIND with DN from search result above, if any. Password from user input is given to BIND call as is.

These two both assume the LDAP server to require (non-anonymous) BIND before allowing access to user data, which I think is (or should be) default or at least best practice. So Enabling Bind Authentication should be moved up in the document's focus, i.e. "To prevent LDAP Injection, first the LDAP server needs to be configured properly...". Even if it's out of scope of this document, proper safety precautions should be mentioned - "Simple BIND transmits plain text passwords to the server, so it should only be used with an encrypted connection" or "If you really need to use unencrypted connection, make sure to only use SASL BIND with challenge protocols etc. to prevent password sniffing."

The Safe Java Escaping Example should be updated to reflect either one of the above methods and no longer use the password in a search filter.

Are you specifically talking about this area? I agree after reading this cheatsheet I think there is a lot of misinformation of proper ldap injection prevention. I am slightly concerned though on the search filter which is using an ldap query to identify users based on the password, I feel like a bind request should be the method used here to identify the user password match and then a search filter on the user identity to identify status of that account.

I wouldn't mind taking a stab at updating this, but I will admit I don't do much with ldap queries anymore.