From d34d9258b8420b19ec3f97b4cc5bf7aa7d98e35a Mon Sep 17 00:00:00 2001 From: Michael Buckley Date: Thu, 30 Nov 2023 15:08:02 -0800 Subject: [PATCH] src: add 'strict KEX' to fix CVE-2023-48795 "Terrapin Attack" Refs: https://terrapin-attack.com/ https://seclists.org/oss-sec/2023/q4/292 https://osv.dev/list?ecosystem=&q=CVE-2023-48795 https://github.com/advisories/GHSA-45x7-px36-x8w8 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-48795 Fixes #1290 Closes #1291 --- src/kex.c | 63 +++++++++++++++++++++++------------ src/libssh2_priv.h | 18 +++++++--- src/packet.c | 83 +++++++++++++++++++++++++++++++++++++++++++--- src/packet.h | 2 +- src/session.c | 3 ++ src/transport.c | 12 ++++++- 6 files changed, 149 insertions(+), 32 deletions(-) Index: libssh2-1.11.0/src/kex.c =================================================================== --- libssh2-1.11.0.orig/src/kex.c +++ libssh2-1.11.0/src/kex.c @@ -3037,6 +3037,13 @@ kex_method_extension_negotiation = { 0, }; +static const LIBSSH2_KEX_METHOD +kex_method_strict_client_extension = { + "kex-strict-c-v00@openssh.com", + NULL, + 0, +}; + static const LIBSSH2_KEX_METHOD *libssh2_kex_methods[] = { #if LIBSSH2_ED25519 &kex_method_ssh_curve25519_sha256, @@ -3055,6 +3062,7 @@ static const LIBSSH2_KEX_METHOD *libssh2 &kex_method_diffie_helman_group1_sha1, &kex_method_diffie_helman_group_exchange_sha1, &kex_method_extension_negotiation, + &kex_method_strict_client_extension, NULL }; @@ -3307,13 +3315,13 @@ static int kexinit(LIBSSH2_SESSION * ses return 0; } -/* kex_agree_instr +/* _libssh2_kex_agree_instr * Kex specific variant of strstr() * Needle must be preceded by BOL or ',', and followed by ',' or EOL */ -static unsigned char * -kex_agree_instr(unsigned char *haystack, size_t haystack_len, - const unsigned char *needle, size_t needle_len) +unsigned char * +_libssh2_kex_agree_instr(unsigned char *haystack, size_t haystack_len, + const unsigned char *needle, size_t needle_len) { unsigned char *s; unsigned char *end_haystack; @@ -3398,7 +3406,7 @@ static int kex_agree_hostkey(LIBSSH2_SES while(s && *s) { unsigned char *p = (unsigned char *) strchr((char *) s, ','); size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s)); - if(kex_agree_instr(hostkey, hostkey_len, s, method_len)) { + if(_libssh2_kex_agree_instr(hostkey, hostkey_len, s, method_len)) { const LIBSSH2_HOSTKEY_METHOD *method = (const LIBSSH2_HOSTKEY_METHOD *) kex_get_method_by_name((char *) s, method_len, @@ -3432,9 +3440,9 @@ static int kex_agree_hostkey(LIBSSH2_SES } while(hostkeyp && (*hostkeyp) && (*hostkeyp)->name) { - s = kex_agree_instr(hostkey, hostkey_len, - (unsigned char *) (*hostkeyp)->name, - strlen((*hostkeyp)->name)); + s = _libssh2_kex_agree_instr(hostkey, hostkey_len, + (unsigned char *) (*hostkeyp)->name, + strlen((*hostkeyp)->name)); if(s) { /* So far so good, but does it suit our purposes? (Encrypting vs Signing) */ @@ -3468,6 +3476,12 @@ static int kex_agree_kex_hostkey(LIBSSH2 { const LIBSSH2_KEX_METHOD **kexp = libssh2_kex_methods; unsigned char *s; + const unsigned char *strict = + (unsigned char *)"kex-strict-s-v00@openssh.com"; + + if(_libssh2_kex_agree_instr(kex, kex_len, strict, 28)) { + session->kex_strict = 1; + } if(session->kex_prefs) { s = (unsigned char *) session->kex_prefs; @@ -3475,7 +3489,7 @@ static int kex_agree_kex_hostkey(LIBSSH2 while(s && *s) { unsigned char *q, *p = (unsigned char *) strchr((char *) s, ','); size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s)); - q = kex_agree_instr(kex, kex_len, s, method_len); + q = _libssh2_kex_agree_instr(kex, kex_len, s, method_len); if(q) { const LIBSSH2_KEX_METHOD *method = (const LIBSSH2_KEX_METHOD *) kex_get_method_by_name((char *) s, method_len, @@ -3509,9 +3523,9 @@ static int kex_agree_kex_hostkey(LIBSSH2 } while(*kexp && (*kexp)->name) { - s = kex_agree_instr(kex, kex_len, - (unsigned char *) (*kexp)->name, - strlen((*kexp)->name)); + s = _libssh2_kex_agree_instr(kex, kex_len, + (unsigned char *) (*kexp)->name, + strlen((*kexp)->name)); if(s) { /* We've agreed on a key exchange method, * Can we agree on a hostkey that works with this kex? @@ -3555,7 +3569,7 @@ static int kex_agree_crypt(LIBSSH2_SESSI unsigned char *p = (unsigned char *) strchr((char *) s, ','); size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s)); - if(kex_agree_instr(crypt, crypt_len, s, method_len)) { + if(_libssh2_kex_agree_instr(crypt, crypt_len, s, method_len)) { const LIBSSH2_CRYPT_METHOD *method = (const LIBSSH2_CRYPT_METHOD *) kex_get_method_by_name((char *) s, method_len, @@ -3577,9 +3591,9 @@ static int kex_agree_crypt(LIBSSH2_SESSI } while(*cryptp && (*cryptp)->name) { - s = kex_agree_instr(crypt, crypt_len, - (unsigned char *) (*cryptp)->name, - strlen((*cryptp)->name)); + s = _libssh2_kex_agree_instr(crypt, crypt_len, + (unsigned char *) (*cryptp)->name, + strlen((*cryptp)->name)); if(s) { endpoint->crypt = *cryptp; return 0; @@ -3619,7 +3633,7 @@ static int kex_agree_mac(LIBSSH2_SESSION unsigned char *p = (unsigned char *) strchr((char *) s, ','); size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s)); - if(kex_agree_instr(mac, mac_len, s, method_len)) { + if(_libssh2_kex_agree_instr(mac, mac_len, s, method_len)) { const LIBSSH2_MAC_METHOD *method = (const LIBSSH2_MAC_METHOD *) kex_get_method_by_name((char *) s, method_len, (const LIBSSH2_COMMON_METHOD **) @@ -3640,8 +3654,9 @@ static int kex_agree_mac(LIBSSH2_SESSION } while(*macp && (*macp)->name) { - s = kex_agree_instr(mac, mac_len, (unsigned char *) (*macp)->name, - strlen((*macp)->name)); + s = _libssh2_kex_agree_instr(mac, mac_len, + (unsigned char *) (*macp)->name, + strlen((*macp)->name)); if(s) { endpoint->mac = *macp; return 0; @@ -3672,7 +3687,7 @@ static int kex_agree_comp(LIBSSH2_SESSIO unsigned char *p = (unsigned char *) strchr((char *) s, ','); size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s)); - if(kex_agree_instr(comp, comp_len, s, method_len)) { + if(_libssh2_kex_agree_instr(comp, comp_len, s, method_len)) { const LIBSSH2_COMP_METHOD *method = (const LIBSSH2_COMP_METHOD *) kex_get_method_by_name((char *) s, method_len, @@ -3694,8 +3709,9 @@ static int kex_agree_comp(LIBSSH2_SESSIO } while(*compp && (*compp)->name) { - s = kex_agree_instr(comp, comp_len, (unsigned char *) (*compp)->name, - strlen((*compp)->name)); + s = _libssh2_kex_agree_instr(comp, comp_len, + (unsigned char *) (*compp)->name, + strlen((*compp)->name)); if(s) { endpoint->comp = *compp; return 0; @@ -3876,6 +3892,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session->local.kexinit = key_state->oldlocal; session->local.kexinit_len = key_state->oldlocal_len; key_state->state = libssh2_NB_state_idle; + session->state &= ~LIBSSH2_STATE_INITIAL_KEX; session->state &= ~LIBSSH2_STATE_KEX_ACTIVE; session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; return -1; @@ -3901,6 +3918,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session->local.kexinit = key_state->oldlocal; session->local.kexinit_len = key_state->oldlocal_len; key_state->state = libssh2_NB_state_idle; + session->state &= ~LIBSSH2_STATE_INITIAL_KEX; session->state &= ~LIBSSH2_STATE_KEX_ACTIVE; session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; return -1; @@ -3949,6 +3967,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session->remote.kexinit = NULL; } + session->state &= ~LIBSSH2_STATE_INITIAL_KEX; session->state &= ~LIBSSH2_STATE_KEX_ACTIVE; session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; Index: libssh2-1.11.0/src/libssh2_priv.h =================================================================== --- libssh2-1.11.0.orig/src/libssh2_priv.h +++ libssh2-1.11.0/src/libssh2_priv.h @@ -699,6 +699,9 @@ struct _LIBSSH2_SESSION /* key signing algorithm preferences -- NULL yields server order */ char *sign_algo_prefs; + /* Whether to use the OpenSSH Strict KEX extension */ + int kex_strict; + /* (remote as source of data -- packet_read ) */ libssh2_endpoint_data remote; @@ -870,6 +873,7 @@ struct _LIBSSH2_SESSION int fullpacket_macstate; size_t fullpacket_payload_len; int fullpacket_packet_type; + uint32_t fullpacket_required_type; /* State variables used in libssh2_sftp_init() */ libssh2_nonblocking_states sftpInit_state; @@ -910,10 +914,11 @@ struct _LIBSSH2_SESSION }; /* session.state bits */ -#define LIBSSH2_STATE_EXCHANGING_KEYS 0x00000001 -#define LIBSSH2_STATE_NEWKEYS 0x00000002 -#define LIBSSH2_STATE_AUTHENTICATED 0x00000004 -#define LIBSSH2_STATE_KEX_ACTIVE 0x00000008 +#define LIBSSH2_STATE_INITIAL_KEX 0x00000001 +#define LIBSSH2_STATE_EXCHANGING_KEYS 0x00000002 +#define LIBSSH2_STATE_NEWKEYS 0x00000004 +#define LIBSSH2_STATE_AUTHENTICATED 0x00000008 +#define LIBSSH2_STATE_KEX_ACTIVE 0x00000010 /* session.flag helpers */ #ifdef MSG_NOSIGNAL @@ -1144,6 +1149,11 @@ ssize_t _libssh2_send(libssh2_socket_t s int _libssh2_kex_exchange(LIBSSH2_SESSION * session, int reexchange, key_exchange_state_t * state); +unsigned char *_libssh2_kex_agree_instr(unsigned char *haystack, + size_t haystack_len, + const unsigned char *needle, + size_t needle_len); + /* Let crypt.c/hostkey.c expose their method structs */ const LIBSSH2_CRYPT_METHOD **libssh2_crypt_methods(void); const LIBSSH2_HOSTKEY_METHOD **libssh2_hostkey_methods(void); Index: libssh2-1.11.0/src/packet.c =================================================================== --- libssh2-1.11.0.orig/src/packet.c +++ libssh2-1.11.0/src/packet.c @@ -605,14 +605,13 @@ authagent_exit: * layer when it has received a packet. * * The input pointer 'data' is pointing to allocated data that this function - * is asked to deal with so on failure OR success, it must be freed fine. - * The only exception is when the return code is LIBSSH2_ERROR_EAGAIN. + * will be freed unless return the code is LIBSSH2_ERROR_EAGAIN. * * This function will always be called with 'datalen' greater than zero. */ int _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data, - size_t datalen, int macstate) + size_t datalen, int macstate, uint32_t seq) { int rc = 0; unsigned char *message = NULL; @@ -657,6 +656,70 @@ _libssh2_packet_add(LIBSSH2_SESSION * se break; } + if(session->state & LIBSSH2_STATE_INITIAL_KEX) { + if(msg == SSH_MSG_KEXINIT) { + if(!session->kex_strict) { + if(datalen < 17) { + LIBSSH2_FREE(session, data); + session->packAdd_state = libssh2_NB_state_idle; + return _libssh2_error(session, + LIBSSH2_ERROR_BUFFER_TOO_SMALL, + "Data too short extracting kex"); + } + else { + const unsigned char *strict = + (unsigned char *)"kex-strict-s-v00@openssh.com"; + struct string_buf buf; + unsigned char *algs = NULL; + size_t algs_len = 0; + + buf.data = (unsigned char *)data; + buf.dataptr = buf.data; + buf.len = datalen; + buf.dataptr += 17; /* advance past type and cookie */ + + if(_libssh2_get_string(&buf, &algs, &algs_len)) { + LIBSSH2_FREE(session, data); + session->packAdd_state = libssh2_NB_state_idle; + return _libssh2_error(session, + LIBSSH2_ERROR_BUFFER_TOO_SMALL, + "Algs too short"); + } + + if(algs_len == 0 || + _libssh2_kex_agree_instr(algs, algs_len, strict, 28)) { + session->kex_strict = 1; + } + } + } + + if(session->kex_strict && seq) { + LIBSSH2_FREE(session, data); + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; + session->packAdd_state = libssh2_NB_state_idle; + libssh2_session_disconnect(session, "strict KEX violation: " + "KEXINIT was not the first packet"); + + return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT, + "strict KEX violation: " + "KEXINIT was not the first packet"); + } + } + + if(session->kex_strict && session->fullpacket_required_type && + session->fullpacket_required_type != msg) { + LIBSSH2_FREE(session, data); + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; + session->packAdd_state = libssh2_NB_state_idle; + libssh2_session_disconnect(session, "strict KEX violation: " + "unexpected packet type"); + + return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT, + "strict KEX violation: " + "unexpected packet type"); + } + } + if(session->packAdd_state == libssh2_NB_state_allocated) { /* A couple exceptions to the packet adding rule: */ switch(msg) { @@ -1341,6 +1404,15 @@ _libssh2_packet_ask(LIBSSH2_SESSION * se return 0; } + else if(session->kex_strict && + (session->state & LIBSSH2_STATE_INITIAL_KEX)) { + libssh2_session_disconnect(session, "strict KEX violation: " + "unexpected packet type"); + + return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT, + "strict KEX violation: " + "unexpected packet type"); + } packet = _libssh2_list_next(&packet->node); } return -1; @@ -1402,7 +1474,10 @@ _libssh2_packet_require(LIBSSH2_SESSION } while(session->socket_state == LIBSSH2_SOCKET_CONNECTED) { - int ret = _libssh2_transport_read(session); + int ret; + session->fullpacket_required_type = packet_type; + ret = _libssh2_transport_read(session); + session->fullpacket_required_type = 0; if(ret == LIBSSH2_ERROR_EAGAIN) return ret; else if(ret < 0) { Index: libssh2-1.11.0/src/packet.h =================================================================== --- libssh2-1.11.0.orig/src/packet.h +++ libssh2-1.11.0/src/packet.h @@ -71,6 +71,6 @@ int _libssh2_packet_burn(LIBSSH2_SESSION int _libssh2_packet_write(LIBSSH2_SESSION * session, unsigned char *data, unsigned long data_len); int _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data, - size_t datalen, int macstate); + size_t datalen, int macstate, uint32_t seq); #endif /* __LIBSSH2_PACKET_H */ Index: libssh2-1.11.0/src/session.c =================================================================== --- libssh2-1.11.0.orig/src/session.c +++ libssh2-1.11.0/src/session.c @@ -464,6 +464,8 @@ libssh2_session_init_ex(LIBSSH2_ALLOC_FU session->abstract = abstract; session->api_timeout = 0; /* timeout-free API by default */ session->api_block_mode = 1; /* blocking API by default */ + session->state = LIBSSH2_STATE_INITIAL_KEX; + session->fullpacket_required_type = 0; session->packet_read_timeout = LIBSSH2_DEFAULT_READ_TIMEOUT; session->flag.quote_paths = 1; /* default behavior is to quote paths for the scp subsystem */ @@ -1186,6 +1188,7 @@ libssh2_session_disconnect_ex(LIBSSH2_SE const char *desc, const char *lang) { int rc; + session->state &= ~LIBSSH2_STATE_INITIAL_KEX; session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; BLOCK_ADJUST(rc, session, session_disconnect(session, reason, desc, lang)); Index: libssh2-1.11.0/src/transport.c =================================================================== --- libssh2-1.11.0.orig/src/transport.c +++ libssh2-1.11.0/src/transport.c @@ -187,6 +187,7 @@ fullpacket(LIBSSH2_SESSION * session, in struct transportpacket *p = &session->packet; int rc; int compressed; + uint32_t seq = session->remote.seqno; if(session->fullpacket_state == libssh2_NB_state_idle) { session->fullpacket_macstate = LIBSSH2_MAC_CONFIRMED; @@ -318,7 +319,7 @@ fullpacket(LIBSSH2_SESSION * session, in if(session->fullpacket_state == libssh2_NB_state_created) { rc = _libssh2_packet_add(session, p->payload, session->fullpacket_payload_len, - session->fullpacket_macstate); + session->fullpacket_macstate, seq); if(rc == LIBSSH2_ERROR_EAGAIN) return rc; if(rc) { @@ -329,6 +330,11 @@ fullpacket(LIBSSH2_SESSION * session, in session->fullpacket_state = libssh2_NB_state_idle; + if(session->kex_strict && + session->fullpacket_packet_type == SSH_MSG_NEWKEYS) { + session->remote.seqno = 0; + } + return session->fullpacket_packet_type; } @@ -1091,6 +1097,10 @@ int _libssh2_transport_send(LIBSSH2_SESS session->local.seqno++; + if(session->kex_strict && data[0] == SSH_MSG_NEWKEYS) { + session->local.seqno = 0; + } + ret = LIBSSH2_SEND(session, p->outbuf, total_length, LIBSSH2_SOCKET_SEND_FLAGS(session)); if(ret < 0)