Last Comment Bug 54901 - [security] LDAP Authentication should fail for empty passwords
: [security] LDAP Authentication should fail for empty passwords
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: x86 Linux
: P1 blocker (vote)
: Bugzilla 2.16
Assigned To: Joe Robins
: default-qa
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-10-02 09:22 PDT by David L. Crow
Modified: 2012-12-18 20:46 PST (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
David Crow's patch (1.16 KB, patch)
2001-11-26 16:13 PST, Dave Miller [:justdave] (justdave@bugzilla.org)
justdave: review+
justdave: review+
Details | Diff | Splinter Review

Description David L. Crow 2000-10-02 09:22:25 PDT
When using LDAP and no password is specified in the login screen, the bind
is successful even if the user has a password.  This is because and LDAP bind
with no password (regardless of the bindn value) will be treated as an anonymous
bind.

The Netscape SDK documentation at
<http://docs.iplanet.com/docs/manuals/dirsdk/csdk30/writing.htm>, states

    Note that if you specify a DN but no password, your client will
    bind to the server anonymously. If you want a NULL password to
    be rejected as an incorrect password, you need to write code to
    perform the check before you call the ldap_simple_bind() or
   ldap_simple_bind_s() function.

The following diff shows such a change to CGI.pl:

===================================================================
RCS file: /cvsroot/mozilla/webtools/bugzilla/CGI.pl,v
retrieving revision 1.73
diff -u -r1.73 CGI.pl
--- CGI.pl      2000/09/18 21:29:44     1.73
+++ CGI.pl      2000/10/02 16:18:27
@@ -719,6 +719,21 @@
          exit;
        }
 
+       # if no password was provided, then fail the authentication
+       # while it may be valid to not have an LDAP password, when you
+       # bind without a password (regardless of the binddn value), you
+       # will get an anonymous bind.  I do not know of a way to determine
+       # whether a bind is anonymous or not without making changes to the
+       # LDAP access control settings
+       if ( ! $::FORM{"LDAP_password"} ) {
+         print "Content-type: text/html\n\n";
+         PutHeader("Login Failed");
+        print "You did not provide a password.\n";
+         print "Please click <b>Back</b> and try again.\n";
+         PutFooter();
+         exit;
+       }
+
        # We've got our anonymous bind;  let's look up this user.
        my $dnEntry =
$LDAPconn->search(Param("LDAPBaseDN"),"subtree","uid=".$::FORM{"LDAP_login"});
        if(!$dnEntry) {
Comment 1 Andreas Franke (gone) 2001-08-29 23:24:34 PDT
-> Bugzilla product, General component (LDAP, CGI.pl), reassigning.
Comment 2 Andreas Franke (gone) 2001-08-29 23:25:47 PDT
Oops. Really changing product now.
Comment 3 Gervase Markham [:gerv] 2001-10-14 17:54:31 PDT
Reassigning to LDAP author. Please set this to 2.16 if you feel the patch is
useful and ready to go.

Gerv
Comment 4 Joe Robins 2001-11-14 10:08:14 PST
Sorry for the delay in reviewing this.  The Job That Pays Me has been eating up 
my time.

This looks good to me.  I've tested it on my dev instance, and it works fine.  
You can go ahead and put it in 2.16.
Comment 5 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-11-17 18:08:38 PST
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Comment 6 Joe Robins 2001-11-26 08:18:48 PST
This should probably be marked as higher priority than it is...  It's actually a 
major security issue.  If you're using LDAP, this bug allows anyone to log into 
your Bugzilla instance as any user solely by knowing the username, and not the 
password.  Personally, I consider this to be a giant gaping flaw in the 
security, that should be dealt with as quickly as possible.

The patch below is simple, straightforward, and I've tested it on my instance.  
It remedies this problem with minimal changes, so it should have no effects on 
any other functionality.  (One caveat: I've tested it on my system, but applied 
the changes manually instead of using patch, because I've made a lot of other 
changes to my version.  I'm just assuming that the diff is legitimate w.r.t line 
numbers, etc.)

I know it's late in the game to be pushing for this to be in 2.16, so if it's 
too late, it's too late, but if it can get in there, it'd probably be a good 
thing.
Comment 7 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-11-26 16:13:22 PST
Created attachment 59251 [details] [diff] [review]
David Crow's patch
Comment 8 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-11-26 16:14:10 PST
Comment on attachment 59251 [details] [diff] [review]
David Crow's patch

marking first-review on behalf of jmrobins
Comment 9 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-12-09 07:58:18 PST
Comment on attachment 59251 [details] [diff] [review]
David Crow's patch

doesn't break the existing login stuff.

r= justdave
Comment 10 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-12-09 08:07:40 PST
Checked in on the trunk:

/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.125; previous revision: 1.124

checked in on the 2.14.1 branch:

/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.100.2.1; previous revision: 1.100
Comment 11 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-12-10 17:26:42 PST
Hmm, it seems the bulk change thinks I'm not changing anything if all I do is
add names to the CC list, so I guess I have to make a comment.  Anyhow, adding
the representatives from the organizations we know of that support Bugzilla
distributions so they're aware of our upcoming security release
Comment 12 James A. Laska 2002-01-29 06:35:02 PST
This code is now unnecesary.  Simple change the userbind statement to the
following (see bug#105504):  

       # Now we attempt to bind as the specified user.
       my $retcode = $LDAPconn->simpleAuth($userDN,$::FORM{"LDAP_password"});
       if ( ($::FORM{"LDAP_password"} eq "") ||
            ($userDN eq "") ||
            ($retcode == 0)  )
       {
         $LDAPconn->close();
         print "Content-type: text/html\n\n";
         PutHeader("Login Failed");
         print "The username or password for <font
color=\"blue\">".$::FORM{"LDAP_login"}."</font> was not found in the IBM <a
href=\"http://w3.ibm.com/bluepages/\">bluepages</a> directory.\n";
         print "<br>Please click <b>Back</b> and try again.\n";
         PutFooter();
         exit;
       }

This comes straight from the perLDAP examples.  This will properly fail a login
attempt for a bad passwd/no passwd etc...

Note You need to log in before you can comment on or make changes to this bug.