[MDEV-31585] Stop trusting or relying on client identifying information sent prior to the TLS handshake

The server has heretofore improperly mishandled—and TRUSTED—information sent
in the plaintext login request packet sent prior to the TLS handshake.

As a result of this, the client is *forced* to send excessive and
exploitable identifying information in the pre-TLS-handshake plaintext login
packet. That client-side vulnerability is CONC-654.

This modifies the server to stop relying on any of the information in the
pre-TLS-handshake plaintext login packet EXCEPT for the single bit that
tells it that a TLS handshake will follow.  It furthermore adds a
capability bit to the server greeting packet, which informs the client that
it is safe to send a bare-bones dummy packet containing ONLY the instruction
that a TLS handshake will follow:

    /* This capability is set if:
     *
     * - The CLIENT knows how to send a truncated 2-byte SSLRequest
     *   packet, containing no information other than the CLIENT_SSL flag
     *   which is necessary to trigger the TLS handshake, and to send its
     *   complete capability flags and other identifying information after
     *   the TLS handshake.
     * - The SERVER knows how to receive this truncated 2-byte SSLRequest
     *   packet, and to receive the client's complete capability bits
     *   after the TLS handshake.
     *
     */
    #define CLIENT_CAN_SSL_V2    (1ULL << 37)

Because the client cannot safely send the SSL_V2 SSLRequest packet unless
the server has advertised support for it in its (plaintext) Server Greeting
packet, an active MITM could strip the CLIENT_CAN_SSL_V2 bit from that
Server Greeting packet.  This downgrade attack will force the client to
continue exhibiting the CONC-654 vulnerability.  The server is also modified
to detect this case and abort the connection; this won't fix the one-time
client information leakage of the CONC-654 vulnerability, but it is intended
to discourage the MITM attack by making it highly visible.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.

Author: dlenski

include/mysql_com.h

@@ -274,6 +274,20 @@ enum enum_indicator_type
 /* Client no longer needs EOF packet */
 #define CLIENT_DEPRECATE_EOF (1ULL << 24)
 
+/* This capability is set if:
+ *
+ * - The CLIENT knows how to send a truncated 2-byte SSLRequest
+ *   packet, containing no information other than the CLIENT_SSL flag
+ *   which is necessary to trigger the TLS handshake, and to send its
+ *   complete capability flags and other identifying information after
+ *   the TLS handshake.
+ * - The SERVER knows how to receive this truncated 2-byte SSLRequest
+ *   packet, and to receive the client's complete capability bits
+ *   after the TLS handshake.
+ *
+ */
+#define CLIENT_CAN_SSL_V2    (1ULL << 37)
+
 #define CLIENT_PROGRESS_OBSOLETE  (1ULL << 29)
 #define CLIENT_SSL_VERIFY_SERVER_CERT_OBSOLETE (1ULL << 30)
 /*

libmariadb

@@ -12761,6 +12761,7 @@ static bool send_server_handshake_packet(MPVIO_EXT *mpvio,
   if (ssl_acceptor_fd)
   {
     thd->client_capabilities |= CLIENT_SSL;
+    thd->client_capabilities |= CLIENT_CAN_SSL_V2;  /* See parse_client_handshake_packet */
   }
 
   if (data_len)
@@ -13253,30 +13254,31 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
   */
   DBUG_ASSERT(net->read_pos[pkt_len] == 0);
 
