From aaa3d4fc7d7685748c11f35a74eac49faf40220d Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Fri, 17 Mar 2023 14:05:01 +0100 Subject: [PATCH] CVE-2023-1667:dh: Expose the callback cleanup functions These will be helpful when we already sent the first key exchange packet, but we found out that our guess was wrong and we need to initiate different key exchange method with different callbacks. Signed-off-by: Jakub Jelen Reviewed-by: Norbert Pocs Reviewed-by: Andrew Bartlett --- include/libssh/curve25519.h | 1 + include/libssh/dh-gex.h | 1 + include/libssh/dh.h | 1 + include/libssh/ecdh.h | 1 + src/curve25519.c | 7 ++++++- src/dh-gex.c | 7 ++++++- src/dh.c | 7 ++++++- src/ecdh.c | 7 ++++++- src/kex.c | 38 +++++++++++++++++++++++++++++++++++++ 9 files changed, 66 insertions(+), 4 deletions(-) diff --git a/include/libssh/curve25519.h b/include/libssh/curve25519.h index f0cc6348..77e6c313 100644 --- a/include/libssh/curve25519.h +++ b/include/libssh/curve25519.h @@ -48,6 +48,7 @@ typedef unsigned char ssh_curve25519_privkey[CURVE25519_PRIVKEY_SIZE]; int ssh_client_curve25519_init(ssh_session session); +void ssh_client_curve25519_remove_callbacks(ssh_session session); #ifdef WITH_SERVER void ssh_server_curve25519_init(ssh_session session); diff --git a/include/libssh/dh-gex.h b/include/libssh/dh-gex.h index 4fc23d82..7a91d7d4 100644 --- a/include/libssh/dh-gex.h +++ b/include/libssh/dh-gex.h @@ -24,6 +24,7 @@ #define SRC_DH_GEX_H_ int ssh_client_dhgex_init(ssh_session session); +void ssh_client_dhgex_remove_callbacks(ssh_session session); #ifdef WITH_SERVER void ssh_server_dhgex_init(ssh_session session); diff --git a/include/libssh/dh.h b/include/libssh/dh.h index 390b30da..57f37cdb 100644 --- a/include/libssh/dh.h +++ b/include/libssh/dh.h @@ -65,6 +65,7 @@ int ssh_dh_get_next_server_publickey_blob(ssh_session session, ssh_string *pubkey_blob); int ssh_client_dh_init(ssh_session session); +void ssh_client_dh_remove_callbacks(ssh_session session); #ifdef WITH_SERVER void ssh_server_dh_init(ssh_session session); #endif /* WITH_SERVER */ diff --git a/include/libssh/ecdh.h b/include/libssh/ecdh.h index 17fe02e7..c1f03a9c 100644 --- a/include/libssh/ecdh.h +++ b/include/libssh/ecdh.h @@ -45,6 +45,7 @@ extern struct ssh_packet_callbacks_struct ssh_ecdh_client_callbacks; /* Backend-specific functions. */ int ssh_client_ecdh_init(ssh_session session); +void ssh_client_ecdh_remove_callbacks(ssh_session session); int ecdh_build_k(ssh_session session); #ifdef WITH_SERVER diff --git a/src/curve25519.c b/src/curve25519.c index d2517551..37654438 100644 --- a/src/curve25519.c +++ b/src/curve25519.c @@ -172,6 +172,11 @@ int ssh_client_curve25519_init(ssh_session session) return rc; } +void ssh_client_curve25519_remove_callbacks(ssh_session session) +{ + ssh_packet_remove_callbacks(session, &ssh_curve25519_client_callbacks); +} + static int ssh_curve25519_build_k(ssh_session session) { ssh_curve25519_pubkey k; @@ -285,7 +290,7 @@ static SSH_PACKET_CALLBACK(ssh_packet_client_curve25519_reply){ (void)type; (void)user; - ssh_packet_remove_callbacks(session, &ssh_curve25519_client_callbacks); + ssh_client_curve25519_remove_callbacks(session); pubkey_blob = ssh_buffer_get_ssh_string(packet); if (pubkey_blob == NULL) { diff --git a/src/dh-gex.c b/src/dh-gex.c index 88a97140..4a298542 100644 --- a/src/dh-gex.c +++ b/src/dh-gex.c @@ -238,6 +238,11 @@ error: return SSH_PACKET_USED; } +void ssh_client_dhgex_remove_callbacks(ssh_session session) +{ + ssh_packet_remove_callbacks(session, &ssh_dhgex_client_callbacks); +} + static SSH_PACKET_CALLBACK(ssh_packet_client_dhgex_reply) { struct ssh_crypto_struct *crypto=session->next_crypto; @@ -248,7 +253,7 @@ static SSH_PACKET_CALLBACK(ssh_packet_client_dhgex_reply) (void)user; SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_KEX_DH_GEX_REPLY received"); - ssh_packet_remove_callbacks(session, &ssh_dhgex_client_callbacks); + ssh_client_dhgex_remove_callbacks(session); rc = ssh_buffer_unpack(packet, "SBS", &pubkey_blob, &server_pubkey, diff --git a/src/dh.c b/src/dh.c index 18b71734..c265efcb 100644 --- a/src/dh.c +++ b/src/dh.c @@ -342,6 +342,11 @@ error: return SSH_ERROR; } +void ssh_client_dh_remove_callbacks(ssh_session session) +{ + ssh_packet_remove_callbacks(session, &ssh_dh_client_callbacks); +} + SSH_PACKET_CALLBACK(ssh_packet_client_dh_reply){ struct ssh_crypto_struct *crypto=session->next_crypto; ssh_string pubkey_blob = NULL; @@ -351,7 +356,7 @@ SSH_PACKET_CALLBACK(ssh_packet_client_dh_reply){ (void)type; (void)user; - ssh_packet_remove_callbacks(session, &ssh_dh_client_callbacks); + ssh_client_dh_remove_callbacks(session); rc = ssh_buffer_unpack(packet, "SBS", &pubkey_blob, &server_pubkey, &crypto->dh_server_signature); diff --git a/src/ecdh.c b/src/ecdh.c index a4c07ccb..e5b11ba9 100644 --- a/src/ecdh.c +++ b/src/ecdh.c @@ -43,6 +43,11 @@ struct ssh_packet_callbacks_struct ssh_ecdh_client_callbacks = { .user = NULL }; +void ssh_client_ecdh_remove_callbacks(ssh_session session) +{ + ssh_packet_remove_callbacks(session, &ssh_ecdh_client_callbacks); +} + /** @internal * @brief parses a SSH_MSG_KEX_ECDH_REPLY packet and sends back * a SSH_MSG_NEWKEYS @@ -55,7 +60,7 @@ SSH_PACKET_CALLBACK(ssh_packet_client_ecdh_reply){ (void)type; (void)user; - ssh_packet_remove_callbacks(session, &ssh_ecdh_client_callbacks); + ssh_client_ecdh_remove_callbacks(session); pubkey_blob = ssh_buffer_get_ssh_string(packet); if (pubkey_blob == NULL) { ssh_set_error(session,SSH_FATAL, "No public key in packet"); diff --git a/src/kex.c b/src/kex.c index 794388ca..6962743d 100644 --- a/src/kex.c +++ b/src/kex.c @@ -793,6 +793,44 @@ kex_select_kex_type(const char *kex) return 0; } + +/** @internal + * @brief Reverts guessed callbacks set during the dh_handshake() + * @param session session handle + * @returns void + */ +static void revert_kex_callbacks(ssh_session session) +{ + switch (session->next_crypto->kex_type) { + case SSH_KEX_DH_GROUP1_SHA1: + case SSH_KEX_DH_GROUP14_SHA1: + case SSH_KEX_DH_GROUP14_SHA256: + case SSH_KEX_DH_GROUP16_SHA512: + case SSH_KEX_DH_GROUP18_SHA512: + ssh_client_dh_remove_callbacks(session); + break; +#ifdef WITH_GEX + case SSH_KEX_DH_GEX_SHA1: + case SSH_KEX_DH_GEX_SHA256: + ssh_client_dhgex_remove_callbacks(session); + break; +#endif /* WITH_GEX */ +#ifdef HAVE_ECDH + case SSH_KEX_ECDH_SHA2_NISTP256: + case SSH_KEX_ECDH_SHA2_NISTP384: + case SSH_KEX_ECDH_SHA2_NISTP521: + ssh_client_ecdh_remove_callbacks(session); + break; +#endif +#ifdef HAVE_CURVE25519 + case SSH_KEX_CURVE25519_SHA256: + case SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG: + ssh_client_curve25519_remove_callbacks(session); + break; +#endif + } +} + /** @brief Select the different methods on basis of client's and * server's kex messages, and watches out if a match is possible. */