214 lines
8.2 KiB
Diff
214 lines
8.2 KiB
Diff
From ebd18d65b725948e816be992d1649e7b18e67272 Mon Sep 17 00:00:00 2001
|
|
From: Jakub Jelen <jjelen@redhat.com>
|
|
Date: Wed, 23 Jun 2021 13:16:33 +0200
|
|
Subject: [PATCH] CVE-2021-3634:Create a sepatate 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/datail?id=35485
|
|
|
|
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
|
|
---
|
|
include/libssh/crypto.h | 3 ++-
|
|
src/gssapi.c | 4 ++--
|
|
src/kdf.c | 2 +-
|
|
src/kex.c | 4 +++-
|
|
src/libcrypto.c | 2 +-
|
|
src/messages.c | 4 ++--
|
|
src/packet.c | 9 +++++----
|
|
src/pki.c | 8 ++++----
|
|
src/wrapper.c | 2 +-
|
|
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 ede7166..671cf91 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 two 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 488df58..1d0fb6a 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",
|
|
diff --git a/src/kdf.c b/src/kdf.c
|
|
index 0e90e18..0964473 100644
|
|
--- a/src/kdf.c
|
|
+++ b/src/kdf.c
|
|
@@ -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) {
|
|
diff --git a/src/kex.c b/src/kex.c
|
|
index 80b6e8a..602de1c 100644
|
|
--- a/src/kex.c
|
|
+++ b/src/kex.c
|
|
@@ -1197,11 +1197,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 8ff8a02..3db75df 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 25683b2..2891218 100644
|
|
--- a/src/messages.c
|
|
+++ b/src/messages.c
|
|
@@ -708,8 +708,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 e9ae564..9824fca 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;
|
|
}
|
|
diff --git a/src/pki.c b/src/pki.c
|
|
index 6dcb120..dba305c 100644
|
|
--- a/src/pki.c
|
|
+++ b/src/pki.c
|
|
@@ -2328,11 +2328,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;
|
|
}
|
|
- ssh_string_fill(session_id, crypto->session_id, crypto->digest_len);
|
|
+ ssh_string_fill(session_id, crypto->session_id, crypto->session_id_len);
|
|
|
|
/* Fill the input */
|
|
sign_input = ssh_buffer_new();
|
|
@@ -2389,11 +2389,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;
|
|
}
|
|
- ssh_string_fill(session_id, crypto->session_id, crypto->digest_len);
|
|
+ ssh_string_fill(session_id, crypto->session_id, crypto->session_id_len);
|
|
|
|
sig_buf = ssh_buffer_new();
|
|
if (sig_buf == NULL) {
|
|
diff --git a/src/wrapper.c b/src/wrapper.c
|
|
index 7e57ab5..36dc39c 100644
|
|
--- a/src/wrapper.c
|
|
+++ b/src/wrapper.c
|
|
@@ -183,7 +183,7 @@ void crypto_free(struct ssh_crypto_struct *crypto)
|
|
}
|
|
#endif
|
|
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 f220e01..7a4e7ce 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,
|
|
--
|
|
1.8.3.1
|
|
|