[rt #1279] when logging in (as a siteuser) start a new session object
authorTony Cook <tony@develop-help.com>
Fri, 16 Dec 2011 11:49:17 +0000 (22:49 +1100)
committerTony Cook <tony@develop-help.com>
Fri, 16 Dec 2011 11:49:17 +0000 (22:49 +1100)
This prevents some cookie duplication attacks.

We also do the exchange to the ssl/non-ssl side of the site more
securely.

MANIFEST
site/cgi-bin/modules/BSE/Cfg.pm
site/cgi-bin/modules/BSE/SessionSign.pm [new file with mode: 0644]
site/cgi-bin/modules/BSE/UI/Shop.pm
site/cgi-bin/modules/BSE/UserReg.pm

index b7d56d6..f0e8204 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -136,6 +136,7 @@ site/cgi-bin/modules/BSE/Request/Test.pm
 site/cgi-bin/modules/BSE/Search/Base.pm
 site/cgi-bin/modules/BSE/Search/BSE.pm
 site/cgi-bin/modules/BSE/Session.pm
+site/cgi-bin/modules/BSE/SessionSign.pm
 site/cgi-bin/modules/BSE/Shipping.pm
 site/cgi-bin/modules/BSE/Shop/Util.pm
 site/cgi-bin/modules/BSE/Sort.pm
index 7480b4b..95a3bb8 100644 (file)
@@ -4,7 +4,7 @@ use base "DevHelp::Cfg";
 use Carp qw(confess);
 use constant MAIN_CFG => 'bse.cfg';
 
-our $VERSION = "1.003";
+our $VERSION = "1.004";
 
 my %cache;
 
@@ -103,14 +103,14 @@ sub user_url {
 
   my $secure = $script =~ /^(shop|user)$/;
   $secure = $cfg->entry("secure user url", $script, $secure);
-  my $base = $secure ? $cfg->entryVar('site', 'secureurl') : '';
+  my $base;
   my $template;
   if ($target) {
     if ($script eq 'nuser') {
       $template = "/cgi-bin/nuser.pl/user/TARGET";
     }
     else {
-      $template = "$base/cgi-bin/$script.pl?a_TARGET=1";
+      $template = "/cgi-bin/$script.pl?a_TARGET=1";
     }
     $template = $cfg->entry('targets', $script, $template);
     $template =~ s/TARGET/$target/;
@@ -120,21 +120,30 @@ sub user_url {
       $template = "/cgi-bin/nuser.pl/user";
     }
     else {
-      $template = "$base/cgi-bin/$script.pl";
+      $template = "/cgi-bin/$script.pl";
     }
     $template = $cfg->entry('targets', $script.'_n', $template);
   }
   if (@options) {
-    $template .= $template =~ /\?/ ? '&' : '?';
     my @entries;
     while (my ($key, $value) = splice(@options, 0, 2)) {
       require BSE::Util::HTML;
-      push @entries, "$key=" . BSE::Util::HTML::escape_uri($value);
+      if ($key eq '-base') {
+       $base = $value;
+      }
+      else {
+       push @entries, "$key=" . BSE::Util::HTML::escape_uri($value);
+      }
+    }
+    if (@entries) {
+      $template .= $template =~ /\?/ ? '&' : '?';
+      $template .= join '&', @entries;
     }
-    $template .= join '&', @entries;
   }
 
-  return $template;
+  $base ||= $secure ? $cfg->entryVar('site', 'secureurl') : '';
+
+  return $base . $template;
 }
 
 sub admin_url {
diff --git a/site/cgi-bin/modules/BSE/SessionSign.pm b/site/cgi-bin/modules/BSE/SessionSign.pm
new file mode 100644 (file)
index 0000000..51a21c9
--- /dev/null
@@ -0,0 +1,74 @@
+package BSE::SessionSign;
+use strict;
+use BSE::Cfg;
+
+our $VERSION = "1.000";
+
+sub _sign {
+  my ($self, $sessionid, $when) = @_;
+
+  require Digest::SHA;
+
+  my $secret = BSE::Cfg->single->entryErr("site", "secret");
+  my $sha = Digest::SHA::sha256_base64($secret, $sessionid, $when);
+
+  return $when . "." . $sha;
+}
+
+sub make {
+  my ($self, $sessionid) = @_;
+
+  my $now = time;
+
+  return $self->_sign($sessionid, $now);
+}
+
+sub check {
+  my ($self, $sessionid, $sig, $error) = @_;
+
+  my $now = time;
+  my ($then, $sha) = split /\./, $sig, 2;
+
+  my $good_sig = $self->_sign($sessionid, $then);
+
+  if ($good_sig ne $sig) {
+    require BSE::TB::AuditLog;
+    BSE::TB::AuditLog->log
+       (
+        component => "user::setcookie",
+        level => "warning",
+        actor => "S",
+        msg => "Bad signature setting session cookie",
+        dump => <<DUMP,
+Received:
+sessionid: $sessionid
+sig: $sig
+DUMP
+       );
+    $$error = "BADSIG";
+    return;
+  }
+
+  require BSE::TB::AuditLog;
+  unless ($then + 30 > $now) {
+    require BSE::TB::AuditLog;
+    BSE::TB::AuditLog->log
+       (
+        component => "user::setcookie",
+        level => "warning",
+        actor => "S",
+        msg => "Too old setting session cookie",
+        dump => <<DUMP,
+Received:
+then: $then
+now: $now
+DUMP
+       );
+    $$error = "OLDSIG";
+    return 0;
+  }
+
+  return 1;
+}
+
+1;
index d4f1a03..dd07f43 100644 (file)
@@ -17,7 +17,7 @@ use BSE::Shipping;
 use BSE::Countries qw(bse_country_code);
 use BSE::Util::Secure qw(make_secret);
 
-our $VERSION = "1.029";
+our $VERSION = "1.030";
 
 use constant MSG_SHOP_CART_FULL => 'Your shopping cart is full, please remove an item and try adding an item again';
 
@@ -2028,17 +2028,23 @@ sub _add_refresh {
        print STDERR "not on base host ('$ENV{SERVER_NAME}' cmp '$basehost' '$protocol cmp '$baseprot'  $baseport cmp $port\n" if $debug;
        $onbase = 0;
       }
-      my $url = $onbase ? $secure_url : $base_url;
+      my $base = $onbase ? $secure_url : $base_url;
       my $finalbase = $onbase ? $base_url : $secure_url;
       $refresh = $finalbase . $refresh unless $refresh =~ /^\w+:/;
+      my $sessionid = $req->session->{_session_id};
+      require BSE::SessionSign;
+      my $sig = BSE::SessionSign->make($sessionid);
+      my $url = $cfg->user_url("user", undef,
+                              -base => $base,
+                              setcookie => $sessionid,
+                              s => $sig,
+                              r => $refresh);
       print STDERR "Heading to $url to setcookie\n" if $debug;
-      $url .= "/cgi-bin/user.pl?setcookie=".$req->session->{_session_id};
-      $url .= "&r=".CGI::escape($refresh);
-      return BSE::Template->get_refresh($url, $cfg);
+      return $req->get_refresh($url);
     }
   }
 
