Attachment 292694 Details for Bug 406231 – Pacth to add signing support to bugzilla login cookie

[patch] Pacth to add signing support to bugzilla login cookie

single_signed_cookie.patch (text/plain), 7.57 KB, created by David Lawrence on 2008-01-23 21:01:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Lawrence

Created: 2008-01-23 21:01:10 UTC

Size: 7.57 KB

patch

obsolete

>? patches
>? lib/.htaccess
>Index: Bugzilla/Auth/Login/Cookie.pm
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Login/Cookie.pm,v
>retrieving revision 1.5
>diff -u -r1.5 Cookie.pm
>--- Bugzilla/Auth/Login/Cookie.pm	5 Jul 2006 23:42:47 -0000	1.5
>+++ Bugzilla/Auth/Login/Cookie.pm	23 Jan 2008 20:39:11 -0000
>@@ -23,6 +23,7 @@
> use Bugzilla::Util;
> 
> use List::Util qw(first);
>+use Digest::SHA1;
> 
> use constant requires_persistence  => 0;
> use constant requires_verification => 0;
>@@ -37,21 +38,12 @@
> 
>     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");
> 
>-    # If cookies cannot be found, this could mean that they haven't
>-    # been made available yet. In this case, look at Bugzilla_cookie_list.
>-    unless ($login_cookie) {
>-        my $cookie = first {$_->name eq 'Bugzilla_logincookie'}
>-                            @{$cgi->{'Bugzilla_cookie_list'}};
>-        $login_cookie = $cookie->value if $cookie;
>-    }
>-    unless ($user_id) {
>-        my $cookie = first {$_->name eq 'Bugzilla_login'}
>-                            @{$cgi->{'Bugzilla_cookie_list'}};
>-        $user_id = $cookie->value if $cookie;
>-    }
>+    my $cookie_info = $self->get_cookie_info();
>+    return { failure => AUTH_NODATA } if not $cookie_info->{userid};
>+
>+    my $login_cookie = $cookie_info->{token};
>+    my $user_id      = $cookie_info->{userid};
> 
>     if ($login_cookie && $user_id) {
>         # Anything goes for these params - they're just strings which
>@@ -93,4 +85,56 @@
>     return { failure => AUTH_NODATA };
> }
> 
>+# Returns all attributes from token cookie if key is valid
>+sub get_cookie_info {
>+    my $self = shift;
>+
>+    my $cgi = Bugzilla->cgi;
>+    my @info = $cgi->cookie('Bugzilla_login');
>+
>+    # If cookies cannot be found, this could mean that they haven't
>+    # been made available yet. In this case, look at Bugzilla_cookie_list.
>+    unless (@info) {
>+        my $cookie = first {$_->name eq 'Bugzilla_login'}
>+                            @{$cgi->{'Bugzilla_cookie_list'}};
>+        @info = $cookie->value if $cookie;
>+    }
>+
>+    if ( not( @info == 6 ) ) {
>+        # expect 3 key/val pairs (userid/token/MAC)
>+        # ADD LOGGING HERE
>+        return { userid => 0 };
>+    }
>+
>+    my %info = @info;
>+
>+    # find attribute
>+    for my $attr qw(userid token MAC) {
>+        if ( not $info{$attr} ) {
>+            # ADD LOGGING HERE
>+            return { userid => 0 };
>+        }
>+    }
>+
>+    # This method of using a MAC to make sure a cookie is valid
>+    # is discussed in the Eagle Book.
>+    my $MAC = Digest::SHA1::sha1_hex(
>+        $info{userid} . $info{token} . Bugzilla->params->{mac_secret} );
>+
>+    # If the cookie's MAC matches the one we generate, we know
>+    # that the cookie has not been tampered with.
>+    if ( $info{MAC} ne $MAC ) {
>+        # Cookie was tampered with
>+        # ADD LOGGING HERE
>+        return { userid => 0 };
>+    }
>+
>+    # untaint data and return as hash ref
>+    delete $info{MAC};    # discard
>+    trick_taint( $info{userid} );
>+    trick_taint( $info{token} );
>+
>+    return \%info;
>+}
>+
> 1;
>Index: Bugzilla/Auth/Persist/Cookie.pm
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Auth/Persist/Cookie.pm,v
>retrieving revision 1.5
>diff -u -r1.5 Cookie.pm
>--- Bugzilla/Auth/Persist/Cookie.pm	3 Jul 2006 21:42:46 -0000	1.5
>+++ Bugzilla/Auth/Persist/Cookie.pm	23 Jan 2008 20:39:11 -0000
>@@ -37,6 +37,7 @@
> use Bugzilla::Token;
> 
> use List::Util qw(first);
>+use Digest::SHA1;
> 
> sub new {
>     my ($class) = @_;
>@@ -75,18 +76,24 @@
>           $cgi->param('Bugzilla_remember') eq 'on') ) 
>     {
>         $cgi->send_cookie(-name => 'Bugzilla_login',
>-                          -value => $user->id,
>+                          -value => {
>+                                userid => $user->id,
>+                                token  => $login_cookie,
>+                                MAC    => Digest::SHA1::sha1_hex(
>+                                    $user->id . $login_cookie . Bugzilla->params->{mac_secret} 
>+                                )
>+                           },
>                           -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
>-        $cgi->send_cookie(-name => 'Bugzilla_logincookie',
>-                          -value => $login_cookie,
>-                          -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
>-
>     }
>     else {
>         $cgi->send_cookie(-name => 'Bugzilla_login',
>-                          -value => $user->id);
>-        $cgi->send_cookie(-name => 'Bugzilla_logincookie',
>-                          -value => $login_cookie);
>+                          -value => {
>+                                userid => $user->id,
>+                                token  => $login_cookie,
>+                                MAC    => Digest::SHA1::sha1_hex(
>+                                    $user->id . $login_cookie . Bugzilla->params->{mac_secret}
>+                                )
>+                           });
>     }
> }
> 
>@@ -106,18 +113,8 @@
>     }
> 
>     # The LOGOUT_*_CURRENT options require the current login cookie.
>-    # If a new cookie has been issued during this run, that's the current one.
>-    # If not, it's the one we've received.
>-    my $cookie = first {$_->name eq 'Bugzilla_logincookie'}
>-                       @{$cgi->{'Bugzilla_cookie_list'}};
>-    my $login_cookie;
>-    if ($cookie) {
>-        $login_cookie = $cookie->value;
>-    }
>-    else {
>-        $login_cookie = $cgi->cookie("Bugzilla_logincookie");
>-    }
>-    trick_taint($login_cookie);
>+    my $cookie_info = Bugzilla::Auth::Login::Cookie::get_cookie_info();
>+    my $login_cookie = $cookie_info->{token};
> 
>     # These queries use both the cookie ID and the user ID as keys. Even
>     # though we know the userid must match, we still check it in the SQL
>@@ -145,7 +142,6 @@
> sub clear_browser_cookies {
>     my $cgi = Bugzilla->cgi;
>     $cgi->remove_cookie('Bugzilla_login');
>-    $cgi->remove_cookie('Bugzilla_logincookie');
> }
> 
> 1;
>Index: Bugzilla/Config/Auth.pm
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/Auth.pm,v
>retrieving revision 1.3
>diff -u -r1.3 Auth.pm
>--- Bugzilla/Config/Auth.pm	2 Aug 2007 22:38:39 -0000	1.3
>+++ Bugzilla/Config/Auth.pm	23 Jan 2008 20:39:11 -0000
>@@ -104,6 +104,12 @@
>   },
> 
>   {
>+   name => 'mac_secret',
>+   type => 't',
>+   default => '',
>+  },
>+
>+  {
>    name => 'emailregexp',
>    type => 't',
>    default => q:^[\\w\\.\\+\\-=]+@[\\w\\.\\-]+\\.[\\w\\-]+$:,
>Index: template/en/default/admin/params/auth.html.tmpl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/auth.html.tmpl,v
>retrieving revision 1.4
>diff -u -r1.4 auth.html.tmpl
>--- template/en/default/admin/params/auth.html.tmpl	5 Dec 2007 00:48:30 -0000	1.4
>+++ template/en/default/admin/params/auth.html.tmpl	23 Jan 2008 20:39:12 -0000
>@@ -112,6 +112,9 @@
>                   "front page will require a login. No anonymous users will " _
>                   "be permitted.",
> 
>+  mac_secret => "If the cookie\'s MAC matches the one we generate, " _
>+                "we know that a cookie has not been tampered with.",
>+
>   emailregexp => "This defines the regexp to use for legal email addresses. The " _
>                  "default tries to match fully qualified email addresses. Another " _
>                  "popular value to put here is <tt>^[^@]+$</tt>, which means " _

Actions: View | Diff

Attachments on bug 406231: 292459 | 292694