Add support for PolarSSL 1.1.x branch

PATCH In review Igor Novgorodov 3 participants 5 messages Feb 27, 2012 – Feb 28, 2012

lore

Share & tools

lore

Igor Novgorodov

author

2012-02-27 19:58 UTC #0 lore raw

PATCH 1/2 Add support for PolarSSL 1.1.x branch

Continuing to struggle with making OpenVPN as tiny as possible :)

The attached patch adds checking for PolarSSL version on crypto_polarssl.c and depending on which version we are using (1.0.x or 1.1.x) chooses a new shiny havege_random() function, or an old ugly while{} loop hack to generate randomness.

crypto_polarssl.c +10 ~~-10~~

@@ -41,6 +41,7 @@
 #include <polarssl/md5.h>
 #include <polarssl/cipher.h>
 #include <polarssl/havege.h>
+#include <polarssl/version.h>
 
 /*
  *

@@ -157,25 +158,24 @@
 rand_bytes (uint8_t *output, int len)
 {
   static havege_state hs = {0};
-  static bool hs_initialised = false;
-  const int int_size = sizeof(int);
 
-  if (!hs_initialised)
-    {
-      /* Initialise PolarSSL RNG */
-      havege_init(&hs);
-      hs_initialised = true;
-    }
+  /* Initialise PolarSSL RNG */
+  havege_init(&hs);
+#if (POLARSSL_VERSION_MAJOR >= 1 && POLARSSL_VERSION_MINOR >= 1)
+  havege_random(&hs, output, len);
+#else
+  const int int_size = sizeof(int);
 
   while (len > 0)
     {
-      const int blen 	= min_int (len, int_size);
-      const int rand_int 	= havege_rand(&hs);
+      const int blen    = min_int (len, int_size);
+      const int rand_int        = havege_rand(&hs);
 
       memcpy (output, &rand_int, blen);
       output += blen;
       len -= blen;
     }
+#endif
   return 1;
 }

Fabian Knittel

2012-02-27 21:37 UTC #1 lore raw

Hi Igor,

2012/2/27 Igor Novgorodov <[email protected]...>:

Quoted 4 lines

The attached patch adds checking for PolarSSL version on crypto_polarssl.c and depending on which version we are using (1.0.x or 1.1.x) chooses a new shiny havege_random() function, or an old ugly while{} loop hack to generate randomness.

Your patch removes the code that causes havege_init() to only be called once. You never want to initialise your PRNG more than once, otherwise you increase the risk that your randomness is predictable. So please revert that part of your patch.

(The rest looks fine, although I haven't tested it and don't have any experience with PolarSSL.)

Cheers
Fabian

Igor Novgorodov

author

2012-02-28 05:46 UTC #2 lore raw

Quoted 11 lines · Fabian Knittel

Hi Igor,
2012/2/27 Igor Novgorodov<[email protected]...>:
Quoted 4 lines
The attached patch adds checking for PolarSSL version on crypto_polarssl.c and depending on which version we are using (1.0.x or 1.1.x) chooses a new shiny havege_random() function, or an old ugly while{} loop hack to generate randomness.
Your patch removes the code that causes havege_init() to only be called once. You never want to initialise your PRNG more than once, otherwise you increase the risk that your randomness is predictable. So please revert that part of your patch.

Yes, my fault. I didn't notice that the variable was static, so i though that it was local-scope only and removed the check... The fixed patch is attached

Quoted 5 lines

(The rest looks fine, although I haven't tested it and don't have any experience with PolarSSL.)
Cheers
Fabian

crypto_polarssl.c +9 -3

@@ -41,6 +41,7 @@
 #include <polarssl/md5.h>
 #include <polarssl/cipher.h>
 #include <polarssl/havege.h>
+#include <polarssl/version.h>
 
 /*
  *

@@ -158,7 +159,6 @@
 {
   static havege_state hs = {0};
   static bool hs_initialised = false;
-  const int int_size = sizeof(int);
 
   if (!hs_initialised)
     {

@@ -167,15 +167,21 @@
       hs_initialised = true;
     }
 
+#if (POLARSSL_VERSION_MAJOR >= 1 && POLARSSL_VERSION_MINOR >= 1)
+  havege_random(&hs, output, len);
+#else
+  const int int_size = sizeof(int);
+
   while (len > 0)
     {
-      const int blen 	= min_int (len, int_size);
-      const int rand_int 	= havege_rand(&hs);
+      const int blen    = min_int (len, int_size);
+      const int rand_int        = havege_rand(&hs);
 
       memcpy (output, &rand_int, blen);
       output += blen;
       len -= blen;
     }
+#endif
   return 1;
 }

Fabian Knittel

2012-02-28 07:39 UTC #3 lore raw

Hi Igor,

2012/2/28 Igor Novgorodov <[email protected]...>:

Quoted 8 lines · Fabian Knittel

Quoted 4 lines
Your patch removes the code that causes havege_init() to only be called once. You never want to initialise your PRNG more than once, otherwise you increase the risk that your randomness is predictable. So please revert that part of your patch.
Yes, my fault. I didn't notice that the variable was static, so i though that it was local-scope only and removed the check... The fixed patch is attached

Thanks!

Quoted 2 lines

Quoted 2 lines
([...], although I haven't tested it and don't have any experience with PolarSSL.)

Maybe Adriaan or someone else can take a quick peek and give a full-hearted ACK?

Cheers
Fabian

Adriaan de Jong

2012-02-28 07:50 UTC #4 lore raw

Hi Fabian and Igor,

Thanks for your patch! As the Havege random number generator has some known issues on a (limited) set of virtual machines, there's a brand new RNG in PolarSSL. I'm currently working on a more complete support patch for PolarSSL's new RNG.

Instead of calling Havege directly, this patch calls the NIST DRBG which uses both Havege and the platform RNG as entropy sources (/dev/urandom).

For the moment, I'd prefer to include that here, instead of these more limited patches.

Kind regards,

Adriaan

Quoted 33 lines

-----Original Message-----
From: [email protected]... [mailto:[email protected]...] On Behalf Of Fabian Knittel
Sent: dinsdag 28 februari 2012 8:40
To: Igor Novgorodov
Cc: Adriaan de Jong; [email protected]
Subject: Re: [Openvpn-devel] [PATCH 01/02] Add support for PolarSSL 1.1.x branch
Hi Igor,
2012/2/28 Igor Novgorodov <[email protected]...>:
Quoted 8 lines · Fabian Knittel
Quoted 4 lines
Your patch removes the code that causes havege_init() to only be called once. You never want to initialise your PRNG more than once, otherwise you increase the risk that your randomness is predictable. So please revert that part of your patch.
Yes, my fault. I didn't notice that the variable was static, so i though that it was local-scope only and removed the check... The
fixed
Quoted 1 line
patch is attached
Thanks!
Quoted 2 lines
Quoted 2 lines
([...], although I haven't tested it and don't have any experience with PolarSSL.)
Maybe Adriaan or someone else can take a quick peek and give a full- hearted ACK?
Cheers
Fabian