From 4cef5e965a46e9271aed62631b152e4bd23c1e3c Mon Sep 17 00:00:00 2001 From: Aris Adamantiadis Date: Tue, 12 Dec 2023 23:09:57 +0100 Subject: [PATCH] CVE-2023-48795: client side mitigation Signed-off-by: Aris Adamantiadis Signed-off-by: Jakub Jelen Reviewed-by: Andreas Schneider Upstream-Status: Backport [https://gitlab.com/libssh/libssh-mirror/-/commit/4cef5e965a46e9271aed62631b152e4bd23c1e3c] CVE: CVE-2023-48795 Signed-off-by: Vijay Anusuri --- include/libssh/packet.h | 1 + include/libssh/session.h | 6 +++++ src/curve25519.c | 18 +++---------- src/dh.c | 6 +---- src/ecdh.c | 7 +---- src/ecdh_crypto.c | 10 ++----- src/ecdh_gcrypt.c | 10 +++---- src/ecdh_mbedcrypto.c | 11 +++----- src/kex.c | 34 ++++++++++++++++++++---- src/packet.c | 56 +++++++++++++++++++++++++++++++++++++++- src/packet_cb.c | 12 +++++++++ 11 files changed, 118 insertions(+), 53 deletions(-) diff --git a/include/libssh/packet.h b/include/libssh/packet.h index fbe09700..8800e16b 100644 --- a/include/libssh/packet.h +++ b/include/libssh/packet.h @@ -63,6 +63,7 @@ SSH_PACKET_CALLBACK(ssh_packet_ext_info); SSH_PACKET_CALLBACK(ssh_packet_kexdh_init); #endif +int ssh_packet_send_newkeys(ssh_session session); int ssh_packet_send_unimplemented(ssh_session session, uint32_t seqnum); int ssh_packet_parse_type(ssh_session session); //int packet_flush(ssh_session session, int enforce_blocking); diff --git a/include/libssh/session.h b/include/libssh/session.h index 23633cc2..b8810f54 100644 --- a/include/libssh/session.h +++ b/include/libssh/session.h @@ -69,6 +69,12 @@ enum ssh_pending_call_e { /* Client successfully authenticated */ #define SSH_SESSION_FLAG_AUTHENTICATED 2 +/* The current SSH2 session implements the "strict KEX" feature and should behave + * differently on SSH2_MSG_NEWKEYS. */ +#define SSH_SESSION_FLAG_KEX_STRICT 0x0010 +/* Unexpected packets have been sent while the session was still unencrypted */ +#define SSH_SESSION_FLAG_KEX_TAINTED 0x0020 + /* codes to use with ssh_handle_packets*() */ /* Infinite timeout */ #define SSH_TIMEOUT_INFINITE -1 diff --git a/src/curve25519.c b/src/curve25519.c index 167209f4..6eda5feb 100644 --- a/src/curve25519.c +++ b/src/curve25519.c @@ -166,12 +166,7 @@ int ssh_client_curve25519_reply(ssh_session session, ssh_buffer packet){ } /* Send the MSG_NEWKEYS */ - if (ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS) < 0) { - goto error; - } - - rc=ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); + rc = ssh_packet_send_newkeys(session); return rc; error: return SSH_ERROR; @@ -297,15 +292,10 @@ int ssh_server_curve25519_init(ssh_session session, ssh_buffer packet){ return SSH_ERROR; } - /* Send the MSG_NEWKEYS */ - rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS); - if (rc < 0) { - goto error; - } - session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; - rc = ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); + + /* Send the MSG_NEWKEYS */ + rc = ssh_packet_send_newkeys(session); return rc; error: diff --git a/src/dh.c b/src/dh.c index cc12fd46..33883f2d 100644 --- a/src/dh.c +++ b/src/dh.c @@ -735,11 +735,7 @@ int ssh_client_dh_reply(ssh_session session, ssh_buffer packet){ } /* Send the MSG_NEWKEYS */ - if (ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS) < 0) { - goto error; - } - - rc=ssh_packet_send(session); + rc = ssh_packet_send_newkeys(session); SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); return rc; error: diff --git a/src/ecdh.c b/src/ecdh.c index f7fcaf13..1fef7ec9 100644 --- a/src/ecdh.c +++ b/src/ecdh.c @@ -72,12 +72,7 @@ int ssh_client_ecdh_reply(ssh_session session, ssh_buffer packet){ } /* Send the MSG_NEWKEYS */ - if (ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS) < 0) { - goto error; - } - - rc=ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); + rc = ssh_packet_send_newkeys(session); return rc; error: return SSH_ERROR; diff --git a/src/ecdh_crypto.c b/src/ecdh_crypto.c index 24f21c03..7e5f0cc7 100644 --- a/src/ecdh_crypto.c +++ b/src/ecdh_crypto.c @@ -318,15 +318,9 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet){ return SSH_ERROR; } - /* Send the MSG_NEWKEYS */ - rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS); - if (rc < 0) { - return SSH_ERROR;; - } - session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; - rc = ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); + /* Send the MSG_NEWKEYS */ + rc = ssh_packet_send_newkeys(session); return rc; } diff --git a/src/ecdh_gcrypt.c b/src/ecdh_gcrypt.c index e43cacea..c1db7f5d 100644 --- a/src/ecdh_gcrypt.c +++ b/src/ecdh_gcrypt.c @@ -362,17 +362,13 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet) { goto out; } - + session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; /* Send the MSG_NEWKEYS */ - rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS); - if (rc != SSH_OK) { + rc = ssh_packet_send_newkeys(session); + if (rc == SSH_ERROR) { goto out; } - session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; - rc = ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); - out: gcry_sexp_release(param); gcry_sexp_release(key); diff --git a/src/ecdh_mbedcrypto.c b/src/ecdh_mbedcrypto.c index fa350028..24924508 100644 --- a/src/ecdh_mbedcrypto.c +++ b/src/ecdh_mbedcrypto.c @@ -293,16 +293,13 @@ int ssh_server_ecdh_init(ssh_session session, ssh_buffer packet) goto out; } - rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS); - if (rc < 0) { - rc = SSH_ERROR; + session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; + /* Send the MSG_NEWKEYS */ + rc = ssh_packet_send_newkeys(session); + if (rc == SSH_ERROR) { goto out; } - session->dh_handshake_state = DH_STATE_NEWKEYS_SENT; - rc = ssh_packet_send(session); - SSH_LOG(SSH_LOG_PROTOCOL, "SSH_MSG_NEWKEYS sent"); - out: mbedtls_ecp_group_free(&grp); return rc; diff --git a/src/kex.c b/src/kex.c index 82686e4b..7f1bb324 100644 --- a/src/kex.c +++ b/src/kex.c @@ -105,6 +105,9 @@ /* RFC 8308 */ #define KEX_EXTENSION_CLIENT "ext-info-c" +/* Strict kex mitigation against CVE-2023-48795 */ +#define KEX_STRICT_CLIENT "kex-strict-c-v00@openssh.com" +#define KEX_STRICT_SERVER "kex-strict-s-v00@openssh.com" /* NOTE: This is a fixed API and the index is defined by ssh_kex_types_e */ static const char *default_methods[] = { @@ -521,6 +524,27 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){ goto error; } + /* + * handle the "strict KEX" feature. If supported by peer, then set up the + * flag and verify packet sequence numbers. + */ + if (server_kex) { + ok = ssh_match_group(session->next_crypto->client_kex.methods[SSH_KEX], + KEX_STRICT_CLIENT); + if (ok) { + SSH_LOG(SSH_LOG_DEBUG, "Client supports strict kex, enabling."); + session->flags |= SSH_SESSION_FLAG_KEX_STRICT; + } + } else { + /* client kex */ + ok = ssh_match_group(session->next_crypto->server_kex.methods[SSH_KEX], + KEX_STRICT_SERVER); + if (ok) { + SSH_LOG(SSH_LOG_DEBUG, "Server supports strict kex, enabling."); + session->flags |= SSH_SESSION_FLAG_KEX_STRICT; + } + } + /* * If client sent a ext-info-c message in the kex list, it supports * RFC 8308 extension negotiation. @@ -778,21 +802,21 @@ int ssh_set_client_kex(ssh_session session) return SSH_OK; } - /* Here we append ext-info-c to the list of kex algorithms */ + /* Here we append ext-info-c and kex-strict-c-v00@openssh.com to the list of kex algorithms */ kex = client->methods[SSH_KEX]; len = strlen(kex); - if (len + strlen(KEX_EXTENSION_CLIENT) + 2 < len) { + /* Comma, comma, nul byte */ + kex_len = len + 1 + strlen(KEX_EXTENSION_CLIENT) + 1 + strlen(KEX_STRICT_CLIENT ) + 1; + if (kex_len >= MAX_PACKET_LEN) { /* Overflow */ return SSH_ERROR; } - kex_len = len + strlen(KEX_EXTENSION_CLIENT) + 2; /* comma, NULL */ kex_tmp = realloc(kex, kex_len); if (kex_tmp == NULL) { - free(kex); ssh_set_error_oom(session); return SSH_ERROR; } - snprintf(kex_tmp + len, kex_len - len, ",%s", KEX_EXTENSION_CLIENT); + snprintf(kex_tmp + len, kex_len - len, ",%s,%s", KEX_EXTENSION_CLIENT, KEX_STRICT_CLIENT); client->methods[SSH_KEX] = kex_tmp; return SSH_OK; diff --git a/src/packet.c b/src/packet.c index 61a44237..8025a7ff 100644 --- a/src/packet.c +++ b/src/packet.c @@ -1126,6 +1126,19 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user) } #endif /* WITH_ZLIB */ payloadsize = ssh_buffer_get_len(session->in_buffer); + if (session->recv_seq == UINT32_MAX) { + /* Overflowing sequence numbers is always fishy */ + if (session->current_crypto == NULL) { + /* don't allow sequence number overflow when unencrypted */ + ssh_set_error(session, + SSH_FATAL, + "Incoming sequence number overflow"); + goto error; + } else { + SSH_LOG(SSH_LOG_WARNING, + "Incoming sequence number overflow"); + } + } session->recv_seq++; if (session->raw_counter != NULL) { session->raw_counter->in_bytes += payloadsize; @@ -1141,7 +1154,19 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user) SSH_LOG(SSH_LOG_PACKET, "packet: read type %hhd [len=%d,padding=%hhd,comp=%d,payload=%d]", session->in_packet.type, packet_len, padding, compsize, payloadsize); - + if (session->current_crypto == NULL) { + /* In strict kex, only a few packets are allowed. Taint the session + * if we received packets that are normally allowed but to be + * refused if we are in strict kex when KEX is over. + */ + uint8_t type = session->in_packet.type; + + if (type != SSH2_MSG_KEXINIT && type != SSH2_MSG_NEWKEYS && + (type < SSH2_MSG_KEXDH_INIT || + type > SSH2_MSG_KEX_DH_GEX_REQUEST)) { + session->flags |= SSH_SESSION_FLAG_KEX_TAINTED; + } + } /* Check if the packet is expected */ filter_result = ssh_packet_incoming_filter(session); @@ -1153,6 +1178,9 @@ int ssh_packet_socket_callback(const void *data, size_t receivedlen, void *user) case SSH_PACKET_DENIED: goto error; case SSH_PACKET_UNKNOWN: + if (session->current_crypto == NULL) { + session->flags |= SSH_SESSION_FLAG_KEX_TAINTED; + } ssh_packet_send_unimplemented(session, session->recv_seq - 1); break; } @@ -1276,9 +1304,35 @@ void ssh_packet_process(ssh_session session, uint8_t type){ if(r==SSH_PACKET_NOT_USED){ SSH_LOG(SSH_LOG_RARE,"Couldn't do anything with packet type %d",type); ssh_packet_send_unimplemented(session, session->recv_seq-1); + if (session->current_crypto == NULL) { + session->flags |= SSH_SESSION_FLAG_KEX_TAINTED; + } } } +/** @internal + * @brief sends a SSH_MSG_NEWKEYS when enabling the new negotiated ciphers + * @param session the SSH session + * @return SSH_ERROR on error, else SSH_OK + */ +int ssh_packet_send_newkeys(ssh_session session) +{ + int rc; + + /* Send the MSG_NEWKEYS */ + rc = ssh_buffer_add_u8(session->out_buffer, SSH2_MSG_NEWKEYS); + if (rc < 0) { + return rc; + } + + rc = ssh_packet_send(session); + if (rc == SSH_ERROR) { + return rc; + } + SSH_LOG(SSH_LOG_DEBUG, "SSH_MSG_NEWKEYS sent"); + return rc; +} + /** @internal * @brief sends a SSH_MSG_UNIMPLEMENTED answer to an unhandled packet * @param session the SSH session diff --git a/src/packet_cb.c b/src/packet_cb.c index 6aa64766..de03fb07 100644 --- a/src/packet_cb.c +++ b/src/packet_cb.c @@ -154,6 +154,18 @@ SSH_PACKET_CALLBACK(ssh_packet_newkeys){ goto error; } + if (session->flags & SSH_SESSION_FLAG_KEX_STRICT) { + /* reset packet sequence number when running in strict kex mode */ + session->recv_seq = 0; + /* Check that we aren't tainted */ + if (session->flags & SSH_SESSION_FLAG_KEX_TAINTED) { + ssh_set_error(session, + SSH_FATAL, + "Received unexpected packets in strict KEX mode."); + goto error; + } +} + if(session->server){ /* server things are done in server.c */ session->dh_handshake_state=DH_STATE_FINISHED; -- 2.25.1