| From d4634630432594b139b3af6b9f254b890c0f275d Mon Sep 17 00:00:00 2001 |
| From: Michael Buckley <michael@buckleyisms.com> |
| 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 |
| |
| CVE: CVE-2023-48795 |
| Upstream-Status: Backport |
| Signed-off-by: Ross Burton <ross.burton@arm.com> |
| --- |
| 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(-) |
| |
| diff --git a/src/kex.c b/src/kex.c |
| index d4034a0a..b4b748ca 100644 |
| --- a/src/kex.c |
| +++ b/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_methods[] = { |
| &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 * session) |
| 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_SESSION * session, |
| 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_SESSION * session, |
| } |
| |
| 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_SESSION * session, unsigned char *kex, |
| { |
| 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_SESSION * session, unsigned char *kex, |
| 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_SESSION * session, unsigned char *kex, |
| } |
| |
| 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_SESSION * 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(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_SESSION * session, |
| } |
| |
| 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 * 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 * 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_SESSION *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(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_SESSION *session, |
| } |
| |
| 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, int reexchange, |
| 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, int reexchange, |
| 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, int reexchange, |
| session->remote.kexinit = NULL; |
| } |
| |
| + session->state &= ~LIBSSH2_STATE_INITIAL_KEX; |
| session->state &= ~LIBSSH2_STATE_KEX_ACTIVE; |
| session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS; |
| |
| diff --git a/src/libssh2_priv.h b/src/libssh2_priv.h |
| index 82c3afe2..ee1d8b5c 100644 |
| --- a/src/libssh2_priv.h |
| +++ b/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 socket, const void *buffer, |
| 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); |
| diff --git a/src/packet.c b/src/packet.c |
| index b5b41981..35d4d39e 100644 |
| --- a/src/packet.c |
| +++ b/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 * session, unsigned char *data, |
| 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 * session, unsigned char packet_type, |
| |
| 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 * session, unsigned char packet_type, |
| } |
| |
| 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) { |
| diff --git a/src/packet.h b/src/packet.h |
| index 79018bcf..6ea100a5 100644 |
| --- a/src/packet.h |
| +++ b/src/packet.h |
| @@ -71,6 +71,6 @@ int _libssh2_packet_burn(LIBSSH2_SESSION * 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 */ |
| diff --git a/src/session.c b/src/session.c |
| index a4d602ba..f4bafb57 100644 |
| --- a/src/session.c |
| +++ b/src/session.c |
| @@ -464,6 +464,8 @@ libssh2_session_init_ex(LIBSSH2_ALLOC_FUNC((*my_alloc)), |
| 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_SESSION *session, int reason, |
| 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)); |
| diff --git a/src/transport.c b/src/transport.c |
| index 6d902d33..3b30ff84 100644 |
| --- a/src/transport.c |
| +++ b/src/transport.c |
| @@ -187,6 +187,7 @@ fullpacket(LIBSSH2_SESSION * session, int encrypted /* 1 or 0 */ ) |
| 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, int encrypted /* 1 or 0 */ ) |
| 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, int encrypted /* 1 or 0 */ ) |
| |
| 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_SESSION *session, |
| |
| 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) |
| -- |
| 2.34.1 |
| |