Last Comment Bug 258515 - Errors when accessing Bugzilla over IPv6
: Errors when accessing Bugzilla over IPv6
Status: RESOLVED FIXED
: regression
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: 2.18
: All All
: -- normal (vote)
: Bugzilla 2.18
Assigned To: Marc Schumann [:Wurblzap]
: default-qa
:
Mentors:
http://i.can.give.developers.the.url....
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-09-08 14:45 PDT by Calum
Modified: 2012-12-18 20:46 PST (History)
2 users (show)
justdave: approval+
justdave: blocking2.20+
justdave: approval2.18+
justdave: blocking2.18.1+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
HEAD patch (1.27 KB, patch)
2005-03-15 04:10 PST, Marc Schumann [:Wurblzap]
bugreport: review+
Details | Diff | Splinter Review
Branch patch (1.21 KB, patch)
2005-03-15 04:11 PST, Marc Schumann [:Wurblzap]
bugreport: review+
Details | Diff | Splinter Review

Description Calum 2004-09-08 14:45:22 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040714 Firefox/0.8
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040714 Firefox/0.8

When I access a dual-stack server running Apache 2, with Bugzilla 2.18.0_rc2
using the IPv6 address of the server, I get an error when I try and log in. It
all works OK with IPv4.

Reproducible: Always
Steps to Reproduce:
1. https://my.ipv6.address/bugzilla/
2. Log in
3. See error.

Actual Results:  
index.cgi: DBD::mysql::db selectrow_array failed: called with 4 bind variables
when 3 are needed [for statement ``SELECT profiles.userid, profiles.disabledtext
FROM logincookies, profiles WHERE logincookies.cookie=? AND  
logincookies.userid=profiles.userid AND   logincookies.userid=? AND  
(logincookies.ipaddr=?)'']) at Bugzilla/Auth/Cookie.pm line 67
index.cgi: \tBugzilla::Auth::Cookie::authenticate('Bugzilla::Auth::Cookie',2,7)
called at Bugzilla/Auth/CGI.pm line 104
index.cgi: \tBugzilla::Auth::CGI::login('Bugzilla::Auth::CGI',0) called at
Bugzilla.pm line 74
index.cgi: \tBugzilla::login('Bugzilla',0) called at
/var/www/localhost/htdocs/bugzilla/index.cgi line 41


Expected Results:  
Logged me in as it does with IPv4.
Comment 1 Maxime Labelle 2005-03-14 08:32:03 PST
 my ($userid, $disabledtext) = $dbh->selectrow_array($query, undef,
                                                        $login_cookie,
                                                        $login,
                                                    ->  $ipaddr,
                                                    ->  $netaddr);

If I remove $netaddr , it works with IPv6 but no more IPv4, but here is the
output of the IPv4 error:

DBD::mysql::db selectrow_array failed: called with 3 bind variables when 4 are
needed [for statement ``SELECT profiles.userid, profiles.disabledtext FROM
logincookies, profiles WHERE logincookies.cookie=? AND  
logincookies.userid=profiles.userid AND   logincookies.userid=? AND  
(logincookies.ipaddr=? OR logincookies.ipaddr=?)'']) at Bugzilla/Auth/Cookie.pm
line 67
	Bugzilla::Auth::Cookie::authenticate('Bugzilla::Auth::Cookie',22,4) called at
Bugzilla/Auth/CGI.pm line 104
	Bugzilla::Auth::CGI::login('Bugzilla::Auth::CGI',1) called at Bugzilla.pm line 74
	Bugzilla::login('Bugzilla') called at
/usr/local/bugzilla/UPDATE/bugzilla-2.18/buglist.cgi line 81

Here is what is interesting: 
 (logincookies.ipaddr=? OR logincookies.ipaddr=?)

Seems that bugzilla has some trouble to understand IPv6, lets take a look at this:

 my $query = "SELECT profiles.userid, profiles.disabledtext " .
                "FROM logincookies, profiles " .
                "WHERE logincookies.cookie=? AND " .
                "  logincookies.userid=profiles.userid AND " .
                "  logincookies.userid=? AND " .
                "  (logincookies.ipaddr=?";
    if (defined $netaddr) {
        trick_taint($netaddr);
        $query .= " OR logincookies.ipaddr=?";
    }

