[PATCH] testbot/web: Use a cryptographically secure random number generator.

[Francois Gouget]

Session ids should really be hard to guess so a user cannot take over
another's session.
This also fixes a bug where the session id length could be less than
32 characters.

Note:
* This introduces a dependency on the Bytes::Random::Secure Perl
  module.

Signed-off-by: Francois Gouget 
---

libbytes-random-secure-perl is already installed so this should just 
take a web server restart.

Perl documents rand() as being not cryptographically secure. That means 
an attacker could cycle through session ids for his own account and 
derive the random number generator internal state, thus allowing him to 
predict the session ids for other users' next login.

Funnily enough the bug that caused session ids to not always have 32 
characters could actually make this type of attack much harder: for this 
attack one needs the sequence of values returned by rand(), here groups 
of 4 hexadecimal characters. But because sprintf("%lx") does not 0-pad 
its result, a 31 charachter session id starting with '123456789ab...' 
could either be '123' + '4567' + '89ab', or '1234' + '567' + '89ab', or 
'1234' + '5678' + '9ab', etc.

But not being a cryptographer and not knowing the internals of rand() I 
don't know how much harder this makes the attacks. So better safe than 
sorry.

testbot/doc/INSTALL.txt                 | 1 +
 testbot/lib/WineTestBot/CGI/Sessions.pm | 8 +++-----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/testbot/doc/INSTALL.txt b/testbot/doc/INSTALL.txt
index 456ffe6957..4d430d9157 100644
--- a/testbot/doc/INSTALL.txt
+++ b/testbot/doc/INSTALL.txt
@@ -6,6 +6,7 @@ Dependencies:
 - MySQL
 - Perl DBD and DBI::mysql modules
 - Sendmail and Procmail
+- Bytes::Random::Secure (libbytes-random-secure-perl)
 - Sys::Virt (libsys-virt-perl, see http://libvirt.org/)
 - Image::Magick (perlmagick)
 - Optional: IO::Socket::IP (for IPv6, libio-socket-ip-perl)
diff --git a/testbot/lib/WineTestBot/CGI/Sessions.pm b/testbot/lib/WineTestBot/CGI/Sessions.pm
index 8cd3c52fb7..4ca62ba76e 100644
--- a/testbot/lib/WineTestBot/CGI/Sessions.pm
+++ b/testbot/lib/WineTestBot/CGI/Sessions.pm
@@ -54,7 +54,9 @@ use WineTestBot::WineTestBotObjects;
 our @ISA = qw(WineTestBot::WineTestBotCollection);
 our @EXPORT = qw(CreateSessions DeleteSessions NewSession);

+use Bytes::Random::Secure;
 use CGI::Cookie;
+
 use ObjectModel::BasicPropertyDescriptor;
 use ObjectModel::ItemrefPropertyDescriptor;
 use WineTestBot::Users;
@@ -121,11 +123,7 @@ sub NewSession($$$)
   my $Id;
   while (defined($Existing))
   {
-    $Id = "";
-    foreach my $i (1..8)
-    {
-      $Id .= sprintf("%lx", int(rand(2 ** 16)));
-    }
+    $Id = Bytes::Random::Secure::random_bytes_hex(16);
     $Existing = $self->GetItem($Id);
   }
   $Session->Id($Id);