Skip to content

Commit d34d925

Browse files
MichaelBuckleyvszakats
MichaelBuckley
authored and
vszakats
committed
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 GHSA-45x7-px36-x8w8 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-48795 Fixes #1290 Closes #1291
1 parent c0f6954 commit d34d925

File tree

6 files changed

+149
-32
lines changed

6 files changed

+149
-32
lines changed

‎src/kex.c‎

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3032,6 +3032,13 @@ kex_method_extension_negotiation = {
30323032
0,
30333033
};
30343034

3035+
static const LIBSSH2_KEX_METHOD
3036+
kex_method_strict_client_extension = {
3037+
"kex-strict-c-v00@openssh.com",
3038+
NULL,
3039+
0,
3040+
};
3041+
30353042
static const LIBSSH2_KEX_METHOD *libssh2_kex_methods[] = {
30363043
#if LIBSSH2_ED25519
30373044
&kex_method_ssh_curve25519_sha256,
@@ -3050,6 +3057,7 @@ static const LIBSSH2_KEX_METHOD *libssh2_kex_methods[] = {
30503057
&kex_method_diffie_helman_group1_sha1,
30513058
&kex_method_diffie_helman_group_exchange_sha1,
30523059
&kex_method_extension_negotiation,
3060+
&kex_method_strict_client_extension,
30533061
NULL
30543062
};
30553063

@@ -3302,13 +3310,13 @@ static int kexinit(LIBSSH2_SESSION * session)
33023310
return 0;
33033311
}
33043312

