Hi, Dmitry, Looks pretty good! Basically no comments, some minor nitpicking below, feel free to ignore. On Oct 10, Dmitry Shulga wrote:

revision-id: 6cb526bd363 (mariadb-11.2.1-9-g6cb526bd363) parent(s): 872ed5342d8 author: Dmitry Shulga committer: Dmitry Shulga timestamp: 2023-10-03 18:07:35 +0700 message:

MDEV-32123: require_secure_transport doesn't allow TCP connections

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.

diff --git a/mysql-test/main/require_secure_transport.result b/mysql-test/main/require_secure_transport.result index 94474d0fa0e..8fa273cbae4 100644 --- a/mysql-test/main/require_secure_transport.result +++ b/mysql-test/main/require_secure_transport.result @@ -1,8 +1,13 @@ -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. +ERROR 08004: Connections using insecure transport are prohibited while --require_secure_transport=ON.

I wonder why sqlstate has changed now, as far as I remember it was the previous patch that changed it. Was the test skipped until now?

+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; +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; diff --git a/mysql-test/main/require_secure_transport.test b/mysql-test/main/require_secure_transport.test index e238e732423..f3da73a5167 100644 --- a/mysql-test/main/require_secure_transport.test +++ b/mysql-test/main/require_secure_transport.test @@ -1,15 +1,19 @@ -- source include/have_ssl_communication.inc -CREATE TABLE t1 (t int(1)); SET GLOBAL require_secure_transport=ON; --disable_query_log --error ER_SECURE_TRANSPORT_REQUIRED connect without_ssl,localhost,root,,,,,TCP NOSSL; + +connect with_ssl,localhost,root,,,,,TCP SSL; --enable_query_log

I'd suggest to remove disable/enable_query_log and let connect commmand show in the result file. It'll make them easier to read. You'll need replace_result before the failing connect to replace the port away.

+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 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;

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org