Skip to content

Commit ea4282e

Browse files
committed
[CONC-654] Stop leaking client identifying information to the server 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: /* 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) 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.
1 parent b090994 commit ea4282e

File tree

2 files changed

+55
-18
lines changed

2 files changed

+55
-18
lines changed

Diff for: include/mariadb_com.h

+15-1
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,19 @@ enum enum_server_command
162162
#define CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS (1UL << 22)
163163
#define CLIENT_SESSION_TRACKING (1UL << 23)
164164
#define CLIENT_ZSTD_COMPRESSION (1UL << 26)
165+
/* This capability is set if:
166+
*
167+
* - The CLIENT knows how to send a truncated 2-byte SSLRequest
168+
* packet, containing no information other than the CLIENT_SSL flag
169+
* which is necessary to trigger the TLS handshake, and to send its
170+
* complete capability flags and other identifying information after
171+
* the TLS handshake.
172+
* - The SERVER knows how to receive this truncated 2-byte SSLRequest
173+
* packet, and to receive the client's complete capability bits
174+
* after the TLS handshake.
175+
*
176+
*/
177+
#define CLIENT_CAN_SSL_V2 (1ULL << 37)
165178
#define CLIENT_PROGRESS (1UL << 29) /* client supports progress indicator */
166179
#define CLIENT_PROGRESS_OBSOLETE CLIENT_PROGRESS
167180
#define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30)
@@ -219,7 +232,8 @@ enum enum_server_command
219232
CLIENT_PLUGIN_AUTH |\
220233
CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \
221234
CLIENT_SESSION_TRACKING |\
222-
CLIENT_CONNECT_ATTRS)
235+
CLIENT_CONNECT_ATTRS |\
236+
CLIENT_CAN_SSL_V2)
223237

224238
#define CLIENT_DEFAULT_FLAGS ((CLIENT_SUPPORTED_FLAGS & ~CLIENT_COMPRESS)\
225239
& ~CLIENT_SSL)

Diff for: plugins/auth/my_auth.c

+40-17
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,11 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
209209
size_t conn_attr_len= (mysql->options.extension) ?
210210
mysql->options.extension->connect_attrs_len : 0;
211211

212+
#if defined(HAVE_TLS) && !defined(EMBEDDED_LIBRARY)
213+
bool server_supports_ssl_v2=
214+
mysql->extension->mariadb_server_capabilities & (MARIADB_CLIENT_CAN_SSL_V2 >> 32);
215+
#endif
216+
212217
/* see end= buff+32 below, fixed size of the packet is 32 bytes */
213218
buff= malloc(33 + USERNAME_LENGTH + data_len + NAME_LEN + NAME_LEN + conn_attr_len + 9);
214219
end= buff;
@@ -320,26 +325,44 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
320325
if (mysql->options.use_ssl &&
321326
(mysql->client_flag & CLIENT_SSL))
322327
{
323-
/*
324-
Send UNENCRYPTED "Login Request" packet with mysql->client_flag
325-
and max_packet_size, but no username; without this, the server
326-
does not know we want to switch to SSL/TLS
327-
328-
FIXME: Sending this packet is a very very VERY bad idea. It
329-
contains the client's preferred charset and flags in plaintext;
330-
this can be used for fingerprinting the client software version,
331-
and probable geographic location.
332-
333-
This offers a glaring opportunity for pervasive attackers to
334-
easily target, intercept, and exploit the client-server
335-
connection (e.g. "MITM all connections from known-vulnerable
336-
client versions originating from countries X, Y, and Z").
337-
*/
338-
if (ma_net_write(net, (unsigned char *)buff, (size_t) (end-buff)) || ma_net_flush(net))
328+
int ret;
329+
if (server_supports_ssl_v2)
330+
{
331+
/*
332+
The server doesn't inadvisably rely on information send in the
333+
pre-TLS-handshake packet. Send it a dummy packet that
334+
contains NOTHING except for the 2-byte client capabilities
335+
with the CLIENT_SSL bit.
336+
*/
337+
unsigned char dummy[2];
338+
int2store(dummy, CLIENT_SSL);
339+
ret= (ma_net_write(net, dummy, sizeof(dummy)) || ma_net_flush(net));
340+
}
341+
else
342+
{
343+
/*
344+
Send UNENCRYPTED "Login Request" packet with mysql->client_flag
345+
and max_packet_size, but no username; without this, the server
346+
does not know we want to switch to SSL/TLS
347+
348+
FIXME: Sending this packet is a very very VERY bad idea. It
349+
contains the client's preferred charset and flags in plaintext;
350+
this can be used for fingerprinting the client software version,
351+
and probable geographic location.
352+
353+
This offers a glaring opportunity for pervasive attackers to
354+
easily target, intercept, and exploit the client-server
355+
connection (e.g. "MITM all connections from known-vulnerable
356+
client versions originating from countries X, Y, and Z").
357+
*/
358+
ret= (ma_net_write(net, (unsigned char *)buff, (size_t) (end-buff)) || ma_net_flush(net));
359+
}
360+
361+
if (ret)
339362
{
340363
my_set_error(mysql, CR_SERVER_LOST, SQLSTATE_UNKNOWN,
341364
ER(CR_SERVER_LOST_EXTENDED),
342-
"sending connection information to server",
365+
"sending SSLRequest packet to server",
343366
errno);
344367
goto error;
345368
}

0 commit comments

Comments
 (0)