crypto: whip-up our bignum-from-bin API

The endgame being plugging the leak caused by us misusing libgcrypt
bignums from binary functions. If the DH exchange was interrupted, we'd
reload P & G on top of the still live values.

src/kex.c

@@ -144,12 +144,10 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,
         exchange_state->s_packet = NULL;
         exchange_state->k_value = NULL;
         exchange_state->ctx = _libssh2_bn_ctx_new();
-        libssh2_dh_init(&exchange_state->x);
-        exchange_state->e = _libssh2_bn_init(); /* g^x mod p */
-        exchange_state->f = _libssh2_bn_init_from_bin(); /* g^(Random from
-                                                            server) mod p */
-        exchange_state->k = _libssh2_bn_init(); /* The shared secret: f^x mod
-                                                   p */
+        _libssh2_dh_init(&exchange_state->x);
+        exchange_state->e = _libssh2_bn_new();  /* g^x mod p */
+        exchange_state->f = NULL;               /* g^(server random) mod p */
+        exchange_state->k = _libssh2_bn_new();  /* f^x mod p: shared secret */
 
         /* Zero the whole thing out */
         memset(&exchange_state->req_state, 0, sizeof(packet_require_state_t));
@@ -384,8 +382,9 @@ static int diffie_hellman_sha1(LIBSSH2_SESSION *session,
             goto clean_exit;
         }
 