-  ulonglong client_capabilities= uint2korr(net->read_pos);
-  compile_time_assert(sizeof(client_capabilities) >= 8);
-  if (client_capabilities & CLIENT_PROTOCOL_41)
-  {
-    if (pkt_len < 32)
-      DBUG_RETURN(packet_error);
-    client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16;
-    if (!(client_capabilities & CLIENT_MYSQL))
-    {
-      // it is client with mariadb extensions
-      ulonglong ext_client_capabilities=
-        (((ulonglong)uint4korr(net->read_pos + 28)) << 32);
-      client_capabilities|= ext_client_capabilities;
-    }
-  }
-
-  /* Disable those bits which are not supported by the client. */
-  compile_time_assert(sizeof(thd->client_capabilities) >= 8);
-  thd->client_capabilities&= client_capabilities;
+  ushort first_two_bytes_of_client_capabilities= uint2korr(net->read_pos);
+  bool pre_tls_client_packet_is_ssl_v2= (pkt_len==2 && first_two_bytes_of_client_capabilities == CLIENT_SSL);
+  bool pre_tls_client_packet_wants_ssl= !!(first_two_bytes_of_client_capabilities & CLIENT_SSL);
+
+  if (pre_tls_client_packet_wants_ssl)
+  {
+    /* Client wants to use TLS. This SSLRequest packet, sent in
+     * plaintext before the TLS handshake, is basically just a vestige
+     * that triggers the server (us) to start the TLS handshake.
+     *
+     * We ignore everything else in this pre-TLS packet, even though
+     * older clients send much of the same information that they will
+     * re-send over the TLS channel.
+     *
+     * This pre-TLS packet is untrustworthy AND if the server acts on
+     * its content, that FORCES the client to send more information
+     * in the clear.
+     */
 
-  DBUG_PRINT("info", ("client capabilities: %llu", thd->client_capabilities));
-  if (thd->client_capabilities & CLIENT_SSL)
-  {
     unsigned long errptr __attribute__((unused));
+    if (pre_tls_client_packet_is_ssl_v2)
+        DBUG_PRINT("info", ("client sent SSL_V2 SSLRequest packet (2 bytes with only TLS/SSL bit set)"));
+    else
+        DBUG_PRINT("info", ("client sent old SSLRequest packet (%ld bytes including TLS/SSL bit; capabilities & 0xffff == 0x%04x)",
+                            pkt_len, first_two_bytes_of_client_capabilities));
 
     /* Do the SSL layering. */
     if (!ssl_acceptor_fd)
@@ -13297,6 +13299,10 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
     DBUG_PRINT("info", ("Immediately following IO layer change: vio_type=%s",
                         safe_vio_type_name(thd->net.vio)));
 
+    /* Now we are using TLS. The client will resend its REAL
+     * handshake packet, containing complete credentials and
+     * capability information.
+     */
     DBUG_PRINT("info", ("Reading user information over SSL layer"));
     pkt_len= my_net_read(net);
     if (unlikely(pkt_len == packet_error || pkt_len < NORMAL_HANDSHAKE_SIZE))
@@ -13305,8 +13311,55 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
 			   pkt_len));
       DBUG_RETURN(packet_error);
     }
+
+    /* Re-load the FIRST TWO BYTES of the capabilities from the packet sent over TLS. */
+    first_two_bytes_of_client_capabilities = uint2korr(net->read_pos);
+  }
+
+  ulonglong client_capabilities= (ulonglong) first_two_bytes_of_client_capabilities;
+  compile_time_assert(sizeof(client_capabilities) >= 8);
+
+  DBUG_PRINT("info", ("client capabilities: %llu", thd->client_capabilities));
+  if (client_capabilities & CLIENT_PROTOCOL_41)
+  {
+    if (pkt_len < 32)
+      DBUG_RETURN(packet_error);
+    client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16;
+    if (!(client_capabilities & CLIENT_MYSQL))
+    {
+      // it is client with mariadb extensions
+      ulonglong ext_client_capabilities=
+        (((ulonglong)uint4korr(net->read_pos + 28)) << 32);
+      client_capabilities|= ext_client_capabilities;
+    }
+  }
+  bool post_tls_client_packet_indicates_ssl_v2= (client_capabilities & CLIENT_CAN_SSL_V2);
+
+  if (pre_tls_client_packet_wants_ssl
+      && post_tls_client_packet_indicates_ssl_v2
+      && !pre_tls_client_packet_is_ssl_v2)
+  {
+    /* 1. We told the client in our server greeting that we support the pre-TLS client packet containing only the TLS/SSL flag,
+     *    CLIENT_CAN_SSL_V2. [Server greeting packet is sent in the clear, may be MITM'ed en route to the client.]
+     * 2. The client told us in its pre-TLS SSLRequest packet that it wants to use SSL. (CLIENT_SSL flag)
+     * 3. The client told us in its post-TLS packet that it too supports the pre-TLS client packet containing only the TLS/SSL flag,
+     *    CLIENT_CAN_SSL_V2. [We received this information via TLS; assuming the client validated our server certificate
+     *    to avoid a 2-sided TLS MITM, we know that this packet is authentically from the client.]
+     * 4. Nevertheless, the client DID NOT SEND us an SSL_V2-style SSLRequest packet.
+     *
+     * The only way this can happen is if the client is being downgraded by an active MITM attacker which
+     * disables the CLIENT_CAN_SSL_V2 bit in our server greeting packet.
+     */
+    sql_print_warning("Aborting connection %lld because it is being actively MITM'ed to downgrade TLS security (attacker "
+                      "is stripping the CLIENT_CAN_SSL_V2 bit from our server capabilities)",
+                      thd->thread_id);
+    DBUG_RETURN(packet_error);
   }
 
+  /* Disable those bits which are not supported by the client. */
+  compile_time_assert(sizeof(thd->client_capabilities) >= 8);
+  thd->client_capabilities&= client_capabilities;
+
   if (client_capabilities & CLIENT_PROTOCOL_41)
   {
     thd->max_client_packet_length= uint4korr(net->read_pos+4);