From 8e27439e6cf030838a245f5de6437e9c71d4fca5 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Mon, 24 Jan 2011 13:47:25 -0800 Subject: [PATCH] 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 --- Bugzilla/Install/Localconfig.pm | 13 ++++++- Bugzilla/Install/Requirements.pm | 7 ++++ Bugzilla/Util.pm | 49 +++++++++++++++++++++++- mod_perl.pl | 13 ++++++- template/en/default/setup/strings.txt.pl | 1 + 5 files changed, 78 insertions(+), 5 deletions(-) diff --git a/Bugzilla/Install/Localconfig.pm b/Bugzilla/Install/Localconfig.pm index d5d76cb799..e15e235070 100644 --- a/Bugzilla/Install/Localconfig.pm +++ b/Bugzilla/Install/Localconfig.pm @@ -205,7 +205,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 => <{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 04c3571b81..64a56410ad 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -327,6 +327,13 @@ sub OPTIONAL_MODULES { version => '0.93', feature => ['mod_perl'], }, + { + package => 'Math-Random-Secure', + module => 'Math::Random::Secure', + # This is the first version that installs properly on Windows. + version => '0.05', + feature => ['rand_security'], + }, ); if (ON_WINDOWS) { diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 412ab45e50..b1655f7ca0 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -548,9 +548,56 @@ 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)); + my $rand; + if (Bugzilla->feature('rand_security')) { + $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 { diff --git a/mod_perl.pl b/mod_perl.pl index f12ae93a64..cefe11d4cd 100644 --- a/mod_perl.pl +++ b/mod_perl.pl @@ -44,6 +44,9 @@ use File::Basename (); use Template::Config (); Template::Config->preload(); +# For PerlChildInitHandler +eval { require Math::Random::Secure }; + use Bugzilla (); use Bugzilla::CGI (); use Bugzilla::Extension (); @@ -62,8 +65,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 = < AddHandler perl-script .cgi # No need to PerlModule these because they're already defined in mod_perl.pl diff --git a/template/en/default/setup/strings.txt.pl b/template/en/default/setup/strings.txt.pl index 1cf399c960..5824f9579e 100644 --- a/template/en/default/setup/strings.txt.pl +++ b/template/en/default/setup/strings.txt.pl @@ -68,6 +68,7 @@ END feature_mod_perl => 'mod_perl', feature_moving => 'Move Bugs Between Installations', feature_patch_viewer => 'Patch Viewer', + feature_rand_security => 'Improve cookie and token security', feature_smtp_auth => 'SMTP Authentication', feature_updates => 'Automatic Update Notifications', feature_xmlrpc => 'XML-RPC Interface', -- 2.47.2