Skip to content

Commit

Permalink
[CONC-654] Stop leaking client identifying information to the server …
Browse files Browse the repository at this point in the history
…before the TLS handshake

The server implementation here was incorrect as well, unnecessarily
reading—and TRUSTING—client identifying information sent before the TLS
handshake.  That's in MDEV-31585.

As a result of the server's mishandling of this information, it's not
possible for the client to fix this in a way that's backwards-compatible
with old servers.

We rely on the server sending a capability bit to indicate that the
server-side bug has been fixed:

    /* Server does not mishandle information sent in the plaintext
     * login request packet sent prior to the TLS handshake. As a result, the
     * client can safely send an empty/dummy packet contianing no
     * identifying information. Indicates that MDEV-31585 has been fixed.
     * Since ??.?.
     */
    #define MARIADB_CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET    (1ULL << 37)

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.
  • Loading branch information
dlenski committed Jul 3, 2023
1 parent b090994 commit 1b38393
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 17 deletions.
7 changes: 7 additions & 0 deletions include/mariadb_com.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ enum enum_server_command
#define MARIADB_CLIENT_EXTENDED_METADATA (1ULL << 35)
/* Do not resend metadata for prepared statements, since 10.6*/
#define MARIADB_CLIENT_CACHE_METADATA (1ULL << 36)
/* Server does not mishandle information sent in the plaintext
* login request packet sent prior to the TLS handshake. As a result, the
* client can safely send an empty/dummy packet contianing no
* identifying information. Indicates that MDEV-31585 has been fixed.
* Since ??.?.
*/
#define MARIADB_CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET (1ULL << 37)

#define IS_MARIADB_EXTENDED_SERVER(mysql)\
(!(mysql->server_capabilities & CLIENT_MYSQL))
Expand Down
57 changes: 40 additions & 17 deletions plugins/auth/my_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_allows_dummy_packet=
mysql->extension->mariadb_server_capabilities & (MARIADB_CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET >> 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;
Expand Down Expand Up @@ -320,26 +325,44 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
if (mysql->options.use_ssl &&
(mysql->client_flag & CLIENT_SSL))
{
/*
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").
*/
if (ma_net_write(net, (unsigned char *)buff, (size_t) (end-buff)) || ma_net_flush(net))
int ret;
if (server_allows_dummy_packet)
{
/*
The server has been disemborked so that it 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 dummy client reply packet to server",
errno);
goto error;
}
Expand Down

0 comments on commit 1b38393

Please sign in to comment.