Skip to content

Commit

Permalink
[MDEV-28634] Move check for server TLS/SSL capability to mthd_my_real…
Browse files Browse the repository at this point in the history
…_connect

Two reasons:

1. Reduction of attack surface

   As soon as the client receives the server's capability flags, it knows
   whether the server supports TLS/SSL.

   If the server does not support TLS/SSL, but the client expects and
   requires it, the client should immediately abort at this point in order
   to truncate any code paths by which it could inadvertently continue to
   communicate without TLS/SSL.

2. Separation of concerns

   Whether or not the server supports TLS/SSL encryption at the transport
   layer (TLS stands for TRANSPORT-layer security) is a logically separate
   issue from what APPLICATION-layer authentication modes the client and
   server support or should use.

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 Jun 28, 2023
1 parent 165110d commit 2ebbeb9
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
10 changes: 10 additions & 0 deletions libmariadb/mariadb_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1923,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);
Expand Down
9 changes: 0 additions & 9 deletions plugins/auth/my_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,15 +245,6 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
mysql->server_capabilities &= ~(CLIENT_SSL);
}

/* if server doesn't support SSL, we need to return an error */
if (mysql->options.use_ssl && !(mysql->server_capabilities & CLIENT_SSL))
{
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)
Expand Down

0 comments on commit 2ebbeb9

Please sign in to comment.