aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Jelen <jjelen@redhat.com>2021-06-23 13:16:33 +0200
committerJakub Jelen <jjelen@redhat.com>2021-08-18 14:16:18 +0200
commitd3060bc84ed4e160082e819b4d404f76df7c8063 (patch)
treeded6214dfa8f9692dce1cfed783e5df4aef8929d
parent948bcb773e99139a22adf7be07fb079ab2c3ac5e (diff)
downloadlibssh-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.h3
-rw-r--r--src/gssapi.c4
-rw-r--r--src/kdf.c2
-rw-r--r--src/kex.c4
-rw-r--r--src/libcrypto.c2
-rw-r--r--src/messages.c4
-rw-r--r--src/packet.c9
-rw-r--r--src/pki.c8
-rw-r--r--src/wrapper.c2
-rw-r--r--tests/unittests/torture_session_keys.c3
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",
diff --git a/src/kdf.c b/src/kdf.c
index 0e90e188..09644739 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 c2ca77f1..82071c74 100644
--- a/src/kex.c
+++ b/src/kex.c
@@ -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;
}
diff --git a/src/pki.c b/src/pki.c
index 86e94724..932abf2c 100644
--- a/src/pki.c
+++ b/src/pki.c
@@ -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,