297 lines
12 KiB
Diff
297 lines
12 KiB
Diff
From 70565ac43867053871f47378c53e5d90ba9007d8 Mon Sep 17 00:00:00 2001
|
|
From: Jakub Jelen <jjelen@redhat.com>
|
|
Date: Thu, 16 Mar 2023 11:55:12 +0100
|
|
Subject: [PATCH] CVE-2023-1667:kex: Add support for sending
|
|
first_kex_packet_follows flag
|
|
|
|
This is not completely straightforward as it requires us to do some
|
|
state
|
|
shuffling.
|
|
|
|
We introduce internal flag that can turn this on in client side, so far
|
|
for
|
|
testing only as we do not want to universally enable this. We also
|
|
repurpose the
|
|
server flag indicating the guess was wrong also for the client to make
|
|
desired
|
|
decisions.
|
|
|
|
If we found out our guess was wrong, we need to hope the server was able
|
|
to
|
|
figure out this much, we need to revert the DH FSM state, drop the
|
|
callbacks
|
|
from the "wrong" key exchange method and initiate the right one.
|
|
|
|
The server side is already tested by the pkd_hello_i1, which is
|
|
executing tests
|
|
against dropbrear clients, which is using this flag by default out of
|
|
the box.
|
|
|
|
Tested manually also with the pkd_hello --rekey to make sure the server
|
|
is able
|
|
to handle the rekeying with all key exchange methods.
|
|
|
|
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
|
|
Reviewed-by: Norbert Pocs <npocs@redhat.com>
|
|
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
|
|
|
|
Conflict:NA
|
|
Reference:https://gitlab.com/libssh/libssh-mirror/commit/70565ac43867053871f47378c53e5d90ba9007d8
|
|
---
|
|
include/libssh/dh.h | 1 +
|
|
include/libssh/session.h | 13 +++++--
|
|
src/client.c | 10 ++++-
|
|
src/kex.c | 82 +++++++++++++++++++++++++++++++++++-----
|
|
4 files changed, 92 insertions(+), 14 deletions(-)
|
|
|
|
diff --git a/include/libssh/dh.h b/include/libssh/dh.h
|
|
index 4a78738..08fe5d4 100644
|
|
--- a/include/libssh/dh.h
|
|
+++ b/include/libssh/dh.h
|
|
@@ -73,6 +73,7 @@ int ssh_dh_get_current_server_publickey_blob(ssh_session session,
|
|
ssh_key ssh_dh_get_next_server_publickey(ssh_session session);
|
|
int ssh_dh_get_next_server_publickey_blob(ssh_session session,
|
|
ssh_string *pubkey_blob);
|
|
+int dh_handshake(ssh_session session);
|
|
|
|
int ssh_client_dh_init(ssh_session session);
|
|
void ssh_client_dh_remove_callbacks(ssh_session session);
|
|
diff --git a/include/libssh/session.h b/include/libssh/session.h
|
|
index f1c8792..8bfca1f 100644
|
|
--- a/include/libssh/session.h
|
|
+++ b/include/libssh/session.h
|
|
@@ -165,14 +165,21 @@ struct ssh_session_struct {
|
|
uint32_t current_method;
|
|
} auth;
|
|
|
|
+ /* Sending this flag before key exchange to save one round trip during the
|
|
+ * key exchange. This might make sense on high-latency connections.
|
|
+ * So far internal only for testing. Usable only on the client side --
|
|
+ * there is no key exchange method that would start with server message */
|
|
+ bool send_first_kex_follows;
|
|
/*
|
|
* RFC 4253, 7.1: if the first_kex_packet_follows flag was set in
|
|
* the received SSH_MSG_KEXINIT, but the guess was wrong, this
|
|
* field will be set such that the following guessed packet will
|
|
- * be ignored. Once that packet has been received and ignored,
|
|
- * this field is cleared.
|
|
+ * be ignored on the receiving side. Once that packet has been received and
|
|
+ * ignored, this field is cleared.
|
|
+ * On the sending side, this is set after we got peer KEXINIT message and we
|
|
+ * need to resend the initial message of the negotiated KEX algorithm.
|
|
*/
|
|
- int first_kex_follows_guess_wrong;
|
|
+ bool first_kex_follows_guess_wrong;
|
|
|
|
ssh_buffer in_hashbuf;
|
|
ssh_buffer out_hashbuf;
|
|
diff --git a/src/client.c b/src/client.c
|
|
index 855fee8..f4b8f32 100644
|
|
--- a/src/client.c
|
|
+++ b/src/client.c
|
|
@@ -246,10 +246,13 @@ end:
|
|
* @warning this function returning is no proof that DH handshake is
|
|
* completed
|
|
*/
|
|
-static int dh_handshake(ssh_session session)
|
|
+int dh_handshake(ssh_session session)
|
|
{
|
|
int rc = SSH_AGAIN;
|
|
|
|
+ SSH_LOG(SSH_LOG_TRACE, "dh_handshake_state = %d, kex_type = %d",
|
|
+ session->dh_handshake_state, session->next_crypto->kex_type);
|
|
+
|
|
switch (session->dh_handshake_state) {
|
|
case DH_STATE_INIT:
|
|
switch(session->next_crypto->kex_type){
|
|
@@ -392,6 +395,8 @@ static void ssh_client_connection_callback(ssh_session session)
|
|
{
|
|
int rc;
|
|
|
|
+ SSH_LOG(SSH_LOG_DEBUG, "session_state=%d", session->session_state);
|
|
+
|
|
switch(session->session_state) {
|
|
case SSH_SESSION_STATE_NONE:
|
|
case SSH_SESSION_STATE_CONNECTING:
|
|
@@ -454,6 +459,9 @@ static void ssh_client_connection_callback(ssh_session session)
|
|
goto error;
|
|
set_status(session,0.8f);
|
|
session->session_state=SSH_SESSION_STATE_DH;
|
|
+
|
|
+ /* If the init packet was already sent in previous step, this will be no
|
|
+ * operation */
|
|
if (dh_handshake(session) == SSH_ERROR) {
|
|
goto error;
|
|
}
|
|
diff --git a/src/kex.c b/src/kex.c
|
|
index 5ace6d3..b4c9d8f 100644
|
|
--- a/src/kex.c
|
|
+++ b/src/kex.c
|
|
@@ -28,6 +28,7 @@
|
|
#include <stdio.h>
|
|
#include <stdbool.h>
|
|
|
|
+#include "libssh/libssh.h"
|
|
#include "libssh/priv.h"
|
|
#include "libssh/buffer.h"
|
|
#include "libssh/dh.h"
|
|
@@ -355,14 +356,19 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit)
|
|
(void)type;
|
|
(void)user;
|
|
|
|
+ SSH_LOG(SSH_LOG_TRACE, "KEXINIT received");
|
|
+
|
|
if (session->session_state == SSH_SESSION_STATE_AUTHENTICATED) {
|
|
if (session->dh_handshake_state == DH_STATE_FINISHED) {
|
|
SSH_LOG(SSH_LOG_DEBUG, "Peer initiated key re-exchange");
|
|
/* Reset the sent flag if the re-kex was initiated by the peer */
|
|
session->flags &= ~SSH_SESSION_FLAG_KEXINIT_SENT;
|
|
- } else if (session->dh_handshake_state == DH_STATE_INIT_SENT) {
|
|
- SSH_LOG(SSH_LOG_DEBUG, "Receeved peer kexinit answer");
|
|
- } else {
|
|
+ } else if (session->flags & SSH_SESSION_FLAG_KEXINIT_SENT &&
|
|
+ session->dh_handshake_state == DH_STATE_INIT_SENT) {
|
|
+ /* This happens only when we are sending our-guessed first kex
|
|
+ * packet right after our KEXINIT packet. */
|
|
+ SSH_LOG(SSH_LOG_DEBUG, "Received peer kexinit answer.");
|
|
+ } else if (session->session_state != SSH_SESSION_STATE_INITIAL_KEX) {
|
|
ssh_set_error(session, SSH_FATAL,
|
|
"SSH_KEXINIT received in wrong state");
|
|
goto error;
|
|
@@ -470,9 +476,10 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit)
|
|
/*
|
|
* Remember whether 'first_kex_packet_follows' was set and the client
|
|
* guess was wrong: in this case the next SSH_MSG_KEXDH_INIT message
|
|
- * must be ignored.
|
|
+ * must be ignored on the server side.
|
|
+ * Client needs to start the Key exchange over with the correct method
|
|
*/
|
|
- if (first_kex_packet_follows) {
|
|
+ if (first_kex_packet_follows || session->send_first_kex_follows) {
|
|
char **client_methods = session->next_crypto->client_kex.methods;
|
|
char **server_methods = session->next_crypto->server_kex.methods;
|
|
session->first_kex_follows_guess_wrong =
|
|
@@ -480,6 +487,8 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit)
|
|
server_methods[SSH_KEX]) ||
|
|
cmp_first_kex_algo(client_methods[SSH_HOSTKEYS],
|
|
server_methods[SSH_HOSTKEYS]);
|
|
+ SSH_LOG(SSH_LOG_DEBUG, "The initial guess was %s.",
|
|
+ session->first_kex_follows_guess_wrong ? "wrong" : "right");
|
|
}
|
|
|
|
if (server_kex) {
|
|
@@ -559,7 +568,12 @@ SSH_PACKET_CALLBACK(ssh_packet_kexinit)
|
|
|
|
/* Note, that his overwrites authenticated state in case of rekeying */
|
|
session->session_state = SSH_SESSION_STATE_KEXINIT_RECEIVED;
|
|
- session->dh_handshake_state = DH_STATE_INIT;
|
|
+ /* if we already sent our initial key exchange packet, do not reset the
|
|
+ * DH state. We will know if we were right with our guess only in
|
|
+ * dh_handshake_state() */
|
|
+ if (session->send_first_kex_follows == false) {
|
|
+ session->dh_handshake_state = DH_STATE_INIT;
|
|
+ }
|
|
session->ssh_connection_callback(session);
|
|
return SSH_PACKET_USED;
|
|
|
|
@@ -869,6 +883,7 @@ int ssh_kex_select_methods (ssh_session session)
|
|
struct ssh_kex_struct *client = &session->next_crypto->client_kex;
|
|
char *ext_start = NULL;
|
|
const char *aead_hmac = NULL;
|
|
+ enum ssh_key_exchange_e kex_type;
|
|
int i;
|
|
|
|
/* Here we should drop the ext-info-c from the list so we avoid matching.
|
|
@@ -897,8 +912,18 @@ int ssh_kex_select_methods (ssh_session session)
|
|
session->next_crypto->kex_methods[i] = strdup("");
|
|
}
|
|
}
|
|
- kex = session->next_crypto->kex_methods[SSH_KEX];
|
|
- session->next_crypto->kex_type = kex_select_kex_type(kex);
|
|
+
|
|
+ /* We can not set this value directly as the old value is needed to revert
|
|
+ * callbacks if we are client */
|
|
+ kex_type = kex_select_kex_type(session->next_crypto->kex_methods[SSH_KEX]);
|
|
+ if (session->client && session->first_kex_follows_guess_wrong) {
|
|
+ SSH_LOG(SSH_LOG_DEBUG, "Our guess was wrong. Restarting the KEX");
|
|
+ /* We need to remove the wrong callbacks and start kex again */
|
|
+ revert_kex_callbacks(session);
|
|
+ session->dh_handshake_state = DH_STATE_INIT;
|
|
+ session->first_kex_follows_guess_wrong = false;
|
|
+ }
|
|
+ session->next_crypto->kex_type = kex_type;
|
|
|
|
SSH_LOG(SSH_LOG_INFO, "Negotiated %s,%s,%s,%s,%s,%s,%s,%s,%s,%s",
|
|
session->next_crypto->kex_methods[SSH_KEX],
|
|
@@ -925,6 +950,19 @@ int ssh_send_kex(ssh_session session)
|
|
ssh_string str = NULL;
|
|
int i;
|
|
int rc;
|
|
+ int first_kex_packet_follows = 0;
|
|
+
|
|
+ /* Only client can initiate the handshake methods we implement. If we
|
|
+ * already received the peer mechanisms, there is no point in guessing */
|
|
+ if (session->client &&
|
|
+ session->session_state != SSH_SESSION_STATE_KEXINIT_RECEIVED &&
|
|
+ session->send_first_kex_follows) {
|
|
+ first_kex_packet_follows = 1;
|
|
+ }
|
|
+
|
|
+ SSH_LOG(SSH_LOG_TRACE,
|
|
+ "Sending KEXINIT packet, first_kex_packet_follows = %d",
|
|
+ first_kex_packet_follows);
|
|
|
|
rc = ssh_buffer_pack(session->out_buffer,
|
|
"bP",
|
|
@@ -957,14 +995,14 @@ int ssh_send_kex(ssh_session session)
|
|
|
|
rc = ssh_buffer_pack(session->out_buffer,
|
|
"bd",
|
|
- 0,
|
|
+ first_kex_packet_follows,
|
|
0);
|
|
if (rc != SSH_OK) {
|
|
goto error;
|
|
}
|
|
|
|
/* Prepare also the first_kex_packet_follows and reserved to 0 */
|
|
- rc = ssh_buffer_add_u8(session->out_hashbuf, 0);
|
|
+ rc = ssh_buffer_add_u8(session->out_hashbuf, first_kex_packet_follows);
|
|
if (rc < 0) {
|
|
goto error;
|
|
}
|
|
@@ -979,6 +1017,30 @@ int ssh_send_kex(ssh_session session)
|
|
|
|
session->flags |= SSH_SESSION_FLAG_KEXINIT_SENT;
|
|
SSH_LOG(SSH_LOG_PACKET, "SSH_MSG_KEXINIT sent");
|
|
+
|
|
+ /* If we indicated that we are sending the guessed key exchange packet,
|
|
+ * do it now. The packet is simple, but we need to do some preparations */
|
|
+ if (first_kex_packet_follows) {
|
|
+ char *list = kex->methods[SSH_KEX];
|
|
+ char *colon = strchr(list, ',');
|
|
+ size_t kex_name_len = colon ? (size_t)(colon - list) : strlen(list);
|
|
+ char *kex_name = calloc(kex_name_len + 1, 1);
|
|
+ if (kex_name == NULL) {
|
|
+ ssh_set_error_oom(session);
|
|
+ goto error;
|
|
+ }
|
|
+ snprintf(kex_name, kex_name_len + 1, "%.*s", (int)kex_name_len, list);
|
|
+ SSH_LOG(SSH_LOG_TRACE, "Sending the first kex packet for %s", kex_name);
|
|
+
|
|
+ session->next_crypto->kex_type = kex_select_kex_type(kex_name);
|
|
+ free(kex_name);
|
|
+
|
|
+ /* run the first step of the DH handshake */
|
|
+ session->dh_handshake_state = DH_STATE_INIT;
|
|
+ if (dh_handshake(session) == SSH_ERROR) {
|
|
+ goto error;
|
|
+ }
|
|
+ }
|
|
return 0;
|
|
error:
|
|
ssh_buffer_reinit(session->out_buffer);
|
|
--
|
|
2.23.0
|
|
|