ecdh: enable ecdh_sha2_nistp{384,521} kex methods

Summary:
Based on Dirkjan's original patch series here:

 * https://www.libssh.org/archive/libssh/2015-08/0000029.html

Here the changes are adapted for the current master
branch, and expanded to include libgcrypt support.

Co-Authored-By: Dirkjan Bussink <d.bussink@gmail.com>
Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>

Test Plan:
 * Ran pkd tests for libcrypto and libgcrypt builds.
 * Ran client torture_algorithms.c tests for libcrypto and libgcrypt builds.
 * Tested across multiple libgcrypts ("1.6.3" and "1.7.6-beta").

Reviewers: aris, asn

Tags: #libssh

Differential Revision: https://bugs.libssh.org/D7

include/libssh/crypto.h

@@ -55,6 +55,10 @@ enum ssh_key_exchange_e {
   SSH_KEX_DH_GROUP14_SHA1,
   /* ecdh-sha2-nistp256 */
   SSH_KEX_ECDH_SHA2_NISTP256,
+  /* ecdh-sha2-nistp384 */
+  SSH_KEX_ECDH_SHA2_NISTP384,
+  /* ecdh-sha2-nistp521 */
+  SSH_KEX_ECDH_SHA2_NISTP521,
   /* curve25519-sha256@libssh.org */
   SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG
 };

src/client.c

@@ -260,6 +260,8 @@ static int dh_handshake(ssh_session session) {
           break;
 #ifdef HAVE_ECDH
         case SSH_KEX_ECDH_SHA2_NISTP256:
+        case SSH_KEX_ECDH_SHA2_NISTP384:
+        case SSH_KEX_ECDH_SHA2_NISTP521:
           rc = ssh_client_ecdh_init(session);
           break;
 #endif

src/dh.c

@@ -608,7 +608,9 @@ int ssh_make_sessionid(ssh_session session) {
         }
 
 #ifdef HAVE_ECDH
-    } else if (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256) {
+    } else if ((session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256) ||
+               (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP384) ||
+               (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP521)) {
         if (session->next_crypto->ecdh_client_pubkey == NULL ||
             session->next_crypto->ecdh_server_pubkey == NULL) {
             SSH_LOG(SSH_LOG_WARNING, "ECDH parameted missing");
@@ -670,6 +672,28 @@ int ssh_make_sessionid(ssh_session session) {
         sha256(ssh_buffer_get(buf), ssh_buffer_get_len(buf),
                                      session->next_crypto->secret_hash);
         break;
+    case SSH_KEX_ECDH_SHA2_NISTP384:
+        session->next_crypto->digest_len = SHA384_DIGEST_LENGTH;
+        session->next_crypto->mac_type = SSH_MAC_SHA384;
+        session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len);
+        if (session->next_crypto->secret_hash == NULL) {
+            ssh_set_error_oom(session);
+            goto error;
+        }
+        sha384(ssh_buffer_get(buf), ssh_buffer_get_len(buf),
+                                     session->next_crypto->secret_hash);
+        break;
+    case SSH_KEX_ECDH_SHA2_NISTP521:
+        session->next_crypto->digest_len = SHA512_DIGEST_LENGTH;
+        session->next_crypto->mac_type = SSH_MAC_SHA512;
+        session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len);
+        if (session->next_crypto->secret_hash == NULL) {
+            ssh_set_error_oom(session);
+            goto error;
+        }
+        sha512(ssh_buffer_get(buf), ssh_buffer_get_len(buf),
+                                     session->next_crypto->secret_hash);
+        break;
     }
     /* During the first kex, secret hash and session ID are equal. However, after
      * a key re-exchange, a new secret hash is calculated. This hash will not replace

src/ecdh_crypto.c

@@ -35,6 +35,20 @@
 #define NISTP384 NID_secp384r1
 #define NISTP521 NID_secp521r1
 
+/** @internal
+ * @brief Map the given key exchange enum value to its curve name.
+ */
+static int ecdh_kex_type_to_curve(enum ssh_key_exchange_e kex_type) {
+    if (kex_type == SSH_KEX_ECDH_SHA2_NISTP256) {
+        return NISTP256;
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP384) {
+        return NISTP384;
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP521) {
+        return NISTP521;
+    }
+    return SSH_ERROR;
+}
+
 /** @internal
  * @brief Starts ecdh-sha2-nistp256 key exchange
  */
