From 2ebbeb9139815757c0017e4a5a14615c33880ad4 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Wed, 14 Jun 2023 12:25:39 -0700 Subject: [PATCH] [MDEV-28634] Move check for server TLS/SSL capability to mthd_my_real_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. --- libmariadb/mariadb_lib.c | 10 ++++++++++ plugins/auth/my_auth.c | 9 --------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/libmariadb/mariadb_lib.c b/libmariadb/mariadb_lib.c index 5f922efe4..e0f33c4ba 100644 --- a/libmariadb/mariadb_lib.c +++ b/libmariadb/mariadb_lib.c @@ -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); diff --git a/plugins/auth/my_auth.c b/plugins/auth/my_auth.c index daeb3074a..991306d97 100644 --- a/plugins/auth/my_auth.c +++ b/plugins/auth/my_auth.c @@ -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)