Skip to content

Commit

Permalink
src: add 'strict KEX' to fix CVE-2023-48795 "Terrapin Attack"
Browse files Browse the repository at this point in the history 
Refs:
https://terrapin-attack.com/
https://seclists.org/oss-sec/2023/q4/292
https://osv.dev/list?ecosystem=&q=CVE-2023-48795
GHSA-45x7-px36-x8w8
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-48795

Fixes libssh2#1290
Closes libssh2#1291

# Conflicts:
#	src/kex.c
#	src/session.c
  • Loading branch information
@MichaelBuckley @palmin
MichaelBuckley authored and palmin committed Dec 19, 2023
1 parent 93e0f85 commit 4abdf4c
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 36 deletions.
68 changes: 44 additions & 24 deletions src/kex.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3028,14 +3028,21 @@ kex_method_ssh_curve25519_sha256 = {

/* this kex method signals that client can receive extensions
* as described in https://datatracker.ietf.org/doc/html/rfc8308
*/
*/
static const LIBSSH2_KEX_METHOD
kex_method_extension_negotiation = {
"ext-info-c",
NULL,
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[] = {
&kex_method_extension_negotiation,
#if LIBSSH2_ED25519
Expand All @@ -3054,7 +3061,9 @@ static const LIBSSH2_KEX_METHOD *libssh2_kex_methods[] = {
&kex_method_diffie_helman_group14_sha1,
&kex_method_diffie_helman_group1_sha1,
&kex_method_diffie_helman_group_exchange_sha1,
NULL
&kex_method_extension_negotiation,
&kex_method_strict_client_extension,
NULL
};

typedef struct _LIBSSH2_COMMON_METHOD
Expand Down Expand Up @@ -3292,13 +3301,13 @@ static int kexinit(LIBSSH2_SESSION * session)
return 0;
}

/* kex_agree_instr
/* _libssh2_kex_agree_instr
* Kex specific variant of strstr()
* Needle must be precede by BOL or ',', and followed by ',' or EOL
*/
static unsigned char *
kex_agree_instr(unsigned char *haystack, unsigned long haystack_len,
const unsigned char *needle, unsigned long 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;
Expand Down Expand Up @@ -3382,7 +3391,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,
Expand Down Expand Up @@ -3416,9 +3425,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) */
Expand Down Expand Up @@ -3452,14 +3461,20 @@ 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;

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,
Expand Down Expand Up @@ -3493,9 +3508,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?
Expand Down Expand Up @@ -3539,7 +3554,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,
Expand All @@ -3561,9 +3576,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;
Expand Down Expand Up @@ -3603,7 +3618,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 **)
Expand All @@ -3624,8 +3639,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;
Expand Down Expand Up @@ -3656,7 +3672,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,
Expand All @@ -3678,8 +3694,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;
Expand Down Expand Up @@ -3853,6 +3870,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;
Expand All @@ -3878,6 +3896,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;
Expand Down Expand Up @@ -3926,6 +3945,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;

Expand Down
18 changes: 14 additions & 4 deletions src/libssh2_priv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,9 @@ struct _LIBSSH2_SESSION
unsigned char *server_sign_algorithms;
size_t server_sign_algorithms_len;

/* Whether to use the OpenSSH Strict KEX extension */
int kex_strict;

/* (remote as source of data -- packet_read ) */
libssh2_endpoint_data remote;

Expand Down Expand Up @@ -824,6 +827,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;
Expand Down Expand Up @@ -871,10 +875,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
Expand Down Expand Up @@ -1116,6 +1121,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);
Expand Down
83 changes: 79 additions & 4 deletions src/packet.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,13 @@ packet_x11_open(LIBSSH2_SESSION * session, unsigned char *data,
* 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;
Expand Down Expand Up @@ -517,6 +516,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) {
Expand Down Expand Up @@ -1178,6 +1241,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;
Expand Down Expand Up @@ -1239,7 +1311,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) {
Expand Down
2 changes: 1 addition & 1 deletion src/packet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
10 changes: 8 additions & 2 deletions src/session.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,13 @@ 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 */
_libssh2_debug(session, LIBSSH2_TRACE_TRANS,
"New session resource allocated");
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 */
_libssh2_debug((session, LIBSSH2_TRACE_TRANS,
"New session resource allocated"));
_libssh2_init_if_needed();
}
return session;
Expand Down Expand Up @@ -1175,6 +1180,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));
Expand Down

0 comments on commit 4abdf4c

Please sign in to comment.