diff options
-rw-r--r-- | Bugzilla/Auth/Login/Cookie.pm | 25 | ||||
-rw-r--r-- | Bugzilla/Auth/Persist/Cookie.pm | 15 | ||||
-rw-r--r-- | Bugzilla/Config/Auth.pm | 7 | ||||
-rw-r--r-- | Bugzilla/Config/Common.pm | 17 | ||||
-rw-r--r-- | Bugzilla/DB/Schema.pm | 2 | ||||
-rw-r--r-- | Bugzilla/Install/DB.pm | 14 | ||||
-rw-r--r-- | Bugzilla/Util.pm | 32 | ||||
-rw-r--r-- | docs/en/xml/troubleshooting.xml | 90 | ||||
-rw-r--r-- | template/en/default/account/auth/login.html.tmpl | 20 | ||||
-rw-r--r-- | template/en/default/admin/params/auth.html.tmpl | 5 |
10 files changed, 48 insertions, 179 deletions
diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm index e2cd8f5ee..0b002168e 100644 --- a/Bugzilla/Auth/Login/Cookie.pm +++ b/Bugzilla/Auth/Login/Cookie.pm @@ -36,7 +36,6 @@ sub get_login_info { my $dbh = Bugzilla->dbh; my $ip_addr = $cgi->remote_addr(); - my $net_addr = get_netaddr($ip_addr); my $login_cookie = $cgi->cookie("Bugzilla_logincookie"); my $user_id = $cgi->cookie("Bugzilla_login"); @@ -60,24 +59,16 @@ sub get_login_info { trick_taint($login_cookie); detaint_natural($user_id); - my $query = "SELECT userid - FROM logincookies - WHERE logincookies.cookie = ? - AND logincookies.userid = ? - AND (logincookies.ipaddr = ?"; - - # If we have a network block that's allowed to use this cookie, - # as opposed to just a single IP. - my @params = ($login_cookie, $user_id, $ip_addr); - if (defined $net_addr) { - trick_taint($net_addr); - $query .= " OR logincookies.ipaddr = ?"; - push(@params, $net_addr); - } - $query .= ")"; + my $is_valid = + $dbh->selectrow_array('SELECT 1 + FROM logincookies + WHERE cookie = ? + AND userid = ? + AND (ipaddr = ? OR ipaddr IS NULL)', + undef, ($login_cookie, $user_id, $ip_addr)); # If the cookie is valid, return a valid username. - if ($dbh->selectrow_array($query, undef, @params)) { + if ($is_valid) { # If we logged in successfully, then update the lastused # time on the login cookie $dbh->do("UPDATE logincookies SET lastused = NOW() diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 60f90925e..4458e31b5 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -49,17 +49,14 @@ sub persist_login { my $dbh = Bugzilla->dbh; my $cgi = Bugzilla->cgi; - my $ip_addr = $cgi->remote_addr; - unless ($cgi->param('Bugzilla_restrictlogin') || - Bugzilla->params->{'loginnetmask'} == 32) - { - $ip_addr = get_netaddr($ip_addr); + my $ip_addr; + if ($cgi->param('Bugzilla_restrictlogin')) { + $ip_addr = $cgi->remote_addr; + # The IP address is valid, at least for comparing with itself in a + # subsequent login + trick_taint($ip_addr); } - # The IP address is valid, at least for comparing with itself in a - # subsequent login - trick_taint($ip_addr); - $dbh->bz_start_transaction(); my $login_cookie = diff --git a/Bugzilla/Config/Auth.pm b/Bugzilla/Config/Auth.pm index cbd94617a..1af808eaa 100644 --- a/Bugzilla/Config/Auth.pm +++ b/Bugzilla/Config/Auth.pm @@ -91,13 +91,6 @@ sub get_param_list { }, { - name => 'loginnetmask', - type => 't', - default => '0', - checker => \&check_netmask - }, - - { name => 'requirelogin', type => 'b', default => '0' diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 90a5a6c76..b722795d4 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -47,7 +47,7 @@ use base qw(Exporter); qw(check_multi check_numeric check_regexp check_url check_group check_sslbase check_priority check_severity check_platform check_opsys check_shadowdb check_urlbase check_webdotbase - check_netmask check_user_verify_class + check_user_verify_class check_mail_delivery_method check_notification check_utf8 check_bug_status check_smtp_auth check_theschwartz_available check_maxattachmentsize @@ -248,21 +248,6 @@ sub check_webdotbase { return ""; } -sub check_netmask { - my ($mask) = @_; - my $res = check_numeric($mask); - return $res if $res; - if ($mask < 0 || $mask > 32) { - return "an IPv4 netmask must be between 0 and 32 bits"; - } - # Note that if we changed the netmask from anything apart from 32, then - # existing logincookies which aren't for a single IP won't work - # any more. We can't know which ones they are, though, so they'll just - # take space until they're periodically cleared, later. - - return ""; -} - sub check_user_verify_class { # doeditparams traverses the list of params, and for each one it checks, # then updates. This means that if one param checker wants to look at diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 2bd95d501..c5003f798 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -974,7 +974,7 @@ use constant ABSTRACT_SCHEMA => { REFERENCES => {TABLE => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE'}}, - ipaddr => {TYPE => 'varchar(40)', NOTNULL => 1}, + ipaddr => {TYPE => 'varchar(40)'}, lastused => {TYPE => 'DATETIME', NOTNULL => 1}, ], INDEXES => [ diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index e6b577526..df6296056 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -580,6 +580,9 @@ sub update_table_definitions { # 2009-09-28 LpSolit@gmail.com - Bug 519032 $dbh->bz_drop_column('series', 'last_viewed'); + # 2009-09-28 LpSolit@gmail.com - Bug 399073 + _fix_logincookies_ipaddr(); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -1249,7 +1252,7 @@ sub _use_ip_instead_of_hostname_in_logincookies { # Now update the logincookies schema $dbh->bz_drop_column("logincookies", "hostname"); $dbh->bz_add_column("logincookies", "ipaddr", - {TYPE => 'varchar(40)', NOTNULL => 1}, ''); + {TYPE => 'varchar(40)'}); } } @@ -3207,6 +3210,15 @@ sub _convert_disallownew_to_isactive { } } +sub _fix_logincookies_ipaddr { + my $dbh = Bugzilla->dbh; + return if !$dbh->bz_column_info('logincookies', 'ipaddr')->{NOTNULL}; + + $dbh->bz_alter_column('logincookies', 'ipaddr', {TYPE => 'varchar(40)'}); + $dbh->do('UPDATE logincookies SET ipaddr = NULL WHERE ipaddr = ?', + undef, '0.0.0.0'); +} + 1; __END__ diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 90525b9d4..a36b22c37 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -35,7 +35,7 @@ use base qw(Exporter); detaint_signed html_quote url_quote xml_quote css_class_quote html_light_quote url_decode - i_am_cgi get_netaddr correct_urlbase + i_am_cgi correct_urlbase lsearch do_ssl_redirect_if_required use_attachbase diff_arrays trim wrap_hard wrap_comment find_wrap_point @@ -601,28 +601,6 @@ sub get_text { return $message; } - -sub get_netaddr { - my $ipaddr = shift; - - # Check for a valid IPv4 addr which we know how to parse - if (!$ipaddr || $ipaddr !~ /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/) { - return undef; - } - - my $addr = unpack("N", pack("CCCC", split(/\./, $ipaddr))); - - my $maskbits = Bugzilla->params->{'loginnetmask'}; - - # Make Bugzilla ignore the IP address if loginnetmask is set to 0 - return "0.0.0.0" if ($maskbits == 0); - - $addr >>= (32-$maskbits); - - $addr <<= (32-$maskbits); - return join(".", unpack("CCCC", pack("N", $addr))); -} - sub disable_utf8 { if (Bugzilla->params->{'utf8'}) { binmode STDOUT, ':bytes'; # Turn off UTF8 encoding. @@ -657,7 +635,6 @@ Bugzilla::Util - Generic utility functions for bugzilla # Functions that tell you about your environment my $is_cgi = i_am_cgi(); - my $net_addr = get_netaddr($ip_addr); my $urlbase = correct_urlbase(); # Functions for searching @@ -788,13 +765,6 @@ Tells you whether or not you are being run as a CGI script in a web server. For example, it would return false if the caller is running in a command-line script. -=item C<get_netaddr($ipaddr)> - -Given an IP address, this returns the associated network address, using -C<Bugzilla->params->{'loginnetmask'}> as the netmask. This can be used -to obtain data in order to restrict weak authentication methods (such as -cookies) to only some addresses. - =item C<correct_urlbase()> Returns either the C<sslbase> or C<urlbase> parameter, depending on the diff --git a/docs/en/xml/troubleshooting.xml b/docs/en/xml/troubleshooting.xml index 7a4a08ef5..0c20b71d1 100644 --- a/docs/en/xml/troubleshooting.xml +++ b/docs/en/xml/troubleshooting.xml @@ -1,5 +1,5 @@ <!-- <!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.1.2//EN"> --> -<!-- $Id: troubleshooting.xml,v 1.11 2008/04/04 06:48:23 uid623 Exp $ --> +<!-- $Id: troubleshooting.xml,v 1.14 2009/10/18 23:35:00 lpsolit%gmail.com Exp $ --> <appendix id="troubleshooting"> <title>Troubleshooting</title> @@ -22,7 +22,7 @@ <para>If you have made it all the way through <xref linkend="installation"/> (Installation) and <xref linkend="configuration"/> (Configuration) but accessing the Bugzilla - URL doesn't work, the first thing to do is to check your webserver error + URL doesn't work, the first thing to do is to check your web server error log. For Apache, this is often located at <filename>/etc/logs/httpd/error_log</filename>. The error messages you see may be self-explanatory enough to enable you to diagnose and @@ -32,7 +32,7 @@ <para> Bugzilla can also log all user-based errors (and many code-based errors) - that occur, without polluting the web server error log. To enable + that occur, without polluting the web server's error log. To enable Bugzilla error logging, create a file that Bugzilla can write to, named <filename>errorlog</filename>, in the Bugzilla <filename>data</filename> directory. Errors will be logged as they occur, and will include the type @@ -45,10 +45,10 @@ </section> <section id="trbl-testserver"> - <title>The Apache webserver is not serving Bugzilla pages</title> + <title>The Apache web server is not serving Bugzilla pages</title> <para>After you have run <command>checksetup.pl</command> twice, run <command>testserver.pl http://yoursite.yourdomain/yoururl</command> - to confirm that your webserver is configured properly for + to confirm that your web server is configured properly for Bugzilla. </para> <programlisting> @@ -75,9 +75,9 @@ TEST-OK Webserver is preventing fetch of http://landfill.bugzilla.org/bugzilla-t </para> </listitem> <listitem> - <para>The permissions on your library directories are set incorrectly. - They must, at the very least, be readable by the webserver user or - group. It is recommended that they be world readable. + <para>The permissions on your library directories are set incorrectly. + They must, at the very least, be readable by the web server user or + group. It is recommended that they be world readable. </para> </listitem> </orderedlist> @@ -139,55 +139,12 @@ TEST-OK Webserver is preventing fetch of http://landfill.bugzilla.org/bugzilla-t </para> </section> - <section id="trouble-filetemp"> - <title>Your vendor has not defined Fcntl macro O_NOINHERIT</title> - - <para>This is caused by a bug in the version of - <productname>File::Temp</productname> that is distributed with perl - 5.6.0. Many minor variations of this error have been reported: - </para> - - <programlisting>Your vendor has not defined Fcntl macro O_NOINHERIT, used -at /usr/lib/perl5/site_perl/5.6.0/File/Temp.pm line 208. - -Your vendor has not defined Fcntl macro O_EXLOCK, used -at /usr/lib/perl5/site_perl/5.6.0/File/Temp.pm line 210. - -Your vendor has not defined Fcntl macro O_TEMPORARY, used -at /usr/lib/perl5/site_perl/5.6.0/File/Temp.pm line 233.</programlisting> - - <para>Numerous people have reported that upgrading to version 5.6.1 - or higher solved the problem for them. A less involved fix is to apply - the following patch, which is also - available as a <ulink url="../xml/filetemp.patch">patch file</ulink>. - </para> - - <programlisting><![CDATA[--- File/Temp.pm.orig Thu Feb 6 16:26:00 2003 -+++ File/Temp.pm Thu Feb 6 16:26:23 2003 -@@ -205,6 +205,7 @@ - # eg CGI::Carp - local $SIG{__DIE__} = sub {}; - local $SIG{__WARN__} = sub {}; -+ local *CORE::GLOBAL::die = sub {}; - $bit = &$func(); - 1; - }; -@@ -226,6 +227,7 @@ - # eg CGI::Carp - local $SIG{__DIE__} = sub {}; - local $SIG{__WARN__} = sub {}; -+ local *CORE::GLOBAL::die = sub {}; - $bit = &$func(); - 1; - };]]></programlisting> - </section> - <section id="trbl-relogin-everyone"> <title>Everybody is constantly being forced to relogin</title> <para>The most-likely cause is that the <quote>cookiepath</quote> parameter is not set correctly in the Bugzilla configuration. You can change this (if - you're a Bugzilla administrator) from the editparams.cgi page via the web. + you're a Bugzilla administrator) from the editparams.cgi page via the web interface. </para> <para>The value of the cookiepath parameter should be the actual directory @@ -256,35 +213,6 @@ at /usr/lib/perl5/site_perl/5.6.0/File/Temp.pm line 233.</programlisting> </para> </section> - <section id="trbl-relogin-some"> - <title>Some users are constantly being forced to relogin</title> - - <para>First, make sure cookies are enabled in the user's browser. - </para> - - <para>If that doesn't fix the problem, it may be that the user's ISP - implements a rotating proxy server. This causes the user's effective IP - address (the address which the Bugzilla server perceives him coming from) - to change periodically. Since Bugzilla cookies are tied to a specific IP - address, each time the effective address changes, the user will have to - log in again. - </para> - - <para>If you are using 2.18 (or later), there is a - parameter called <quote>loginnetmask</quote>, which you can use to set - the number of bits of the user's IP address to require to be matched when - authenticating the cookies. If you set this to something less than 32, - then the user will be given a checkbox for <quote>Restrict this login to - my IP address</quote> on the login screen, which defaults to checked. If - they leave the box checked, Bugzilla will behave the same as it did - before, requiring an exact match on their IP address to remain logged in. - If they uncheck the box, then only the left side of their IP address (up - to the number of bits you specified in the parameter) has to match to - remain logged in. - </para> - - </section> - <section id="trbl-index"> <title><filename>index.cgi</filename> doesn't show up unless specified in the URL</title> <para> diff --git a/template/en/default/account/auth/login.html.tmpl b/template/en/default/account/auth/login.html.tmpl index e4adfdcb6..9a043e4f4 100644 --- a/template/en/default/account/auth/login.html.tmpl +++ b/template/en/default/account/auth/login.html.tmpl @@ -69,17 +69,15 @@ </tr> [% END %] - [% IF Param('loginnetmask') < 32 %] - <tr> - <th> </th> - <td> - <input type="checkbox" id="Bugzilla_restrictlogin" name="Bugzilla_restrictlogin" - checked="checked"> - <label for="Bugzilla_restrictlogin">Restrict this session to this IP address - (using this option improves security)</label> - </td> - </tr> - [% END %] + <tr> + <th> </th> + <td> + <input type="checkbox" id="Bugzilla_restrictlogin" name="Bugzilla_restrictlogin" + checked="checked"> + <label for="Bugzilla_restrictlogin">Restrict this session to this IP address + (using this option improves security)</label> + </td> + </tr> </table> [% PROCESS "global/hidden-fields.html.tmpl" diff --git a/template/en/default/admin/params/auth.html.tmpl b/template/en/default/admin/params/auth.html.tmpl index 94a748b25..d2cb3e5b5 100644 --- a/template/en/default/admin/params/auth.html.tmpl +++ b/template/en/default/admin/params/auth.html.tmpl @@ -103,11 +103,6 @@ </li> </ul>", - loginnetmask => "The number of bits for the netmask used if a user chooses to " _ - "allow a login to be valid for more than a single IP. Setting " _ - "this to 32 disables this feature.<br> " _ - "Note that enabling this may decrease the security of your system.", - requirelogin => "If this option is set, all access to the system beyond the " _ "front page will require a login. No anonymous users will " _ "be permitted.", |