binding as an existing user "None"

Hi,

nginx-ldap-auth/nginx-ldap-auth-daemon.py

Line 231 in 850f5ea

if len(results) < 1:

Wouldn't it be supposed to check not only the length of the results, but also what's inside?
if len(results) < 1:

The following is the output from the daemon:

results with an existent user

[('CN=my.username,OU=AADDC Users,DC=test01ad,DC=me,DC=net',
  {'objectClass': []}),
 (None,
  ['ldaps://DomainDnsZones.test01ad.me.net/DC=DomainDnsZones,DC=test01ad,DC=me,DC=net']),
 (None,
  ['ldaps://ForestDnsZones.test01ad.me.net/DC=ForestDnsZones,DC=test01ad,DC=me,DC=net']),
 (None,
  ['ldaps://test01ad.me.net/CN=Configuration,DC=test01ad,DC=me,DC=net'])]

results with a non-existent user

[(None,
  ['ldaps://ForestDnsZones.test01ad.me.net/DC=ForestDnsZones,DC=test01ad,DC=me,DC=net']),
 (None,
  ['ldaps://DomainDnsZones.test01ad.me.net/DC=DomainDnsZones,DC=test01ad,DC=me,DC=net']),
 (None,
  ['ldaps://test01ad.me.net/CN=Configuration,DC=test01ad,DC=me,DC=net'])]

Even the latter case, the script falls through and returns 200(Auth OK) which is wrong as the user in question does not exist(in my case in MS Active Directory).

The len() check is just a quick test for results to give better error to user if nothing was found.
The code relies on successful bind operation with a found user with provided credentials.
What filter are you using to find users and why is it returning results in case of wrong username? I.e. I'd expect that proper filter will return zero results with non-existent user.

The following is the filter I am using

proxy_set_header X-Ldap-Template "(&(sAMAccountName=%(username)s)(memberOf=CN=groupx,OU=AADDC Users,DC=test01ad,DC=me,DC=net)(objectClass=user))"

I thought that this would suffice to filter out the non-existent users as they would match nither of the attributes specified above.

Is my setting wrong or missing something?

Looks like you are hitting this issue:
https://stackoverflow.com/questions/9441594/python-ldap-and-active-directory-issue
I.e. such an answer is a referral.
You may want to disable them, but setting 'disable_referrals' options.
Since I don't have access to advanced AD installation, I can only suggest how
it works in your case. Can you please check results with 'disable_referrals' option?
If this is the case, I think we need to force stricter checks on results and give some
warning/error.
So what happens next is that passing 'None' to bind_s leads to anonymous bind (which is enabled by default, at least on AD 2003). You may want to check in server logs if this is what actually happening.

results with a non-existent user
[(None,
['ldaps://ForestDnsZones.test01ad.me.net/DC=ForestDnsZones,DC=test01ad,DC=me,DC=net']),
(None,
['ldaps://DomainDnsZones.test01ad.me.net/DC=DomainDnsZones,DC=test01ad,DC=me,DC=net']),
(None,
['ldaps://test01ad.me.net/CN=Configuration,DC=test01ad,DC=me,DC=net'])]

..and this is strange, because I see the error in such case:

[(None, ['ldap://127.0.0.1:8085/ou=more,ou=Users,dc=test,dc=local??sub'])]
User entry for 'userx' is: (None, ['ldap://127.0.0.1:8085/ou=more,ou=Users,dc=test,dc=local??sub']) dn is 'None'
localhost.localdomain - userx [24/Oct/2018 18:20:15] Error while binding as an existing user "None": argument 1 must be string, not None, server="ldap://127.0.0.1:8083", login="userx"

Which python/python_ldap versions are used?

You may want to disable them, but setting 'disable_referrals' options.

I have already set it like the following, but as per my understanding, the referrals are disabled by default in the module.

proxy_set_header X-Ldap-DisableReferrals "true";

I do not have the granular level of control over AD as it is the managed service on MS Azure.

Which python/python_ldap versions are used?

python: 2.7.14
python-ldap: 3.1.0

Based on the post on StackOverflow you are referring to, the "None" should be handled by the client side, shouldn't it?

Thank you for the information,
It looks like python-ldap indeed changed the behaviour somewhere between 2.4 and 3.1.0
to allow None to be passed into bind as valid value (meaning anonymous, while previously it resulted in type error check).
I've added some additional checks and more logging. Can you please test the patch below?
Regarding the last question: yes, it should be handled on the client side.

diff --git a/nginx-ldap-auth-daemon.py b/nginx-ldap-auth-daemon.py
index 46daf3b..bdfafff 100755
--- a/nginx-ldap-auth-daemon.py
+++ b/nginx-ldap-auth-daemon.py
@@ -228,13 +228,27 @@ class LDAPAuthHandler(AuthHandler):
                                           searchfilter, ['objectclass'], 1)
 
             ctx['action'] = 'verifying search query results'
-            if len(results) < 1:
+
+            nres = len(results)
+
+            if nres < 1:
                 self.auth_failed(ctx, 'no objects found')
                 return
 
-            ctx['action'] = 'binding as an existing user'
-            ldap_dn = results[0][0]
-            ctx['action'] += ' "%s"' % ldap_dn
+            if nres > 1:
+                self.log_message("note: filter match multiple objects: %d, using first" % nres)
+
+            user_entry = results[0]
+            ldap_dn = user_entry[0]
+
+            if ldap_dn == None:
+                self.auth_failed(ctx, 'matched object has no dn')
+                return
+
+            self.log_message('attempting to bind using dn "%s"' % (ldap_dn))
+
+            ctx['action'] = 'binding as an existing user "%s"' % ldap_dn
+
             ldap_obj.bind_s(ldap_dn, ctx['pass'], ldap.AUTH_SIMPLE)
 
             self.log_message('Auth OK for user "%s"' % (ctx['user']))

Thank you for the patch.

I have tested it on my machine and confirmed that the daemon works fine now.