Skip to content

Commit

Permalink
MDEV-32123: require_secure_transport doesn't allow TCP connections
Browse files Browse the repository at this point in the history
In case the option require_secure_transport is on the user can't
establish a secure ssl connection over TCP protocol. Inability to set up
a ssl session over TCP was caused by the fact that a type of client's
connection was checked before ssl handshake performed (ssl handshake
happens at the function acl_authenticate()). At that moment vio type has
the value VIO_TYPE_TCPIP for client connection that uses TCP transport.
In result, checking for allowable vio type for fails despite the fact
that SSL session being established. To fix the issue move checking of
vio type for allowable values inside the function
  parse_client_handshake_packet()
right after client's capabilities discovered that SSL is not requested
by the client.
  • Loading branch information
dmitryshulga committed Oct 11, 2023
1 parent 872ed53 commit a05b5dd
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 36 deletions.
15 changes: 12 additions & 3 deletions mysql-test/main/require_secure_transport.result
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
CREATE TABLE t1 (t int(1));
SET GLOBAL require_secure_transport=ON;
ERROR HY000: Connections using insecure transport are prohibited while --require_secure_transport=ON.
connect(localhost,root,,test,MASTER_PORT,MASTER_SOCKET);
connect without_ssl,localhost,root,,,,,TCP NOSSL;
ERROR 08004: Connections using insecure transport are prohibited while --require_secure_transport=ON.
connect with_ssl,localhost,root,,,,,TCP SSL;
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
have_ssl
1
disconnect with_ssl;
connection default;
SET GLOBAL require_secure_transport=OFF;
connect without_ssl,localhost,root,,,,,TCP NOSSL;
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
have_ssl
0
disconnect without_ssl;
connection default;
DROP TABLE t1;
15 changes: 9 additions & 6 deletions mysql-test/main/require_secure_transport.test
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
-- source include/have_ssl_communication.inc
CREATE TABLE t1 (t int(1));
SET GLOBAL require_secure_transport=ON;
--disable_query_log
--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
--error ER_SECURE_TRANSPORT_REQUIRED
connect without_ssl,localhost,root,,,,,TCP NOSSL;
--enable_query_log

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
connect with_ssl,localhost,root,,,,,TCP SSL;
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
disconnect with_ssl;

connection default;
SET GLOBAL require_secure_transport=OFF;
--disable_query_log
--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
connect without_ssl,localhost,root,,,,,TCP NOSSL;
--enable_query_log
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
disconnect without_ssl;
connection default;
DROP TABLE t1;
1 change: 1 addition & 0 deletions mysql-test/main/require_secure_transport_on.opt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--require-secure-transport=TRUE
9 changes: 9 additions & 0 deletions mysql-test/main/require_secure_transport_on.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
connect(localhost,root,,test,MASTER_PORT,MASTER_SOCKET);
connect without_ssl,localhost,root,,,,,TCP NOSSL;
ERROR 08004: Connections using insecure transport are prohibited while --require_secure_transport=ON.
connect with_ssl,localhost,root,,,,,TCP SSL;
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
have_ssl
1
disconnect with_ssl;
connection default;
14 changes: 14 additions & 0 deletions mysql-test/main/require_secure_transport_on.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--source include/not_windows.inc
--source include/have_ssl_communication.inc

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
--error ER_SECURE_TRANSPORT_REQUIRED
connect without_ssl,localhost,root,,,,,TCP NOSSL;

--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
connect with_ssl,localhost,root,,,,,TCP SSL;
SELECT (VARIABLE_VALUE <> '') AS have_ssl FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='Ssl_cipher';
disconnect with_ssl;

connection default;

44 changes: 44 additions & 0 deletions sql/sql_acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13727,6 +13727,34 @@ static bool parse_com_change_user_packet(MPVIO_EXT *mpvio, uint packet_length)
}


#ifndef EMBEDDED_LIBRARY
/**
Check that a client uses secure connection type in case the option
require_secure_transport is on.
@param thd thread handle
@return true in case the option require_secure_transport is on and the client
uses euther named pipe or unix socket or ssl, else return false
*/

static bool check_require_secured_transport(THD *thd)
{
Vio *vio= thd->net.vio;
if (opt_require_secure_transport)
{
enum enum_vio_type type= vio_type(vio);

return
(type != VIO_TYPE_SSL) &&
(type != VIO_TYPE_NAMEDPIPE) &&
(type != VIO_TYPE_SOCKET);
}
return 0;
}
#endif


/* the packet format is described in send_client_reply_packet() */
static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
uchar **buff, ulong pkt_len)
Expand Down Expand Up @@ -13796,6 +13824,22 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
return packet_error;
}
}
/*
Check whether the option require_secure_transport is on and in case
it is true that the secured connection type is used, that is either
unix socket or named pipe or ssl is in use.
*/
else if (check_require_secured_transport(thd))
{
Host_errors errors;

errors.m_ssl= 1;
inc_host_errors(mpvio->auth_info.thd->security_ctx->ip, &errors);
status_var_increment(thd->status_var.access_denied_errors);
my_error(ER_SECURE_TRANSPORT_REQUIRED, MYF(0));

return packet_error;
}

if (client_capabilities & CLIENT_PROTOCOL_41)
{
Expand Down
27 changes: 0 additions & 27 deletions sql/sql_connect.cc
Original file line number Diff line number Diff line change
Expand Up @@ -829,21 +829,6 @@ bool init_new_connection_handler_thread()
return 0;
}

static bool check_require_secured_transport(THD *thd)
{
Vio *vio= thd->net.vio;
if (opt_require_secure_transport)
{
enum enum_vio_type type= vio_type(vio);

return
(type != VIO_TYPE_SSL) &&
(type != VIO_TYPE_NAMEDPIPE) &&
(type != VIO_TYPE_SOCKET);
}
return 0;
}

/**
Set client address during authentication.
Expand Down Expand Up @@ -1095,18 +1080,6 @@ static int check_connection(THD *thd)
statistic_increment(aborted_connects_preauth, &LOCK_status);
return 1; /* The error is set by alloc(). */
}

if(check_require_secured_transport(thd))
{
Host_errors errors;
errors.m_ssl= 1;
inc_host_errors(thd->main_security_ctx.ip, &errors);
status_var_increment(thd->status_var.access_denied_errors);
my_error(ER_SECURE_TRANSPORT_REQUIRED, MYF(0));

return 1;
}

auth_rc= acl_authenticate(thd, 0);
if (auth_rc == 0 && connect_errors != 0)
{
Expand Down

0 comments on commit a05b5dd

Please sign in to comment.