Browse code

openssl: properly check return value of RAND_bytes()

This patch is in response to an off-list report by Sebastian Krahmer of
the SuSE security team. Sebastian noticed we do not check the return
value of RAND_bytes() correctly.

The RAND_bytes() man page first says "RAND_bytes() returns 1 on success,
0 otherwise.", but then a bit later "Both functions return -1 if they are
not supported by the current RAND method.". This second case was not
covered by our return value checking.

Note that if RAND_bytes() would return -1, it would *always* return -1 and
fail to generate random.

Also note that if RAND_bytes() would return -1, it would do so too in the
openssl internal ssl funtions. The openssl internal function do check the
return value properly, and connection setup would fail all together. If
that would be at least somewhat common, we would have received a *lot* of
bug reports. In other words, the error affects static key setups only,
and seems highly unlikely to occur in actual setups.

Only builds using OpenSSL as the crypto backend are affected.

This patch:
1. Changes the behaviour of rand_bytes() in openssl builds to match what
the doxygen claims (and polarssl builds already do).
2. Adds error reporting for RAND_bytes() failures.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1448707105-10753-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10637
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2015/11/28 19:38:24
Showing 1 changed files
... ...
@@ -352,7 +352,12 @@ show_available_engines ()
352 352
 
353 353
 int rand_bytes(uint8_t *output, int len)
354 354
 {
355
-  return RAND_bytes (output, len);
355
+  if (unlikely(1 != RAND_bytes (output, len)))
356
+    {
357
+      crypto_msg(D_CRYPT_ERRORS, "RAND_bytes() failed");
358
+      return 0;
359
+    }
360
+  return 1;
356 361
 }
357 362
 
358 363
 /*