[go: nahoru, domu]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Without bind_dn_template, any password is authenticated successfully! #44

Closed
djrobstep opened this issue Aug 25, 2017 · 10 comments · Fixed by #95
Closed

Without bind_dn_template, any password is authenticated successfully! #44

djrobstep opened this issue Aug 25, 2017 · 10 comments · Fixed by #95
Labels
¡bug! needs-verification

Comments

@djrobstep
Copy link
djrobstep commented Aug 25, 2017 •
edited
Loading

When initially configuring ldapauthenticator, I accidentally left bind_dn_template out of my config. When I tested it, I noticed it was authenticating me even with incorrect passwords!

This seems very dangerous. Although this parameter is required, authentication should probably always fail rather than always succeeding (or jupyter hub should simply refuse to start if required values are missing).

This happened when I had the following config vars set: server_address, lookup_dn, user_search_base, user_attribute.
@dhirschfeld
Copy link
Collaborator

This sounds like a bug. I'll check if I can reproduce to confirm...

@ermakovpetr
Copy link

this is fact!
after update 1.1->1.2.2 have this problem

@ermakovpetr
Copy link
ermakovpetr commented Jun 22, 2018 •
edited by dhirschfeld
Loading

If don't set bind_dn_template - bind_dn_template=[]
and isBound don't set True
https://github.com/jupyterhub/ldapauthenticator/blob/master/ldapauthenticator/ldapauthenticator.py#L315-L335

ldapauthenticator/ldapauthenticator/ldapauthenticator.py

Lines 315 to 335 in 718eb59

for dn in bind_dn_template:
userdn = dn.format(username=resolved_username)
msg = 'Status of user bind {username} with {userdn} : {isBound}'
try:
conn = getConnection(userdn, username, password)
except ldap3.core.exceptions.LDAPBindError as exc:
isBound = False
msg += '\n{exc_type}: {exc_msg}'.format(
exc_type=exc.__class__.__name__,
exc_msg=exc.args[0] if exc.args else ''
)
else:
isBound = conn.bind()
msg = msg.format(
username=username,
userdn=userdn,
isBound=isBound
)
self.log.debug(msg)
if isBound:
break

@ermakovpetr
Copy link

But we can take userdn from connection after search user conn.response[0]['dn']
my temporarily dirty fix :
#88
If you ok about the idea, I try to make pull-request correctly and beautifully

@ermakovpetr ermakovpetr mentioned this issue Jun 22, 2018
Closed
@AnirudhVyas
Copy link

How do we set bind_dn_template?

@dhirschfeld
Copy link
Collaborator

@AnirudhVyas - bind_dn_template is set in your jupyterhub_config.py file.
https://github.com/jupyterhub/ldapauthenticator#ldapauthenticatorbind_dn_template

@dhirschfeld
Copy link
Collaborator

I can't reproduce and I can't follow the logic how this could be the case.

isBound is initially set to False:

ldapauthenticator/ldapauthenticator/ldapauthenticator.py

Line 298 in 718eb59

isBound = False

If bind_dn_template is empty this entire block is skipped:

ldapauthenticator/ldapauthenticator/ldapauthenticator.py

Line 315 in 718eb59

for dn in bind_dn_template:

therefore isBound is still False here and this block is also skipped:

ldapauthenticator/ldapauthenticator/ldapauthenticator.py

Line 337 in 718eb59

if isBound:

which then falls through to the else clause which blocks access:

ldapauthenticator/ldapauthenticator/ldapauthenticator.py

Lines 377 to 381 in 718eb59

else:
self.log.warn('Invalid password for user {username}'.format(
username=username,
))
return None

@dhirschfeld
Copy link
Collaborator

Poking around some more, it seems that if your bind_dn_template is an empty string '' then it does indeed authenticate any password :(

dhirschfeld pushed a commit to dhirschfeld/ldapauthenticator that referenced this issue Jul 19, 2018
Misc cleanup + fixes
0d5d903 
- Fixes jupyterhub#44
  Empty DN templates are now ignored
- Fixes jupyterhub#93
  search_filter and allowed_groups are no longer mutually exclusive
@dhirschfeld dhirschfeld mentioned this issue Jul 19, 2018
Merged
@ermakovpetr
Copy link
ermakovpetr commented Jul 19, 2018 •
edited
Loading

@dhirschfeld
correctly I understand that my PR (#88) doesn't conflict your PR. and I can add functionality (get bind_dn_template from first request's response)
?

@dhirschfeld
Copy link
Collaborator

@ermakovpetr - yes, I think the functionality in #88 is orthogonal to that in #95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
¡bug! needs-verification
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Misc cleanup + fixes dhirschfeld/ldapauthenticator
4 participants
@AnirudhVyas @ermakovpetr @dhirschfeld @djrobstep