-  return BSE::Template->get_refresh($refresh, $cfg);
+  return $req->get_refresh($refresh);
 }
 
 sub _same_options {
index f0b6137..a19f8f0 100644 (file)
@@ -18,7 +18,7 @@ use BSE::Util::Iterate;
 use base 'BSE::UI::UserCommon';
 use Carp qw(confess);
 
-our $VERSION = "1.020";
+our $VERSION = "1.021";
 
 use constant MAX_UNACKED_CONF_MSGS => 3;
 use constant MIN_UNACKED_CONF_GAP => 2 * 24 * 60 * 60;
@@ -204,22 +204,33 @@ sub req_logon {
     return $self->req_show_opts($req, undef, \%errors);
   }
 
-  $session->{userid} = $user->id;
   $user->{previousLogon} = $user->{lastLogon};
   $user->{lastLogon} = now_datetime;
   $user->save;
 
+  my $cart = delete $session->{cart};
+  undef $session;
+  $req->clear_session;
+  $session = $req->session;
+
+  $session->{cart} = $cart;
+  $session->{userid} = $user->id;
+
   if ($custom->can('siteuser_login')) {
     $custom->siteuser_login($session->{_session_id}, $session->{userid}, 
                            $cfg, $session);
   }
+
   $self->_send_user_cookie($user);
 
-  return _got_user_refresh($session, $cgi, $cfg);
+  return $self->_got_user_refresh($req);
 }
 
 sub _got_user_refresh {
-  my ($session, $cgi, $cfg) = @_;
+  my ($self, $req) = @_;
+
+  my $cfg = $req->cfg;
+  my $session = $req->session;
 
   my $baseurl = $cfg->entryVar('site', 'url');
   my $securl = $cfg->entryVar('site', 'secureurl');
@@ -253,29 +264,33 @@ sub _got_user_refresh {
       $onbase = 0;
     }
   }
