Skip to content

Commit d89589b

Browse files
committed
[CONC-654, MDEV-31585] Ensure that client is not downgraded by an active MITM
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 3323e90 commit d89589b

File tree

1 file changed

+37
-5
lines changed

1 file changed

+37
-5
lines changed

sql/sql_acl.cc

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13255,18 +13255,29 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
1325513255
DBUG_ASSERT(net->read_pos[pkt_len] == 0);
1325613256

1325713257
ushort first_two_bytes_of_client_capabilities= uint2korr(net->read_pos);
13258-
if (first_two_bytes_of_client_capabilities & CLIENT_SSL)
13258+
bool pre_tls_client_packet_is_dummy= (pkt_len==2 && first_two_bytes_of_client_capabilities == CLIENT_SSL);
13259+
bool pre_tls_client_packet_wants_ssl= first_two_bytes_of_client_capabilities & CLIENT_SSL;
13260+
13261+
if (pre_tls_client_packet_wants_ssl)
1325913262
{
1326013263
/* Client wants to use TLS. This packet is really just a
1326113264
* dummy which gets sent before the TLS handshake, in order
1326213265
* to trigger the server (us) to start the TLS handshake.
1326313266
*
13264-
* We should ignore everything else in this packet until
13265-
* after the TLS handshake.
13267+
* We ignore everything else in this pre-TLS packet, even though
13268+
* older clients send much of the same information that they will
13269+
* re-send over the TLS channel.
13270+
*
13271+
* This pre-TLS packet is untrustworthy AND if the server acts on
13272+
* its content, that FORCES the client to send more information
13273+
* in the clear.
1326613274
*/
1326713275

1326813276
unsigned long errptr __attribute__((unused));
13269-
DBUG_PRINT("info", ("client capabilities have TLS/SSL bit set"));
13277+
if (pre_tls_client_packet_is_dummy)
13278+
DBUG_PRINT("info", ("client sent 2-byte dummy packet with only the TLS/SSL bit set"));
13279+
else
13280+
DBUG_PRINT("info", ("client capabilities have TLS/SSL bit set"));
1327013281

1327113282
/* Do the SSL layering. */
1327213283
if (!ssl_acceptor_fd)
@@ -13300,7 +13311,7 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
1330013311
DBUG_RETURN(packet_error);
1330113312
}
1330213313

13303-
/* Re-load the FIRST TWO BYTES of the client handshake packet */
13314+
/* Re-load the FIRST TWO BYTES of the capabilities from the packet sent over TLS. */
1330413315
first_two_bytes_of_client_capabilities = uint2korr(net->read_pos);
1330513316
}
1330613317

@@ -13321,6 +13332,27 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
1332113332
client_capabilities|= ext_client_capabilities;
1332213333
}
1332313334
}
13335+
bool post_tls_client_packet_indicates_dummy_support= (client_capabilities & CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET);
13336+
13337+
if (pre_tls_client_packet_wants_ssl
13338+
&& post_tls_client_packet_indicates_dummy_support
13339+
&& !pre_tls_client_packet_is_dummy)
13340+
{
13341+
/* 1. We told the client in our server greeting that we support the pre-TLS client packet containing only the TLS/SSL flag.
13342+
* [Our server greeting packet is sent in the clear; no guarantee that it is delivered unmodified to the client.]
13343+
* 2. The client told us in its pre-TLS packet that it wants to use SSL.
13344+
* 3. The client told us in its post-TLS packet that it knows how to send the pre-TLS client packet containing only the TLS/SSL flag.
13345+
* [We received this information via TLS; it is authentic.]
13346+
* 4. Nevertheless, the client DID NOT SEND a pre-TLS client packet containing only the TLS/SSL flag.
13347+
*
13348+
* The only way this can happen is if the client is being downgraded by an active MITM attacker which
13349+
* disables the CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET bit in our server greeting packet.
13350+
*/
13351+
sql_print_warning("Aborting connection %lld because it is being actively MITM'ed to downgrade TLS security (attacker "
13352+
"is stripping the CLIENT_CAN_SEND_DUMMY_HANDSHAKE_PACKET bit from our server capabilities)",
13353+
thd->thread_id);
13354+
DBUG_RETURN(packet_error);
13355+
}
1332413356

1332513357
/* Disable those bits which are not supported by the client. */
1332613358
compile_time_assert(sizeof(thd->client_capabilities) >= 8);

0 commit comments

Comments
 (0)