[CONC-654] Stop leaking client identifying information to the server before the TLS handshake

include/mariadb_com.h

@@ -162,6 +162,19 @@ enum enum_server_command
 #define CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS (1UL << 22)
 #define CLIENT_SESSION_TRACKING  (1UL << 23)
 #define CLIENT_ZSTD_COMPRESSION  (1UL << 26)
+/* 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          (1UL << 29) /* client supports progress indicator */
 #define CLIENT_PROGRESS_OBSOLETE  CLIENT_PROGRESS 
 #define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30)
@@ -219,7 +232,8 @@ enum enum_server_command
                                  CLIENT_PLUGIN_AUTH |\
                                  CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \
                                  CLIENT_SESSION_TRACKING |\
-                                 CLIENT_CONNECT_ATTRS)
+                                 CLIENT_CONNECT_ATTRS |\
+                                 CLIENT_CAN_SSL_V2)
 
 #define CLIENT_DEFAULT_FLAGS ((CLIENT_SUPPORTED_FLAGS & ~CLIENT_COMPRESS)\
                                                       & ~CLIENT_SSL)

libmariadb/mariadb_lib.c

@@ -1787,7 +1787,12 @@ MYSQL *mthd_my_real_connect(MYSQL *mysql, const char *host, const char *user,
  */
   if ((pkt_length=ma_net_safe_read(mysql)) == packet_error)
   {
-    if (mysql->net.last_errno == CR_SERVER_LOST)
+    if (mysql->options.use_ssl)
+      my_set_error(mysql, CR_CONNECTION_ERROR, SQLSTATE_UNKNOWN,
+                   "Received error packet before completion of TLS handshake. "
+                   "Suppressing its details so that the client cannot vary its behavior "
+                   "based on this UNTRUSTED input.");
+    else if (mysql->net.last_errno == CR_SERVER_LOST)
       my_set_error(mysql, CR_SERVER_LOST, SQLSTATE_UNKNOWN,
                  ER(CR_SERVER_LOST_EXTENDED),
                  "handshake: reading initial communication packet",
@@ -1918,6 +1923,16 @@ MYSQL *mthd_my_real_connect(MYSQL *mysql, const char *host, const char *user,
     }
   }
 
+  /* We now know the server's capabilities. If the client wants TLS/SSL,
+   * but the server doesn't support it, we should immediately abort.
+   */
+  if (mysql->options.use_ssl && !(mysql->server_capabilities & CLIENT_SSL))
+  {
+    SET_CLIENT_ERROR(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN,
+                     "Client requires TLS/SSL, but the server does not support it");
+    goto error;
+  }
+
   /* Set character set */
   if (mysql->options.charset_name)
     mysql->charset= mysql_find_charset_name(mysql->options.charset_name);
@@ -1936,6 +1951,17 @@ MYSQL *mthd_my_real_connect(MYSQL *mysql, const char *host, const char *user,
 
   mysql->client_flag= client_flag;
 
+  /* Until run_plugin_auth has completed, the connection
+   * cannot have been secured with TLS/SSL.
+   *
+   * This means that any client which expects to use a
+   * TLS/SSL-secured connection SHOULD NOT trust any
+   * communication received from the server prior to this
+   * point as being genuine; nor should either the client
+   * or the server send any confidential information up
+   * to this point.
+   */
+
   if (run_plugin_auth(mysql, scramble_data, scramble_len,
                              scramble_plugin, db))
     goto error;

plugins/auth/my_auth.c

@@ -209,6 +209,11 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
   size_t conn_attr_len= (mysql->options.extension) ? 
                          mysql->options.extension->connect_attrs_len : 0;
 
+#if defined(HAVE_TLS) && !defined(EMBEDDED_LIBRARY)
+  bool server_supports_ssl_v2=
+      !!(mysql->extension->mariadb_server_capabilities & (CLIENT_CAN_SSL_V2 >> 32));
+#endif
+
   /* see end= buff+32 below, fixed size of the packet is 32 bytes */
   buff= malloc(33 + USERNAME_LENGTH + data_len + NAME_LEN + NAME_LEN + conn_attr_len + 9);
   end= buff;
@@ -245,22 +250,6 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
     mysql->server_capabilities &= ~(CLIENT_SSL);
   }
 
-  /* if server doesn't support SSL and verification of server certificate
-     was set to mandatory, we need to return an error */
-  if (mysql->options.use_ssl && !(mysql->server_capabilities & CLIENT_SSL))
-  {
-    if ((mysql->client_flag & CLIENT_SSL_VERIFY_SERVER_CERT) ||
-        (mysql->options.extension && (mysql->options.extension->tls_fp || 
-                                      mysql->options.extension->tls_fp_list)))
-    {
-      my_set_error(mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN,
-                          ER(CR_SSL_CONNECTION_ERROR), 
-                          "SSL is required, but the server does not support it");
-      goto error;
-    }
-  }
-
-
   /* Remove options that server doesn't support */
   mysql->client_flag= mysql->client_flag &
                        (~(CLIENT_COMPRESS | CLIENT_ZSTD_COMPRESSION | CLIENT_SSL | CLIENT_PROTOCOL_41) 
@@ -336,18 +325,50 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
   if (mysql->options.use_ssl &&
       (mysql->client_flag & CLIENT_SSL))
   {
-    /*
-      Send mysql->client_flag, max_packet_size - unencrypted otherwise
-      the server does not know we want to do SSL
-    */
-    if (ma_net_write(net, (unsigned char *)buff, (size_t) (end-buff)) || ma_net_flush(net))
+    int ret;
+    if (server_supports_ssl_v2)
+    {
+      /*
+        The server doesn't inadvisably rely on information send in the
+        pre-TLS-handshake packet.  Send it a dummy packet that
+        contains NOTHING except for the 2-byte client capabilities
+        with the CLIENT_SSL bit.
+      */
+      unsigned char dummy[2];
+      int2store(dummy, CLIENT_SSL);
+      ret= (ma_net_write(net, dummy, sizeof(dummy)) || ma_net_flush(net));
+    }
+    else
+    {
+      /*
+        Send UNENCRYPTED "Login Request" packet with mysql->client_flag
+        and max_packet_size, but no username; without this, the server
+        does not know we want to switch to SSL/TLS
+
+        FIXME: Sending this packet is a very very VERY bad idea. It
+        contains the client's preferred charset and flags in plaintext;
+        this can be used for fingerprinting the client software version,
+        and probable geographic location.
+
+        This offers a glaring opportunity for pervasive attackers to
+        easily target, intercept, and exploit the client-server
+        connection (e.g. "MITM all connections from known-vulnerable
+        client versions originating from countries X, Y, and Z").
+      */
+      ret= (ma_net_write(net, (unsigned char *)buff, (size_t) (end-buff)) || ma_net_flush(net));
+    }
+
+    if (ret)
     {
       my_set_error(mysql, CR_SERVER_LOST, SQLSTATE_UNKNOWN,
                           ER(CR_SERVER_LOST_EXTENDED),
-                          "sending connection information to server",
+                          "sending SSLRequest packet to server",
                           errno);
       goto error;
     }
+    /* This is where the socket is actually converted from a plain
+     * TCP/IP socket to a TLS/SSL-wrapped socket.
+     */
     if (ma_pvio_start_ssl(mysql->net.pvio))
       goto error;
   }