From a70d016949ee62930b4c4a4d1228cd72f997178d Mon Sep 17 00:00:00 2001 From: Roman Tkachenko Date: Tue, 14 Dec 2021 15:46:10 -0800 Subject: [PATCH] Do not parse MySQL server packets --- lib/srv/db/access_test.go | 20 ++++++++++++++++++++ lib/srv/db/mysql/engine.go | 4 ++-- lib/srv/db/mysql/test.go | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/srv/db/access_test.go b/lib/srv/db/access_test.go index 691787a64d72f..6d185866b09b7 100644 --- a/lib/srv/db/access_test.go +++ b/lib/srv/db/access_test.go @@ -337,6 +337,26 @@ func TestAccessMySQLChangeUser(t *testing.T) { require.Error(t, err) } +// TestAccessMySQLServerPacket verifies some edge-cases related to reading +// wire packets sent by the MySQL server. +func TestAccessMySQLServerPacket(t *testing.T) { + ctx := context.Background() + testCtx := setupTestContext(ctx, t, withSelfHostedMySQL("mysql")) + go testCtx.startHandlingConnections() + + // Create user/role with access permissions. + testCtx.createUserAndRole(ctx, t, "alice", "admin", []string{"alice"}, []string{types.Wildcard}) + + // Connect to the database as this user. + mysqlConn, err := testCtx.mysqlClient("alice", "mysql", "alice") + require.NoError(t, err) + + // Execute "show tables" command which will make the test server to reply + // in a way that previously would cause our packet parsing logic to fail. + _, err = mysqlConn.Execute("show tables") + require.NoError(t, err) +} + // TestAccessMongoDB verifies access scenarios to a MongoDB database based // on the configured RBAC rules. func TestAccessMongoDB(t *testing.T) { diff --git a/lib/srv/db/mysql/engine.go b/lib/srv/db/mysql/engine.go index 454217b59153b..48a5ba4e6f867 100644 --- a/lib/srv/db/mysql/engine.go +++ b/lib/srv/db/mysql/engine.go @@ -281,7 +281,7 @@ func (e *Engine) receiveFromServer(serverConn, clientConn net.Conn, serverErrCh close(serverErrCh) }() for { - packet, err := protocol.ParsePacket(serverConn) + packet, _, err := protocol.ReadPacket(serverConn) if err != nil { if utils.IsOKNetworkError(err) { log.Debug("Server connection closed.") @@ -291,7 +291,7 @@ func (e *Engine) receiveFromServer(serverConn, clientConn net.Conn, serverErrCh serverErrCh <- err return } - _, err = protocol.WritePacket(packet.Bytes(), clientConn) + _, err = protocol.WritePacket(packet, clientConn) if err != nil { log.WithError(err).Error("Failed to write client packet.") serverErrCh <- err diff --git a/lib/srv/db/mysql/test.go b/lib/srv/db/mysql/test.go index d5d21271e40fe..55f6836570077 100644 --- a/lib/srv/db/mysql/test.go +++ b/lib/srv/db/mysql/test.go @@ -198,6 +198,24 @@ type testHandler struct { func (h *testHandler) HandleQuery(query string) (*mysql.Result, error) { h.log.Debugf("Received query %q.", query) atomic.AddUint32(&h.queryCount, 1) + // When getting a "show tables" query, construct the response in a way + // which previously caused server packets parsing logic to fail. + if query == "show tables" { + resultSet, err := mysql.BuildSimpleTextResultset( + []string{"Tables_in_test"}, + [][]interface{}{ + // In raw bytes, this table name starts with 0x11 which used to + // cause server packet parsing issues since it clashed with + // COM_CHANGE_USER packet type. + {"metadata_md_table"}, + }) + if err != nil { + return nil, trace.Wrap(err) + } + return &mysql.Result{ + Resultset: resultSet, + }, nil + } return TestQueryResponse, nil }