search

korylprince / go-ad-auth

Simple Active Directory authentication library for Go
MIT License
169 stars 28 forks source link

Escape LDAP filter arguments #12

Open BZValoche opened 3 years ago

BZValoche commented 3 years ago

Hi,

When entry.DN contains a backslash, a parenthesis, an asterisk or a null character, the following query fails because the filter is invalid. https://github.com/korylprince/go-ad-auth/blob/0ee4caa3e0d969ebe086e28cf082fe17f63ee6a7/auth.go#L60

You should escape entry.DN using the rules detailed here, paragraph "The String Representation of LDAP Filters": https://ldap.com/ldap-filters/

korylprince commented 3 years ago

@BZValoche good catch! You found an edge case that I certainly didn't expect.

I believe this issue is fixed in v3.1.3. I've also added a test case for this issue. Can you please test the new version and reopen this case if it didn't fix the issue for you?

korylprince commented 3 years ago

Actually, after further research, this doesn't fully resolve the issue, though it does seem to cover some cases. See this go-ldap issue.

I think this issue would be better resolved in go-ldap, but I'll entertain a PR if someone wants to send one. As of right now, I don't have the time to devote to this.

BZValoche commented 3 years ago

I just tested, it resolves the issue. I don't think the issue you are mentioning is worth talking about. When you send a LDAP query, your filter must be properly formed, i.e. parenthesis escaped where needed. go-ldap just cannot guess which parenthesis should be escaped, and which should not. The fix in your package is the proper way to go IMHO. And as a matter of fact, it... just... works... ;-) Thanks for the good work.

korylprince commented 3 years ago

Good to hear!

In testing, there are still issues with CNs like (test, test=, and probably whatever other special symbols should be escaped in the DN. The filter compiles, but since the library doesn't properly escape those characters, searches and modifies will fail on them with a "not found" error. While I'd love this library to be feature complete, you're the first who's ever had an issues with special characters. Honestly, if you're naming your objects with weird characters or unclosed parenthesis, you're probably having issues with more than just this library.

I'll keep this issue open if anyone wants to send a PR, or maybe I'll have some time to mess with this in the future.

BZValoche commented 3 years ago

I read more carefully, and of course, you are right. For the record, I ran into this issue because someone from work (whom I haven't found... yet) renamed a number of AD users, who suddenly lost access to an application I wrote.

Oh, and the parenthesis were closed. Like this : cn=Some Guy (foo),dc=bar,dc=com