Bug 619594: (CVE-2010-4568) [SECURITY] Improve the randomness of

generate_random_password, to protect against an account compromise issue
and other critical vulnerabilities.
r=LpSolit, a=LpSolit

https://bugzilla.mozilla.org/show_bug.cgi?id=621591

diff --git a/Bugzilla/Install/Localconfig.pm b/Bugzilla/Install/Localconfig.pm
index 956d3c72e3edc4afcdeed3ce1d9834d92aacaf8c..3ce12207e8f30c4cf12c124098668ecd7510ae1a 100644 (file)
--- a/Bugzilla/Install/Localconfig.pm
+++ b/Bugzilla/Install/Localconfig.pm
@@ -109,7 +109,9 @@ use constant LOCALCONFIG_VARS => (
     },
     {
         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) },
     },
 );
 
@@ -210,7 +212,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';
             if (exists $answer->{$name}) {

diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index f45360916a14cc5341513e41e845ad02ab1c4f8c..e3049f2d5d9d97f252ced59bb5af9263dccaed1d 100644 (file)
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -157,6 +157,12 @@ sub REQUIRED_MODULES {
         module  => 'List::MoreUtils',
         version => 0.22,
     },
+    {
+        package => 'Math-Random-Secure',
+        module  => 'Math::Random::Secure',
+        # This is the first version that installs properly on Windows.
+        version => '0.05',
+    },
     );
 
     my $extra_modules = _get_extension_requirements('REQUIRED_MODULES');

diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 457eb7d02228be17ae11732752c7130e45194878..f9e8d12f79bd556b9d40c9d6438e068a133e9298 100644 (file)
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -54,6 +54,7 @@ use DateTime::TimeZone;
 use Digest;
 use Email::Address;
 use List::Util qw(first);
+use Math::Random::Secure qw(irand);
 use Scalar::Util qw(tainted);
 use Template::Filters;
 use Text::Wrap;
@@ -535,9 +536,15 @@ sub bz_crypt {
     return $crypted_password;
 }
 
+# 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));
+    return join("", map{ ('0'..'9','a'..'z','A'..'Z')[irand 62] } (1..$size));
 }
 
 sub validate_email_syntax {

diff --git a/mod_perl.pl b/mod_perl.pl
index 3551abdaf3a9d0023a8feeb3a29b53971b4234bb..0c7caf173b3891ffb35c74db85ec6e9ce64a32c6 100644 (file)
--- a/mod_perl.pl
+++ b/mod_perl.pl
@@ -37,6 +37,7 @@ use lib Bugzilla::Constants::bz_locations()->{'ext_libpath'};
 use Apache2::ServerUtil;
 use ModPerl::RegistryLoader ();
 use File::Basename ();
+use Math::Random::Secure;
 
 # This loads most of our modules.
 use Bugzilla ();
@@ -60,8 +61,12 @@ 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.)
+PerlChildInitHandler "sub { 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