diff options
author | Jakub Jelen <jjelen@redhat.com> | 2021-06-23 13:16:33 +0200 |
---|---|---|
committer | Jakub Jelen <jjelen@redhat.com> | 2021-08-18 14:16:18 +0200 |
commit | d3060bc84ed4e160082e819b4d404f76df7c8063 (patch) | |
tree | ded6214dfa8f9692dce1cfed783e5df4aef8929d | |
parent | 948bcb773e99139a22adf7be07fb079ab2c3ac5e (diff) | |
download | libssh-d3060bc84ed4e160082e819b4d404f76df7c8063.tar.gz libssh-d3060bc84ed4e160082e819b4d404f76df7c8063.tar.xz libssh-d3060bc84ed4e160082e819b4d404f76df7c8063.zip |
CVE-2021-3634: Create a separate length for session_id
Normally, the length of session_id and secret_hash is the same,
but if we will get into rekeying with a peer that changes preference
of key exchange algorithm, the new secret hash can be larger or
smaller than the previous session_id causing invalid reads or writes.
Resolves https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=35485
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
-rw-r--r-- | include/libssh/crypto.h | 3 | ||||
-rw-r--r-- | src/gssapi.c | 4 | ||||
-rw-r--r-- | src/kdf.c | 2 | ||||
-rw-r--r-- | src/kex.c | 4 | ||||
-rw-r--r-- | src/libcrypto.c | 2 | ||||
-rw-r--r-- | src/messages.c | 4 | ||||
-rw-r--r-- | src/packet.c | 9 | ||||
-rw-r--r-- | src/pki.c | 8 | ||||
-rw-r--r-- | src/wrapper.c | 2 | ||||
-rw-r--r-- | tests/unittests/torture_session_keys.c | 3 |
10 files changed, 23 insertions, 18 deletions
diff --git a/include/libssh/crypto.h b/include/libssh/crypto.h index ede71661..489b4905 100644 --- a/include/libssh/crypto.h +++ b/include/libssh/crypto.h @@ -126,8 +126,9 @@ struct ssh_crypto_struct { ssh_curve25519_pubkey curve25519_server_pubkey; #endif ssh_string dh_server_signature; /* information used by dh_handshake. */ - size_t digest_len; /* len of the two fields below */ + size_t session_id_len; unsigned char *session_id; + size_t digest_len; /* len of the secret hash */ unsigned char *secret_hash; /* Secret hash is same as session id until re-kex */ unsigned char *encryptIV; unsigned char *decryptIV; diff --git a/src/gssapi.c b/src/gssapi.c index 488df582..1d0fb6ae 100644 --- a/src/gssapi.c +++ b/src/gssapi.c @@ -465,8 +465,8 @@ static ssh_buffer ssh_gssapi_build_mic(ssh_session session) rc = ssh_buffer_pack(mic_buffer, "dPbsss", - crypto->digest_len, - (size_t)crypto->digest_len, crypto->session_id, + crypto->session_id_len, + crypto->session_id_len, crypto->session_id, SSH2_MSG_USERAUTH_REQUEST, session->gssapi->user, "ssh-connection", @@ -138,7 +138,7 @@ int sshkdf_derive_key(struct ssh_crypto_struct *crypto, ssh_mac_update(ctx, key, key_len); ssh_mac_update(ctx, crypto->secret_hash, crypto->digest_len); ssh_mac_update(ctx, &letter, 1); - ssh_mac_update(ctx, crypto->session_id, crypto->digest_len); + ssh_mac_update(ctx, crypto->session_id, crypto->session_id_len); ssh_mac_final(digest, ctx); if (requested_len < output_len) { @@ -1222,11 +1222,13 @@ int ssh_make_sessionid(ssh_session session) } memcpy(session->next_crypto->session_id, session->next_crypto->secret_hash, session->next_crypto->digest_len); + /* Initial length is the same as secret hash */ + session->next_crypto->session_id_len = session->next_crypto->digest_len; } #ifdef DEBUG_CRYPTO printf("Session hash: \n"); ssh_log_hexdump("secret hash", session->next_crypto->secret_hash, session->next_crypto->digest_len); - ssh_log_hexdump("session id", session->next_crypto->session_id, session->next_crypto->digest_len); + ssh_log_hexdump("session id", session->next_crypto->session_id, session->next_crypto->session_id_len); #endif rc = SSH_OK; diff --git a/src/libcrypto.c b/src/libcrypto.c index 8ff8a02b..3db75df6 100644 --- a/src/libcrypto.c +++ b/src/libcrypto.c @@ -392,7 +392,7 @@ int ssh_kdf(struct ssh_crypto_struct *crypto, goto out; } rc = EVP_KDF_ctrl(ctx, EVP_KDF_CTRL_SET_SSHKDF_SESSION_ID, - crypto->session_id, crypto->digest_len); + crypto->session_id, crypto->session_id_len); if (rc != 1) { goto out; } diff --git a/src/messages.c b/src/messages.c index c7fcc887..a772d488 100644 --- a/src/messages.c +++ b/src/messages.c @@ -714,8 +714,8 @@ static ssh_buffer ssh_msg_userauth_build_digest(ssh_session session, rc = ssh_buffer_pack(buffer, "dPbsssbsS", - crypto->digest_len, /* session ID string */ - (size_t)crypto->digest_len, crypto->session_id, + crypto->session_id_len, /* session ID string */ + crypto->session_id_len, crypto->session_id, SSH2_MSG_USERAUTH_REQUEST, /* type */ msg->auth_request.username, service, diff --git a/src/packet.c b/src/packet.c index fcaace4a..ec4a7203 100644 --- a/src/packet.c +++ b/src/packet.c @@ -1899,7 +1899,7 @@ ssh_packet_set_newkeys(ssh_session session, /* Both sides switched: do the actual switch now */ if (session->next_crypto->used == SSH_DIRECTION_BOTH) { - size_t digest_len; + size_t session_id_len; if (session->current_crypto != NULL) { crypto_free(session->current_crypto); @@ -1916,8 +1916,8 @@ ssh_packet_set_newkeys(ssh_session session, return SSH_ERROR; } - digest_len = session->current_crypto->digest_len; - session->next_crypto->session_id = malloc(digest_len); + session_id_len = session->current_crypto->session_id_len; + session->next_crypto->session_id = malloc(session_id_len); if (session->next_crypto->session_id == NULL) { ssh_set_error_oom(session); return SSH_ERROR; @@ -1925,7 +1925,8 @@ ssh_packet_set_newkeys(ssh_session session, memcpy(session->next_crypto->session_id, session->current_crypto->session_id, - digest_len); + session_id_len); + session->next_crypto->session_id_len = session_id_len; return SSH_OK; } @@ -2336,11 +2336,11 @@ ssh_string ssh_pki_do_sign(ssh_session session, } /* Get the session ID */ - session_id = ssh_string_new(crypto->digest_len); + session_id = ssh_string_new(crypto->session_id_len); if (session_id == NULL) { return NULL; } - rc = ssh_string_fill(session_id, crypto->session_id, crypto->digest_len); + rc = ssh_string_fill(session_id, crypto->session_id, crypto->session_id_len); if (rc < 0) { goto end; } @@ -2400,11 +2400,11 @@ ssh_string ssh_pki_do_sign_agent(ssh_session session, } /* prepend session identifier */ - session_id = ssh_string_new(crypto->digest_len); + session_id = ssh_string_new(crypto->session_id_len); if (session_id == NULL) { return NULL; } - rc = ssh_string_fill(session_id, crypto->session_id, crypto->digest_len); + rc = ssh_string_fill(session_id, crypto->session_id, crypto->session_id_len); if (rc < 0) { SSH_STRING_FREE(session_id); return NULL; diff --git a/src/wrapper.c b/src/wrapper.c index 06fbe5f9..05c820da 100644 --- a/src/wrapper.c +++ b/src/wrapper.c @@ -184,7 +184,7 @@ void crypto_free(struct ssh_crypto_struct *crypto) #endif SAFE_FREE(crypto->dh_server_signature); if (crypto->session_id != NULL) { - explicit_bzero(crypto->session_id, crypto->digest_len); + explicit_bzero(crypto->session_id, crypto->session_id_len); SAFE_FREE(crypto->session_id); } if (crypto->secret_hash != NULL) { diff --git a/tests/unittests/torture_session_keys.c b/tests/unittests/torture_session_keys.c index 6ae58831..a5ec3d89 100644 --- a/tests/unittests/torture_session_keys.c +++ b/tests/unittests/torture_session_keys.c @@ -48,8 +48,9 @@ struct ssh_cipher_struct fake_out_cipher = { }; struct ssh_crypto_struct test_crypto = { - .digest_len = 32, + .session_id_len = 32, .session_id = secret, + .digest_len = 32, .secret_hash = secret, .in_cipher = &fake_in_cipher, .out_cipher = &fake_out_cipher, |