diff options
author | Andreas Schneider <asn@cryptomilk.org> | 2014-04-15 09:49:25 +0200 |
---|---|---|
committer | Andreas Schneider <asn@cryptomilk.org> | 2014-04-15 09:49:25 +0200 |
commit | e2805abbf7b4976a7eeeee3a215d00bd23aa14ef (patch) | |
tree | 17ea4e189f998f257918a2651cf6e51e0ca78879 | |
parent | 79d51099ac3273aa73c3bd3bd047b3fa996fef18 (diff) | |
download | libssh-e2805abbf7b4976a7eeeee3a215d00bd23aa14ef.tar.gz libssh-e2805abbf7b4976a7eeeee3a215d00bd23aa14ef.tar.xz libssh-e2805abbf7b4976a7eeeee3a215d00bd23aa14ef.zip |
Revert "kex: server fix for first_kex_packet_follows"
The patch breaks the client with ECDSA.
This reverts commit 5865b9436fda96ac9fc7c18e4dffe5fb12dcc515.
-rw-r--r-- | include/libssh/session.h | 9 | ||||
-rw-r--r-- | src/dh.c | 358 | ||||
-rw-r--r-- | src/kex.c | 213 | ||||
-rw-r--r-- | src/server.c | 9 |
4 files changed, 235 insertions, 354 deletions
diff --git a/include/libssh/session.h b/include/libssh/session.h index 29bdd60b..8480135d 100644 --- a/include/libssh/session.h +++ b/include/libssh/session.h @@ -127,15 +127,6 @@ struct ssh_session_struct { struct ssh_agent_state_struct *agent_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 out_hashbuf; struct ssh_crypto_struct *current_crypto; @@ -587,234 +587,218 @@ 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) { - ssh_string num = NULL; - ssh_string str = NULL; - ssh_buffer server_hash = NULL; - ssh_buffer client_hash = NULL; - ssh_buffer buf = NULL; - uint32_t len; - int rc = SSH_ERROR; - - buf = ssh_buffer_new(); - if (buf == NULL) { - return rc; - } + ssh_string num = NULL; + ssh_string str = NULL; + ssh_buffer server_hash = NULL; + ssh_buffer client_hash = NULL; + ssh_buffer buf = NULL; + uint32_t len; + int rc = SSH_ERROR; + + buf = ssh_buffer_new(); + if (buf == NULL) { + return rc; + } - str = ssh_string_from_char(session->clientbanner); - if (str == NULL) { - goto error; - } + str = ssh_string_from_char(session->clientbanner); + if (str == NULL) { + goto error; + } - rc = buffer_add_ssh_string(buf, str); - if (rc < 0) { - goto error; - } - ssh_string_free(str); + if (buffer_add_ssh_string(buf, str) < 0) { + goto error; + } + ssh_string_free(str); - str = ssh_string_from_char(session->serverbanner); - if (str == NULL) { - goto error; - } + str = ssh_string_from_char(session->serverbanner); + if (str == NULL) { + goto error; + } - rc = buffer_add_ssh_string(buf, str); - if (rc < 0) { - goto error; - } + if (buffer_add_ssh_string(buf, str) < 0) { + goto error; + } - if (session->client) { - server_hash = session->in_hashbuf; - client_hash = session->out_hashbuf; - } else { - server_hash = session->out_hashbuf; - client_hash = session->in_hashbuf; - } + if (session->client) { + server_hash = session->in_hashbuf; + client_hash = session->out_hashbuf; + } else { + server_hash = session->out_hashbuf; + client_hash = session->in_hashbuf; + } - /* - * 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; - } - rc = buffer_add_u32(server_hash, 0); - if (rc < 0) { - goto error; + if (buffer_add_u32(server_hash, 0) < 0) { + goto error; + } + if (buffer_add_u8(server_hash, 0) < 0) { + goto error; + } + if (buffer_add_u32(client_hash, 0) < 0) { + goto error; + } + if (buffer_add_u8(client_hash, 0) < 0) { + goto error; + } + + len = ntohl(buffer_get_rest_len(client_hash)); + if (buffer_add_u32(buf,len) < 0) { + goto error; + } + if (ssh_buffer_add_data(buf, buffer_get_rest(client_hash), + buffer_get_rest_len(client_hash)) < 0) { + goto error; + } + + len = ntohl(buffer_get_rest_len(server_hash)); + if (buffer_add_u32(buf, len) < 0) { + goto error; + } + if (ssh_buffer_add_data(buf, buffer_get_rest(server_hash), + buffer_get_rest_len(server_hash)) < 0) { + goto error; + } + + len = ssh_string_len(session->next_crypto->server_pubkey) + 4; + if (ssh_buffer_add_data(buf, session->next_crypto->server_pubkey, len) < 0) { + goto error; + } + if(session->next_crypto->kex_type == SSH_KEX_DH_GROUP1_SHA1 || + session->next_crypto->kex_type == SSH_KEX_DH_GROUP14_SHA1) { + + num = make_bignum_string(session->next_crypto->e); + if (num == NULL) { + goto error; } - /* These fields are handled for the server case in ssh_packet_kexinit. */ - if (session->client) { - rc = buffer_add_u8(client_hash, 0); - if (rc < 0) { - goto error; - } - rc = buffer_add_u32(client_hash, 0); - if (rc < 0) { - goto error; - } + len = ssh_string_len(num) + 4; + if (ssh_buffer_add_data(buf, num, len) < 0) { + goto error; } - len = ntohl(buffer_get_rest_len(client_hash)); - rc = buffer_add_u32(buf,len); - if (rc < 0) { - goto error; + ssh_string_free(num); + num = make_bignum_string(session->next_crypto->f); + if (num == NULL) { + goto error; } - rc = ssh_buffer_add_data(buf, buffer_get_rest(client_hash), - buffer_get_rest_len(client_hash)); - if (rc < 0) { - goto error; + + len = ssh_string_len(num) + 4; + if (ssh_buffer_add_data(buf, num, len) < 0) { + goto error; } - len = ntohl(buffer_get_rest_len(server_hash)); - rc = buffer_add_u32(buf, len); - if (rc < 0) { + ssh_string_free(num); +#ifdef HAVE_ECDH + } else if (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256){ + if(session->next_crypto->ecdh_client_pubkey == NULL || + session->next_crypto->ecdh_server_pubkey == NULL){ + SSH_LOG(SSH_LOG_WARNING, "ECDH parameted missing"); goto error; } - rc = ssh_buffer_add_data(buf, buffer_get_rest(server_hash), - buffer_get_rest_len(server_hash)); + rc = buffer_add_ssh_string(buf,session->next_crypto->ecdh_client_pubkey); if (rc < 0) { goto error; } - - len = ssh_string_len(session->next_crypto->server_pubkey) + 4; - rc = ssh_buffer_add_data(buf, session->next_crypto->server_pubkey, len); + rc = buffer_add_ssh_string(buf,session->next_crypto->ecdh_server_pubkey); if (rc < 0) { goto error; } - - if (session->next_crypto->kex_type == SSH_KEX_DH_GROUP1_SHA1 || - session->next_crypto->kex_type == SSH_KEX_DH_GROUP14_SHA1) { - - num = make_bignum_string(session->next_crypto->e); - if (num == NULL) { - goto error; - } - - len = ssh_string_len(num) + 4; - rc = ssh_buffer_add_data(buf, num, len); - if (rc < 0) { - goto error; - } - - ssh_string_free(num); - num = make_bignum_string(session->next_crypto->f); - if (num == NULL) { - goto error; - } - - len = ssh_string_len(num) + 4; - rc = ssh_buffer_add_data(buf, num, len); - if (rc < 0) { - goto error; - } - - ssh_string_free(num); -#ifdef HAVE_ECDH - } else if (session->next_crypto->kex_type == SSH_KEX_ECDH_SHA2_NISTP256) { - if (session->next_crypto->ecdh_client_pubkey == NULL || - session->next_crypto->ecdh_server_pubkey == NULL) { - SSH_LOG(SSH_LOG_WARNING, "ECDH parameted missing"); - goto error; - } - rc = buffer_add_ssh_string(buf,session->next_crypto->ecdh_client_pubkey); - if (rc < 0) { - goto error; - } - rc = buffer_add_ssh_string(buf,session->next_crypto->ecdh_server_pubkey); - if (rc < 0) { - goto error; - } #endif #ifdef HAVE_CURVE25519 - } else if (session->next_crypto->kex_type == SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG) { - rc = buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE)); - rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_client_pubkey, - CURVE25519_PUBKEY_SIZE); - rc += buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE)); - rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_server_pubkey, - CURVE25519_PUBKEY_SIZE); - if (rc != SSH_OK) { - goto error; - } + } else if(session->next_crypto->kex_type == SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG){ + rc = buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE)); + rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_client_pubkey, + CURVE25519_PUBKEY_SIZE); + rc += buffer_add_u32(buf, htonl(CURVE25519_PUBKEY_SIZE)); + rc += ssh_buffer_add_data(buf, session->next_crypto->curve25519_server_pubkey, + CURVE25519_PUBKEY_SIZE); + if (rc != SSH_OK) { + goto error; + } #endif - } - - num = make_bignum_string(session->next_crypto->k); - if (num == NULL) { - goto error; - } + } + num = make_bignum_string(session->next_crypto->k); + if (num == NULL) { + goto error; + } - len = ssh_string_len(num) + 4; - rc = ssh_buffer_add_data(buf, num, len); - if (rc < 0) { - goto error; - } + len = ssh_string_len(num) + 4; + if (ssh_buffer_add_data(buf, num, len) < 0) { + goto error; + } #ifdef DEBUG_CRYPTO - 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 - switch (session->next_crypto->kex_type) { + switch(session->next_crypto->kex_type){ case SSH_KEX_DH_GROUP1_SHA1: case SSH_KEX_DH_GROUP14_SHA1: - session->next_crypto->digest_len = SHA_DIGEST_LENGTH; - session->next_crypto->mac_type = SSH_MAC_SHA1; - session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); - if (session->next_crypto->secret_hash == NULL) { - ssh_set_error_oom(session); - goto error; - } - sha1(buffer_get_rest(buf), buffer_get_rest_len(buf), - session->next_crypto->secret_hash); - break; + session->next_crypto->digest_len = SHA_DIGEST_LENGTH; + session->next_crypto->mac_type = SSH_MAC_SHA1; + session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); + if(session->next_crypto->secret_hash == NULL){ + ssh_set_error_oom(session); + goto error; + } + sha1(buffer_get_rest(buf), buffer_get_rest_len(buf), + session->next_crypto->secret_hash); + break; case SSH_KEX_ECDH_SHA2_NISTP256: case SSH_KEX_CURVE25519_SHA256_LIBSSH_ORG: - session->next_crypto->digest_len = SHA256_DIGEST_LENGTH; - session->next_crypto->mac_type = SSH_MAC_SHA256; - session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); - if (session->next_crypto->secret_hash == NULL) { - ssh_set_error_oom(session); - goto error; - } - sha256(buffer_get_rest(buf), buffer_get_rest_len(buf), - session->next_crypto->secret_hash); - break; - } - /* During the first kex, secret hash and session ID are equal. However, after - * a key re-exchange, a new secret hash is calculated. This hash will not replace - * but complement existing session id. - */ - if (!session->next_crypto->session_id) { - session->next_crypto->session_id = malloc(session->next_crypto->digest_len); - if (session->next_crypto->session_id == NULL) { - ssh_set_error_oom(session); - goto error; - } - memcpy(session->next_crypto->session_id, session->next_crypto->secret_hash, - session->next_crypto->digest_len); - } + session->next_crypto->digest_len = SHA256_DIGEST_LENGTH; + session->next_crypto->mac_type = SSH_MAC_SHA256; + session->next_crypto->secret_hash = malloc(session->next_crypto->digest_len); + if(session->next_crypto->secret_hash == NULL){ + ssh_set_error_oom(session); + goto error; + } + sha256(buffer_get_rest(buf), buffer_get_rest_len(buf), + session->next_crypto->secret_hash); + break; + } + /* During the first kex, secret hash and session ID are equal. However, after + * a key re-exchange, a new secret hash is calculated. This hash will not replace + * but complement existing session id. + */ + if (!session->next_crypto->session_id){ + session->next_crypto->session_id = malloc(session->next_crypto->digest_len); + if (session->next_crypto->session_id == NULL){ + ssh_set_error_oom(session); + goto error; + } + memcpy(session->next_crypto->session_id, session->next_crypto->secret_hash, + session->next_crypto->digest_len); + } #ifdef DEBUG_CRYPTO - printf("Session hash: \n"); - ssh_print_hexa("secret hash", session->next_crypto->secret_hash, session->next_crypto->digest_len); - ssh_print_hexa("session id", session->next_crypto->session_id, session->next_crypto->digest_len); + printf("Session hash: \n"); + ssh_print_hexa("secret hash", session->next_crypto->secret_hash, session->next_crypto->digest_len); + ssh_print_hexa("session id", session->next_crypto->session_id, session->next_crypto->digest_len); #endif - rc = SSH_OK; + rc = SSH_OK; error: - ssh_buffer_free(buf); - ssh_buffer_free(client_hash); - ssh_buffer_free(server_hash); + ssh_buffer_free(buf); + ssh_buffer_free(client_hash); + ssh_buffer_free(server_hash); - session->in_hashbuf = NULL; - session->out_hashbuf = NULL; + session->in_hashbuf = NULL; + session->out_hashbuf = NULL; - ssh_string_free(str); - ssh_string_free(num); + ssh_string_free(str); + ssh_string_free(num); - return rc; + return rc; } int hashbufout_add_cookie(ssh_session session) { @@ -275,172 +275,87 @@ char *ssh_find_matching(const char *available_d, const char *preferred_d){ 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){ - int i; - int server_kex=session->server; - ssh_string str = NULL; - char *strings[KEX_METHODS_SIZE]; - int rc = SSH_ERROR; - - uint8_t first_kex_packet_follows = 0; - uint32_t kexinit_reserved = 0; - - (void)type; - (void)user; - - memset(strings, 0, sizeof(strings)); - if (session->session_state == SSH_SESSION_STATE_AUTHENTICATED){ - SSH_LOG(SSH_LOG_WARNING, "Other side initiating key re-exchange"); - } else if(session->session_state != SSH_SESSION_STATE_INITIAL_KEX){ - ssh_set_error(session,SSH_FATAL,"SSH_KEXINIT received in wrong state"); - goto error; - } - - if (server_kex) { - 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"); - goto error; - } - - 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"); - goto error; - } - } else { - 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"); - goto error; - } - - 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"); - goto error; - } - } + int server_kex=session->server; + ssh_string str = NULL; + char *strings[KEX_METHODS_SIZE]; + int i; - for (i = 0; i < KEX_METHODS_SIZE; i++) { - str = buffer_get_ssh_string(packet); - if (str == NULL) { - break; - } + (void)type; + (void)user; + memset(strings, 0, sizeof(strings)); + if (session->session_state == SSH_SESSION_STATE_AUTHENTICATED){ + SSH_LOG(SSH_LOG_WARNING, "Other side initiating key re-exchange"); + } else if(session->session_state != SSH_SESSION_STATE_INITIAL_KEX){ + ssh_set_error(session,SSH_FATAL,"SSH_KEXINIT received in wrong state"); + goto error; + } + if (server_kex) { + if (buffer_get_data(packet,session->next_crypto->client_kex.cookie,16) != 16) { + ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet"); + goto error; + } - rc = buffer_add_ssh_string(session->in_hashbuf, str); - if (rc < 0) { - ssh_set_error(session, SSH_FATAL, "Error adding string in hash buffer"); - goto error; - } + if (hashbufin_add_cookie(session, session->next_crypto->client_kex.cookie) < 0) { + ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed"); + goto error; + } + } else { + if (buffer_get_data(packet,session->next_crypto->server_kex.cookie,16) != 16) { + ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: no cookie in packet"); + goto error; + } - strings[i] = ssh_string_to_char(str); - if (strings[i] == NULL) { - ssh_set_error_oom(session); - goto error; - } - ssh_string_free(str); - str = NULL; - } + if (hashbufin_add_cookie(session, session->next_crypto->server_kex.cookie) < 0) { + ssh_set_error(session, SSH_FATAL, "ssh_packet_kexinit: adding cookie failed"); + goto error; + } + } - /* copy the server kex info into an array of strings */ - if (server_kex) { - for (i = 0; i < SSH_KEX_METHODS; i++) { - session->next_crypto->client_kex.methods[i] = strings[i]; - } - } else { /* client */ - for (i = 0; i < SSH_KEX_METHODS; i++) { - session->next_crypto->server_kex.methods[i] = strings[i]; - } + for (i = 0; i < KEX_METHODS_SIZE; i++) { + str = buffer_get_ssh_string(packet); + if (str == NULL) { + break; } - /* - * 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; + if (buffer_add_ssh_string(session->in_hashbuf, str) < 0) { + ssh_set_error(session, SSH_FATAL, "Error adding string in hash buffer"); + goto error; } - rc = buffer_add_u8(session->in_hashbuf, first_kex_packet_follows); - if (rc < 0) { - goto error; + strings[i] = ssh_string_to_char(str); + if (strings[i] == NULL) { + ssh_set_error_oom(session); + goto error; } + ssh_string_free(str); + str = NULL; + } - rc = buffer_add_u32(session->in_hashbuf, kexinit_reserved); - if (rc < 0) { - goto error; + /* copy the server kex info into an array of strings */ + if (server_kex) { + for (i = 0; i < SSH_KEX_METHODS; i++) { + session->next_crypto->client_kex.methods[i] = strings[i]; } - - /* - * 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]); + } else { /* client */ + for (i = 0; i < SSH_KEX_METHODS; i++) { + session->next_crypto->server_kex.methods[i] = strings[i]; } + } - session->session_state = SSH_SESSION_STATE_KEXINIT_RECEIVED; - session->dh_handshake_state = DH_STATE_INIT; - session->ssh_connection_callback(session); - return SSH_PACKET_USED; - + session->session_state=SSH_SESSION_STATE_KEXINIT_RECEIVED; + session->dh_handshake_state=DH_STATE_INIT; + session->ssh_connection_callback(session); + return SSH_PACKET_USED; error: - ssh_string_free(str); - for (i = 0; i < SSH_KEX_METHODS; i++) { - SAFE_FREE(strings[i]); - } + ssh_string_free(str); + for (i = 0; i < SSH_KEX_METHODS; i++) { + SAFE_FREE(strings[i]); + } - session->session_state = SSH_SESSION_STATE_ERROR; + session->session_state = SSH_SESSION_STATE_ERROR; - return SSH_PACKET_USED; + return SSH_PACKET_USED; } void ssh_list_kex(struct ssh_kex_struct *kex) { diff --git a/src/server.c b/src/server.c index 005effe1..b87c6e57 100644 --- a/src/server.c +++ b/src/server.c @@ -174,15 +174,6 @@ SSH_PACKET_CALLBACK(ssh_packet_kexdh_init){ SSH_LOG(SSH_LOG_RARE,"Invalid state for SSH_MSG_KEXDH_INIT"); 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){ case SSH_KEX_DH_GROUP1_SHA1: case SSH_KEX_DH_GROUP14_SHA1: |