Maybe that query should always include the optional query?

    if (defined $netaddr) {
        trick_taint($netaddr);
    }
        $query .= " OR logincookies.ipaddr=?";

Thats better! All right, now bugzilla works both with IPv4 and IPv6 ! But why?
Well, 

my $netaddr = Bugzilla::Auth::get_netaddr($ipaddr);

Seem to be confused.

(from Auth.pm)

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 = Param('loginnetmask');

    $addr >>= (32-$maskbits);
    $addr <<= (32-$maskbits);
    return join(".", unpack("CCCC", pack("N", $addr)));
}

Maybe a patch could be written to enable get_netaddr to deal with IPv6 address?
Comment 2 Marc Schumann [:Wurblzap] 2005-03-15 03:58:14 PST
Confirming. Happens on trunk and 2.18 branch.
Comment 3 Marc Schumann [:Wurblzap] 2005-03-15 04:10:34 PST
Created attachment 177474 [details] [diff] [review]
HEAD patch
Comment 4 Marc Schumann [:Wurblzap] 2005-03-15 04:11:38 PST
Created attachment 177475 [details] [diff] [review]
Branch patch
Comment 5 Dave Miller [:justdave] (justdave@bugzilla.org) 2005-03-15 11:46:54 PST
I believe this got broken when we introduced the ability to restrict the number
of bits we honor in an IP address, which is a very IPv4-centric thing to do.
Comment 6 Max Kanat-Alexander 2005-03-16 21:14:53 PST
Actually, you can honor a number of bits in an IPv6 address, too, if you want.
It's just that the range extends much further, so a /32 is a huge range. :-)
Comment 7 Joel Peshkin 2005-03-18 01:28:54 PST
Comment on attachment 177474 [details] [diff] [review]
HEAD patch

r=joel
Comment 8 Joel Peshkin 2005-03-18 01:30:07 PST
FYI - to test this on an IPv4 machine, force getnetaddr to return an undef.
Comment 9 Joel Peshkin 2005-03-18 01:31:21 PST
Comment on attachment 177475 [details] [diff] [review]
Branch patch

r=joel by inspection
(same fix as HEAD)
Comment 10 Dave Miller [:justdave] (justdave@bugzilla.org) 2005-03-21 20:44:01 PST
(In reply to comment #6)
> Actually, you can honor a number of bits in an IPv6 address, too, if you want.
> It's just that the range extends much further, so a /32 is a huge range. :-)

OK, so before I approve this, is our netaddr code actually working on IPv6?  Or
are IPv6 folks going to be stuck with the entire address being bound to the
cookie?  Is our bit width preference only affecting IPv4 addresses then?
Comment 11 Marc Schumann [:Wurblzap] 2005-03-22 01:52:07 PST
(In reply to comment #10)
> OK, so before I approve this, is our netaddr code actually working on IPv6?
> Or are IPv6 folks going to be stuck with the entire address being bound to
> the cookie?  Is our bit width preference only affecting IPv4 addresses then?

No, yes, and yes :)
Fixing this is not part of this bug; this bug takes care of the logincookies
query not handling its bound parameters correctly.
Comment 12 Dave Miller [:justdave] (justdave@bugzilla.org) 2005-03-22 13:44:37 PST
OK, so anyone accessing over IPv6 is still stuck with the cookie bound to their
entire IP address, but this bug fixes it so it no longer causes an SQL error
trying to find that out.  Got it.  Just trying to make sure I have the story
straight :)
Comment 13 Frédéric Buclin 2005-03-22 14:48:58 PST
Tip:

Checking in Bugzilla/Auth/Login/WWW/CGI/Cookie.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Login/WWW/CGI/Cookie.pm,v  <--
 Cookie.pm
new revision: 1.3; previous revision: 1.2
done


2.18 branch:

Checking in Bugzilla/Auth/Cookie.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Attic/Cookie.pm,v  <--  Cookie.pm
new revision: 1.2.2.1; previous revision: 1.2
done

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