2011-01-24 William Siegrist <wsiegrist@apple.com>
authorwsiegrist@apple.com <wsiegrist@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Jan 2011 22:01:56 +0000 (22:01 +0000)
committerwsiegrist@apple.com <wsiegrist@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Jan 2011 22:01:56 +0000 (22:01 +0000)
Apply fix for https://bugzilla.mozilla.org/show_bug.cgi?id=621591

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76545 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/bugs.webkit.org/Bugzilla/Install/Localconfig.pm
Websites/bugs.webkit.org/Bugzilla/Install/Requirements.pm
Websites/bugs.webkit.org/Bugzilla/Util.pm
Websites/bugs.webkit.org/mod_perl.pl

index 8857b75..97d93ae 100644 (file)
@@ -188,7 +188,9 @@ EOT
     },
     {
         name    => 'site_wide_secret',
-        default => sub { generate_random_password(256) },
+        # 64 characters is roughly the equivalent of a 384-bit key, which
+        # is larger than anybody would ever be able to brute-force.
+        default => sub { generate_random_password(64) },
         desc    => <<EOT
 # This secret key is used by your installation for the creation and
 # validation of encrypted tokens to prevent unsolicited changes,
@@ -295,7 +297,14 @@ sub update_localconfig {
     my @new_vars;
     foreach my $var (LOCALCONFIG_VARS) {
         my $name = $var->{name};
-        if (!defined $localconfig->{$name}) {
+        my $value = $localconfig->{$name};
+        # Regenerate site_wide_secret if it was made by our old, weak
+        # generate_random_password. Previously we used to generate
+        # a 256-character string for site_wide_secret.
+        $value = undef if ($name eq 'site_wide_secret' and defined $value
+                           and length($value) == 256);
+        
+        if (!defined $value) {
             push(@new_vars, $name);
             $var->{default} = &{$var->{default}} if ref($var->{default}) eq 'CODE';
             $localconfig->{$name} = $answer->{$name} || $var->{default};
@@ -342,6 +351,18 @@ EOT
                                          ["*$var->{name}"]);
             }
         }
+        # When updating site_wide_secret to the new value, don't
+        # leave the old value behind.
+        if (grep { $_ eq 'site_wide_secret' } @new_vars) {
+            my $read = new IO::File($filename, '<') || die "$filename: $!";
+            my $text;
+            { local $/; $text = <$read> }
+            $read->close;
+            $text =~ s/^\$site_wide_secret = '\w{256}';$//ms;
+            my $write = new IO::File($filename, '>') || die "$filename: $!";
+            print $write $text;
+            $write->close;
+        }
 
         my $newstuff = join(', ', @new_vars);
         print <<EOT;
index 47699e4..d018139 100644 (file)
@@ -232,6 +232,12 @@ sub OPTIONAL_MODULES {
         version => '1.999022',
         feature => 'mod_perl'
     },
+    {
+        package => 'Math-Random-Secure',
+        module  => 'Math::Random::Secure',
+        version => '0.05',
+        feature => 'Improve cookie and token security',
+    },
     );
 
     my $all_modules = _get_extension_requirements(
index a872eb2..0d0c970 100644 (file)
@@ -504,9 +504,56 @@ sub bz_crypt {
     return $cryptedpassword;
 }
 
+# If you want to understand the security of strings generated by this
+# function, here's a quick formula that will help you estimate:
+# We pick from 62 characters, which is close to 64, which is 2^6.
+# So 8 characters is (2^6)^8 == 2^48 combinations. Just multiply 6
+# by the number of characters you generate, and that gets you the equivalent
+# strength of the string in bits.
 sub generate_random_password {
     my $size = shift || 10; # default to 10 chars if nothing specified
-    return join("", map{ ('0'..'9','a'..'z','A'..'Z')[rand 62] } (1..$size));
+    my $rand;
+    if (eval { require Math::Random::Secure; 1; }) {
+        $rand = \&Math::Random::Secure::irand;
+    }
+    else {
+        # For details on why this block works the way it does, see bug 619594.
+        # (Note that we don't do this if Math::Random::Secure is installed,
+        # because we don't need to.)
+        my $counter = 0;
+        $rand = sub {
+            # If we regenerate the seed every 5 characters, our seed is roughly
+            # as strong (in terms of bit size) as our randomly-generated
+            # string itself.
+            _do_srand() if ($counter % 5) == 0;
+            $counter++;
+            return int(rand $_[0]);
+        };
+    }
+    return join("", map{ ('0'..'9','a'..'z','A'..'Z')[$rand->(62)] } 
+                       (1..$size));
+}
+
+sub _do_srand {
+    # On Windows, calling srand over and over in the same process produces
+    # very bad results. We need a stronger seed.
+    if (ON_WINDOWS) {
+        require Win32;
+        # GuidGen generates random data via Windows's CryptGenRandom
+        # interface, which is documented as being cryptographically secure.
+        my $guid = Win32::GuidGen();
+        # GUIDs look like:
+        # {09531CF1-D0C7-4860-840C-1C8C8735E2AD}
+        $guid =~ s/[-{}]+//g;
+        # Get a 32-bit integer using the first eight hex digits.
+        my $seed = hex(substr($guid, 0, 8));
+        srand($seed);
+        return;
+    }
+
+    # On *nix-like platforms, this uses /dev/urandom, so the seed changes
+    # enough on every invocation.
+    srand();
 }
 
 sub validate_email_syntax {
index 6a9bc5c..7cc647c 100644 (file)
@@ -44,6 +44,9 @@ use Bugzilla::Mailer ();
 use Bugzilla::Template ();
 use Bugzilla::Util ();
 
+# For PerlChildInitHandler
+eval { require Math::Random::Secure };
+
 # This means that every httpd child will die after processing
 # a CGI if it is taking up more than 70MB of RAM all by itself.
 $Apache2::SizeLimit::MAX_UNSHARED_SIZE = 70000;
@@ -53,8 +56,14 @@ my $cgi_path = Bugzilla::Constants::bz_locations()->{'cgi_path'};
 # Set up the configuration for the web server
 my $server = Apache2::ServerUtil->server;
 my $conf = <<EOT;
-# Make sure each httpd child receives a different random seed (bug 476622)
-PerlChildInitHandler "sub { srand(); }"
+# Make sure each httpd child receives a different random seed (bug 476622).
+# Math::Random::Secure has one srand that needs to be called for
+# every process, and Perl has another. (Various Perl modules still use
+# the built-in rand(), even though we only use Math::Random::Secure in
+# Bugzilla itself, so we need to srand() both of them.) However, 
+# Math::Random::Secure may not be installed, so we call its srand in an
+# eval.
+PerlChildInitHandler "sub { eval { Math::Random::Secure::srand() }; srand(); }"
 <Directory "$cgi_path">
     AddHandler perl-script .cgi
     # No need to PerlModule these because they're already defined in mod_perl.pl