node-ldapauth-fork icon indicating copy to clipboard operation
node-ldapauth-fork copied to clipboard

Possible mishandling of backslashes in CNs

Open s100 opened this issue 8 months ago • 1 comments

We want to support CNs with unusual characters. For testing purposes, we have a user with a CN which is the following string:

const cn = '<User>,,<With>,,<Special>++<Chars>\\'

Notice that the final character in this string is a single backslash.

I have been having trouble getting our unit test to work after upgrading from ldapauth-fork@5 to ldapauth-fork@6. After a long investigation I have discovered that under ldapauth-fork@5 this CN was being converted to the following DN:

const dn = 'cn=\\3CUser\\3E\\2C\\2C\\3CWith\\3E\\2C\\2C\\3CSpecial\\3E\\2B\\2B\\3CChars\\3E\\5C,ou=users,o=mqst'

whereas under ldapauth-fork@6 the CN is converted to:

const dn = 'cn=\\3cUser\\3e\\2c\\2c\\3cWith\\3e\\2c\\2c\\3cSpecial\\3e\\2b\\2b\\3cChars\\3e\\,ou=users,o=mqst'

See how under ldapauth-fork@5 the backslash has been escaped as '\\5C' - backslash, "5", "C", as one would expect. However, with ldapauth-fork@6 the backslash has been passed through unmodified and unescaped, as '\\'. This causes authentication to fail.

Ultimately I believe the issue is here, in the source code of @ldapjs/dn. Notice that the backslash is omitted from the embeddedReservedChars array, and a comment states "We will handle the reverse solidus ('\') on its own." I'm not entirely sure how this is meant to work, but I see no code in evidence which actually handles backslashes, whereas adding an entry 0x5c to the array causes a properly escaped DN to appear, and fixes the issue.

However, @ldapjs/dn is no longer maintained. Nor is ldapjs, which is what uses @ldapjs/dn internally. So unfortunately the bug comes to ldapauth-fork, which uses ldapjs internally. Is there something that we can do in ldapauth-fork to repair this issue? Do we need to fork ldapjs and the whole @ldapjs/* ecosystem to get this fixed? (And is this even a correct diagnosis of the problem, or would my suggested fix cause different problems?)

I would be interested to hear your thoughts on this. Thank you for an excellent project!

s100 avatar Jun 28 '24 11:06 s100