[Bug 6979] New: LDAP authentication fails during password comparison
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Bug #: 6979 Summary: LDAP authentication fails during password comparison Classification: Unclassified Change sponsored?: --- Product: Koha Version: rel_3_4 Platform: Other OS/Version: Linux Status: NEW Severity: major Priority: P5 Component: Architecture, internals, and plumbing AssignedTo: gmcharlt@gmail.com ReportedBy: rfox2@nd.edu QAContact: koha-bugs@lists.koha-community.org Password is failing during comparison in Auth_with_ldap.pm in code (between lines 140 and 165) in this call: my $cmpmesg = $db->compare( $userldapentry, attr=>'userpassword', value => $password ); This was failing 100% of the time, even if a correct password was submitted with: "LDAP Auth rejected : invalid password for user ..." The attribute comparison is not always a valid way to check the password validity because not all LDAP databases support the userpassword attribute. Also, many LDAP databases require a valid DN string from the user as opposed to the uid for authentication purposes. I have a fix for this that does a recursive lookup of the user's DN in the LDAP database, and then uses that DN to perform a bind in a similar manner to the auth_by_bind method. The auth_by_bind method should also be changed so that it uses the retrieved DN of the user to perform a bind against the LDAP server. I plan on submitting a code revision for this and soliciting feedback. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #1 from Robert Fox <rfox2@nd.edu> 2011-10-05 21:00:03 UTC --- Created attachment 5723 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=5723 Patch for Bug 6979 - Auth_with_ldap.pm in C4 directory This patch is being submitted in order to address a bug we encountered in the checkpw_ldap subroutine in the C4/Auth_with_ldap.pm module. I did not touch the part of the conditional that obtains if the auth_by_bind variable is set to true in the configuration. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Robert Fox <rfox2@nd.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 |PATCH-Sent Patch Status|--- |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com Patch Status|Needs Signoff |Failed QA --- Comment #2 from Paul Poulain <paul.poulain@biblibre.com> 2011-10-19 13:44:58 UTC --- QA comment investigating this bug before the signoff : 2 comments : === COMMENT 1 === + # BUG 6979 + # 2011-09-29 Robert Fox (rfox2@nd.edu) => those comments should not be in the code. Git is here to keep track of those informations. I agree you've reindented + # BUG #5094 + # 2010-08-04 JeremyC but it should not have been here either (and now we have a strong QA, it would not have been accepted) So, please resubmit without those comments. === COMMENT 2 === Replacing compare by a bind is not a good solution. Some LDAPs are configured to let no-one (except some specific accounts) bind. Some are configured to require binding. It means you'll solve a problem (for you probably, but not only, I agree), and create another problem for some other libraries that have Auth_with_ldap working now. A better patch would be : * to test compare, and if it fails, test binding (acceptable, although dirty & less secure I feel) or * add an entry in the ldap config file to select between bind & compare method (better but more work) So I think we should not integrate this patch for now, and mark as failed QA. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 evandro@ipb.pt changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |evandro@ipb.pt -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m, | |martin.renvoize@ptfs-europe | |.com --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Is it still valid or can be closed? cc Martin -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Mike Gabriel <mike.gabriel@das-netzwerkteam.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mike.gabriel@das-netzwerkte | |am.de --- Comment #4 from Mike Gabriel <mike.gabriel@das-netzwerkteam.de> --- Hi, (In reply to Jonathan Druart from comment #3)
Is it still valid or can be closed? cc Martin
I have recently been contracting for making KOHA LDAP authentication against a Debian Edu (aka Skolelinux) main server work. The customer also finances upstream communication and asked me to do my best to get whatever solution I come up with into upstream KOHA. After auditing the Auth_with_LDAP.pm code, I come to these conclusions: 1. The customer runs a Koha 3.08.01.002 [1]. In the meantime, Koha 3.20.something is out. However, the Auth_with_LDAP.pm file in latest HEAD (master branch) is still at version 3.07.00.049 [2]. Also the Auth_with_LDAP.pm code looks far more advanced than the Auth_with_LDAP.pm code on latest HEAD. Is is possible that some branch merging did not happen for the Auth_with_LDAP.pm file? It feels like this requires some portion of investigation. Thanks. [1]http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Auth_with_ldap.... [2] http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=C4/Auth_with_ldap.... 2. KOHA LDAP Auth seems to be working fine for AD authentication using userPrincipal attribute description and a valid password. The default AD setup always allows user binding to their own account's DN. So that should work out well. 3. Authentication against openLDAP with clear text passwords stored in LDAP should also work fine as long as an administrative DN object is used for binding (e.g. cn=admin of objectClass simpleSecurityObject or such). However, storing clear text passwords in an LDAP tree is really really old school and should neither happen nor be expected anymore. On most setups, using $db->compare() will be unusable as passwords in most recent openLDAP setups are stored in a hashed way (and have been salted before hashing). 4. In KOHA, it even seems to be an option to use anonymous bind and $db->compare() for LDAP authentication. This should not be allowed at all, as it requires (a) an anonymous bind LDAP configuration that reveals the userPassword field (to everyone!!!) and requires the value in the userPassword attribute description to be stored in clear text. Nothing people really want... The approach for my customer (and also my proposal for getting the above sorted out in KOHA, if devs here agree) is this: o drop anonymous bind + userPassword LDAP CompareRequest completely o keep admin-bind + userPassword LDAP CompareRequest o keep AD authentication as is o try an auth for a specific user against LDAP using the user's DN (as proposed by a patch similar to the patch provided by Robert Fox) o make the openLDAP user-login-via-test-authbind method configurable via koha-conf.xml Any feedback on this is highly welcome. I am also open to discuss a different approach (as long as it works against openLDAP deployed in Debian Edu / Skolelinux setups). Greets, Mike PS: I am also a Debian Developer, being interested in getting KOHA into Debian in the long run... -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hi Mike, Great to have some new blood on board; I totally agree that the Auth_with_ldap code needs a major rethink and would support such a piece of work. It's great to have your insight regarding best/worst practice's in the LDAP space and i'd be OK with deprecating some feature and clarifying the code.. though we'de need a good strong DEPRECATION warning because not all koha users are as technically able as yourself and may not be following current bets practice.. That's why it's always hard to get rid of some of these ldap related features. It might also be worth you taking a little look at bug 8993 as it was a piece of work aimed at re-working the LDAP code. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- On the debian front, Robin is your man there.. it's always good to get some extra feedback on our packaging approach. Are you on the Koha IRC channel yet.. that's probably your best place to start getting involved? -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #7 from Mike Gabriel <mike.gabriel@das-netzwerkteam.de> --- (In reply to Martin Renvoize from comment #5)
It might also be worth you taking a little look at bug 8993 as it was a piece of work aimed at re-working the LDAP code.
The patches in #8993 look indeed promising. I will need some time to review and get back to you via #8993 or here (depends where it fits best). Mike -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #8 from Mike Gabriel <mike.gabriel@das-netzwerkteam.de> --- (In reply to Martin Renvoize from comment #6)
On the debian front, Robin is your man there.. it's always good to get some extra feedback on our packaging approach. Are you on the Koha IRC channel yet.. that's probably your best place to start getting involved?
Showing up on IRC now (my nick is around 24/7, nick is: sunweaver)... Mike -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex.arnaud@biblibre.com --- Comment #9 from Alex Arnaud <alex.arnaud@biblibre.com> --- Hello Mike, Nice to read your comment 4. I totally agree with you but i have a question: What do you mean by "openLDAP user-login-via-test-authbind method" ? For me, there is no difference between AD and openLDAP binds. Net::LDAP should work with both, right ? I think bug 8983 is quite tricky. It has the advantage that we can make more complex/useful mapping when replicating users from LDAP but it needs librian create a package with perl code. The attached patch here is more simple and could solve (with a little change) the problem of multiple branche. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #5723|0 |1 is obsolete| | --- Comment #10 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46646 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46646&action=edit Bug #6979 I removed several lines of code in the checkpw_ldap subroutine where LDAP authentication takes place, in the "else" part of the conditional that checks for the auth_by_bind config parameter. I added several lines to check whether the user can log in to LDAP using their DN and the password supplied in the login form. If they are able to bind, login contiues as normal and the LDAP attributes can be harvested as normal if the update options are turned on. The routine that was in place was failing because it was trying to check against a non-existent LDAP entry attribute called 'userpassword'. Instead of checking against a 'userpassword' attribute, the routine really should be checking to make sure the user can actually bind to LDAP. That's what I set up, and it is a safer way to test authentication against LDAP. -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46646|0 |1 is obsolete| | --- Comment #11 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46654 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46654&action=edit Bug #6979 I removed several lines of code in the checkpw_ldap subroutine where LDAP authentication takes place, in the "else" part of the conditional that checks for the auth_by_bind config parameter. I added several lines to check whether the user can log in to LDAP using their DN and the password supplied in the login form. If they are able to bind, login contiues as normal and the LDAP attributes can be harvested as normal if the update options are turned on. The routine that was in place was failing because it was trying to check against a non-existent LDAP entry attribute called 'userpassword'. Instead of checking against a 'userpassword' attribute, the routine really should be checking to make sure the user can actually bind to LDAP. That's what I set up, and it is a safer way to test authentication against LDAP. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #12 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46659 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46659&action=edit Bug 6979 - Update tests -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 --- Comment #13 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46698 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46698&action=edit Bug 6979 - Handle multiple branches in non-auth_by_bin -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|PATCH-Sent (DO NOT USE) |P1 - high Status|Failed QA |Needs Signoff Version|3.4 |master -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |alex.arnaud@biblibre.com -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6979 Alex Arnaud <alex.arnaud@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #46698|0 |1 is obsolete| | --- Comment #14 from Alex Arnaud <alex.arnaud@biblibre.com> --- Created attachment 46863 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46863&action=edit Bug 6979 - Handle multiple branches in non-auth_by_bin -- You are receiving this mail because: You are watching all bug changes. You are the QA Contact for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org