Fix _libssh2_random() silently discarding errors (#520)

Notes:
* Make _libssh2_random return code consistent

Previously, _libssh2_random was advertized in HACKING.CRYPTO as
returning `void` (and was implemented that way in os400qc3.c), but that
was in other crypto backends a lie; _libssh2_random is (a macro
expanding) to an int-value expression or function.

Moreover, that returned code was:
  — 0 or success, -1 on error for the MbedTLS & WinCNG crypto backends
But also:
  — 1 on success, -1 or 0 on error for the OpenSSL backend!
  – 1 on success, error cannot happen for libgcrypt!

This commit makes explicit that _libssh2_random can fail (because most of
the underlying crypto functions can indeed fail!), and it makes its result
code consistent: 0 on success, -1 on error.

This is related to issue #519 https://github.com/libssh2/libssh2/issues/519
It fixes the first half of it.

* Don't silent errors of _libssh2_random

Make sure to check the returned code of _libssh2_random(), and
propagates any failure.

A new LIBSSH_ERROR_RANDGEN constant is added to libssh2.h
None of the existing error constants seemed fit.

This commit is related to d74285b68450c0e9ea6d5f8070450837fb1e74a7
and to https://github.com/libssh2/libssh2/issues/519 (see the issue
for more info.)  It closes #519.

Credit:
Paul Capron

docs/HACKING.CRYPTO

@@ -897,5 +897,6 @@ In example, this is needed to preset unused structure slacks on platforms
 requiring it.
 If this is not needed, it should be defined as an empty macro.
 
-void _libssh2_random(unsigned char *buf, int len);
+int _libssh2_random(unsigned char *buf, int len);
 Store len random bytes at buf.
+Returns 0 if OK, else -1.

include/libssh2.h

@@ -505,6 +505,7 @@ typedef struct _LIBSSH2_POLLFD {
 #define LIBSSH2_ERROR_KNOWN_HOSTS               -46
 #define LIBSSH2_ERROR_CHANNEL_WINDOW_FULL       -47
 #define LIBSSH2_ERROR_KEYFILE_AUTH_FAILED       -48
+#define LIBSSH2_ERROR_RANDGEN                   -49
 
 /* this is a define to provide the old (<= 1.2.7) name */
 #define LIBSSH2_ERROR_BANNER_NONE LIBSSH2_ERROR_BANNER_RECV

src/channel.c

@@ -1338,7 +1338,11 @@ channel_x11_req(LIBSSH2_CHANNEL *channel, int single_connection,
                border */
             unsigned char buffer[(LIBSSH2_X11_RANDOM_COOKIE_LEN / 2) + 1];
 
-            _libssh2_random(buffer, LIBSSH2_X11_RANDOM_COOKIE_LEN / 2);
+            if(_libssh2_random(buffer, LIBSSH2_X11_RANDOM_COOKIE_LEN / 2)) {
+                return _libssh2_error(session, LIBSSH2_ERROR_RANDGEN,
+                                      "Unable to get random bytes "
+                                      "for x11-req cookie");
+            }
             for(i = 0; i < (LIBSSH2_X11_RANDOM_COOKIE_LEN / 2); i++) {
                 snprintf((char *)&s[i*2], 3, "%02X", buffer[i]);
             }

src/kex.c

@@ -3176,7 +3176,11 @@ static int kexinit(LIBSSH2_SESSION * session)
 
         *(s++) = SSH_MSG_KEXINIT;
 
-        _libssh2_random(s, 16);
+        if(_libssh2_random(s, 16)) {
+            return _libssh2_error(session, LIBSSH2_ERROR_RANDGEN,
+                                  "Unable to get random bytes "
+                                  "for KEXINIT cookie");
+        }
         s += 16;
 
         /* Ennumerating through these lists twice is probably (certainly?)

src/libgcrypt.h

@@ -68,7 +68,7 @@
 #define EC_MAX_POINT_LEN ((528 * 2 / 8) + 1)
 
 #define _libssh2_random(buf, len)                \
-  (gcry_randomize ((buf), (len), GCRY_STRONG_RANDOM), 1)
+  (gcry_randomize ((buf), (len), GCRY_STRONG_RANDOM), 0)
 
 #define libssh2_prepare_iovec(vec, len)  /* Empty. */
 

src/openssl.h

@@ -137,7 +137,7 @@
 
 #define EC_MAX_POINT_LEN ((528 * 2 / 8) + 1)
 
-#define _libssh2_random(buf, len) RAND_bytes ((buf), (len))
+#define _libssh2_random(buf, len) (RAND_bytes((buf), (len)) == 1 ? 0 : -1)
 
 #define libssh2_prepare_iovec(vec, len)  /* Empty. */
 

src/os400qc3.c

@@ -884,11 +884,14 @@ _libssh2_bn_from_bn(_libssh2_bn *to, _libssh2_bn *from)
     return 0;
 }
 
-void
+int
 _libssh2_random(unsigned char *buf, int len)
 {
     Qc3GenPRNs(buf, len,
         Qc3PRN_TYPE_NORMAL, Qc3PRN_NO_PARITY, (char *) &ecnull);
+    /* FIXME: any error is silently discarded! But Qc3GenPRNs could fail,
+       including if "The system seed digest is not ready" dixit IBM doc. */
+    return 0;
 }
 
 

src/os400qc3.h

@@ -360,7 +360,7 @@ extern int      _libssh2_bn_from_bin(_libssh2_bn *bn, int len,
                                      const unsigned char *v);
 extern int      _libssh2_bn_set_word(_libssh2_bn *bn, unsigned long val);
 extern int      _libssh2_bn_to_bin(_libssh2_bn *bn, unsigned char *val);
-extern void     _libssh2_random(unsigned char *buf, int len);
+extern int      _libssh2_random(unsigned char *buf, int len);
 extern void     _libssh2_os400qc3_crypto_dtor(_libssh2_os400qc3_crypto_ctx *x);
 extern int      libssh2_os400qc3_hash_init(Qc3_Format_ALGD0100_T *x,
                                            unsigned int algo);

src/transport.c

@@ -862,7 +862,10 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
     p->outbuf[4] = (unsigned char)padding_length;
 
     /* fill the padding area with random junk */
-    _libssh2_random(p->outbuf + 5 + data_len, padding_length);
+    if(_libssh2_random(p->outbuf + 5 + data_len, padding_length)) {
+        return _libssh2_error(session, LIBSSH2_ERROR_RANDGEN,
+                              "Unable to get random bytes for packet padding");
+    }
 
     if(encrypted) {
         size_t i;