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>

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;

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,
+                         crypto->session_id_len,
-                         (size_t)crypto->digest_len, crypto->session_id,
+                         crypto->session_id_len, crypto->session_id,
                          SSH2_MSG_USERAUTH_REQUEST,
                          session->gssapi->user,
                          "ssh-connection",

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) {

src/kex.c

@@ -1233,11 +1233,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
     SSH_LOG(SSH_LOG_DEBUG, "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;

src/libcrypto.c

@@ -388,7 +388,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;
     }

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 */
+                         crypto->session_id_len, /* session ID string */
-                         (size_t)crypto->digest_len, crypto->session_id,
+                         crypto->session_id_len, crypto->session_id,
                          SSH2_MSG_USERAUTH_REQUEST, /* type */
                          msg->auth_request.username,
                          service,

src/packet.c

@@ -1903,7 +1903,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);
@@ -1920,8 +1920,8 @@ ssh_packet_set_newkeys(ssh_session session,
             return SSH_ERROR;
         }
 
-        digest_len = session->current_crypto->digest_len;
+        session_id_len = session->current_crypto->session_id_len;
-        session->next_crypto->session_id = malloc(digest_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;
@@ -1929,7 +1929,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;
     }

src/pki.c

@@ -2562,11 +2562,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;
     }
@@ -2626,11 +2626,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;

src/wrapper.c

@@ -187,7 +187,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) {

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,