ecdh: Factor out keypair generation

This adds a new internal API function (ssh_ecdh_init),
similar to how it's done in curve25519 implementation.
The new function can be used in hybrid key exchange
constructions.

Signed-off-by: Pavol Žáčik <pzacik@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>

include/libssh/ecdh.h

@@ -48,6 +48,7 @@ extern "C" {
 
 extern struct ssh_packet_callbacks_struct ssh_ecdh_client_callbacks;
 /* Backend-specific functions.  */
+int ssh_ecdh_init(ssh_session session);
 int ssh_client_ecdh_init(ssh_session session);
 void ssh_client_ecdh_remove_callbacks(ssh_session session);
 int ecdh_build_k(ssh_session session);

src/ecdh_crypto.c

@@ -206,12 +206,36 @@ static ssh_string ssh_ecdh_generate(ssh_session session)
     return pubkey_string;
 }
 
+/** @internal
+ * @brief Set up a nistp{256,384,521} key pair for ECDH key exchange.
+ */
+int ssh_ecdh_init(ssh_session session)
+{
+    ssh_string pubkey = NULL;
+    ssh_string *pubkey_loc = NULL;
+
+    pubkey = ssh_ecdh_generate(session);
+    if (pubkey == NULL) {
+        return SSH_ERROR;
+    }
+
+    if (session->server) {
+        pubkey_loc = &session->next_crypto->ecdh_server_pubkey;
+    } else {
+        pubkey_loc = &session->next_crypto->ecdh_client_pubkey;
+    }
+
+    ssh_string_free(*pubkey_loc);
+    *pubkey_loc = pubkey;
+
+    return SSH_OK;
+}
+
 /** @internal
  * @brief Starts ecdh-sha2-nistp256 key exchange
  */
 int ssh_client_ecdh_init(ssh_session session)
 {
-    ssh_string client_pubkey = NULL;
     int rc;
 
     rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
@@ -219,19 +243,16 @@ int ssh_client_ecdh_init(ssh_session session)
         return SSH_ERROR;
     }
 
-    client_pubkey = ssh_ecdh_generate(session);
+    rc = ssh_ecdh_init(session);
-    if (client_pubkey == NULL) {
-        return SSH_ERROR;
-    }
-
-    rc = ssh_buffer_add_ssh_string(session->out_buffer, client_pubkey);
     if (rc < 0) {
-        ssh_string_free(client_pubkey);
         return SSH_ERROR;
     }
 
-    ssh_string_free(session->next_crypto->ecdh_client_pubkey);
+    rc = ssh_buffer_add_ssh_string(session->out_buffer,
-    session->next_crypto->ecdh_client_pubkey = client_pubkey;
+                                   session->next_crypto->ecdh_client_pubkey);
+    if (rc < 0) {
+        return SSH_ERROR;
+    }
 
     /* register the packet callbacks */
     ssh_packet_set_callbacks(session, &ssh_ecdh_client_callbacks);
@@ -454,7 +475,6 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init)
 {
     /* ECDH keys */
     ssh_string q_c_string = NULL;
-    ssh_string q_s_string = NULL;
     /* SSH host keys (rsa, ed25519 and ecdsa) */
     ssh_key privkey = NULL;
     enum ssh_digest_e digest = SSH_DIGEST_AUTO;
@@ -475,13 +495,11 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init)
     }
     session->next_crypto->ecdh_client_pubkey = q_c_string;
 
-    q_s_string = ssh_ecdh_generate(session);
+    rc = ssh_ecdh_init(session);
-    if (q_s_string == NULL) {
+    if (rc < 0) {
         goto error;
     }
 
-    session->next_crypto->ecdh_server_pubkey = q_s_string;
-
     /* build k and session_id */
     rc = ecdh_build_k(session);
     if (rc < 0) {
@@ -518,7 +536,7 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init)
                          "bSSS",
                          SSH2_MSG_KEXDH_REPLY,
                          pubkey_blob, /* host's pubkey */
-                         q_s_string, /* ecdh public key */
+                         session->next_crypto->ecdh_server_pubkey, /* ecdh public key */
                          sig_blob); /* signature blob */
 
     SSH_STRING_FREE(sig_blob);

src/ecdh_gcrypt.c

@@ -48,16 +48,26 @@ static const char *ecdh_kex_type_to_curve(enum ssh_key_exchange_e kex_type) {
 }
 
 /** @internal
- * @brief Starts ecdh-sha2-nistp{256,384,521} key exchange.
+ * @brief Set up a nistp{256,384,521} key pair for ECDH key exchange.
  */