@@ -43,6 +57,7 @@ int ssh_client_ecdh_init(ssh_session session){
   const EC_GROUP *group;
   const EC_POINT *pubkey;
   ssh_string client_pubkey;
+  int curve;
   int len;
   int rc;
   bignum_CTX ctx = BN_CTX_new();
@@ -53,7 +68,13 @@ int ssh_client_ecdh_init(ssh_session session){
       return SSH_ERROR;
   }
 
-  key = EC_KEY_new_by_curve_name(NISTP256);
+  curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
+  if (curve == SSH_ERROR) {
+      BN_CTX_free(ctx);
+      return SSH_ERROR;
+  }
+
+  key = EC_KEY_new_by_curve_name(curve);
   if (key == NULL) {
       BN_CTX_free(ctx);
       return SSH_ERROR;
@@ -185,6 +206,7 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
     /* SSH host keys (rsa,dsa,ecdsa) */
     ssh_key privkey;
     ssh_string sig_blob = NULL;
+    int curve;
     int len;
     int rc;
 
@@ -199,7 +221,14 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){
     /* Build server's keypair */
 
     ctx = BN_CTX_new();
-    ecdh_key = EC_KEY_new_by_curve_name(NISTP256);
+
+    curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
+    if (curve == SSH_ERROR) {
+        BN_CTX_free(ctx);
+        return SSH_ERROR;
+    }
+
+    ecdh_key = EC_KEY_new_by_curve_name(curve);
     if (ecdh_key == NULL) {
         ssh_set_error_oom(session);
         BN_CTX_free(ctx);

src/ecdh_gcrypt.c

@@ -34,7 +34,21 @@
 #include <gcrypt.h>
 
 /** @internal
- * @brief Starts ecdh-sha2-nistp256 key exchange
+ * @brief Map the given key exchange enum value to its curve name.
+ */
+static const char *ecdh_kex_type_to_curve(enum ssh_key_exchange_e kex_type) {
+    if (kex_type == SSH_KEX_ECDH_SHA2_NISTP256) {
+        return "NIST P-256";
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP384) {
+        return "NIST P-384";
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP521) {
+        return "NIST P-521";
+    }
+    return NULL;
+}
+
+/** @internal
+ * @brief Starts ecdh-sha2-nistp{256,384,521} key exchange.
  */
 int ssh_client_ecdh_init(ssh_session session)
 {
@@ -43,6 +57,13 @@ int ssh_client_ecdh_init(ssh_session session)
     ssh_string client_pubkey = NULL;
     gcry_sexp_t param = NULL;
     gcry_sexp_t key = NULL;
+    const char *curve = NULL;
+
+    curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
+    if (curve == NULL) {
+        rc = SSH_ERROR;
+        goto out;
+    }
 
     rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
     if (rc < 0) {
@@ -53,7 +74,7 @@ int ssh_client_ecdh_init(ssh_session session)
     err = gcry_sexp_build(&param,
                           NULL,
                           "(genkey(ecdh(curve %s)))",
-                          "NIST P-256");
+                          curve);
     if (err) {
         rc = SSH_ERROR;
         goto out;
@@ -105,12 +126,20 @@ int ecdh_build_k(ssh_session session)
     gcry_mpi_t s = NULL;
     gcry_mpi_point_t point;
 #else
+    size_t k_len = 0;
+    enum ssh_key_exchange_e kex_type = session->next_crypto->kex_type;
     ssh_string s;
 #endif
     ssh_string pubkey_raw;
     gcry_sexp_t pubkey = NULL;
     ssh_string privkey = NULL;
     int rc = SSH_ERROR;
+    const char *curve = NULL;
+
+    curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
+    if (curve == NULL) {
+        goto out;
+    }
 
     pubkey_raw = session->server
         ? session->next_crypto->ecdh_client_pubkey
@@ -119,7 +148,7 @@ int ecdh_build_k(ssh_session session)
     err = gcry_sexp_build(&pubkey,
                           NULL,
                           "(key-data(public-key(ecdh(curve %s)(q %b))))",
-                          "NIST P-256",
+                          curve,
                           ssh_string_len(pubkey_raw),
                           ssh_string_data(pubkey_raw));
     if (err) {
@@ -173,7 +202,19 @@ int ecdh_build_k(ssh_session session)
         goto out;
     }
 
-    if (ssh_string_len(s) != 65) {
+    if (kex_type == SSH_KEX_ECDH_SHA2_NISTP256) {
+        k_len = 65;
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP384) {
+        k_len = 97;
+    } else if (kex_type == SSH_KEX_ECDH_SHA2_NISTP521) {
+        k_len = 133;
+    } else {
+        ssh_string_burn(s);
+        ssh_string_free(s);
+        goto out;
+    }
+
+    if (ssh_string_len(s) != k_length) {
         ssh_string_burn(s);
         ssh_string_free(s);
         goto out;
@@ -182,7 +223,7 @@ int ecdh_build_k(ssh_session session)
     err = gcry_mpi_scan(&session->next_crypto->k,
                         GCRYMPI_FMT_USG,
                         (const char *)ssh_string_data(s) + 1,
-                        32,
+                        k_length / 2,
                         NULL);
     ssh_string_burn(s);
     ssh_string_free(s);
@@ -228,6 +269,12 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet) {
     ssh_key privkey;
     ssh_string sig_blob = NULL;
     int rc = SSH_ERROR;
+    const char *curve = NULL;
+
+    curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
+    if (curve == NULL) {
+        goto out;
+    }
 
     /* Extract the client pubkey from the init packet */
     q_c_string = ssh_buffer_get_ssh_string(packet);
@@ -239,7 +286,7 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet) {
 
     /* Build server's keypair */
     err = gcry_sexp_build(&param, NULL, "(genkey(ecdh(curve %s)))",
-                          "NIST P-256");
+                          curve);
     if (err) {
         goto out;
     }

src/kex.c

@@ -78,7 +78,7 @@
 #endif
 
 #ifdef HAVE_ECDH
-#define ECDH "ecdh-sha2-nistp256,"
+#define ECDH "ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,"
 #define HOSTKEYS "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-rsa,ssh-dss"
 #else
 #define HOSTKEYS "ssh-ed25519,ssh-rsa,ssh-dss"
@@ -589,6 +589,10 @@ int ssh_kex_select_methods (ssh_session session){
       session->next_crypto->kex_type=SSH_KEX_DH_GROUP14_SHA1;
     } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp256") == 0){
       session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP256;
+    } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp384") == 0){
+      session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP384;
+    } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "ecdh-sha2-nistp521") == 0){
+      session->next_crypto->kex_type=SSH_KEX_ECDH_SHA2_NISTP521;
     } else if(strcmp(session->next_crypto->kex_methods[SSH_KEX], "curve25519-sha256@libssh.org") == 0){
       session->next_crypto->kex_type=SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG;
     }

src/packet_cb.c

@@ -110,6 +110,8 @@ SSH_PACKET_CALLBACK(ssh_packet_dh_reply){
       break;
 #ifdef HAVE_ECDH
     case SSH_KEX_ECDH_SHA2_NISTP256:
+    case SSH_KEX_ECDH_SHA2_NISTP384:
+    case SSH_KEX_ECDH_SHA2_NISTP521:
       rc = ssh_client_ecdh_reply(session, packet);
       break;
 #endif

src/server.c

@@ -198,6 +198,8 @@ SSH_PACKET_CALLBACK(ssh_packet_kexdh_init){
         break;
   #ifdef HAVE_ECDH
       case SSH_KEX_ECDH_SHA2_NISTP256:
+      case SSH_KEX_ECDH_SHA2_NISTP384:
+      case SSH_KEX_ECDH_SHA2_NISTP521:
         rc = ssh_server_ecdh_init(session, packet);
         break;
   #endif

src/session.c

@@ -357,6 +357,10 @@ const char* ssh_get_kex_algo(ssh_session session) {
             return "diffie-hellman-group14-sha1";
         case SSH_KEX_ECDH_SHA2_NISTP256:
             return "ecdh-sha2-nistp256";
+        case SSH_KEX_ECDH_SHA2_NISTP384:
+            return "ecdh-sha2-nistp384";
+        case SSH_KEX_ECDH_SHA2_NISTP521:
+            return "ecdh-sha2-nistp521";
         case SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG:
             return "curve25519-sha256@libssh.org";
         default:

tests/client/torture_algorithms.c

@@ -325,7 +325,7 @@ static void torture_algorithms_zlib_openssh(void **state) {
     ssh_disconnect(session);
 }
 
-#if defined(HAVE_LIBCRYPTO) && defined(HAVE_ECC)
+#if defined(HAVE_ECC)
 static void torture_algorithms_ecdh_sha2_nistp256(void **state) {
     struct torture_state *s = *state;
     ssh_session session = s->ssh.session;
@@ -344,6 +344,44 @@ static void torture_algorithms_ecdh_sha2_nistp256(void **state) {
 
     ssh_disconnect(session);
 }
+
+static void torture_algorithms_ecdh_sha2_nistp384(void **state) {
+    struct torture_state *s = *state;
+    ssh_session session = s->ssh.session;
+    int rc;
+
+    rc = ssh_options_set(session, SSH_OPTIONS_KEY_EXCHANGE, "ecdh-sha2-nistp384");
+    assert_int_equal(rc, SSH_OK);
+
+    rc = ssh_connect(session);
+    assert_int_equal(rc, SSH_OK);
+    rc = ssh_userauth_none(session, NULL);
+    if (rc != SSH_OK) {
+      rc = ssh_get_error_code(session);
+      assert_int_equal(rc, SSH_REQUEST_DENIED);
+    }
+
+    ssh_disconnect(session);
+}
+
+static void torture_algorithms_ecdh_sha2_nistp521(void **state) {
+    struct torture_state *s = *state;
+    ssh_session session = s->ssh.session;
+    int rc;
+
+    rc = ssh_options_set(session, SSH_OPTIONS_KEY_EXCHANGE, "ecdh-sha2-nistp521");
+    assert_int_equal(rc, SSH_OK);
+
+    rc = ssh_connect(session);
+    assert_int_equal(rc, SSH_OK);
+    rc = ssh_userauth_none(session, NULL);
+    if (rc != SSH_OK) {
+      rc = ssh_get_error_code(session);
+      assert_int_equal(rc, SSH_REQUEST_DENIED);
+    }
+
+    ssh_disconnect(session);
+}
 #endif
 
 static void torture_algorithms_dh_group1(void **state) {
@@ -448,10 +486,16 @@ int torture_run_tests(void) {
         cmocka_unit_test_setup_teardown(torture_algorithms_dh_group1,
                                         session_setup,
                                         session_teardown),
-#if defined(HAVE_LIBCRYPTO) && defined(HAVE_ECC)
+#if defined(HAVE_ECC)
         cmocka_unit_test_setup_teardown(torture_algorithms_ecdh_sha2_nistp256,
                                         session_setup,
                                         session_teardown),
+        cmocka_unit_test_setup_teardown(torture_algorithms_ecdh_sha2_nistp384,
+                                        session_setup,
+                                        session_teardown),
+        cmocka_unit_test_setup_teardown(torture_algorithms_ecdh_sha2_nistp521,
+                                        session_setup,
+                                        session_teardown),
 #endif
     };
 

tests/pkd/pkd_hello.c

@@ -190,22 +190,32 @@ static int torture_pkd_setup_ecdsa_521(void **state) {
     /* Kex algorithms. */ \
     f(client, rsa_curve25519_sha256,                  kexcmd("curve25519-sha256@libssh.org"),  setup_rsa,        teardown) \
     f(client, rsa_ecdh_sha2_nistp256,                 kexcmd("ecdh-sha2-nistp256 "),           setup_rsa,        teardown) \
+    f(client, rsa_ecdh_sha2_nistp384,                 kexcmd("ecdh-sha2-nistp384 "),           setup_rsa,        teardown) \
+    f(client, rsa_ecdh_sha2_nistp521,                 kexcmd("ecdh-sha2-nistp521 "),           setup_rsa,        teardown) \
     f(client, rsa_diffie_hellman_group14_sha1,        kexcmd("diffie-hellman-group14-sha1"),   setup_rsa,        teardown) \
     f(client, rsa_diffie_hellman_group1_sha1,         kexcmd("diffie-hellman-group1-sha1"),    setup_rsa,        teardown) \
     f(client, dsa_curve25519_sha256,                  kexcmd("curve25519-sha256@libssh.org"),  setup_dsa,        teardown) \
     f(client, dsa_ecdh_sha2_nistp256,                 kexcmd("ecdh-sha2-nistp256 "),           setup_dsa,        teardown) \
+    f(client, dsa_ecdh_sha2_nistp384,                 kexcmd("ecdh-sha2-nistp384 "),           setup_dsa,        teardown) \
+    f(client, dsa_ecdh_sha2_nistp521,                 kexcmd("ecdh-sha2-nistp521 "),           setup_dsa,        teardown) \
     f(client, dsa_diffie_hellman_group14_sha1,        kexcmd("diffie-hellman-group14-sha1"),   setup_dsa,        teardown) \
     f(client, dsa_diffie_hellman_group1_sha1,         kexcmd("diffie-hellman-group1-sha1"),    setup_dsa,        teardown) \
     f(client, ecdsa_256_curve25519_sha256,            kexcmd("curve25519-sha256@libssh.org"),  setup_ecdsa_256,  teardown) \
     f(client, ecdsa_256_ecdh_sha2_nistp256,           kexcmd("ecdh-sha2-nistp256 "),           setup_ecdsa_256,  teardown) \
+    f(client, ecdsa_256_ecdh_sha2_nistp384,           kexcmd("ecdh-sha2-nistp384 "),           setup_ecdsa_256,  teardown) \
+    f(client, ecdsa_256_ecdh_sha2_nistp521,           kexcmd("ecdh-sha2-nistp521 "),           setup_ecdsa_256,  teardown) \
     f(client, ecdsa_256_diffie_hellman_group14_sha1,  kexcmd("diffie-hellman-group14-sha1"),   setup_ecdsa_256,  teardown) \
     f(client, ecdsa_256_diffie_hellman_group1_sha1,   kexcmd("diffie-hellman-group1-sha1"),    setup_ecdsa_256,  teardown) \
     f(client, ecdsa_384_curve25519_sha256,            kexcmd("curve25519-sha256@libssh.org"),  setup_ecdsa_384,  teardown) \
     f(client, ecdsa_384_ecdh_sha2_nistp256,           kexcmd("ecdh-sha2-nistp256 "),           setup_ecdsa_384,  teardown) \
+    f(client, ecdsa_384_ecdh_sha2_nistp384,           kexcmd("ecdh-sha2-nistp384 "),           setup_ecdsa_384,  teardown) \
+    f(client, ecdsa_384_ecdh_sha2_nistp521,           kexcmd("ecdh-sha2-nistp521 "),           setup_ecdsa_384,  teardown) \
     f(client, ecdsa_384_diffie_hellman_group14_sha1,  kexcmd("diffie-hellman-group14-sha1"),   setup_ecdsa_384,  teardown) \
     f(client, ecdsa_384_diffie_hellman_group1_sha1,   kexcmd("diffie-hellman-group1-sha1"),    setup_ecdsa_384,  teardown) \
     f(client, ecdsa_521_curve25519_sha256,            kexcmd("curve25519-sha256@libssh.org"),  setup_ecdsa_521,  teardown) \
     f(client, ecdsa_521_ecdh_sha2_nistp256,           kexcmd("ecdh-sha2-nistp256 "),           setup_ecdsa_521,  teardown) \
+    f(client, ecdsa_521_ecdh_sha2_nistp384,           kexcmd("ecdh-sha2-nistp384 "),           setup_ecdsa_521,  teardown) \
+    f(client, ecdsa_521_ecdh_sha2_nistp521,           kexcmd("ecdh-sha2-nistp521 "),           setup_ecdsa_521,  teardown) \
     f(client, ecdsa_521_diffie_hellman_group14_sha1,  kexcmd("diffie-hellman-group14-sha1"),   setup_ecdsa_521,  teardown) \
     f(client, ecdsa_521_diffie_hellman_group1_sha1,   kexcmd("diffie-hellman-group1-sha1"),    setup_ecdsa_521,  teardown)