-  my $refresh = $cgi->param('r');
+  my $refresh = $req->cgi->param('r');
   unless ($refresh) {
     if ($session->{userid}) {
-      $refresh = "$ENV{SCRIPT_NAME}?userpage=1";
+      $refresh = $cfg->user_url("user", "userpage");
     }
     else {
-      $refresh = "$ENV{SCRIPT_NAME}?show_logon=1";
+      $refresh = $cfg->user_url("user", "show_logon");
     }
   }
   if ($need_magic) {
-    my $url = $onbase ? $securl : $baseurl;
+    my $base = $onbase ? $securl : $baseurl;
     my $finalbase = $onbase ? $baseurl : $securl;
     $refresh = $finalbase . $refresh unless $refresh =~ /^\w+:/;
+    my $sessionid = $session->{_session_id};
+    require BSE::SessionSign;
+    my $sig = BSE::SessionSign->make($sessionid);
+    my $url = $cfg->user_url("user", undef,
+                            -base => $base,
+                            setcookie => $sessionid,
+                            s => $sig,
+                            r => $refresh);
     print STDERR "Heading to $url to setcookie\n" if $debug;
-    $url .= "$ENV{SCRIPT_NAME}?setcookie=".$session->{_session_id};
-    $url .= "&r=".escape_uri($refresh);
-    refresh_to($url);
+    return $req->get_refresh($url);
   }
   else {
-    refresh_to($refresh);
+    return $req->get_refresh($refresh);
   }
-
-  return;
 }
 
 sub req_setcookie {
@@ -290,6 +305,16 @@ sub req_setcookie {
   my $cookie = $cgi->param('setcookie')
     or return $self->req_show_logon($req, 
                                $msgs->(nocookie=>"No cookie provided"));
+  my $sig = $cgi->param("s")
+    or return $self->req_show_logon($req,
+                                   $msgs->(nosig => "No signature for setcookie"));
+
+  require BSE::SessionSign;
+  my $error;
+  unless (BSE::SessionSign->check($cookie, $sig, \$error)) {
+    return $self->req_show_logon($req,
+                                   $msgs->(badsig => "Invalid signature for setcookie"));
+  }
   print STDERR "Setting sessionid to $cookie for $ENV{HTTP_HOST}\n";
   my %newsession;
   BSE::Session->change_cookie($session, $cfg, $cookie, \%newsession);
@@ -337,9 +362,7 @@ sub req_logoff {
     $custom->siteuser_logout($session_id, $cfg);
   }
 
-  my $session = $req->session;
-
-  return _got_user_refresh($session, $cgi, $cfg);
+  return $self->_got_user_refresh($req);
 }
 
 sub tag_if_subscribed_register {
@@ -795,7 +818,7 @@ sub req_saveopts {
 
     $self->_send_user_cookie($user);
 
-    return _got_user_refresh($session, $cgi, $cfg);
+    return $self->_got_user_refresh($req);
   }
 
   my $url = $cgi->param('r');
@@ -1035,7 +1058,7 @@ sub req_register {
       $self->_notify_registration($req, $user);
     }
 
-    return _got_user_refresh($session, $cgi, $cfg);
+    return $self->_got_user_refresh($req);
   }
   else {
     return $self->req_show_register($req, $msgs->(regdberr=> "Database error $@"));