-int ssh_client_ecdh_init(ssh_session session)
+int ssh_ecdh_init(ssh_session session)
 {
-    int rc;
+    int rc = SSH_OK;
+    const char *curve = NULL;
+    const char *genstring = NULL;
     gpg_error_t err;
-    ssh_string client_pubkey = NULL;
     gcry_sexp_t param = NULL;
     gcry_sexp_t key = NULL;
-    const char *curve = NULL;
+    ssh_string pubkey = NULL;
+    ssh_string *pubkey_loc = NULL;
+
+    if (session->server) {
+        pubkey_loc = &session->next_crypto->ecdh_server_pubkey;
+        genstring = "(genkey(ecdh(curve %s) (flags transient-key)))";
+    } else {
+        pubkey_loc = &session->next_crypto->ecdh_client_pubkey;
+        genstring = "(genkey(ecdh(curve %s)))";
+    }
 
     curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
     if (curve == NULL) {
@@ -65,16 +75,7 @@ int ssh_client_ecdh_init(ssh_session session)
         goto out;
     }
 
-    rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
+    err = gcry_sexp_build(&param, NULL, genstring, curve);
-    if (rc < 0) {
-        rc = SSH_ERROR;
-        goto out;
-    }
-
-    err = gcry_sexp_build(&param,
-                          NULL,
-                          "(genkey(ecdh(curve %s)))",
-                          curve);
     if (err) {
         rc = SSH_ERROR;
         goto out;
@@ -86,17 +87,8 @@ int ssh_client_ecdh_init(ssh_session session)
         goto out;
     }
 
-    client_pubkey = ssh_sexp_extract_mpi(key,
+    pubkey = ssh_sexp_extract_mpi(key, "q", GCRYMPI_FMT_USG, GCRYMPI_FMT_STD);
-                                         "q",
+    if (pubkey == NULL) {
-                                         GCRYMPI_FMT_USG,
-                                         GCRYMPI_FMT_STD);
-    if (client_pubkey == NULL) {
-        rc = SSH_ERROR;
-        goto out;
-    }
-
-    rc = ssh_buffer_add_ssh_string(session->out_buffer, client_pubkey);
-    if (rc < 0) {
         rc = SSH_ERROR;
         goto out;
     }
@@ -109,20 +101,45 @@ int ssh_client_ecdh_init(ssh_session session)
     session->next_crypto->ecdh_privkey = key;
     key = NULL;
 
-    SSH_STRING_FREE(session->next_crypto->ecdh_client_pubkey);
+    SSH_STRING_FREE(*pubkey_loc);
-    session->next_crypto->ecdh_client_pubkey = client_pubkey;
+    *pubkey_loc = pubkey;
-    client_pubkey = NULL;
+    pubkey = NULL;
+
+out:
+    gcry_sexp_release(param);
+    gcry_sexp_release(key);
+    SSH_STRING_FREE(pubkey);
+    return rc;
+}
+
+/** @internal
+ * @brief Starts ecdh-sha2-nistp{256,384,521} key exchange.
+ */
+int ssh_client_ecdh_init(ssh_session session)
+{
+    int rc;
+
+    rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
+    if (rc < 0) {
+        return SSH_ERROR;
+    }
+
+    rc = ssh_ecdh_init(session);
+    if (rc < 0) {
+        return SSH_ERROR;
+    }
+
+    rc = ssh_buffer_add_ssh_string(session->out_buffer,
+                                   session->next_crypto->ecdh_client_pubkey);
+    if (rc < 0) {
+        return SSH_ERROR;
+    }
 
     /* register the packet callbacks */
     ssh_packet_set_callbacks(session, &ssh_ecdh_client_callbacks);
     session->dh_handshake_state = DH_STATE_INIT_SENT;
 
     rc = ssh_packet_send(session);
-
- out:
-    gcry_sexp_release(param);
-    gcry_sexp_release(key);
-    SSH_STRING_FREE(client_pubkey);
     return rc;
 }
 
@@ -272,27 +289,16 @@ int ecdh_build_k(ssh_session session)
  * SSH_MSG_KEXDH_REPLY
  */
 SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
-    gpg_error_t err;
-    /* ECDH keys */
     ssh_string q_c_string = NULL;
-    ssh_string q_s_string = NULL;
-    gcry_sexp_t param = NULL;
-    gcry_sexp_t key = NULL;
-    /* SSH host keys (rsa, ed25519 and ecdsa) */
     ssh_key privkey = NULL;
     enum ssh_digest_e digest = SSH_DIGEST_AUTO;
     ssh_string sig_blob = NULL;
     ssh_string pubkey_blob = NULL;
     int rc = SSH_ERROR;
-    const char *curve = NULL;
     (void)type;
     (void)user;
 
     ssh_packet_remove_callbacks(session, &ssh_ecdh_server_callbacks);
-    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);
@@ -302,29 +308,12 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
     }
     session->next_crypto->ecdh_client_pubkey = q_c_string;
 
