From 4f2a0e8eeb0eccdb596a56167626d1561a0e4d70 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 1 Jul 2024 11:50:09 +0200 Subject: [PATCH] Fix possible issues with MySQL server protocol TLS connections The problem here is that retries (in SecureSocketImpl.cpp::mustRetry()) relies on non-zero socket timeout, but MySQL handler does not set timeouts for the socket (like other does), and this leads to a problem when OpenSSL returns SSL_ERROR_WANT_READ/SSL_ERROR_WANT_WRITE, the connection will be simply terminated. I've played with this patch, by hacking the openssl sources: diff --git a/crypto/bio/bss_sock.c b/crypto/bio/bss_sock.c index 82f7be85ae..a399291ff4 100644 --- a/crypto/bio/bss_sock.c +++ b/crypto/bio/bss_sock.c @@ -124,7 +125,18 @@ static int sock_read(BIO *b, char *out, int outl) ret = ktls_read_record(b->num, out, outl); else # endif - ret = readsocket(b->num, out, outl); + { + /* pthread_kill(pthread_self(), SIGUSR1); */ + static int i = 0; + if (!(++i % 2)) + { + fprintf(stderr, "sock_read: inject EAGAIN\n"); + ret = -1; + errno = EAGAIN; + } + else + ret = readsocket(b->num, out, outl); + } BIO_clear_retry_flags(b); if (ret <= 0) { if (BIO_sock_should_retry(ret)) And after this patch this succeed without errors: ch benchmark -c10 -q "SELECT * FROM mysql('127.0.0.1:9004', system, one, 'default', '', SETTINGS connection_pool_size=1, connect_timeout = 100, connection_wait_timeout = 100)" Note, that this also fixes the timeouts for plain (non-TLS) connections Signed-off-by: Azat Khuzhin --- src/Server/MySQLHandler.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Server/MySQLHandler.cpp b/src/Server/MySQLHandler.cpp index 9471509ad4ba..c0f015bfcd5b 100644 --- a/src/Server/MySQLHandler.cpp +++ b/src/Server/MySQLHandler.cpp @@ -204,6 +204,10 @@ void MySQLHandler::run() session->setClientConnectionId(connection_id); + const Settings & settings = server.context()->getSettingsRef(); + socket().setReceiveTimeout(settings.receive_timeout); + socket().setSendTimeout(settings.send_timeout); + in = std::make_shared(socket(), read_event); out = std::make_shared(socket(), write_event); packet_endpoint = std::make_shared(*in, *out, sequence_id); @@ -451,6 +455,7 @@ void MySQLHandler::comQuery(ReadBuffer & payload, bool binary_protocol) // Settings replacements if (!should_replace) + { for (auto const & [mysql_setting, clickhouse_setting] : settings_replacements) { const auto replacement_query_opt = setSettingReplacementQuery(query, mysql_setting, clickhouse_setting); @@ -461,6 +466,7 @@ void MySQLHandler::comQuery(ReadBuffer & payload, bool binary_protocol) break; } } + } auto query_context = session->makeQueryContext(); query_context->setCurrentQueryId(fmt::format("mysql:{}:{}", connection_id, toString(UUIDHelpers::generateV4()))); @@ -470,6 +476,10 @@ void MySQLHandler::comQuery(ReadBuffer & payload, bool binary_protocol) settings.prefer_column_name_to_alias = true; query_context->setSettings(settings); + /// Update timeouts + socket().setReceiveTimeout(settings.receive_timeout); + socket().setSendTimeout(settings.send_timeout); + CurrentThread::QueryScope query_scope{query_context}; std::atomic affected_rows {0}; @@ -643,7 +653,11 @@ void MySQLHandlerSSL::finishHandshakeSSL( client_capabilities = ssl_request.capability_flags; max_packet_size = ssl_request.max_packet_size ? ssl_request.max_packet_size : MAX_PACKET_LENGTH; secure_connection = true; + ss = std::make_shared(SecureStreamSocket::attach(socket(), SSLManager::instance().defaultServerContext())); + ss->setReceiveTimeout(socket().getReceiveTimeout()); + ss->setSendTimeout(socket().getSendTimeout()); + in = std::make_shared(*ss); out = std::make_shared(*ss); sequence_id = 2;