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

[PATCH] Allow SSH2_MSG_EXT_INFO when authenticated


Hello,

Continuing the investigation of the curl issue [1], I found the actual problem which is a regression introduced by the CVE-2018-10933 fix.
The SSH_MSG_EXT_INFO, used in key exchange, is being filtered when the user is already authenticated. This breaks the re-keying.
Follows attached a patch to fix this regression. It can be reviewed in gitlab [2].

Regards,
Anderson

[1] https://github.com/curl/curl/issues/3310
[2] https://gitlab.com/ansasaki/libssh-mirror/merge_requests/20
From 8efb7ae348dd9f82440867b86c90f40959c6d738 Mon Sep 17 00:00:00 2001
From: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
Date: Fri, 7 Dec 2018 18:19:33 +0100
Subject: [PATCH] packet: Allow SSH2_MSG_EXT_INFO when authenticated

When the server requests rekey, it can send the SSH2_MSG_EXT_INFO.  This
message was being filtered out by the packet filtering.  This includes a
test to enforce the filtering rules for this packet type.

Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@xxxxxxxxxx>
---
 src/packet.c                            |  6 ++++-
 tests/unittests/torture_packet_filter.c | 31 +++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/packet.c b/src/packet.c
index 706a451f..8fc589bb 100644
--- a/src/packet.c
+++ b/src/packet.c
@@ -264,13 +264,17 @@ static enum ssh_packet_filter_result_e ssh_packet_incoming_filter(ssh_session se
         /*
          * States required:
          * - session_state == SSH_SESSION_STATE_AUTHENTICATING
+         *   or session->session_state == SSH_SESSION_STATE_AUTHENTICATED
+         *   (re-exchange)
          * - dh_handshake_state == DH_STATE_FINISHED
          *
          * Transitions:
          * - None
          * */
 
-        if (session->session_state != SSH_SESSION_STATE_AUTHENTICATING) {
+        if ((session->session_state != SSH_SESSION_STATE_AUTHENTICATING) &&
+            (session->session_state != SSH_SESSION_STATE_AUTHENTICATED))
+        {
             rc = SSH_PACKET_DENIED;
             break;
         }
diff --git a/tests/unittests/torture_packet_filter.c b/tests/unittests/torture_packet_filter.c
index 871eb431..85fb5c1b 100644
--- a/tests/unittests/torture_packet_filter.c
+++ b/tests/unittests/torture_packet_filter.c
@@ -464,6 +464,36 @@ static void torture_packet_filter_check_auth_success(void **state)
     assert_int_equal(rc, 0);
 }
 
+static void torture_packet_filter_check_msg_ext_info(void **state)
+{
+    int rc;
+
+    global_state accepted[] = {
+        {
+            .flags = (COMPARE_SESSION_STATE |
+                    COMPARE_DH_STATE),
+            .session = SSH_SESSION_STATE_AUTHENTICATING,
+            .dh = DH_STATE_FINISHED,
+        },
+        {
+            .flags = (COMPARE_SESSION_STATE |
+                    COMPARE_DH_STATE),
+            .session = SSH_SESSION_STATE_AUTHENTICATED,
+            .dh = DH_STATE_FINISHED,
+        },
+    };
+
+    int accepted_count = 2;
+
+    /* Unused */
+    (void) state;
+
+    rc = check_message_in_all_states(accepted, accepted_count,
+            SSH2_MSG_EXT_INFO);
+
+    assert_int_equal(rc, 0);
+}
+
 static void torture_packet_filter_check_channel_open(void **state)
 {
     int rc;
@@ -494,6 +524,7 @@ int torture_run_tests(void)
         cmocka_unit_test(torture_packet_filter_check_auth_success),
         cmocka_unit_test(torture_packet_filter_check_channel_open),
         cmocka_unit_test(torture_packet_filter_check_unfiltered),
+        cmocka_unit_test(torture_packet_filter_check_msg_ext_info)
     };
 
     ssh_init();
-- 
2.19.1


Follow-Ups:
Re: [PATCH] Allow SSH2_MSG_EXT_INFO when authenticatedJakub Jelen <jjelen@xxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org