-    /* Build server's key pair */
+    rc = ssh_ecdh_init(session);
-    err = gcry_sexp_build(&param, NULL, "(genkey(ecdh(curve %s) (flags transient-key)))",
+    if (rc != SSH_OK) {
-                          curve);
+        ssh_set_error(session, SSH_FATAL, "Failed to generate a key pair");
-    if (err) {
         goto out;
     }
 
-    err = gcry_pk_genkey(&key, param);
-    if (err)
-        goto out;
-
-    q_s_string = ssh_sexp_extract_mpi(key,
-                                      "q",
-                                      GCRYMPI_FMT_USG,
-                                      GCRYMPI_FMT_STD);
-    if (q_s_string == NULL) {
-        goto out;
-    }
-
-    session->next_crypto->ecdh_privkey = key;
-    key = NULL;
-    session->next_crypto->ecdh_server_pubkey = q_s_string;
-
     /* build k and session_id */
     rc = ecdh_build_k(session);
     if (rc != SSH_OK) {
@@ -362,7 +351,7 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
                          "bSSS",
                          SSH2_MSG_KEXDH_REPLY,
                          pubkey_blob, /* host's pubkey */
-                         q_s_string, /* ecdh public key */
+                         session->next_crypto->ecdh_server_pubkey, /* ecdh public key */
                          sig_blob); /* signature blob */
 
     SSH_STRING_FREE(sig_blob);
@@ -387,8 +376,6 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
     }
 
  out:
-    gcry_sexp_release(param);
-    gcry_sexp_release(key);
     if (rc == SSH_ERROR) {
         ssh_buffer_reinit(session->out_buffer);
         session->session_state = SSH_SESSION_STATE_ERROR;

src/ecdh_mbedcrypto.c

@@ -49,14 +49,22 @@ static mbedtls_ecp_group_id ecdh_kex_type_to_curve(enum ssh_key_exchange_e kex_t
 
     return MBEDTLS_ECP_DP_NONE;
 }
-int ssh_client_ecdh_init(ssh_session session)
+
+int ssh_ecdh_init(ssh_session session)
 {
-    ssh_string client_pubkey = NULL;
-    mbedtls_ecp_group grp;
     int rc;
+    mbedtls_ecp_group grp;
     mbedtls_ecp_group_id curve;
     mbedtls_ctr_drbg_context *ctr_drbg = NULL;
     mbedtls_ecp_keypair *ecdh_privkey = NULL;
+    ssh_string pubkey = NULL;
+    ssh_string *pubkey_loc = NULL;
+
+    if (session->server) {
+        pubkey_loc = &session->next_crypto->ecdh_server_pubkey;
+    } else {
+        pubkey_loc = &session->next_crypto->ecdh_client_pubkey;
+    }
 
     ctr_drbg = ssh_get_mbedtls_ctr_drbg_context();
 
@@ -65,11 +73,6 @@ int ssh_client_ecdh_init(ssh_session session)
         return SSH_ERROR;
     }
 
-    rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
-    if (rc < 0) {
-        return SSH_ERROR;
-    }
-
     /* Free any previously allocated privkey */
     if (session->next_crypto->ecdh_privkey != NULL) {
         mbedtls_ecp_keypair_free(session->next_crypto->ecdh_privkey);
@@ -97,38 +100,52 @@ int ssh_client_ecdh_init(ssh_session session)
                                  &ecdh_privkey->MBEDTLS_PRIVATE(Q),
                                  mbedtls_ctr_drbg_random,
                                  ctr_drbg);
-
     if (rc != 0) {
         rc = SSH_ERROR;
         goto out;
     }
 
-    client_pubkey = make_ecpoint_string(&grp,
+    pubkey = make_ecpoint_string(&grp, &ecdh_privkey->MBEDTLS_PRIVATE(Q));
-            &ecdh_privkey->MBEDTLS_PRIVATE(Q));
+    if (pubkey == NULL) {
-    if (client_pubkey == NULL) {
         rc = SSH_ERROR;
         goto out;
     }
 
-    rc = ssh_buffer_add_ssh_string(session->out_buffer, client_pubkey);
+    SSH_STRING_FREE(*pubkey_loc);
+    *pubkey_loc = pubkey;
+    pubkey = NULL;
+
+out:
+    mbedtls_ecp_group_free(&grp);
+    SSH_STRING_FREE(pubkey);
+    return rc;
+}
+
+int ssh_client_ecdh_init(ssh_session session)
+{
+    int rc;
+
+    rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_KEX_ECDH_INIT);
     if (rc < 0) {
-        rc = SSH_ERROR;
+        return SSH_ERROR;
-        goto out;
     }
 
-    SSH_STRING_FREE(session->next_crypto->ecdh_client_pubkey);
+    rc = ssh_ecdh_init(session);
-    session->next_crypto->ecdh_client_pubkey = client_pubkey;
+    if (rc < 0) {
-    client_pubkey = NULL;
+        return SSH_ERROR;
+    }
+
+    rc = ssh_buffer_add_ssh_string(session->out_buffer,
+                                   session->next_crypto->ecdh_client_pubkey);
+    if (rc < 0) {
+        return SSH_ERROR;
+    }
 
     /* register the packet callbacks */
     ssh_packet_set_callbacks(session, &ssh_ecdh_client_callbacks);
     session->dh_handshake_state = DH_STATE_INIT_SENT;
     rc = ssh_packet_send(session);
 
-out:
-    mbedtls_ecp_group_free(&grp);
-    SSH_STRING_FREE(client_pubkey);
-
     return rc;
 }
 
