diff --git a/plugins/mysqlx/include/mysqlx_session.h b/plugins/mysqlx/include/mysqlx_session.h index df07fbadc..d51696f5c 100644 --- a/plugins/mysqlx/include/mysqlx_session.h +++ b/plugins/mysqlx/include/mysqlx_session.h @@ -279,6 +279,16 @@ private: std::vector compress_batch_framed_; uint32_t compress_batch_count_; + // Pre-auth capability-message replay counter. A hostile or buggy + // client can replay CapabilitiesGet / CapabilitiesSet arbitrarily + // many times before authenticating; each one runs through protobuf + // parsing and a small allocation. Bound the count per session so + // the path is not a free CPU/memory amplifier. Reset to zero on + // successful auth; bumps on every CapabilitiesGet/Set seen while + // status_ is still pre-auth (CONNECTING_CLIENT / X_CAPABILITIES_*). + uint32_t pre_auth_cap_msgs_; + static constexpr uint32_t MAX_PRE_AUTH_CAP_MSGS = 64; + public: // Test-only accessors for compression negotiation outcome. MysqlxCompressionAlgo compression_algo_for_test() const { return compression_algo_; } diff --git a/plugins/mysqlx/src/mysqlx_connection.cpp b/plugins/mysqlx/src/mysqlx_connection.cpp index 78f9af61b..2cefda975 100644 --- a/plugins/mysqlx/src/mysqlx_connection.cpp +++ b/plugins/mysqlx/src/mysqlx_connection.cpp @@ -140,10 +140,23 @@ std::optional MysqlxConnection::read_auth_frame() { // emit a session-state-change notice before AuthenticateContinue / Ok; // returning nullopt on a NOTICE without re-trying caused the auth state // machine to spin until the 10s handshake timeout. + // + // Cap the NOTICE drain so a misbehaving or hostile backend can't pin + // this worker thread by streaming NOTICEs forever. 64 is more than any + // legitimate backend would emit during auth — typical is 1-2. + constexpr int MAX_LEADING_NOTICES = 64; + int notice_count = 0; while (true) { auto frame = backend_ds_.try_read_one_frame(); if (!frame) return std::nullopt; if (frame->size() >= 5 && (*frame)[4] == Mysqlx::ServerMessages_Type_NOTICE) { + if (++notice_count > MAX_LEADING_NOTICES) { + // Treat as auth failure; the caller will mark the session + // unhealthy and the chassis will return the connection to + // the pool / drop it. + auth_state_ = BACKEND_AUTH_ERROR; + return std::nullopt; + } continue; } return frame; diff --git a/plugins/mysqlx/src/mysqlx_data_stream.cpp b/plugins/mysqlx/src/mysqlx_data_stream.cpp index 24ec8c539..f281a6ea5 100644 --- a/plugins/mysqlx/src/mysqlx_data_stream.cpp +++ b/plugins/mysqlx/src/mysqlx_data_stream.cpp @@ -135,7 +135,19 @@ void MysqlxDataStream::pop_frame() { } void MysqlxDataStream::enqueue_frame(uint8_t msg_type, const uint8_t* body, size_t body_len) { - if (body_len + 1 > X_MAX_PAYLOAD_SIZE) return; + if (body_len + 1 > X_MAX_PAYLOAD_SIZE) { + // Server attempted to enqueue a frame larger than the X-Protocol + // 16 MiB cap. Silently dropping the body would leave the client + // expecting a frame that never arrives — usually a protocol + // stall. Mark the parser as broken so the session loop tears + // down cleanly instead of half-working. The caller (always a + // server-side path: send_error, send_capabilities, compression + // emit) is responsible for never feeding bodies anywhere near + // this size; reaching here means an internal bug, not a + // protocol-level event. + parse_error_ = true; + return; + } uint32_t payload_size = static_cast(body_len) + 1; write_buf_.push_back(static_cast(payload_size & 0xFF)); write_buf_.push_back(static_cast((payload_size >> 8) & 0xFF)); diff --git a/plugins/mysqlx/src/mysqlx_session.cpp b/plugins/mysqlx/src/mysqlx_session.cpp index d96421b9b..1ad39979c 100644 --- a/plugins/mysqlx/src/mysqlx_session.cpp +++ b/plugins/mysqlx/src/mysqlx_session.cpp @@ -104,7 +104,8 @@ MysqlxSession::MysqlxSession() , compression_max_combine_messages_(0) , zstd_dctx_(nullptr) , zstd_cctx_(nullptr) - , compress_batch_count_(0) { + , compress_batch_count_(0) + , pre_auth_cap_msgs_(0) { } MysqlxSession::~MysqlxSession() { @@ -167,6 +168,7 @@ void MysqlxSession::reset() { compression_combine_mixed_messages_ = false; compression_max_combine_messages_ = 0; reset_compression_state(); + pre_auth_cap_msgs_ = 0; } int MysqlxSession::handler() { @@ -265,6 +267,22 @@ void MysqlxSession::handler_connecting_client() { void MysqlxSession::handler_capabilities_get() { if (!client_ds_.has_complete_frame()) return; + // Bound how many pre-auth capability messages a client can ship per + // session. Each one runs through frame parsing + send_capabilities() + // allocations; without a cap, an idle hostile client can pin a + // worker on the cap-replay path forever. The counter applies only + // while still pre-auth; post-auth callers (dispatch_client_message + // → CON_CAPABILITIES_GET) routinely query capabilities and should + // not trip the bound. status_ == WAITING_CLIENT_XMSG indicates auth + // completed. + if (status_ != WAITING_CLIENT_XMSG && + ++pre_auth_cap_msgs_ > MAX_PRE_AUTH_CAP_MSGS) { + client_ds_.pop_frame(); + send_error(5008, "Too many pre-auth capability messages", true); + healthy = false; + return; + } + client_ds_.pop_frame(); send_capabilities(); status_ = CONNECTING_CLIENT; @@ -352,6 +370,16 @@ static bool parse_compression_capability( void MysqlxSession::handler_capabilities_set() { if (!client_ds_.has_complete_frame()) return; + // Same per-session bound as handler_capabilities_get(); see comment + // there. Counter only applies pre-auth. + if (status_ != WAITING_CLIENT_XMSG && + ++pre_auth_cap_msgs_ > MAX_PRE_AUTH_CAP_MSGS) { + client_ds_.pop_frame(); + send_error(5008, "Too many pre-auth capability messages", true); + healthy = false; + return; + } + const auto& frame = client_ds_.front_frame(); if (frame.size() > 5) { Mysqlx::Connection::CapabilitiesSet cap_set; @@ -398,6 +426,26 @@ void MysqlxSession::handler_capabilities_set() { for (const auto& cap : cap_set.capabilities().capabilities()) { if (cap.name() == "tls") { client_ds_.pop_frame(); + // Reject TLS upgrade post-auth or on an already-encrypted + // channel. The X Protocol forbids TLS negotiation after + // AuthenticateOk, and a second tls=true on an already-TLS + // channel would desync the state machine (the server + // expects a fresh handshake; the client expects to keep + // the existing TLS session). Either case is a hostile or + // confused client; respond with 5052 and drop the + // session before driving SSL_do_handshake. + if (client_ds_.is_encrypted()) { + send_error(5052, "TLS already negotiated on this session", true); + healthy = false; + return; + } + if (status_ != CONNECTING_CLIENT && + status_ != X_CAPABILITIES_GET && + status_ != X_CAPABILITIES_SET) { + send_error(5052, "TLS negotiation not allowed after authentication", true); + healthy = false; + return; + } SSL_CTX* ctx = thread_ptr_ ? thread_ptr_->get_ssl_ctx() : nullptr; if (!ctx) { send_error(3150, "TLS is not configured on server");