[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] kex: server fix to include first_kex_packet_follows


Hi,

Attached is a patch which fixes a host key signature validation bug that can
be hit when testing with dropbear clients at or beyond version 2013.57.  The
issue is that dropbear now always sets the 'first_kex_packet_follows' field
in its KEXINIT message.  Until now libssh would assume this field is zero;
but, it needs to be used when computing the session ID.

Before the patch I'm able to hit 'Bad hostkey signature' errors with dbclient;
after, dbclient is working for me.


Thanks,
-Jon
From 9961da6770cbbb43e9b9ab6e2648a4a1e7307c97 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Tue, 18 Mar 2014 13:26:43 -0700
Subject: [PATCH] kex: server fix to include first_kex_packet_follows

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 src/dh.c  | 24 ++++++++++++++++++------
 src/kex.c | 17 +++++++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/src/dh.c b/src/dh.c
index 3c2e5ad..c72f63e 100644
--- a/src/dh.c
+++ b/src/dh.c
@@ -638,17 +638,29 @@ int make_sessionid(ssh_session session) {
     client_hash = session->in_hashbuf;
   }
 
-  if (buffer_add_u32(server_hash, 0) < 0) {
-    goto error;
-  }
+  /*
+   * Handle the two final fields for the KEXINIT message (RFC 4253 7.1):
+   *
+   *      boolean      first_kex_packet_follows
+   *      uint32       0 (reserved for future extension)
+   */
   if (buffer_add_u8(server_hash, 0) < 0) {
     goto error;
   }
-  if (buffer_add_u32(client_hash, 0) < 0) {
+  if (buffer_add_u32(server_hash, 0) < 0) {
     goto error;
   }
-  if (buffer_add_u8(client_hash, 0) < 0) {
-    goto error;
+
+  /*
+   * These fields are handled for the server case in ssh_packet_kexinit.
+   */
+  if (session->client) {
+    if (buffer_add_u8(client_hash, 0) < 0) {
+      goto error;
+    }
+    if (buffer_add_u32(client_hash, 0) < 0) {
+      goto error;
+    }
   }
 
   len = ntohl(buffer_get_rest_len(client_hash));
diff --git a/src/kex.c b/src/kex.c
index f19beb8..4629363 100644
--- a/src/kex.c
+++ b/src/kex.c
@@ -281,6 +281,9 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
   char *strings[KEX_METHODS_SIZE];
   int i;
 
+  uint8_t first_kex_packet_follows = 0;
+  uint32_t kexinit_reserved = 0;
+
   (void)type;
   (void)user;
   memset(strings, 0, sizeof(strings));
@@ -343,6 +346,20 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit){
     }
   }
 
+  /*
+   * 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').
+   */
+  buffer_get_u8(packet, &first_kex_packet_follows);
+  buffer_add_u8(session->in_hashbuf, first_kex_packet_follows);
+  buffer_add_u32(session->in_hashbuf, kexinit_reserved);
+
   session->session_state=SSH_SESSION_STATE_KEXINIT_RECEIVED;
   session->dh_handshake_state=DH_STATE_INIT;
   session->ssh_connection_callback(session);
-- 
1.8.4.21.g992c386


Follow-Ups:
Re: [PATCH] kex: server fix to include first_kex_packet_followsAndreas Schneider <asn@xxxxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org