dh: generalize group usage

4 files changed, 86 insertions, 16 deletions

diff --git a/include/libssh/crypto.h b/include/libssh/crypto.h
index e370c748..770995d0 100644
--- a/include/libssh/crypto.h
+++ b/include/libssh/crypto.h
@@ -75,6 +75,9 @@ enum ssh_cipher_e {
 
 struct ssh_crypto_struct {
     bignum e,f,x,k,y;
+    bignum g, p;
+    int dh_group_is_mutable; /* do free group parameters */
+    int dh_group_bits; /* size of p in bits */
 #ifdef HAVE_ECDH
     EC_KEY *ecdh_privkey;
     ssh_string ecdh_client_pubkey;
diff --git a/include/libssh/dh.h b/include/libssh/dh.h
index 101ceecd..04816068 100644
--- a/include/libssh/dh.h
+++ b/include/libssh/dh.h
@@ -33,4 +33,9 @@ int ssh_client_dh_init(ssh_session session);
 #ifdef WITH_SERVER
 void ssh_server_dh_init(ssh_session session);
 #endif /* WITH_SERVER */
+
+int ssh_dh_init_common(ssh_session session);
+void ssh_dh_cleanup(ssh_session session);
+int ssh_dh_generate_secret(ssh_session session, bignum dest);
+
 #endif /* DH_H_ */
diff --git a/include/libssh/libcrypto.h b/include/libssh/libcrypto.h
index 428f0495..407ae708 100644
--- a/include/libssh/libcrypto.h
+++ b/include/libssh/libcrypto.h
@@ -82,16 +82,19 @@ typedef BN_CTX* bignum_CTX;
 #define bignum_bn2dec(num) BN_bn2dec(num)
 #define bignum_dec2bn(bn,data) BN_dec2bn(data,bn)
 #define bignum_bn2hex(num, dest) (*dest)=(unsigned char *)BN_bn2hex(num)
-#define bignum_rand(rnd, bits) BN_rand(rnd,bits,0,1)
+#define bignum_rand_range(rnd, max) BN_rand_range(rnd, max)
 #define bignum_ctx_new() BN_CTX_new()
 #define bignum_ctx_free(num) BN_CTX_free(num)
 #define bignum_ctx_invalid(ctx) ((ctx) == NULL)
 #define bignum_mod_exp(dest,generator,exp,modulo,ctx) BN_mod_exp(dest,generator,exp,modulo,ctx)
+#define bignum_add(dest, a, b) BN_add(dest, a, b)
+#define bignum_sub(dest, a, b) BN_sub(dest, a, b)
 #define bignum_num_bytes(num) BN_num_bytes(num)
 #define bignum_num_bits(num) BN_num_bits(num)
 #define bignum_is_bit_set(num,bit) BN_is_bit_set(num,bit)
 #define bignum_bn2bin(num,len, ptr) BN_bn2bin(num,ptr)
 #define bignum_cmp(num1,num2) BN_cmp(num1,num2)
+#define bignum_rshift1(dest, src) BN_rshift1(dest, src)
 
 #endif /* HAVE_LIBCRYPTO */
 
diff --git a/src/dh.c b/src/dh.c
index a6d10263..b553e3ee 100644
--- a/src/dh.c
+++ b/src/dh.c
@@ -117,10 +117,6 @@ static bignum p_group1;
 static bignum p_group14;
 static int dh_crypto_initialized;
 
-static bignum select_p(enum ssh_key_exchange_e type) {
-    return type == SSH_KEX_DH_GROUP14_SHA1 ? p_group14 : p_group1;
-}
-
 /**
  * @internal
  * @brief Initialize global constants used in DH key agreement
@@ -166,13 +162,25 @@ void ssh_dh_finalize(void) {
  * @internal
  * @brief allocate and initialize ephemeral values used in dh kex
  */
-static int ssh_dh_init_common(ssh_session session){
+int ssh_dh_init_common(ssh_session session){
     struct ssh_crypto_struct *crypto=session->next_crypto;
     crypto->x = bignum_new();
     crypto->y = bignum_new();
     crypto->e = bignum_new();
     crypto->f = bignum_new();
     crypto->k = bignum_new();
+    if (session->next_crypto->kex_type == SSH_KEX_DH_GROUP1_SHA1){
+        session->next_crypto->p = p_group1;
+        session->next_crypto->dh_group_bits = 1024;
+        session->next_crypto->g = g;
+        session->next_crypto->dh_group_is_mutable = 0;
+    } else if (session->next_crypto->kex_type == SSH_KEX_DH_GROUP14_SHA1){
+        session->next_crypto->p = p_group14;
+        session->next_crypto->dh_group_bits = 2048;
+        session->next_crypto->g = g;
+        session->next_crypto->dh_group_is_mutable = 0;
+    }
+
     if (crypto->x == NULL || crypto->y == NULL || crypto->e == NULL ||
             crypto->f == NULL || crypto->k == NULL){
         return SSH_ERROR;
@@ -181,16 +189,22 @@ static int ssh_dh_init_common(ssh_session session){
     }
 }
 
-static void ssh_dh_cleanup(ssh_session session){
+void ssh_dh_cleanup(ssh_session session){
     struct ssh_crypto_struct *crypto=session->next_crypto;
     bignum_safe_free(crypto->x);
     bignum_safe_free(crypto->y);
     bignum_safe_free(crypto->e);
     bignum_safe_free(crypto->f);
+    if (crypto->dh_group_is_mutable){
+        bignum_safe_free(crypto->p);
+        bignum_safe_free(crypto->g);
+    }
 }
 
 #ifdef DEBUG_CRYPTO
 static void ssh_dh_debug(ssh_session session){
+    ssh_print_bignum("p", session->next_crypto->p);
+    ssh_print_bignum("g", session->next_crypto->g);
     ssh_print_bignum("x", session->next_crypto->x);
     ssh_print_bignum("y", session->next_crypto->y);
     ssh_print_bignum("e", session->next_crypto->e);
@@ -206,6 +220,46 @@ static void ssh_dh_debug(ssh_session session){
 #define ssh_dh_debug(session)
 #endif
 
+/** @internal
+ * @brief generate a secret DH parameter between 1 and (p-1)/2
+ * @param[out] dest preallocated bignum where to store the parameter
+ * @return SSH_OK on success, SSH_ERROR on error
+ */
+int ssh_dh_generate_secret(ssh_session session, bignum dest){
+    bignum one = NULL, p_half=NULL;
+    int rc = 0;
+
+    one = bignum_new();
+    p_half = bignum_new();
+    if (one == NULL || p_half == NULL){
+        goto error;
+    }
+
+    /* creating a random between [0;(p-1)/2 -1[ */
+    rc = bignum_set_word(one, 1);
+    if (rc == 1) {
+        /* p_half = (p-1)/2. p is prime so p>>1 == p-1 >> 1 */
+        rc = bignum_rshift1(p_half, session->next_crypto->p);
+    }
+    if (rc == 1)
+        rc = bignum_sub(p_half, p_half, one);
+    if (rc == 1)
+        rc = bignum_rand_range(dest, p_half);
+    if (rc == 1)
+        bignum_add(dest, dest, one);
+    error:
+    if (rc != 1){
+        ssh_set_error_oom(session);
+    }
+    bignum_safe_free(one);
+    bignum_safe_free(p_half);
+    if (rc == 1){
+        return SSH_OK;
+    } else {
+        return SSH_ERROR;
+    }
+}
+
 static int dh_build_k(ssh_session session) {
   bignum_CTX ctx = bignum_ctx_new();
   if (bignum_ctx_invalid(ctx)) {
@@ -215,10 +269,10 @@ static int dh_build_k(ssh_session session) {
   /* the server and clients don't use the same numbers */
   if (session->client) {
     bignum_mod_exp(session->next_crypto->k, session->next_crypto->f,
-        session->next_crypto->x, select_p(session->next_crypto->kex_type), ctx);
+        session->next_crypto->x, session->next_crypto->p, ctx);
   } else {
     bignum_mod_exp(session->next_crypto->k, session->next_crypto->e,
-        session->next_crypto->y, select_p(session->next_crypto->kex_type), ctx);
+        session->next_crypto->y, session->next_crypto->p, ctx);
   }
   bignum_ctx_free(ctx);
   ssh_dh_debug();
@@ -253,10 +307,12 @@ int ssh_client_dh_init(ssh_session session){
   if (rc == SSH_ERROR){
     goto error;
   }
-  bignum_rand(session->next_crypto->x, 128);
-
-  bignum_mod_exp(session->next_crypto->e, g, session->next_crypto->x,
-      select_p(session->next_crypto->kex_type), ctx);
+  rc = ssh_dh_generate_secret(session, session->next_crypto->x);
+  if (rc == SSH_ERROR){
+      goto error;
+  }
+  bignum_mod_exp(session->next_crypto->e, session->next_crypto->g, session->next_crypto->x,
+      session->next_crypto->p, ctx);
 
   bignum_ctx_free(ctx);
 
@@ -363,10 +419,13 @@ static SSH_PACKET_CALLBACK(ssh_packet_server_dh_init){
         goto error;
     }
 
-    bignum_rand(session->next_crypto->y, 128);
+    rc = ssh_dh_generate_secret(session, session->next_crypto->y);
+    if (rc == SSH_ERROR){
+        goto error;
+    }
 
-    bignum_mod_exp(session->next_crypto->f, g, session->next_crypto->y,
-            select_p(session->next_crypto->kex_type), ctx);
+    bignum_mod_exp(session->next_crypto->f, session->next_crypto->g, session->next_crypto->y,
+            session->next_crypto->p, ctx);
     bignum_ctx_free(ctx);
     ctx = NULL;