@@ -204,24 +221,15 @@ out:
 
 SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
     ssh_string q_c_string = NULL;
-    ssh_string q_s_string = NULL;
-    mbedtls_ecp_group grp;
-    mbedtls_ctr_drbg_context *ctr_drbg = NULL;
-    mbedtls_ecp_keypair *ecdh_privkey = NULL;
     ssh_key privkey = NULL;
     enum ssh_digest_e digest = SSH_DIGEST_AUTO;
     ssh_string sig_blob = NULL;
     ssh_string pubkey_blob = NULL;
     int rc;
-    mbedtls_ecp_group_id curve;
     (void)type;
     (void)user;
 
     ssh_packet_remove_callbacks(session, &ssh_ecdh_server_callbacks);
-    curve = ecdh_kex_type_to_curve(session->next_crypto->kex_type);
-    if (curve == MBEDTLS_ECP_DP_NONE) {
-        return SSH_ERROR;
-    }
 
     q_c_string = ssh_buffer_get_ssh_string(packet);
     if (q_c_string == NULL) {
@@ -229,45 +237,13 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
         return SSH_ERROR;
     }
 
-    session->next_crypto->ecdh_privkey = malloc(sizeof(mbedtls_ecp_keypair));
-    if (session->next_crypto->ecdh_privkey == NULL) {
-        ssh_set_error_oom(session);
-        return SSH_ERROR;
-    }
-
-    ecdh_privkey = session->next_crypto->ecdh_privkey;
-
     session->next_crypto->ecdh_client_pubkey = q_c_string;
 
-    ctr_drbg = ssh_get_mbedtls_ctr_drbg_context();
+    rc = ssh_ecdh_init(session);
-
+    if (rc < 0) {
-    mbedtls_ecp_group_init(&grp);
+        return SSH_ERROR;
-    mbedtls_ecp_keypair_init(ecdh_privkey);
-
-    rc = mbedtls_ecp_group_load(&grp, curve);
-    if (rc != 0) {
-        rc = SSH_ERROR;
-        goto out;
     }
 
-    rc = mbedtls_ecp_gen_keypair(&grp,
-                     &ecdh_privkey->MBEDTLS_PRIVATE(d),
-                     &ecdh_privkey->MBEDTLS_PRIVATE(Q),
-                     mbedtls_ctr_drbg_random,
-                     ctr_drbg);
-    if (rc != 0) {
-        rc = SSH_ERROR;
-        goto out;
-    }
-
-    q_s_string = make_ecpoint_string(&grp, &ecdh_privkey->MBEDTLS_PRIVATE(Q));
-    if (q_s_string == NULL) {
-        rc = SSH_ERROR;
-        goto out;
-    }
-
-    session->next_crypto->ecdh_server_pubkey = q_s_string;
-
     /* build k and session_id */
     rc = ecdh_build_k(session);
     if (rc != SSH_OK) {
@@ -306,7 +282,7 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
     rc = ssh_buffer_pack(session->out_buffer, "bSSS",
                          SSH2_MSG_KEXDH_REPLY,
                          pubkey_blob, /* host's pubkey */
-                         q_s_string, /* ecdh public key */
+                         session->next_crypto->ecdh_server_pubkey, /* ecdh public key */
                          sig_blob); /* signature blob */
 
     SSH_STRING_FREE(sig_blob);
@@ -333,7 +309,6 @@ SSH_PACKET_CALLBACK(ssh_packet_server_ecdh_init){
     }
 
 out:
-    mbedtls_ecp_group_free(&grp);
     if (rc == SSH_ERROR) {
         ssh_buffer_reinit(session->out_buffer);
         session->session_state = SSH_SESSION_STATE_ERROR;