From a7310a522e4ac2b24d01f0cdf44e132d0db8f73b Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Sun, 18 Oct 2009 23:34:57 +0000 Subject: Bug 399073: Remove the 'loginnetmask' parameter - Patch by Frédéric Buclin r/a=mkanat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Auth/Login/Cookie.pm | 25 +++---- Bugzilla/Auth/Persist/Cookie.pm | 15 ++-- Bugzilla/Config/Auth.pm | 7 -- Bugzilla/Config/Common.pm | 17 +---- Bugzilla/DB/Schema.pm | 2 +- Bugzilla/Install/DB.pm | 14 +++- Bugzilla/Util.pm | 32 +-------- docs/en/xml/troubleshooting.xml | 90 +++--------------------- template/en/default/account/auth/login.html.tmpl | 20 +++--- 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 @@ -90,13 +90,6 @@ sub get_param_list { checker => \&check_multi }, - { - name => 'loginnetmask', - type => 't', - default => '0', - checker => \&check_netmask - }, - { name => 'requirelogin', type => 'b', 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 - -Given an IP address, this returns the associated network address, using -Cparams->{'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 Returns either the C or C 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 @@ - + Troubleshooting @@ -22,7 +22,7 @@ If you have made it all the way through (Installation) and (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 /etc/logs/httpd/error_log. The error messages you see may be self-explanatory enough to enable you to diagnose and @@ -32,7 +32,7 @@ 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 errorlog, in the Bugzilla data directory. Errors will be logged as they occur, and will include the type @@ -45,10 +45,10 @@
- The Apache webserver is not serving Bugzilla pages + The Apache web server is not serving Bugzilla pages After you have run checksetup.pl twice, run testserver.pl http://yoursite.yourdomain/yoururl - to confirm that your webserver is configured properly for + to confirm that your web server is configured properly for Bugzilla. @@ -75,9 +75,9 @@ TEST-OK Webserver is preventing fetch of http://landfill.bugzilla.org/bugzilla-t - 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. + 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. @@ -139,55 +139,12 @@ TEST-OK Webserver is preventing fetch of http://landfill.bugzilla.org/bugzilla-t
-
- Your vendor has not defined Fcntl macro O_NOINHERIT - - This is caused by a bug in the version of - File::Temp that is distributed with perl - 5.6.0. Many minor variations of this error have been reported: - - - 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. - - 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 patch file. - - - -
-
Everybody is constantly being forced to relogin The most-likely cause is that the cookiepath 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. 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.
-
- Some users are constantly being forced to relogin - - First, make sure cookies are enabled in the user's browser. - - - 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. - - - If you are using 2.18 (or later), there is a - parameter called loginnetmask, 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 Restrict this login to - my IP address 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. - - -
-
<filename>index.cgi</filename> doesn't show up unless specified in the URL 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 @@ [% END %] - [% IF Param('loginnetmask') < 32 %] - -   - - - - - - [% END %] + +   + + + + + [% 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 @@ ", - 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.
" _ - "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.", -- cgit v1.2.3-24-g4f1b