-        _libssh2_bn_from_bin(exchange_state->f, exchange_state->f_value_len,
-                             exchange_state->f_value);
+        exchange_state->f = _libssh2_bn_new_from_bin(
+            exchange_state->f_value_len,
+            exchange_state->f_value);
 
         if(_libssh2_get_string(&buf, &(exchange_state->h_sig),
                                &(exchange_state->h_sig_len))) {
@@ -835,12 +834,10 @@ static int diffie_hellman_sha256(LIBSSH2_SESSION *session,
         exchange_state->s_packet = NULL;
         exchange_state->k_value = NULL;
         exchange_state->ctx = _libssh2_bn_ctx_new();
-        libssh2_dh_init(&exchange_state->x);
-        exchange_state->e = _libssh2_bn_init(); /* g^x mod p */
-        exchange_state->f = _libssh2_bn_init_from_bin(); /* g^(Random from
-                                                            server) mod p */
-        exchange_state->k = _libssh2_bn_init(); /* The shared secret: f^x mod
-                                                   p */
+        _libssh2_dh_init(&exchange_state->x);
+        exchange_state->e = _libssh2_bn_new();  /* g^x mod p */
+        exchange_state->f = NULL;               /* g^(server random) mod p */
+        exchange_state->k = _libssh2_bn_new();  /* f^x mod p: shared secret */
 
         /* Zero the whole thing out */
         memset(&exchange_state->req_state, 0, sizeof(packet_require_state_t));
@@ -1074,8 +1071,9 @@ static int diffie_hellman_sha256(LIBSSH2_SESSION *session,
             goto clean_exit;
         }
 
-        _libssh2_bn_from_bin(exchange_state->f, exchange_state->f_value_len,
-                             exchange_state->f_value);
+        exchange_state->f = _libssh2_bn_new_from_bin(
+            exchange_state->f_value_len,
+            exchange_state->f_value);
 
         if(_libssh2_get_string(&buf, &(exchange_state->h_sig),
                                &(exchange_state->h_sig_len))) {
@@ -1528,14 +1526,13 @@ kex_method_diffie_hellman_group1_sha1_key_exchange(LIBSSH2_SESSION *session,
     int ret;
 
     if(key_state->state == libssh2_NB_state_idle) {
-        /* g == 2 */
-        key_state->p = _libssh2_bn_init_from_bin(); /* SSH2 defined value
-                                                       (p_value) */
-        key_state->g = _libssh2_bn_init();      /* SSH2 defined value (2) */
-
         /* Initialize P and G */
+        /* SSH2 defined value (2) */
+        /* g == 2 */
+        key_state->g = _libssh2_bn_new();
         _libssh2_bn_set_word(key_state->g, 2);
-        _libssh2_bn_from_bin(key_state->p, 128, p_value);
+        /* SSH2 defined value (p_value) */
+        key_state->p = _libssh2_bn_new_from_bin(128, p_value);
 
         _libssh2_debug(session, LIBSSH2_TRACE_KEX,
                        "Initiating Diffie-Hellman Group1 Key Exchange");
@@ -1605,14 +1602,13 @@ kex_method_diffie_hellman_group14_sha1_key_exchange(LIBSSH2_SESSION *session,
     int ret;
 
     if(key_state->state == libssh2_NB_state_idle) {
-        key_state->p = _libssh2_bn_init_from_bin(); /* SSH2 defined value
-                                                       (p_value) */
-        key_state->g = _libssh2_bn_init();      /* SSH2 defined value (2) */
-
-        /* g == 2 */
         /* Initialize P and G */
+        /* SSH2 defined value (2) */
+        /* g == 2 */
+        key_state->g = _libssh2_bn_new();
         _libssh2_bn_set_word(key_state->g, 2);
-        _libssh2_bn_from_bin(key_state->p, 256, p_value);
+        /* SSH2 defined value (p_value) */
+        key_state->p = _libssh2_bn_new_from_bin(256, p_value);
 
         _libssh2_debug(session, LIBSSH2_TRACE_KEX,
                        "Initiating Diffie-Hellman Group14 Key Exchange");
@@ -1649,8 +1645,8 @@ kex_method_diffie_hellman_group_exchange_sha1_key_exchange
     int rc;
 
     if(key_state->state == libssh2_NB_state_idle) {
-        key_state->p = _libssh2_bn_init_from_bin();
-        key_state->g = _libssh2_bn_init_from_bin();
+        key_state->p = NULL;
+        key_state->g = NULL;
         /* Ask for a P and G pair */
 #ifdef LIBSSH2_DH_GEX_NEW
         key_state->request[0] = SSH_MSG_KEX_DH_GEX_REQUEST;
@@ -1733,8 +1729,10 @@ kex_method_diffie_hellman_group_exchange_sha1_key_exchange
             goto dh_gex_clean_exit;
         }
 
-        _libssh2_bn_from_bin(key_state->p, p_len, p);
-        _libssh2_bn_from_bin(key_state->g, g_len, g);
+        _libssh2_bn_free(key_state->p);
+        _libssh2_bn_free(key_state->g);
+        key_state->p = _libssh2_bn_new_from_bin(p_len, p);
+        key_state->g = _libssh2_bn_new_from_bin(g_len, g);
 
         ret = diffie_hellman_sha1(session, key_state->g, key_state->p, p_len,
                                   SSH_MSG_KEX_DH_GEX_INIT,
@@ -1773,8 +1771,8 @@ kex_method_diffie_hellman_group_exchange_sha256_key_exchange
     int rc;
 
     if(key_state->state == libssh2_NB_state_idle) {
-        key_state->p = _libssh2_bn_init();
-        key_state->g = _libssh2_bn_init();
+        key_state->p = NULL;
+        key_state->g = NULL;
         /* Ask for a P and G pair */
 #ifdef LIBSSH2_DH_GEX_NEW
         key_state->request[0] = SSH_MSG_KEX_DH_GEX_REQUEST;
@@ -1858,8 +1856,10 @@ kex_method_diffie_hellman_group_exchange_sha256_key_exchange
             goto dh_gex_clean_exit;
         }
 
-        _libssh2_bn_from_bin(key_state->p, p_len, p);
-        _libssh2_bn_from_bin(key_state->g, g_len, g);
+        _libssh2_bn_free(key_state->p);
+        _libssh2_bn_free(key_state->g);
+        key_state->p = _libssh2_bn_new_from_bin(p_len, p);
+        key_state->g = _libssh2_bn_new_from_bin(g_len, g);
 
         ret = diffie_hellman_sha256(session, key_state->g, key_state->p, p_len,
                                     SSH_MSG_KEX_DH_GEX_INIT,
@@ -2049,7 +2049,7 @@ static int ecdh_sha2_nistp(LIBSSH2_SESSION *session, libssh2_curve_type type,
     if(exchange_state->state == libssh2_NB_state_idle) {
 
         /* Setup initial values */
-        exchange_state->k = _libssh2_bn_init();
+        exchange_state->k = _libssh2_bn_new();
 
         exchange_state->state = libssh2_NB_state_created;
     }
@@ -2663,7 +2663,7 @@ curve25519_sha256(LIBSSH2_SESSION *session, unsigned char *data,
     if(exchange_state->state == libssh2_NB_state_idle) {
 
         /* Setup initial values */
-        exchange_state->k = _libssh2_bn_init();
+        exchange_state->k = _libssh2_bn_new();
 
         exchange_state->state = libssh2_NB_state_created;
     }

src/libgcrypt.c

@@ -42,6 +42,14 @@
 
 #include <string.h>
 
+_libssh2_bn *_libssh2_bn_new_from_bin_(size_t len, const void *val)
+{
+    _libssh2_bn *bn;
+    if(gcry_mpi_scan(&bn, GCRYMPI_FMT_USG, val, len, NULL) < 0)
+        return NULL;
+    return bn;
+}
+
 int
 _libssh2_rsa_new(libssh2_rsa_ctx ** rsa,
                  const unsigned char *edata,

src/libgcrypt.h

@@ -203,12 +203,11 @@
 #define _libssh2_bn_ctx int
 #define _libssh2_bn_ctx_new() 0
 #define _libssh2_bn_ctx_free(bnctx) ((void)0)
-#define _libssh2_bn_init() gcry_mpi_new(0)
-#define _libssh2_bn_init_from_bin() NULL /* because gcry_mpi_scan() creates a
-                                            new bignum */
+#define _libssh2_bn_new() gcry_mpi_new(0)
+    _libssh2_bn *_libssh2_bn_new_from_bin_(size_t len, const void *val);
+#define _libssh2_bn_new_from_bin(len, val) \
+    _libssh2_bn_new_from_bin_(len, val)
 #define _libssh2_bn_set_word(bn, val) gcry_mpi_set_ui(bn, val)
-#define _libssh2_bn_from_bin(bn, len, val)                      \
-    gcry_mpi_scan(&((bn)), GCRYMPI_FMT_USG, val, len, NULL)
 #define _libssh2_bn_to_bin(bn, val)                                     \
     gcry_mpi_print(GCRYMPI_FMT_USG, val, _libssh2_bn_bytes(bn), NULL, bn)
 #define _libssh2_bn_bytes(bn)                                           \

src/mbedtls.c

@@ -247,6 +247,21 @@ _libssh2_mbedtls_bignum_init(void)
     return bignum;
 }
 
+_libssh2_bn *
+_libssh2_mbedtls_bignum_init_from_bin(size_t len, const void *bin)
+{
+    _libssh2_bn *bn = _libssh2_bn_new();
+    if(bn == NULL)
+        return NULL;
+
+    if(mbedtls_mpi_read_binary(bn, bin, len) < 0) {
+        _libssh2_bn_free(bn);
+        return NULL;
+    }
+
+    return bn;
+}
+
 void
 _libssh2_mbedtls_bignum_free(_libssh2_bn *bn)
 {

src/mbedtls.h

@@ -293,14 +293,12 @@
 
 #define _libssh2_bn mbedtls_mpi
 
-#define _libssh2_bn_init() \
-  _libssh2_mbedtls_bignum_init()
-#define _libssh2_bn_init_from_bin() \
+#define _libssh2_bn_new() \
   _libssh2_mbedtls_bignum_init()
+#define _libssh2_bn_new_from_bin(len, val) \
+  _libssh2_mbedtls_bignum_init_from_bin(len, val)
 #define _libssh2_bn_set_word(bn, word) \
   mbedtls_mpi_lset(bn, word)
-#define _libssh2_bn_from_bin(bn, len, bin) \
-  mbedtls_mpi_read_binary(bn, bin, len)
 #define _libssh2_bn_to_bin(bn, bin) \
   mbedtls_mpi_write_binary(bn, bin, mbedtls_mpi_size(bn))
 #define _libssh2_bn_bytes(bn) \
@@ -367,6 +365,9 @@ _libssh2_mbedtls_hash(const unsigned char *data, unsigned long datalen,
 _libssh2_bn *
 _libssh2_mbedtls_bignum_init(void);
 
+_libssh2_bn *
+_libssh2_mbedtls_bignum_init_from_bin(size_t len, const void *bin);
+
 void
 _libssh2_mbedtls_bignum_free(_libssh2_bn *bn);
 

src/openssl.h

@@ -378,10 +378,9 @@ typedef struct {
 #define _libssh2_bn_ctx BN_CTX
 #define _libssh2_bn_ctx_new() BN_CTX_new()
 #define _libssh2_bn_ctx_free(bnctx) BN_CTX_free(bnctx)
-#define _libssh2_bn_init() BN_new()
-#define _libssh2_bn_init_from_bin() _libssh2_bn_init()
+#define _libssh2_bn_new() BN_new()
+#define _libssh2_bn_new_from_bin(val, len) BN_bin2bn(val, len, NULL)
 #define _libssh2_bn_set_word(bn, val) BN_set_word(bn, val)
-#define _libssh2_bn_from_bin(bn, len, val) BN_bin2bn(val, len, bn)
 #define _libssh2_bn_to_bin(bn, val) BN_bn2bin(bn, val)
 #define _libssh2_bn_bytes(bn) BN_num_bytes(bn)
 #define _libssh2_bn_bits(bn) BN_num_bits(bn)

src/wincng.c

@@ -1865,7 +1865,7 @@ _libssh2_wincng_cipher_dtor(_libssh2_cipher_ctx *ctx)
  */
 
 _libssh2_bn *
-_libssh2_wincng_bignum_init(void)
+_libssh2_wincng_bignum_new(void)
 {
     _libssh2_bn *bignum;
 
@@ -2070,18 +2070,19 @@ _libssh2_wincng_bignum_bits(const _libssh2_bn *bn)
     return bits;
 }
 
-void
-_libssh2_wincng_bignum_from_bin(_libssh2_bn *bn, unsigned long len,
-                                const unsigned char *bin)
+_libssh2_bn *
+_libssh2_wincng_bignum_new_from_bin(unsigned long len,
+                                const void *bin)
 {
+    _libssh2_bn *bn = _libssh2_bn_new();
     unsigned char *bignum;
     unsigned long offset, length, bits;
 
     if(!bn || !bin || !len)
-        return;
+        return NULL;
 
     if(_libssh2_wincng_bignum_resize(bn, len))
-        return;
+        return NULL;
 
     memcpy(bn->bignum, bin, len);
 
@@ -2103,6 +2104,7 @@ _libssh2_wincng_bignum_from_bin(_libssh2_bn *bn, unsigned long len,
             bn->length = length;
         }
     }
+    return bn;
 }
 
 void
@@ -2134,7 +2136,7 @@ _libssh2_wincng_bignum_free(_libssh2_bn *bn)
 void
 _libssh2_dh_init(_libssh2_dh_ctx *dhctx)
 {
-    *dhctx = _libssh2_wincng_bignum_init();     /* Random from client */
+    *dhctx = _libssh2_wincng_bignum_new();     /* Random from client */
 }
 
 int

src/wincng.h

@@ -363,16 +363,16 @@ struct _libssh2_wincng_bignum {
  * Windows CNG backend: BigNumber functions
  */
 
-_libssh2_bn *_libssh2_wincng_bignum_init(void);
-
-#define _libssh2_bn_init() \
-  _libssh2_wincng_bignum_init()
-#define _libssh2_bn_init_from_bin() \
-  _libssh2_bn_init()
+_libssh2_bn *_libssh2_wincng_bignum_new(void);
+_libssh2_bn *_libssh2_wincng_bignum_new_from_bin(unsigned long len,
+    const void *val);
+
+#define _libssh2_bn_new() \
+  _libssh2_wincng_bignum_new()
+#define _libssh2_bn_new_from_bin(len, val) \
+  _libssh2_wincng_bignum_new_from_bin(len, val)
 #define _libssh2_bn_set_word(bn, word) \
   _libssh2_wincng_bignum_set_word(bn, word)
-#define _libssh2_bn_from_bin(bn, len, bin) \
-  _libssh2_wincng_bignum_from_bin(bn, len, bin)
 #define _libssh2_bn_to_bin(bn, bin) \
   _libssh2_wincng_bignum_to_bin(bn, bin)
 #define _libssh2_bn_bytes(bn) bn->length
@@ -547,7 +547,7 @@ void
 _libssh2_wincng_cipher_dtor(_libssh2_cipher_ctx *ctx);
 
 _libssh2_bn *
-_libssh2_wincng_bignum_init(void);
+_libssh2_wincng_bignum_new(void);
 int
 _libssh2_wincng_bignum_set_word(_libssh2_bn *bn, unsigned long word);
 unsigned long