lib/libssh
1
0
Fork 0
mirror of https://git.libssh.org/projects/libssh.git synced 2025-11-30 13:01:23 +03:00
Code Activity

kex: server fix for first_kex_packet_follows

Browse Source 
Ensure to honor the 'first_kex_packet_follow' field when processing
KEXINIT messages in the 'ssh_packet_kexinit' callback.  Until now
libssh would assume that this field is always unset (zero).  But
some clients may set this (dropbear at or beyond version 2013.57),
and it needs to be included when computing the session ID.

Also include logic for handling wrongly-guessed key exchange algorithms.
Save whether a client's guess is wrong in a new field in the session
struct: when set, the next KEX_DHINIT message to be processed will be
ignored per RFC 4253, 7.1.

While here, update both 'ssh_packet_kexinit' and 'make_sessionid' to
use softabs with a 4 space indent level throughout, and also convert
various error-checking to store intermediate values into an explicit
'rc'.

Signed-off-by: Jon Simons <jon@jonsimons.org>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This commit is contained in:
Jon Simons
2014-04-09 15:24:04 -07:00
committed by Andreas Schneider
parent ad1313c2e5
commit 5865b9436f
4 changed files with 366 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
include/libssh/session.h
View File

@@ -127,6 +127,15 @@ struct ssh_session_struct {
struct ssh_agent_state_struct *agent_state; struct ssh_agent_state_struct *agent_state;
struct ssh_auth_auto_state_struct *auth_auto_state; struct ssh_auth_auto_state_struct *auth_auto_state;
/*
* RFC 4253, 7.1: if the first_kex_packet_follows flag was set in
* the received SSH_MSG_KEXINIT, but the guess was wrong, this
* field will be set such that the following guessed packet will
* be ignored. Once that packet has been received and ignored,
* this field is cleared.
*/
int first_kex_follows_guess_wrong;
ssh_buffer in_hashbuf; ssh_buffer in_hashbuf;
ssh_buffer out_hashbuf; ssh_buffer out_hashbuf;
struct ssh_crypto_struct *current_crypto; struct ssh_crypto_struct *current_crypto;

98
src/dh.c
View File

@@ -587,16 +587,6 @@ error:
return SSH_ERROR; return SSH_ERROR;
} }
/*
static void sha_add(ssh_string str,SHACTX ctx){
sha1_update(ctx,str,string_len(str)+4);
#ifdef DEBUG_CRYPTO
ssh_print_hexa("partial hashed sessionid",str,string_len(str)+4);
#endif
}
*/
int make_sessionid(ssh_session session) { int make_sessionid(ssh_session session) {
ssh_string num = NULL; ssh_string num = NULL;
ssh_string str = NULL; ssh_string str = NULL;
@@ -616,7 +606,8 @@ int make_sessionid(ssh_session session) {
goto error; goto error;
} }
if (buffer_add_ssh_string(buf, str) < 0) { rc = buffer_add_ssh_string(buf, str);
if (rc < 0) {
goto error; goto error;
} }
ssh_string_free(str); ssh_string_free(str);
@@ -626,7 +617,8 @@ int make_sessionid(ssh_session session) {
goto error; goto error;
} }
if (buffer_add_ssh_string(buf, str) < 0) { rc = buffer_add_ssh_string(buf, str);
if (rc < 0) {
goto error; goto error;
} }
@@ -638,42 +630,62 @@ int make_sessionid(ssh_session session) {
client_hash = session->in_hashbuf; client_hash = session->in_hashbuf;
} }
if (buffer_add_u32(server_hash, 0) < 0) { /*
* Handle the two final fields for the KEXINIT message (RFC 4253 7.1):
*
* boolean first_kex_packet_follows
* uint32 0 (reserved for future extension)
*/
rc = buffer_add_u8(server_hash, 0);
if (rc < 0) {
goto error; goto error;
} }
if (buffer_add_u8(server_hash, 0) < 0) { rc = buffer_add_u32(server_hash, 0);
goto error; if (rc < 0) {
}
if (buffer_add_u32(client_hash, 0) < 0) {
goto error;
}
if (buffer_add_u8(client_hash, 0) < 0) {
goto error; goto error;
} }
len = ntohl(buffer_get_rest_len(client_hash)); /* These fields are handled for the server case in ssh_packet_kexinit. */
if (buffer_add_u32(buf,len) < 0) { if (session->client) {
rc = buffer_add_u8(client_hash, 0);
if (rc < 0) {
goto error; goto error;
} }
if (ssh_buffer_add_data(buf, buffer_get_rest(client_hash), rc = buffer_add_u32(client_hash, 0);
buffer_get_rest_len(client_hash)) < 0) { if (rc < 0) {
goto error;
}
}
len = ntohl(buffer_get_rest_len(client_hash));
rc = buffer_add_u32(buf,len);
if (rc < 0) {
goto error;
}
rc = ssh_buffer_add_data(buf, buffer_get_rest(client_hash),
buffer_get_rest_len(client_hash));
if (rc < 0) {
goto error; goto error;
} }
len = ntohl(buffer_get_rest_len(server_hash)); len = ntohl(buffer_get_rest_len(server_hash));
if (buffer_add_u32(buf, len) < 0) { rc = buffer_add_u32(buf, len);
if (rc < 0) {
goto error; goto error;
} }
if (ssh_buffer_add_data(buf, buffer_get_rest(server_hash), rc = ssh_buffer_add_data(buf, buffer_get_rest(server_hash),
buffer_get_rest_len(server_hash)) < 0) { buffer_get_rest_len(server_hash));
if (rc < 0) {
goto error; goto error;
} }
len = ssh_string_len(session->next_crypto->server_pubkey) + 4; len = ssh_string_len(session->next_crypto->server_pubkey) + 4;
if (ssh_buffer_add_data(buf, session->next_crypto->server_pubkey, len) < 0) { rc = ssh_buffer_add_data(buf, session->next_crypto->server_pubkey, len);
if (rc < 0) {
goto error; goto error;
} }
if(session->next_crypto->kex_type == SSH_KEX_DH_GROUP1_SHA1 ||
if (session->next_crypto->kex_type == SSH_KEX_DH_GROUP1_SHA1 ||
session->next_crypto->kex_type == SSH_KEX_DH_GROUP14_SHA1) { session->next_crypto->kex_type == SSH_KEX_DH_GROUP14_SHA1) {
num = make_bignum_string(session->next_crypto->e); num = make_bignum_string(session->next_crypto->e);
@@ -682,7 +694,8 @@ int make_sessionid(ssh_session session) {
} }
len = ssh_string_len(num) + 4; len = ssh_string_len(num) + 4;
if (ssh_buffer_add_data(buf, num, len) < 0) { rc = ssh_buffer_add_data(buf, num, len);
if (rc < 0) {
goto error; goto error;
} }
@@ -693,15 +706,16 @@ int make_sessionid(ssh_session session) {
} }
len = ssh_string_len(num) + 4; len = ssh_string_len(num) + 4;
if (ssh_buffer_add_data(buf, num, len) < 0) { rc = ssh_buffer_add_data(buf, num, len);
if (rc < 0) {
goto error; goto error;
} }
ssh_string_free(num); ssh_string_free(num);
#ifdef HAVE_ECDH #ifdef HAVE_ECDH
} else if (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256){ } else if (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256) {
if(session->next_crypto->ecdh_client_pubkey == NULL || if (session->next_crypto->ecdh_client_pubkey == NULL ||
session->next_crypto->ecdh_server_pubkey == NULL){ session->next_crypto->ecdh_server_pubkey == NULL) {
SSH_LOG(SSH_LOG_WARNING, "ECDH parameted missing"); SSH_LOG(SSH_LOG_WARNING, "ECDH parameted missing");
goto error; goto error;
} }
@@ -715,7 +729,7 @@ int make_sessionid(ssh_session session) {
} }
#endif #endif
#ifdef HAVE_CURVE25519 #ifdef HAVE_CURVE25519
} else if(session->next_crypto->kex_type == SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG){ } else if (session->next_crypto->kex_type == SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG) {
rc = buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE)); rc = buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE));
rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_client_pubkey, rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_client_pubkey,
CURVE25519_PUBKEY_SIZE); CURVE25519_PUBKEY_SIZE);
@@ -727,13 +741,15 @@ int make_sessionid(ssh_session session) {
} }
#endif #endif
} }
num = make_bignum_string(session->next_crypto->k); num = make_bignum_string(session->next_crypto->k);
if (num == NULL) { if (num == NULL) {
goto error; goto error;
} }
len = ssh_string_len(num) + 4; len = ssh_string_len(num) + 4;
if (ssh_buffer_add_data(buf, num, len) < 0) { rc = ssh_buffer_add_data(buf, num, len);
if (rc < 0) {
goto error; goto error;
} }
@@ -741,13 +757,13 @@ int make_sessionid(ssh_session session) {
ssh_print_hexa("hash buffer", ssh_buffer_get_begin(buf), ssh_buffer_get_len(buf)); ssh_print_hexa("hash buffer", ssh_buffer_get_begin(buf), ssh_buffer_get_len(buf));
#endif #endif
switch(session->next_crypto->kex_type){ switch (session->next_crypto->kex_type) {
case SSH_KEX_DH_GROUP1_SHA1: case SSH_KEX_DH_GROUP1_SHA1:
case SSH_KEX_DH_GROUP14_SHA1: case SSH_KEX_DH_GROUP14_SHA1:
session->next_crypto->digest_len = SHA_DIGEST_LENGTH; session->next_crypto->digest_len = SHA_DIGEST_LENGTH;
session->next_crypto->mac_type = SSH_MAC_SHA1; session->next_crypto->mac_type = SSH_MAC_SHA1;
session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len);
if(session->next_crypto->secret_hash == NULL){ if (session->next_crypto->secret_hash == NULL) {
ssh_set_error_oom(session); ssh_set_error_oom(session);
goto error; goto error;
} }
@@ -759,7 +775,7 @@ int make_sessionid(ssh_session session) {
session->next_crypto->digest_len = SHA256_DIGEST_LENGTH; session->next_crypto->digest_len = SHA256_DIGEST_LENGTH;
session->next_crypto->mac_type = SSH_MAC_SHA256; session->next_crypto->mac_type = SSH_MAC_SHA256;
session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len);
if(session->next_crypto->secret_hash == NULL){ if (session->next_crypto->secret_hash == NULL) {
ssh_set_error_oom(session); ssh_set_error_oom(session);
goto error; goto error;
} }
@@ -771,9 +787,9 @@ int make_sessionid(ssh_session session) {
* a key re-exchange, a new secret hash is calculated. This hash will not replace * a key re-exchange, a new secret hash is calculated. This hash will not replace
* but complement existing session id. * but complement existing session id.
*/ */
if (!session->next_crypto->session_id){ if (!session->next_crypto->session_id) {
session->next_crypto->session_id = malloc(session->next_crypto->digest_len); session->next_crypto->session_id = malloc(session->next_crypto->digest_len);
if (session->next_crypto->session_id == NULL){ if (session->next_crypto->session_id == NULL) {
ssh_set_error_oom(session); ssh_set_error_oom(session);
goto error; goto error;
} }

101
src/kex.c
View File

@@ -275,14 +275,56 @@ char *ssh_find_matching(const char *available_d, const char *preferred_d){
return NULL; return NULL;
} }
/**
* @internal
* @brief returns whether the first client key exchange algorithm matches
* the first server key exchange algorithm
* @returns whether the first client key exchange algorithm matches
* the first server key exchange algorithm
*/
static int is_first_kex_packet_follows_guess_wrong(const char *client_kex,
const char *server_kex) {
int is_wrong = 1;
char **server_kex_tokens = NULL;
char **client_kex_tokens = tokenize(client_kex);
if (client_kex_tokens == NULL) {
goto out;
}
if (client_kex_tokens[0] == NULL) {
goto freeout;
}
server_kex_tokens = tokenize(server_kex);
if (server_kex_tokens == NULL) {
goto freeout;
}
is_wrong = (strcmp(client_kex_tokens[0], server_kex_tokens[0]) != 0);
SAFE_FREE(server_kex_tokens[0]);
SAFE_FREE(server_kex_tokens);
freeout:
SAFE_FREE(client_kex_tokens[0]);
SAFE_FREE(client_kex_tokens);
out:
return is_wrong;
}
SSH_PACKET_CALLBACK(ssh_packet_kexinit){ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
int i;
int server_kex=session->server; int server_kex=session->server;
ssh_string str = NULL; ssh_string str = NULL;
char *strings[KEX_METHODS_SIZE]; char *strings[KEX_METHODS_SIZE];
int i; int rc = SSH_ERROR;
uint8_t first_kex_packet_follows = 0;
uint32_t kexinit_reserved = 0;
(void)type; (void)type;
(void)user; (void)user;
memset(strings, 0, sizeof(strings)); memset(strings, 0, sizeof(strings));
if (session->session_state == SSH_SESSION_STATE_AUTHENTICATED){ if (session->session_state == SSH_SESSION_STATE_AUTHENTICATED){
SSH_LOG(SSH_LOG_WARNING, "Other side initiating key re-exchange"); SSH_LOG(SSH_LOG_WARNING, "Other side initiating key re-exchange");
@@ -290,23 +332,28 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
ssh_set_error(session,SSH_FATAL,"SSH_KEXINIT received in wrong state"); ssh_set_error(session,SSH_FATAL,"SSH_KEXINIT received in wrong state");
goto error; goto error;
} }
if (server_kex) { if (server_kex) {
if (buffer_get_data(packet,session->next_crypto->client_kex.cookie,16) != 16) { rc = buffer_get_data(packet,session->next_crypto->client_kex.cookie, 16);
if (rc != 16) {
ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet"); ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet");
goto error; goto error;
} }
if (hashbufin_add_cookie(session, session->next_crypto->client_kex.cookie) < 0) { rc = hashbufin_add_cookie(session, session->next_crypto->client_kex.cookie);
if (rc < 0) {
ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed"); ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed");
goto error; goto error;
} }
} else { } else {
if (buffer_get_data(packet,session->next_crypto->server_kex.cookie,16) != 16) { rc = buffer_get_data(packet,session->next_crypto->server_kex.cookie, 16);
if (rc != 16) {
ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet"); ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet");
goto error; goto error;
} }
if (hashbufin_add_cookie(session, session->next_crypto->server_kex.cookie) < 0) { rc = hashbufin_add_cookie(session, session->next_crypto->server_kex.cookie);
if (rc < 0) {
ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed"); ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed");
goto error; goto error;
} }
@@ -318,7 +365,8 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
break; break;
} }
if (buffer_add_ssh_string(session->in_hashbuf, str) < 0) { rc = buffer_add_ssh_string(session->in_hashbuf, str);
if (rc < 0) {
ssh_set_error(session, SSH_FATAL, "Error adding string in hash buffer"); ssh_set_error(session, SSH_FATAL, "Error adding string in hash buffer");
goto error; goto error;
} }
@@ -343,10 +391,47 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
} }
} }
session->session_state=SSH_SESSION_STATE_KEXINIT_RECEIVED; /*
session->dh_handshake_state=DH_STATE_INIT; * Handle the two final fields for the KEXINIT message (RFC 4253 7.1):
*
* boolean first_kex_packet_follows
* uint32 0 (reserved for future extension)
*
* Notably if clients set 'first_kex_packet_follows', it is expected
* that its value is included when computing the session ID (see
* 'make_sessionid').
*/
rc = buffer_get_u8(packet, &first_kex_packet_follows);
if (rc != 1) {
goto error;
}
rc = buffer_add_u8(session->in_hashbuf, first_kex_packet_follows);
if (rc < 0) {
goto error;
}
rc = buffer_add_u32(session->in_hashbuf, kexinit_reserved);
if (rc < 0) {
goto error;
}
/*
* Remember whether 'first_kex_packet_follows' was set and the client
* guess was wrong: in this case the next SSH_MSG_KEXDH_INIT message
* must be ignored.
*/
if (server_kex && first_kex_packet_follows) {
session->first_kex_follows_guess_wrong =
is_first_kex_packet_follows_guess_wrong(session->next_crypto->client_kex.methods[SSH_KEX],
session->next_crypto->server_kex.methods[SSH_KEX]);
}
session->session_state = SSH_SESSION_STATE_KEXINIT_RECEIVED;
session->dh_handshake_state = DH_STATE_INIT;
session->ssh_connection_callback(session); session->ssh_connection_callback(session);
return SSH_PACKET_USED; return SSH_PACKET_USED;
error: error:
ssh_string_free(str); ssh_string_free(str);
for (i = 0; i < SSH_KEX_METHODS; i++) { for (i = 0; i < SSH_KEX_METHODS; i++) {

9
src/server.c
View File

@@ -174,6 +174,15 @@ SSH_PACKET_CALLBACK(ssh_packet_kexdh_init){
SSH_LOG(SSH_LOG_RARE,"Invalid state for SSH_MSG_KEXDH_INIT"); SSH_LOG(SSH_LOG_RARE,"Invalid state for SSH_MSG_KEXDH_INIT");
goto error; goto error;
} }
/* If first_kex_packet_follows guess was wrong, ignore this message. */
if (session->first_kex_follows_guess_wrong != 0) {
SSH_LOG(SSH_LOG_RARE, "first_kex_packet_follows guess was wrong, "
"ignoring first SSH_MSG_KEXDH_INIT message");
session->first_kex_follows_guess_wrong = 0;
goto error;
}
switch(session->next_crypto->kex_type){ switch(session->next_crypto->kex_type){
case SSH_KEX_DH_GROUP1_SHA1: case SSH_KEX_DH_GROUP1_SHA1:
case SSH_KEX_DH_GROUP14_SHA1: case SSH_KEX_DH_GROUP14_SHA1: