Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: add 'strict KEX' to fix CVE-2023-48795 "Terrapin Attack" #1291

Closed
wants to merge 6 commits into from
Closed

src: add 'strict KEX' to fix CVE-2023-48795 "Terrapin Attack" #1291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ab44b09
src: fix CVE-2023-48795
MichaelBuckley Nov 30, 2023
113e839
fix length types in decl
vszakats Dec 13, 2023
3d0a579
Free p->payload after an error in _libssh2_packet_add
MichaelBuckley Dec 18, 2023
7bfd610
Revert "Free p->payload after an error in _libssh2_packet_add"
MichaelBuckley Dec 19, 2023
ca4e287
Always free data and reset packet add state in _libssh2_packet_add()
MichaelBuckley Dec 19, 2023
63acd5e
Remove double free
MichaelBuckley Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 41 additions & 22 deletions src/kex.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3032,6 +3032,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,
Expand All @@ -3050,6 +3057,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
};

Expand Down Expand Up @@ -3302,13 +3310,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;
Expand Down Expand Up @@ -3393,7 +3401,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 @@ -3427,9 +3435,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 @@ -3463,14 +3471,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 @@ -3504,9 +3518,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 @@ -3550,7 +3564,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 @@ -3572,9 +3586,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 @@ -3614,7 +3628,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 @@ -3635,8 +3649,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 @@ -3667,7 +3682,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 @@ -3689,8 +3704,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 @@ -3871,6 +3887,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 @@ -3896,6 +3913,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 @@ -3944,6 +3962,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,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;

Expand Down Expand Up @@ -908,6 +911,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 @@ -948,10 +952,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 @@ -1182,6 +1187,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,14 +624,13 @@ packet_authagent_open(LIBSSH2_SESSION * session,
* 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 @@ -676,6 +675,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 @@ -1364,6 +1427,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 @@ -1425,7 +1497,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,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 */
3 changes: 3 additions & 0 deletions src/session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,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 */
Expand Down Expand Up @@ -1223,6 +1225,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