3305-
/* kex_agree_instr
3313+
/* _libssh2_kex_agree_instr
33063314
* Kex specific variant of strstr()
33073315
* Needle must be preceded by BOL or ',', and followed by ',' or EOL
33083316
*/
3309-
static unsigned char *
3310-
kex_agree_instr(unsigned char *haystack, size_t haystack_len,
3311-
const unsigned char *needle, size_t needle_len)
3317+
unsigned char *
3318+
_libssh2_kex_agree_instr(unsigned char *haystack, size_t haystack_len,
3319+
const unsigned char *needle, size_t needle_len)
33123320
{
33133321
unsigned char *s;
33143322
unsigned char *end_haystack;
@@ -3393,7 +3401,7 @@ static int kex_agree_hostkey(LIBSSH2_SESSION * session,
33933401
while(s && *s) {
33943402
unsigned char *p = (unsigned char *) strchr((char *) s, ',');
33953403
size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s));
3396-
if(kex_agree_instr(hostkey, hostkey_len, s, method_len)) {
3404+
if(_libssh2_kex_agree_instr(hostkey, hostkey_len, s, method_len)) {
33973405
const LIBSSH2_HOSTKEY_METHOD *method =
33983406
(const LIBSSH2_HOSTKEY_METHOD *)
33993407
kex_get_method_by_name((char *) s, method_len,
@@ -3427,9 +3435,9 @@ static int kex_agree_hostkey(LIBSSH2_SESSION * session,
34273435
}
34283436

34293437
while(hostkeyp && (*hostkeyp) && (*hostkeyp)->name) {
3430-
s = kex_agree_instr(hostkey, hostkey_len,
3431-
(unsigned char *) (*hostkeyp)->name,
3432-
strlen((*hostkeyp)->name));
3438+
s = _libssh2_kex_agree_instr(hostkey, hostkey_len,
3439+
(unsigned char *) (*hostkeyp)->name,
3440+
strlen((*hostkeyp)->name));
34333441
if(s) {
34343442
/* So far so good, but does it suit our purposes? (Encrypting vs
34353443
Signing) */
@@ -3463,14 +3471,20 @@ static int kex_agree_kex_hostkey(LIBSSH2_SESSION * session, unsigned char *kex,
34633471
{
34643472
const LIBSSH2_KEX_METHOD **kexp = libssh2_kex_methods;
34653473
unsigned char *s;
3474+
const unsigned char *strict =
3475+
(unsigned char *)"kex-strict-s-v00@openssh.com";
3476+
3477+
if(_libssh2_kex_agree_instr(kex, kex_len, strict, 28)) {
3478+
session->kex_strict = 1;
3479+
}
34663480

34673481
if(session->kex_prefs) {
34683482
s = (unsigned char *) session->kex_prefs;
34693483

34703484
while(s && *s) {
34713485
unsigned char *q, *p = (unsigned char *) strchr((char *) s, ',');
34723486
size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s));
3473-
q = kex_agree_instr(kex, kex_len, s, method_len);
3487+
q = _libssh2_kex_agree_instr(kex, kex_len, s, method_len);
34743488
if(q) {
34753489
const LIBSSH2_KEX_METHOD *method = (const LIBSSH2_KEX_METHOD *)
34763490
kex_get_method_by_name((char *) s, method_len,
@@ -3504,9 +3518,9 @@ static int kex_agree_kex_hostkey(LIBSSH2_SESSION * session, unsigned char *kex,
35043518
}
35053519

35063520
while(*kexp && (*kexp)->name) {
3507-
s = kex_agree_instr(kex, kex_len,
3508-
(unsigned char *) (*kexp)->name,
3509-
strlen((*kexp)->name));
3521+
s = _libssh2_kex_agree_instr(kex, kex_len,
3522+
(unsigned char *) (*kexp)->name,
3523+
strlen((*kexp)->name));
35103524
if(s) {
35113525
/* We've agreed on a key exchange method,
35123526
* Can we agree on a hostkey that works with this kex?
@@ -3550,7 +3564,7 @@ static int kex_agree_crypt(LIBSSH2_SESSION * session,
35503564
unsigned char *p = (unsigned char *) strchr((char *) s, ',');
35513565
size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s));
35523566

3553-
if(kex_agree_instr(crypt, crypt_len, s, method_len)) {
3567+
if(_libssh2_kex_agree_instr(crypt, crypt_len, s, method_len)) {
35543568
const LIBSSH2_CRYPT_METHOD *method =
35553569
(const LIBSSH2_CRYPT_METHOD *)
35563570
kex_get_method_by_name((char *) s, method_len,
@@ -3572,9 +3586,9 @@ static int kex_agree_crypt(LIBSSH2_SESSION * session,
35723586
}
35733587

35743588
while(*cryptp && (*cryptp)->name) {
3575-
s = kex_agree_instr(crypt, crypt_len,
3576-
(unsigned char *) (*cryptp)->name,
3577-
strlen((*cryptp)->name));
3589+
s = _libssh2_kex_agree_instr(crypt, crypt_len,
3590+
(unsigned char *) (*cryptp)->name,
3591+
strlen((*cryptp)->name));
35783592
if(s) {
35793593
endpoint->crypt = *cryptp;
35803594
return 0;
@@ -3614,7 +3628,7 @@ static int kex_agree_mac(LIBSSH2_SESSION * session,
36143628
unsigned char *p = (unsigned char *) strchr((char *) s, ',');
36153629
size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s));
36163630

3617-
if(kex_agree_instr(mac, mac_len, s, method_len)) {
3631+
if(_libssh2_kex_agree_instr(mac, mac_len, s, method_len)) {
36183632
const LIBSSH2_MAC_METHOD *method = (const LIBSSH2_MAC_METHOD *)
36193633
kex_get_method_by_name((char *) s, method_len,
36203634
(const LIBSSH2_COMMON_METHOD **)
@@ -3635,8 +3649,9 @@ static int kex_agree_mac(LIBSSH2_SESSION * session,
36353649
}
36363650

36373651
while(*macp && (*macp)->name) {
3638-
s = kex_agree_instr(mac, mac_len, (unsigned char *) (*macp)->name,
3639-
strlen((*macp)->name));
3652+
s = _libssh2_kex_agree_instr(mac, mac_len,
3653+
(unsigned char *) (*macp)->name,
3654+
strlen((*macp)->name));
36403655
if(s) {
36413656
endpoint->mac = *macp;
36423657
return 0;
@@ -3667,7 +3682,7 @@ static int kex_agree_comp(LIBSSH2_SESSION *session,
36673682
unsigned char *p = (unsigned char *) strchr((char *) s, ',');
36683683
size_t method_len = (p ? (size_t)(p - s) : strlen((char *) s));
36693684

3670-
if(kex_agree_instr(comp, comp_len, s, method_len)) {
3685+
if(_libssh2_kex_agree_instr(comp, comp_len, s, method_len)) {
36713686
const LIBSSH2_COMP_METHOD *method =
36723687
(const LIBSSH2_COMP_METHOD *)
36733688
kex_get_method_by_name((char *) s, method_len,
@@ -3689,8 +3704,9 @@ static int kex_agree_comp(LIBSSH2_SESSION *session,
36893704
}
36903705

36913706
while(*compp && (*compp)->name) {
3692-
s = kex_agree_instr(comp, comp_len, (unsigned char *) (*compp)->name,
3693-
strlen((*compp)->name));
3707+
s = _libssh2_kex_agree_instr(comp, comp_len,
3708+
(unsigned char *) (*compp)->name,
3709+
strlen((*compp)->name));
36943710
if(s) {
36953711
endpoint->comp = *compp;
36963712
return 0;
@@ -3871,6 +3887,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session, int reexchange,
38713887
session->local.kexinit = key_state->oldlocal;
38723888
session->local.kexinit_len = key_state->oldlocal_len;
38733889
key_state->state = libssh2_NB_state_idle;
3890+
session->state &= ~LIBSSH2_STATE_INITIAL_KEX;
38743891
session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
38753892
session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
38763893
return -1;
@@ -3896,6 +3913,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session, int reexchange,
38963913
session->local.kexinit = key_state->oldlocal;
38973914
session->local.kexinit_len = key_state->oldlocal_len;
38983915
key_state->state = libssh2_NB_state_idle;
3916+
session->state &= ~LIBSSH2_STATE_INITIAL_KEX;
38993917
session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
39003918
session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
39013919
return -1;
@@ -3944,6 +3962,7 @@ _libssh2_kex_exchange(LIBSSH2_SESSION * session, int reexchange,
39443962
session->remote.kexinit = NULL;
39453963
}
39463964

3965+
session->state &= ~LIBSSH2_STATE_INITIAL_KEX;
39473966
session->state &= ~LIBSSH2_STATE_KEX_ACTIVE;
39483967
session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
39493968

‎src/libssh2_priv.h‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,9 @@ struct _LIBSSH2_SESSION
736736
/* key signing algorithm preferences -- NULL yields server order */
737737
char *sign_algo_prefs;
738738

739+
/* Whether to use the OpenSSH Strict KEX extension */
740+
int kex_strict;
741+
739742
/* (remote as source of data -- packet_read ) */
740743
libssh2_endpoint_data remote;
741744

@@ -908,6 +911,7 @@ struct _LIBSSH2_SESSION
908911
int fullpacket_macstate;
909912
size_t fullpacket_payload_len;
910913
int fullpacket_packet_type;
914+
uint32_t fullpacket_required_type;
911915

912916
/* State variables used in libssh2_sftp_init() */
913917
libssh2_nonblocking_states sftpInit_state;
@@ -948,10 +952,11 @@ struct _LIBSSH2_SESSION
948952
};
949953

950954
/* session.state bits */
951-
#define LIBSSH2_STATE_EXCHANGING_KEYS 0x00000001
952-
#define LIBSSH2_STATE_NEWKEYS 0x00000002
953-
#define LIBSSH2_STATE_AUTHENTICATED 0x00000004
954-
#define LIBSSH2_STATE_KEX_ACTIVE 0x00000008
955+
#define LIBSSH2_STATE_INITIAL_KEX 0x00000001
956+
#define LIBSSH2_STATE_EXCHANGING_KEYS 0x00000002
957+
#define LIBSSH2_STATE_NEWKEYS 0x00000004
958+
#define LIBSSH2_STATE_AUTHENTICATED 0x00000008
959+
#define LIBSSH2_STATE_KEX_ACTIVE 0x00000010
955960

956961
/* session.flag helpers */
957962
#ifdef MSG_NOSIGNAL
@@ -1182,6 +1187,11 @@ ssize_t _libssh2_send(libssh2_socket_t socket, const void *buffer,
11821187
int _libssh2_kex_exchange(LIBSSH2_SESSION * session, int reexchange,
11831188
key_exchange_state_t * state);
11841189

1190+
unsigned char *_libssh2_kex_agree_instr(unsigned char *haystack,
1191+
size_t haystack_len,
1192+
const unsigned char *needle,
1193+
size_t needle_len);
1194+
11851195
/* Let crypt.c/hostkey.c expose their method structs */
11861196
const LIBSSH2_CRYPT_METHOD **libssh2_crypt_methods(void);
11871197
const LIBSSH2_HOSTKEY_METHOD **libssh2_hostkey_methods(void);

‎src/packet.c‎

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -624,14 +624,13 @@ packet_authagent_open(LIBSSH2_SESSION * session,
624624
* layer when it has received a packet.
625625
*
626626
* The input pointer 'data' is pointing to allocated data that this function
627-
* is asked to deal with so on failure OR success, it must be freed fine.
628-
* The only exception is when the return code is LIBSSH2_ERROR_EAGAIN.
627+
* will be freed unless return the code is LIBSSH2_ERROR_EAGAIN.
629628
*
630629
* This function will always be called with 'datalen' greater than zero.
631630
*/
632631
int
633632
_libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
634-
size_t datalen, int macstate)
633+
size_t datalen, int macstate, uint32_t seq)
635634
{
636635
int rc = 0;
637636
unsigned char *message = NULL;
@@ -676,6 +675,70 @@ _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
676675
break;
677676
}
678677

678+
if(session->state & LIBSSH2_STATE_INITIAL_KEX) {
679+
if(msg == SSH_MSG_KEXINIT) {
680+
if(!session->kex_strict) {
681+
if(datalen < 17) {
682+
LIBSSH2_FREE(session, data);
683+
session->packAdd_state = libssh2_NB_state_idle;
684+
return _libssh2_error(session,
685+
LIBSSH2_ERROR_BUFFER_TOO_SMALL,
686+
"Data too short extracting kex");
687+
}
688+
else {
689+
const unsigned char *strict =
690+
(unsigned char *)"kex-strict-s-v00@openssh.com";
691+
struct string_buf buf;
692+
unsigned char *algs = NULL;
693+
size_t algs_len = 0;
694+
695+
buf.data = (unsigned char *)data;
696+
buf.dataptr = buf.data;
697+
buf.len = datalen;
698+
buf.dataptr += 17; /* advance past type and cookie */
699+
700+
if(_libssh2_get_string(&buf, &algs, &algs_len)) {
701+
LIBSSH2_FREE(session, data);
702+
session->packAdd_state = libssh2_NB_state_idle;
703+
return _libssh2_error(session,
704+
LIBSSH2_ERROR_BUFFER_TOO_SMALL,
705+
"Algs too short");
706+
}
707+
708+
if(algs_len == 0 ||
709+
_libssh2_kex_agree_instr(algs, algs_len, strict, 28)) {
710+
session->kex_strict = 1;
711+
}
712+
}
713+
}
714+
715+
if(session->kex_strict && seq) {
716+
LIBSSH2_FREE(session, data);
717+
session->socket_state = LIBSSH2_SOCKET_DISCONNECTED;
718+
session->packAdd_state = libssh2_NB_state_idle;
719+
libssh2_session_disconnect(session, "strict KEX violation: "
720+
"KEXINIT was not the first packet");
721+
722+
return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT,
723+
"strict KEX violation: "
724+
"KEXINIT was not the first packet");
725+
}
726+
}
727+
728+
if(session->kex_strict && session->fullpacket_required_type &&
729+
session->fullpacket_required_type != msg) {
730+
LIBSSH2_FREE(session, data);
731+
session->socket_state = LIBSSH2_SOCKET_DISCONNECTED;
732+
session->packAdd_state = libssh2_NB_state_idle;
733+
libssh2_session_disconnect(session, "strict KEX violation: "
734+
"unexpected packet type");
735+
736+
return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT,
737+
"strict KEX violation: "
738+
"unexpected packet type");
739+
}
740+
}
741+
679742
if(session->packAdd_state == libssh2_NB_state_allocated) {
680743
/* A couple exceptions to the packet adding rule: */
681744
switch(msg) {
@@ -1364,6 +1427,15 @@ _libssh2_packet_ask(LIBSSH2_SESSION * session, unsigned char packet_type,
13641427

13651428
return 0;
13661429
}
1430+
else if(session->kex_strict &&
1431+
(session->state & LIBSSH2_STATE_INITIAL_KEX)) {
1432+
libssh2_session_disconnect(session, "strict KEX violation: "
1433+
"unexpected packet type");
1434+
1435+
return _libssh2_error(session, LIBSSH2_ERROR_SOCKET_DISCONNECT,
1436+
"strict KEX violation: "
1437+
"unexpected packet type");
1438+
}
13671439
packet = _libssh2_list_next(&packet->node);
13681440
}
13691441
return -1;
@@ -1425,7 +1497,10 @@ _libssh2_packet_require(LIBSSH2_SESSION * session, unsigned char packet_type,
14251497
}
14261498

14271499
while(session->socket_state == LIBSSH2_SOCKET_CONNECTED) {
1428-
int ret = _libssh2_transport_read(session);
1500+
int ret;
1501+
session->fullpacket_required_type = packet_type;
1502+
ret = _libssh2_transport_read(session);
1503+
session->fullpacket_required_type = 0;
14291504
if(ret == LIBSSH2_ERROR_EAGAIN)
14301505
return ret;
14311506
else if(ret < 0) {

‎src/packet.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,6 @@ int _libssh2_packet_burn(LIBSSH2_SESSION * session,
7272
int _libssh2_packet_write(LIBSSH2_SESSION * session, unsigned char *data,
7373
unsigned long data_len);
7474
int _libssh2_packet_add(LIBSSH2_SESSION * session, unsigned char *data,
75-
size_t datalen, int macstate);
75+
size_t datalen, int macstate, uint32_t seq);
7676

7777
#endif /* LIBSSH2_PACKET_H */

‎src/session.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ libssh2_session_init_ex(LIBSSH2_ALLOC_FUNC((*my_alloc)),
469469
session->abstract = abstract;
470470
session->api_timeout = 0; /* timeout-free API by default */
471471
session->api_block_mode = 1; /* blocking API by default */
472+
session->state = LIBSSH2_STATE_INITIAL_KEX;
473+
session->fullpacket_required_type = 0;
472474
session->packet_read_timeout = LIBSSH2_DEFAULT_READ_TIMEOUT;
473475
session->flag.quote_paths = 1; /* default behavior is to quote paths
474476
for the scp subsystem */
@@ -1223,6 +1225,7 @@ libssh2_session_disconnect_ex(LIBSSH2_SESSION *session, int reason,
12231225
const char *desc, const char *lang)
12241226
{
12251227
int rc;
1228+
session->state &= ~LIBSSH2_STATE_INITIAL_KEX;
12261229
session->state &= ~LIBSSH2_STATE_EXCHANGING_KEYS;
12271230
BLOCK_ADJUST(rc, session,
12281231
session_disconnect(session, reason, desc, lang));

0 commit comments

Comments
 (0)