Enforce access control for MySQL COM_FIELD_LIST and COM_INIT_DB#108508
Enforce access control for MySQL COM_FIELD_LIST and COM_INIT_DB#108508groeneai wants to merge 4 commits into
Conversation
The MySQL wire protocol handler executes two metadata commands that skipped the access control enforced by their SQL equivalents: - COM_FIELD_LIST (mysql_list_fields) emitted every column name of a table with no check, while DESCRIBE / SHOW COLUMNS require SHOW_COLUMNS. - COM_INIT_DB (USE database) set the current database with no check, while the USE statement requires SHOW_DATABASES. The same omission existed for the database taken from the client handshake response. A user could therefore read column names of tables they only hold partial column grants on, and switch into databases they have no grants on, over the MySQL protocol. Add the same checks the SQL statements perform. The SHOW_COLUMNS check is done before getTable() so the command does not become a table-existence oracle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
cc @vitlibar @yakov-olkhovskiy — could you review this? It adds the missing access-control checks to the MySQL wire protocol metadata commands: |
alexey-milovidov
added
the
can be tested
|
Workflow [PR], commit [88795dc] Summary: ❌
AI ReviewSummaryThis PR adds access checks to the MySQL protocol metadata commands: PR Metadata
Missing context / blind spots
Final VerdictStatus: ✅ Approve |
clickhouse-gh
Bot
added
the
pr-bugfix
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
The handshake-time SHOW_DATABASES check threw on denial, but the surrounding setup try/catch sent an ERR packet and then fell through to the normal handshake OK packet and the command loop. A client that ignores the error therefore still proceeded on an authenticated session with an unauthorized initial database (two terminal handshake packets, ERR followed by OK). Return after sending the setup error so any session-setup failure (including the access denial) cleanly fails the handshake: no OK packet, no command loop. sendPacket flushes by default, so the ERR has already reached the client before run() returns and SCOPE_EXIT closes the connection. comInitDB and comFieldList denials are unaffected: they throw inside the command loop, whose catch sends a per-command ERR and keeps the connection open, matching real MySQL behaviour for a denied USE/DESCRIBE. Extend the regression test with a raw-socket client that drives the handshake with a denied initial database and asserts the server does not send an OK or leave a usable session after the error, plus a sanity check that an allowed database still completes the handshake. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Fixed in a67121e. The handshake-setup Added a regression that drives the handshake over a raw socket with Session id: cron:clickhouse-worker-slot-6:20260625-164900 |
The previous commit added a SHOW_DATABASES check on the database selected in
the MySQL handshake (CLIENT_CONNECT_WITH_DB) and returned from run() on denial.
That made the MySQL protocol the only one that access-checks the connection-time
database: native TCP (TCPHandler), HTTP (HTTPHandler) and PostgreSQL
(PostgreSQLHandler) all setCurrentDatabase() for the connect-time database with
no SHOW_DATABASES check. Only the explicit USE statement / COM_INIT_DB is checked
(InterpreterUseQuery). As a result a user with no grants could no longer connect
over MySQL to the default database, which broke test_session_log::test_mysql_session
(a freshly created, ungranted user connecting to default and running SELECT 1).
Revert the handshake check and the early return so the handshake database matches
every other protocol. This does not reintroduce a disclosure: COM_FIELD_LIST is
access-checked against the current database regardless of how it was set, so
connecting to e.g. system in the handshake still cannot list a system table the
user lacks SHOW COLUMNS on. The COM_INIT_DB (SHOW_DATABASES) and COM_FIELD_LIST
(SHOW_COLUMNS) checks are kept.
Tests:
- test_mysql_metadata_commands_access_control: the default user in this test's
users.xml has password 123, so the server-side setup queries now pass
settings={"password": "123"} (they failed native auth before).
- Replace test_mysql_handshake_database_access_control with
test_mysql_handshake_database_does_not_leak_columns: a low-privilege user can
open a handshake to system (parity with other protocols), but COM_FIELD_LIST on
a system table is still rejected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Pushed The handshake initial database ( Removing the handshake check does not reintroduce the disclosure: The handshake regression test is replaced accordingly: Local validation: full Session id: cron:clickhouse-worker-slot-2:20260625-212600 |
CI finish ledger — adfe4b4Every failure below has an owner: a fixing PR (ours or external), or a full-effort fix task whose fixing-PR link will be posted here when it opens. Only
Neither failure is in this PR's changed files ( Session id: cron:our-pr-ci-monitor:20260626-023000 |
LLVM Coverage Report
Changed lines: Changed C/C++ lines covered by tests: 13/13 (100.00%) | Lost baseline coverage: none · Uncovered code |
CI finish ledger — 88795dcRe-validated after merging master (474a3c1, which contains the merged TSAN fix #108391) into this branch. The two failures from the prior ledger (
PR diff is Session id: cron:our-pr-ci-monitor:20260626-070000 |
2 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
The MySQL wire protocol commands
COM_FIELD_LIST(mysql_list_fields) andCOM_INIT_DB(USE database) now enforce the same access control as their SQL equivalents (SHOW COLUMNS/DESCRIBEandUSE). Previously they could disclose column names of tables the user only had partial column grants on, and switch the current database without theSHOW DATABASESprivilege.Description
src/Server/MySQLHandler.cppexecutes two metadata commands on the MySQL protocol that bypassed access control that the equivalent SQL statements enforce (comQueryroutes throughexecuteQuery, where ACL is checked; these did not):comFieldList(COM_FIELD_LIST, opcode0x04) read table metadata viaDatabaseCatalog::getTable(...)and emitted every column name with no check. The SQL equivalentDESCRIBE/SHOW COLUMNSis access-checked inInterpreterDescribeQueryviacheckAccess(SHOW_COLUMNS, table_id).comInitDB(COM_INIT_DB, opcode0x02) calledsetCurrentDatabase(database)with no check. The SQL equivalentUSE databaseis access-checked inInterpreterUseQueryviacheckAccess(SHOW_DATABASES, new_database).run()set the current database from the client handshake response (setCurrentDatabase(handshake_response.database)) with the same missingSHOW_DATABASEScheck.A user could therefore call
mysql_list_fields("employees")and learn all column names of a table they only hold a column-level grant on (e.g.GRANT SELECT(id, name)), or sendCOM_INIT_DB systemwith zero grants and thenCOM_FIELD_LIST usersto read thesystem.userscolumn names. The disclosure runs under the caller's own session (no privilege escalation), and only column names leak (the column type is hardcoded toMYSQL_TYPE_STRING, no row data).The fix adds the same checks the SQL statements perform:
comInitDBand the handshake path:checkAccess(SHOW_DATABASES, database)beforesetCurrentDatabase.comFieldList:checkAccess(SHOW_COLUMNS, database, table)beforegetTable(), so the command does not become a table-existence oracle.A denied check throws
ACCESS_DENIED, which the handler's existingcatchblock turns into a MySQL error packet (no crash).Design note for reviewers. The table-level
checkAccess(SHOW_COLUMNS, db, table)matchesDESCRIBEand fails closed: a partial-column-grant user (GRANT SELECT(id, name)) is denied entirely, whereasSHOW COLUMNSoverCOM_QUERYreturns the granted subset[id, name]. ExactSHOW COLUMNSparity would require per-column filtering (mirroringStorageSystemColumns:isGranted(SHOW_COLUMNS, db, table, column)per column), but that must enumerate the columns first, which reintroduces a table-existence oracle for zero-grant users. This PR uses the table-level check (simplest, no oracle;mysql_list_fieldsis deprecated since MySQL 8.0). Happy to switch to per-column filtering if preferred.Test.
tests/integration/test_mysql_protocol/test.py::test_mysql_metadata_commands_access_controldrives the rawCOM_FIELD_LIST/COM_INIT_DBopcodes via pymysql for a user with onlySELECT(id, name), asserts both are rejected withACCESS_DENIED(SHOW COLUMNS/SHOW DATABASES), and asserts a user grantedSHOW COLUMNSstill receives all columns.