aherman
/
proxysql
mirror of https://github.com/sysown/proxysql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(mysqlx): preserve backend TLS state past auth handshake

Browse Source 
What:
  Stop rewrapping the raw backend fd in a fresh session-owned
  MysqlxDataStream after backend auth completes. Route every
  data-plane read/write (forward_to_backend, handler_waiting_server_msg,
  handler_session_reset_waiting, and the cached-conn attach path)
  through MysqlxConnection::backend_ds() instead.

Why:
  The optional backend TLS handshake runs against the connection's
  backend_ds_ (see mysqlx_connection.cpp init_ssl_connect) and the
  resulting SSL* lives there. The prior session code discarded that
  SSL* by init()-ing a new plain MysqlxDataStream around the same
  raw fd, then used it for cleartext I/O on a socket where the
  server expects TLS frames. Flagged in the ProtocolX code review
  (item #2).

Testing:
  mysqlx_robustness_unit-t picks up five new assertions that pin
  the invariant: MysqlxSession::server_ds() aliases
  backend_conn_->backend_ds() whenever a backend is attached, and
  falls back to a fd == -1 placeholder otherwise. Baseline 33,
  post-fix 38, all pass.

Out of scope:
  - Connection cache semantics: cached connections keep their
    SSL state; MysqlxConnection::reset() already leaves
    backend_ds_ untouched so the invariant extends to pooled reuse.
  - MysqlxDataStream internals (untouched).
  - The dormant worker path (different PR).

Pre-existing ripple:
  test/tap/tests/unit/Makefile mysqlx_robustness_unit-t link line
  was missing mysqlx_config_store.cpp, same gap other mysqlx tests
  have been closing as they get touched. Added it here so the
  test links.
fix/mysqlx-backend-tls-post-auth
Rene Cannao 1 month ago
parent 923cbfeadc
commit 3e8c3da9de
4 changed files with 132 additions and 35 deletions
Show Stats Download Patch File Download Diff File

18
plugins/mysqlx/include/mysqlx_session.h
Unescape View File

@ -78,7 +78,15 @@ public:
int get_fd() const { return client_ds_.get_fd(); }
MysqlxDataStream& client_ds() { return client_ds_; }
MysqlxDataStream& server_ds() { return server_ds_; }
// Session-level accessor for the backend data stream. When a backend
// connection is attached, this proxies to MysqlxConnection::backend_ds()
// so the SSL* established during the optional backend TLS handshake is
// preserved for the rest of the session. Falls back to an uninitialized
// placeholder (fd == -1) when no backend is attached so that pollers and
// tests can safely query get_fd()/get_status() without crashing.
MysqlxDataStream& server_ds() {
return backend_conn_ ? backend_conn_->backend_ds() : server_ds_placeholder_;
}
MysqlxConnection*& backend_conn() { return backend_conn_; }
void set_credential_lookup(MysqlxCredentialLookup lookup) { credential_lookup_ = lookup; }
@ -123,7 +131,13 @@ private:
bool is_terminal_for_state(uint8_t msg_type) const;
MysqlxDataStream client_ds_;
MysqlxDataStream server_ds_;
// Placeholder stream returned by server_ds() when no backend connection
// is attached. Intentionally never init()'d during the data-plane phase:
// the real backend stream lives on MysqlxConnection::backend_ds_, which
// owns the SSL* from the optional backend TLS handshake. Rewrapping the
// raw fd here after auth would discard that SSL* and silently regress
// TLS-wrapped sessions to cleartext I/O.
MysqlxDataStream server_ds_placeholder_;
MysqlxConnection* backend_conn_;
Mysqlx_Thread* thread_ptr_;
Status status_;

82
plugins/mysqlx/src/mysqlx_session.cpp
Unescape View File

@ -450,26 +450,29 @@ void MysqlxSession::handler_waiting_client_msg() {
}
void MysqlxSession::forward_to_backend() {
if (server_ds_.get_status() != XDS_READY) {
if (!backend_conn_ || backend_conn_->get_state() != MysqlxConnection::IDLE) {
status_ = CONNECTING_SERVER;
to_process = true;
return;
}
server_ds_.init(XDS_BACKEND, backend_conn_->get_fd());
if (!backend_conn_ || backend_conn_->get_state() != MysqlxConnection::IDLE) {
status_ = CONNECTING_SERVER;
to_process = true;
return;
}
// Data-plane reads/writes must go through MysqlxConnection::backend_ds(),
// which owns the SSL* established during the optional backend TLS
// handshake. Wrapping backend_conn_->get_fd() in a fresh MysqlxDataStream
// here would discard that SSL* and silently fall back to cleartext I/O.
MysqlxDataStream& be_ds = backend_conn_->backend_ds();
if (client_ds_.has_complete_frame()) {
const auto& frame = client_ds_.front_frame();
if (frame.size() > 5) {
server_ds_.enqueue_frame(frame[4], frame.data() + 5, frame.size() - 5);
be_ds.enqueue_frame(frame[4], frame.data() + 5, frame.size() - 5);
} else {
server_ds_.enqueue_frame(frame[4], nullptr, 0);
be_ds.enqueue_frame(frame[4], nullptr, 0);
}
client_ds_.pop_frame();
}
server_ds_.write_to_net();
be_ds.write_to_net();
status_ = WAITING_SERVER_XMSG;
}
@ -518,14 +521,16 @@ bool MysqlxSession::is_terminal_for_state(uint8_t msg_type) const {
}
void MysqlxSession::handler_waiting_server_msg() {
if (server_ds_.get_fd() < 0) {
// Go through backend_conn_->backend_ds() so TLS state is preserved.
if (!backend_conn_ || backend_conn_->backend_ds().get_fd() < 0) {
return_backend_to_pool();
status_ = WAITING_CLIENT_XMSG;
to_process = true;
return;
}
MysqlxDataStream& be_ds = backend_conn_->backend_ds();
ssize_t r = server_ds_.read_from_net();
ssize_t r = be_ds.read_from_net();
if (r == 0) {
send_error(2013, "Lost connection to backend during query");
return_backend_to_pool();
@ -540,12 +545,12 @@ void MysqlxSession::handler_waiting_server_msg() {
}
bool got_terminal = false;
while (server_ds_.has_complete_frame()) {
const auto& frame = server_ds_.front_frame();
while (be_ds.has_complete_frame()) {
const auto& frame = be_ds.front_frame();
uint8_t msg_type = frame[4];
forward_frame_to_client(msg_type, frame);
server_ds_.pop_frame();
be_ds.pop_frame();
if (msg_type != Mysqlx::ServerMessages_Type_NOTICE &&
is_terminal_for_state(msg_type)) {
@ -567,14 +572,16 @@ void MysqlxSession::handler_fast_forward() {
}
void MysqlxSession::handler_session_reset_waiting() {
if (server_ds_.get_fd() < 0) {
// Go through backend_conn_->backend_ds() so TLS state is preserved.
if (!backend_conn_ || backend_conn_->backend_ds().get_fd() < 0) {
return_backend_to_pool();
status_ = WAITING_CLIENT_XMSG;
to_process = true;
return;
}
MysqlxDataStream& be_ds = backend_conn_->backend_ds();
ssize_t r = server_ds_.read_from_net();
ssize_t r = be_ds.read_from_net();
if (r == 0 || (r < 0 && errno != EAGAIN && errno != EWOULDBLOCK)) {
return_backend_to_pool();
status_ = WAITING_CLIENT_XMSG;
@ -582,22 +589,20 @@ void MysqlxSession::handler_session_reset_waiting() {
return;
}
while (server_ds_.has_complete_frame()) {
const auto& frame = server_ds_.front_frame();
while (be_ds.has_complete_frame()) {
const auto& frame = be_ds.front_frame();
uint8_t msg_type = frame[4];
if (msg_type == Mysqlx::ServerMessages_Type_NOTICE) {
forward_frame_to_client(msg_type, frame);
server_ds_.pop_frame();
be_ds.pop_frame();
continue;
}
if (msg_type == Mysqlx::ServerMessages_Type_OK) {
server_ds_.pop_frame();
if (backend_conn_) {
backend_conn_->set_has_prepared_statement(false);
backend_conn_->set_in_transaction(false);
}
be_ds.pop_frame();
backend_conn_->set_has_prepared_statement(false);
backend_conn_->set_in_transaction(false);
return_backend_to_pool();
last_active_time_ = monotonic_time_ms();
status_ = WAITING_CLIENT_XMSG;
@ -607,7 +612,7 @@ void MysqlxSession::handler_session_reset_waiting() {
if (msg_type == Mysqlx::ServerMessages_Type_ERROR) {
forward_frame_to_client(msg_type, frame);
server_ds_.pop_frame();
be_ds.pop_frame();
client_ds_.write_to_net();
return_backend_to_pool();
status_ = WAITING_CLIENT_XMSG;
@ -615,7 +620,7 @@ void MysqlxSession::handler_session_reset_waiting() {
return;
}
server_ds_.pop_frame();
be_ds.pop_frame();
}
}
@ -668,7 +673,10 @@ void MysqlxSession::handler_connecting_server() {
}
if (backend_conn_) {
server_ds_.init(XDS_BACKEND, backend_conn_->get_fd());
// Cached connection already has its MysqlxDataStream
// (backend_conn_->backend_ds()) initialized, including any
// SSL* from the original handshake. Do NOT rewrap the raw fd
// here: that would bypass the TLS-aware stream entirely.
status_ = WAITING_CLIENT_XMSG;
to_process = true;
return;
@ -740,7 +748,14 @@ void MysqlxSession::handler_connecting_server() {
}
}
server_ds_.init(XDS_BACKEND, backend_conn_->get_fd());
// Intentionally do NOT reinitialize a session-owned MysqlxDataStream
// around backend_conn_->get_fd() here. The backend auth path, including
// the optional TLS handshake, runs against backend_conn_->backend_ds(),
// which holds the SSL* for the session's entire lifetime. Wrapping the
// raw fd in a fresh plain stream would discard that SSL* and cause
// cleartext I/O on a TLS-wrapped socket. All data-plane code paths
// below (forward_to_backend, handler_waiting_server_msg, etc.) must go
// through backend_conn_->backend_ds() instead.
backend_conn_->set_state(MysqlxConnection::IDLE);
backend_conn_->set_reusable(true);
status_ = WAITING_CLIENT_XMSG;
@ -750,12 +765,19 @@ void MysqlxSession::handler_connecting_server() {
void MysqlxSession::return_backend_to_pool() {
if (!backend_conn_) return;
if (thread_ptr_) {
// The cached connection retains its MysqlxDataStream (including
// the SSL* established during the optional backend TLS handshake),
// so a subsequent session can resume using it without tearing the
// TLS session down. MysqlxConnection::reset() intentionally leaves
// backend_ds_ untouched. Do NOT reset/destroy the SSL state here.
thread_ptr_->return_connection_to_cache(backend_conn_);
} else {
delete backend_conn_;
}
backend_conn_ = nullptr;
server_ds_ = MysqlxDataStream();
// The session-owned placeholder carries no data-plane state and does
// not need to be reset. server_ds() will again return the placeholder
// (with fd == -1) until another backend is attached.
}
void MysqlxSession::send_error(int code, const char* msg, bool fatal) {

4
test/tap/tests/unit/Makefile
Unescape View File

@ -524,8 +524,8 @@ mysqlx_credential_verify_unit-t: mysqlx_credential_verify_unit-t.cpp $(PROXYSQL_
$(IDIRS) $(LDIRS) $(OPT) $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) \
$(MYLIBS) -lprotobuf -lssl -lcrypto $(ALLOW_MULTI_DEF) -o $@
mysqlx_robustness_unit-t: mysqlx_robustness_unit-t.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_session.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_data_stream.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_connection.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_thread.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_protocol.cpp $(MYSQLX_PROTO_OBJS) $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR)
$(CXX) $< $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_session.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_data_stream.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_connection.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_thread.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_protocol.cpp $(MYSQLX_PROTO_OBJS) $(TEST_HELPERS_OBJ) \
mysqlx_robustness_unit-t: mysqlx_robustness_unit-t.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_session.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_data_stream.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_connection.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_thread.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_protocol.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_config_store.cpp $(MYSQLX_PROTO_OBJS) $(TEST_HELPERS_OBJ) $(LIBPROXYSQLAR)
$(CXX) $< $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_session.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_data_stream.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_connection.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_thread.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_protocol.cpp $(PROXYSQL_PATH)/plugins/mysqlx/src/mysqlx_config_store.cpp $(MYSQLX_PROTO_OBJS) $(TEST_HELPERS_OBJ) \
-I$(PROXYSQL_PATH)/plugins/mysqlx/include -I$(MYSQLX_PROTO_DIR) \
$(IDIRS) $(LDIRS) $(OPT) $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) \
$(MYLIBS) -lprotobuf -lssl -lcrypto $(ALLOW_MULTI_DEF) -lpthread -o $@

63
test/tap/tests/unit/mysqlx_robustness_unit-t.cpp
Unescape View File

@ -525,6 +525,66 @@ static void test_client_disconnect_detected() {
close(fds[0]);
}
// Regression guard: after backend auth completes, the data-plane reads/writes
// must route through backend_conn_->backend_ds(), not a session-owned stream
// re-wrapped around the raw backend fd. The TLS handshake (when backend TLS
// is required) runs against backend_conn_->backend_ds() and the resulting
// SSL* lives on that stream; rewrapping the fd would discard it and cause
// cleartext I/O on a TLS-expecting socket.
//
// We can't cheaply drive a full TLS handshake inside a unit test, so we
// instead assert the structural invariant: MysqlxSession::server_ds() is an
// alias for MysqlxConnection::backend_ds() whenever a backend is attached,
// and the session's placeholder stream is never initialized post-auth.
static void test_server_ds_aliases_backend_conn_backend_ds() {
int client_fds[2], backend_fds[2];
socketpair(AF_UNIX, SOCK_STREAM, 0, client_fds);
socketpair(AF_UNIX, SOCK_STREAM, 0, backend_fds);
MysqlxSession sess;
setup_authenticated_session(client_fds, sess);
// With no backend attached yet, server_ds() returns the session's
// placeholder: fd == -1 and status never set to something data-plane
// readers would treat as live.
ok(sess.backend_conn() == nullptr,
"pre-attach: no backend connection is associated with the session");
ok(sess.server_ds().get_fd() == -1,
"pre-attach: server_ds() placeholder has fd == -1");
// Attach a backend and initialize its backend_ds() (as the real auth
// path does via MysqlxConnection::init_backend_ds()).
MysqlxConnection* conn = new MysqlxConnection();
conn->set_fd(backend_fds[0]);
conn->set_state(MysqlxConnection::IDLE);
conn->set_reusable(true);
conn->init_backend_ds(backend_fds[0]);
sess.backend_conn() = conn;
// The critical invariant: server_ds() now aliases backend_conn_->backend_ds().
// If this ever regresses to wrapping the raw fd in a fresh stream, the
// TLS state living on backend_conn_->backend_ds_ would be bypassed.
ok(&sess.server_ds() == &sess.backend_conn()->backend_ds(),
"post-attach: server_ds() aliases backend_conn_->backend_ds() (TLS state preserved)");
ok(sess.server_ds().get_fd() == backend_fds[0],
"post-attach: server_ds().get_fd() reflects the backend fd");
// return_backend_to_pool (via session close) must not tear down the
// TLS-aware stream on the cached connection. Driving this path via the
// session handler while the backend connection is null would invoke
// return_backend_to_pool(); here we simply verify that after clearing
// the backend pointer the placeholder is once again what server_ds()
// returns and its fd is still -1.
delete conn;
sess.backend_conn() = nullptr;
ok(sess.server_ds().get_fd() == -1,
"post-detach: server_ds() placeholder has fd == -1 (never re-initialized)");
detach_session_fds(sess);
close(client_fds[0]); close(client_fds[1]);
close(backend_fds[0]); close(backend_fds[1]);
}
static void test_forward_empty_frame() {
int client_fds[2], backend_fds[2];
socketpair(AF_UNIX, SOCK_STREAM, 0, client_fds);
@ -567,7 +627,7 @@ static void test_forward_empty_frame() {
}
int main() {
plan(33);
plan(38);
test_server_response_terminal_frame();
test_server_response_non_terminal_keeps_waiting();
@ -581,6 +641,7 @@ int main() {
test_return_backend_no_thread();
test_connection_limit_config();
test_client_disconnect_detected();
test_server_ds_aliases_backend_conn_backend_ds();
test_forward_empty_frame();
return exit_status();

Write Preview
Loading…
Cancel
Save