diff --git a/CHANGELOG.md b/CHANGELOG.md index c443d78..e404d56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **`component_mcp_guard` time limit and refusal log** (Phase 2 of RFC 0001, completed). + - **`mcp.max_execution_time_ms`:** sets an MCP session's `max_execution_time` at connect. Attempts to raise it are refused with error 1290: via `SET`, or via the `MAX_EXECUTION_TIME(n)` or `SET_VAR(max_execution_time=n)` hints. Lowering it is allowed. + - **`mcp.log_denials` (default ON):** writes each refusal to the error log as `[MY-011071] [mcp_guard]`, with string literals redacted, at most 10 lines a minute. + - **Tests:** `scripts/test-mcp-guard.sh` now has 80 checks, including a 5 s `SELECT` stopped by a 1 s limit, a tightened limit reaching a connected session, literal redaction in the log and the log rate limit on a fresh server, passing on MySQL 8.4.11, 9.7.2 and 26.10.0. - **`component_mcp_guard` enforcement** (Phase 2 of [RFC 0001](https://github.com/askdba/mysql-mcp-server/blob/main/docs/rfc/0001-mysql-component-architecture.md)). - **MCP sessions:** sessions of accounts holding `MCP_USER` (and not `MCP_ADMIN`) are checked against `mcp.*` settings inside `mysqld`. The checks run on the server's query, table-access and connection events. - **`mcp.read_only` (default ON):** only reads, even with write grants. It uses an allowlist of `sql_command` classes and also refuses `SELECT … INTO OUTFILE`, locking reads, `SET GLOBAL` and `CALL` outside `mysql_mcp`. diff --git a/docs/MCP_COMPONENTS.md b/docs/MCP_COMPONENTS.md index 116c8c7..9aa6f80 100644 --- a/docs/MCP_COMPONENTS.md +++ b/docs/MCP_COMPONENTS.md @@ -4,7 +4,7 @@ Server-side components for [mysql-mcp-server](https://github.com/askdba/mysql-mc | Component | Status | Servers | |---|---|---| -| `component_mcp_guard` | Phase 2: identity, read-only and schema enforcement, status variables | MySQL 8.4, 9.7, 26.10 | +| `component_mcp_guard` | Phase 2: identity, read-only and schema enforcement, time limit, error log, status variables | MySQL 8.4, 9.7, 26.10 | | `mysql_mcp` SQL tool pack ([`sql/mcp/`](https://github.com/askdba/myvector/tree/main/sql/mcp)) | 1.0.0 | MySQL 8.0 to 26.10, MariaDB 11.4, 12.3 | ## component_mcp_guard @@ -39,6 +39,8 @@ A session is an **MCP session** when its account holds `MCP_USER` and not `MCP_A | `mcp.read_only` | `ON` | Only reads, whatever the account's grants (below). | | `mcp.deny_system_schemas` | `ON` | No table access in `mysql` and `sys`. `information_schema` and `performance_schema` stay readable (the tools use them; grants govern them). | | `mcp.allowed_schemas` | empty | Comma-separated schemas whose tables MCP sessions may access; blank means all. A value with no names in it, such as `,`, allows none. Names are compared exactly, or case-insensitively when `lower_case_table_names` is 1 or 2. The system schemas above are governed by `mcp.deny_system_schemas`, and `mysql_mcp` is always allowed. | +| `mcp.max_execution_time_ms` | 0 | Time limit for their `SELECT` statements, which they cannot raise; 0 turns the guard's limit off (the server's own `max_execution_time` default still applies) (below). | +| `mcp.log_denials` | `ON` | Write refusals to the error log, at most 10 lines a minute (below). | `SET GLOBAL` and `SET PERSIST` of `mcp.*` need `MCP_ADMIN` on top of `SYSTEM_VARIABLES_ADMIN`. Persisted values load at startup as usual. Values given in `INSTALL COMPONENT ... SET` are taken from the installer. @@ -64,6 +66,27 @@ Because of the double reading, a `SELECT` whose string literal contains a backsl `CALL` is allowed for any routine in the `mysql_mcp` schema, not only the tool pack's. Keep only the tool pack in `mysql_mcp` (it is installed and dropped as a whole). +### Time limit (mcp.max_execution_time_ms) + +When an MCP session connects, the guard sets its session `max_execution_time` to `mcp.max_execution_time_ms`, so its `SELECT` statements stop after that many milliseconds (error 3024). The session may lower it, but not raise it. These are refused with error 1290 naming `mcp.max_execution_time_ms`: + +- `SET [SESSION] max_execution_time` (or `@@max_execution_time`) to 0, above the limit, or to anything but a literal (`DEFAULT`, an expression, a variable). +- The optimizer hints `MAX_EXECUTION_TIME(n)` and `SET_VAR(max_execution_time = n)` with such a value. + +A changed limit reaches connected MCP sessions at their next statement, and the limit is applied again after `COM_CHANGE_USER` and `COM_RESET_CONNECTION`, which reset session variables. Setting it back to 0 leaves their current session value as it is. + +As with the server's own `max_execution_time`, the limit covers top-level read-only `SELECT` statements. MySQL ignores it inside stored programs, so **`CALL mysql_mcp.*` tool-pack calls are not limited**. Those run catalog queries on `information_schema`. If they must be limited too, revoke `EXECUTE` on `mysql_mcp` from the MCP account: mysql-mcp-server then falls back to its own SQL, whose `SELECT`s the limit covers. + +### Error log (mcp.log_denials) + +Each refusal is written as a warning, with the setting, the connection id and the start of the statement (up to 256 bytes, string literals replaced by `'?'`, line breaks by spaces) or the table: + +``` +[Warning] [MY-011071] [mcp_guard] mcp_guard: refused by mcp.read_only: MCP connection 11: INSERT INTO d.t VALUES (1) +``` + +At most 10 lines are written per minute; the number left out is reported with the next line after the minute. String literals are redacted, but numbers and identifiers are kept; turn `mcp.log_denials` off if even those must not reach the error log. + ### Errors A denial fails the statement with error **1290** (`ER_OPTION_PREVENTS_STATEMENT`), the server's own `read_only` error, naming the setting: @@ -87,8 +110,8 @@ The option is `mcp.read_only`, `mcp.deny_system_schemas`, `mcp.allowed_schemas` - `mcp.allowed_schemas` limits **table access**, not catalog metadata: `information_schema` and `SHOW` still list what the account's grants allow. - With `mcp.deny_system_schemas = ON`, mysql-mcp-server's opt-in `slow_query_log` tool (which reads `mysql.slow_log`) is denied. -- Planned (RFC 0001): a `mcp.max_execution_time_ms` floor, denials in the error log and audit log, `performance_schema.mcp_tool_calls`. +- Planned (RFC 0001 Phase 3): denials in the audit log, `performance_schema.mcp_tool_calls`. ### Tests -`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases against the official image (65 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. +`scripts/test-mcp-guard.sh ` runs install, restart, persistence, uninstall and the enforcement cases against the official image (80 checks, including a `lower_case_table_names=1` server), and `src/mcp/guard/test_mcp_guard_policy.cc` unit-tests the classification and the SQL scanner. CI (`integration-scripts.yml`, job `mcp-guard`) runs both on MySQL 8.4.11, 9.7.2 and 26.10.0. diff --git a/scripts/test-mcp-guard.sh b/scripts/test-mcp-guard.sh index 06efd0b..c439bdb 100755 --- a/scripts/test-mcp-guard.sh +++ b/scripts/test-mcp-guard.sh @@ -144,6 +144,45 @@ expect "mcp.enabled ON again" "$(mcp "SELECT 1")" "^1$" expect "invalid value rejected" "$(sql "SET GLOBAL mcp.read_only = 'maybe'")" "ERROR" +# --- mcp.max_execution_time_ms: a time limit MCP sessions cannot raise ------------------------ +expect "SET GLOBAL mcp.max_execution_time_ms = 1000" "$(sql "SET GLOBAL mcp.max_execution_time_ms = 1000")" "^$" +expect "mcp: session max_execution_time set at connect" "$(mcp "SELECT @@max_execution_time")" "^1000$" +expect "mcp: lowering it is allowed" "$(mcp "SET SESSION max_execution_time = 500; SELECT @@max_execution_time")" "^500$" +denied "mcp: SET max_execution_time = 0" "$(mcp "SET SESSION max_execution_time = 0")" "mcp.max_execution_time_ms" +denied "mcp: SET max_execution_time above the limit" "$(mcp "SET @@max_execution_time = 5000")" "mcp.max_execution_time_ms" +denied "mcp: MAX_EXECUTION_TIME(0) hint" "$(mcp "SELECT /*+ MAX_EXECUTION_TIME(0) */ COUNT(*) FROM d1.t")" "mcp.max_execution_time_ms" +denied "mcp: SET_VAR(max_execution_time=0) hint" "$(mcp "SELECT /*+ SET_VAR(max_execution_time=0) */ COUNT(*) FROM d1.t")" "mcp.max_execution_time_ms" +start=$(date +%s) +slept=$(mcp "SELECT SLEEP(5) FROM d1.t") +took=$(($(date +%s) - start)) +# Interrupted by the time limit: SLEEP() alone returns 1; reading a table, as here, the +# statement fails with 3024 (ER_QUERY_TIMEOUT). Either way it must not run 5s. +if [ "$took" -le 3 ] && { [ "$slept" = 1 ] || printf '%s' "$slept" | grep -q '^ERROR 3024 '; }; then ok "mcp: a 5s SELECT is stopped by the 1s limit (${took}s)"; else bad "mcp: time limit enforced" "took ${took}s, result '$slept'"; fi +expect "app (no MCP_USER) keeps max_execution_time 0" "$(as app apppass1 "SELECT @@max_execution_time")" "^0$" +sql "SET GLOBAL mcp.max_execution_time_ms = 0" >/dev/null +expect "mcp: no limit once the floor is 0" "$(mcp "SELECT @@max_execution_time")" "^0$" +# A session already connected gets a new limit at its next statement. +mcp "SELECT @@max_execution_time; DO SLEEP(3); SELECT @@max_execution_time" >"/tmp/$CNAME-live" 2>&1 & +live=$! +sleep 1 +sql "SET GLOBAL mcp.max_execution_time_ms = 1500" >/dev/null +wait "$live" +expect "mcp: an existing session gets a tightened limit" "$(tr '\n' ' ' <"/tmp/$CNAME-live")" "^0 1500 $" +rm -f "/tmp/$CNAME-live" +sql "SET GLOBAL mcp.max_execution_time_ms = 0" >/dev/null + +# --- mcp.log_denials ----------------------------------------------------------------------------- +expect "refusals are written to the error log" "$(docker logs "$CNAME" 2>&1 | grep 'mcp_guard: refused by' | tail -3)" '\[MY-011071\] \[mcp_guard\] mcp_guard: refused by mcp\.(read_only|max_execution_time_ms): MCP connection [0-9]+' +mcp "SELECT 'token=hunter2' FROM d1.t FOR UPDATE" >/dev/null +mcp "SET SESSION sql_mode = 'NO_BACKSLASH_ESCAPES'; SELECT 'a\\', 'token=hunter2' FROM d1.t FOR UPDATE" >/dev/null +expect "error log redacts string literals" "$(docker logs "$CNAME" 2>&1 | grep 'mcp_guard: refused by' | grep -c 'hunter2')" "^0$" +sql "SET GLOBAL mcp.log_denials = OFF" >/dev/null +before=$(docker logs "$CNAME" 2>&1 | grep -c 'mcp_guard: refused by') +mcp "INSERT INTO d1.t VALUES (8)" >/dev/null +after=$(docker logs "$CNAME" 2>&1 | grep -c 'mcp_guard: refused by') +expect "mcp.log_denials = OFF writes nothing" "$after" "^$before$" +sql "SET GLOBAL mcp.log_denials = ON" >/dev/null + status=$(sql "SHOW GLOBAL STATUS LIKE 'mcp.%'") expect "status: statements_denied counted" "$status" "mcp.statements_denied [1-9]" expect "status: table_access_denied counted" "$status" "mcp.table_access_denied [1-9]" @@ -179,6 +218,9 @@ CREATE USER mcp@'%' IDENTIFIED BY 'mcppass1'; GRANT SELECT ON *.* TO mcp@'%'; GR expect "lower_case_table_names=1: SET GLOBAL mcp.allowed_schemas = 'Shop'" "$(sql "SET GLOBAL mcp.allowed_schemas = 'Shop'")" "^$" expect "lower_case_table_names=1: allowed_schemas 'Shop' allows shop" "$(mcp "SELECT COUNT(*) FROM Shop.t")" "^0$" denied "lower_case_table_names=1: other schemas still denied" "$(mcp "SELECT COUNT(*) FROM other.t")" "mcp.allowed_schemas" + # A fresh server: its first minute of refusals is logged up to 10 lines, then held back. + for _ in $(seq 1 15); do mcp "SELECT COUNT(*) FROM other.t" >/dev/null; done + expect "error log rate limit: 16 refusals in a minute, 10 lines" "$(docker logs "$CNAME" 2>&1 | grep -c 'mcp_guard: refused by')" "^10$" else bad "lower_case_table_names=1 server" "did not start" fi diff --git a/src/mcp/guard/component_mcp_guard.cc b/src/mcp/guard/component_mcp_guard.cc index c2e157c..0806de4 100644 --- a/src/mcp/guard/component_mcp_guard.cc +++ b/src/mcp/guard/component_mcp_guard.cc @@ -23,6 +23,9 @@ // SET, and CALL into the mysql_mcp tool pack; no table writes. // mcp.deny_system_schemas ON (default): no table access in mysql and sys. // mcp.allowed_schemas Comma-separated schemas MCP sessions may read; empty: all. +// mcp.max_execution_time_ms Time limit for their SELECTs (session max_execution_time, +// set at connect), which they cannot raise; 0: none. +// mcp.log_denials ON (default): refusals go to the error log, 10 a minute. // // Denials fail with ER_OPTION_PREVENTS_STATEMENT ("... running with the mcp.read_only // option ..."), like the server's own read_only. SET GLOBAL / PERSIST of mcp.* needs @@ -34,18 +37,24 @@ #include #include #include +#include #include #include +#include +#include #include #include +#include #include #include #include #include #include +#include #include #include +#include #include #include #include @@ -61,6 +70,15 @@ REQUIRES_SERVICE_PLACEHOLDER(component_sys_variable_register); REQUIRES_SERVICE_PLACEHOLDER(component_sys_variable_unregister); REQUIRES_SERVICE_PLACEHOLDER(status_variable_registration); REQUIRES_SERVICE_PLACEHOLDER(mysql_runtime_error); +REQUIRES_SERVICE_PLACEHOLDER(mysql_system_variable_update_integer); +REQUIRES_SERVICE_PLACEHOLDER(mysql_string_factory); +REQUIRES_SERVICE_PLACEHOLDER(mysql_string_converter); +REQUIRES_SERVICE_PLACEHOLDER(log_builtins); +REQUIRES_SERVICE_PLACEHOLDER(log_builtins_string); + +// LogEvent() uses these. +SERVICE_TYPE(log_builtins) *log_bi = nullptr; +SERVICE_TYPE(log_builtins_string) *log_bs = nullptr; namespace { @@ -70,6 +88,9 @@ constexpr int kErOptionPreventsStatement = 1290; // ER_SPECIFIC_ACCESS_DENIED_ERROR: "Access denied; you need (at least one of) the %-.128s // privilege(s) for this operation". constexpr int kErSpecificAccessDenied = 1227; +// ER_LOG_PRINTF_MSG: the error log code for a free-form component message (MY-011071 on +// 8.4, 9.7 and 26.10). +constexpr int kErLogPrintfMsg = 11071; constexpr const char *kMcpUser = "MCP_USER"; constexpr const char *kMcpAdmin = "MCP_ADMIN"; @@ -132,6 +153,8 @@ MYSQL_THD current_thd_or_null() { bool opt_enabled = true; bool opt_read_only = true; bool opt_deny_system_schemas = true; +bool opt_log_denials = true; +unsigned long opt_max_execution_time_ms = 0; // 0: no floor char *opt_allowed_schemas = nullptr; // server-managed (PLUGIN_VAR_MEMALLOC) // Parsed mcp.allowed_schemas, swapped as a whole on update; readers copy the pointer. @@ -169,17 +192,20 @@ struct BoolVar { bool def; }; -constexpr size_t kBoolVarCount = 3; +constexpr size_t kBoolVarCount = 4; const BoolVar kBoolVars[kBoolVarCount] = { {"enabled", "Allow mysql-mcp-server (MCP_USER) sessions; OFF refuses them all", &opt_enabled, true}, {"read_only", "MCP sessions may only read, whatever their grants", &opt_read_only, true}, {"deny_system_schemas", "MCP sessions may not access tables in mysql and sys", &opt_deny_system_schemas, true}, + {"log_denials", "Write refused MCP statements to the error log (at most 10 a minute)", + &opt_log_denials, true}, }; bool bool_registered[kBoolVarCount] = {}; bool allowed_registered = false; +bool time_registered = false; // Returns true if a variable could not be unregistered; it then stays marked as // registered and deinit fails, so the server keeps this library (and the update callback @@ -201,6 +227,13 @@ bool unregister_sysvars() { else allowed_registered = false; } + if (time_registered) { + if (mysql_service_component_sys_variable_unregister->unregister_variable( + "mcp", "max_execution_time_ms")) + failed = true; + else + time_registered = false; + } return failed; } @@ -242,6 +275,19 @@ bool register_sysvars() { nullptr, update_allowed_schemas, &str_arg, &opt_allowed_schemas)) return true; allowed_registered = true; + INTEGRAL_CHECK_ARG(ulong) time_arg; + time_arg.def_val = 0; + time_arg.min_val = 0; + time_arg.max_val = 4294967295UL; + time_arg.blk_sz = 0; + if (mysql_service_component_sys_variable_register->register_variable( + "mcp", "max_execution_time_ms", + PLUGIN_VAR_LONG | PLUGIN_VAR_UNSIGNED | PLUGIN_VAR_RQCMDARG, + "Time limit in milliseconds for SELECT statements of MCP sessions, which they cannot " + "raise; 0: none", + nullptr, nullptr, &time_arg, &opt_max_execution_time_ms)) + return true; + time_registered = true; // A persisted or command-line value is stored without the update callback: parse it. set_allowed_schemas(opt_allowed_schemas); return false; @@ -270,29 +316,120 @@ bool status_registered = false; // ----- MCP session marker (per THD) --------------------------------------------------- +// Per-session state of an MCP session, in a THD slot (null for other sessions). +// applied_time_limit when no limit has been applied to the current session variables. +constexpr unsigned long kTimeLimitNotApplied = ~0UL; + +struct SessionState { + unsigned long applied_time_limit = kTimeLimitNotApplied; // last mcp.max_execution_time_ms set +}; + mysql_thd_store_slot session_slot = nullptr; -char mcp_session_marker; // the slot holds &mcp_session_marker for MCP sessions -int free_session_slot(void *) { return 0; } // the marker is static +// Called by the server when the THD ends with a state still attached. +int free_session_slot(void *state) { + delete static_cast(state); + return 0; +} -bool is_mcp_session(MYSQL_THD thd) { - return thd != nullptr && session_slot != nullptr && - mysql_service_mysql_thd_store->get(thd, session_slot) == &mcp_session_marker; +SessionState *session_state(MYSQL_THD thd) { + if (thd == nullptr || session_slot == nullptr) return nullptr; + return static_cast(mysql_service_mysql_thd_store->get(thd, session_slot)); } -void mark_session(MYSQL_THD thd, bool mcp) { - if (is_mcp_session(thd) == mcp) return; - mysql_service_mysql_thd_store->set(thd, session_slot, mcp ? &mcp_session_marker : nullptr); +bool is_mcp_session(MYSQL_THD thd) { return session_state(thd) != nullptr; } + +// Returns true on failure. An MCP session that cannot be marked would not be guarded, so +// the caller refuses it. +bool mark_session(MYSQL_THD thd, bool mcp) { + if (session_slot == nullptr) return mcp; + SessionState *state = session_state(thd); + if ((state != nullptr) == mcp) return false; + if (mcp) { + state = new (std::nothrow) SessionState(); + if (state == nullptr) return true; + if (mysql_service_mysql_thd_store->set(thd, session_slot, state)) { + delete state; + return true; + } + } else { + mysql_service_mysql_thd_store->set(thd, session_slot, nullptr); + delete state; + } sessions_connected += mcp ? 1 : -1; + return false; } -bool deny(const char *option, std::atomic &counter) { +// Writes a refusal to the error log, at most kLogBurst lines per minute; the number left +// out is reported with the next line after the minute. +constexpr int kLogBurst = 10; +std::mutex log_mutex; +std::chrono::steady_clock::time_point log_window_start; +int log_window_count = 0; +long long log_suppressed = 0; + +void log_denial(const char *option, unsigned long long connection_id, std::string_view what) { + if (!opt_log_denials || log_bi == nullptr) return; + long long suppressed = 0; + { + std::lock_guard lock(log_mutex); + const auto now = std::chrono::steady_clock::now(); + if (now - log_window_start >= std::chrono::minutes(1)) { + suppressed = log_suppressed; + log_suppressed = 0; + log_window_start = now; + log_window_count = 0; + } + if (log_window_count >= kLogBurst) { + ++log_suppressed; + return; + } + ++log_window_count; + } + if (suppressed > 0) + LogEvent() + .type(LOG_TYPE_ERROR) + .prio(WARNING_LEVEL) + .errcode(kErLogPrintfMsg) + .subsys("mcp_guard") + .component("component: mcp_guard") + .message("mcp_guard: %lld more MCP refusals in the last minute were not logged", suppressed); + const std::string shown = mcp_guard::redact_for_log(what.data(), what.size()); + LogEvent() + .type(LOG_TYPE_ERROR) + .prio(WARNING_LEVEL) + .errcode(kErLogPrintfMsg) + .subsys("mcp_guard") + .component("component: mcp_guard") + .message("mcp_guard: refused by %s: MCP connection %llu: %s", option, connection_id, + shown.c_str()); +} + +bool deny(const char *option, std::atomic &counter, unsigned long long connection_id, + std::string_view what) { ++counter; + log_denial(option, connection_id, what); mysql_error_service_emit_printf(mysql_service_mysql_runtime_error, kErOptionPreventsStatement, 0, option); return true; } +// Sets the session's max_execution_time to mcp.max_execution_time_ms when the setting +// differs from what was last applied to this session: at connect, and at the next +// statement after an administrator changes it. Between changes the session may lower +// its own value (and cannot raise it). When the setting goes back to 0 the session +// value is left as it is. +my_h_string max_execution_time_name = nullptr; + +void apply_time_limit(MYSQL_THD thd, SessionState *state) { + const unsigned long limit = opt_max_execution_time_ms; + if (state == nullptr || limit == state->applied_time_limit) return; + state->applied_time_limit = limit; + if (limit == 0 || max_execution_time_name == nullptr) return; + mysql_service_mysql_system_variable_update_integer->set_unsigned( + thd, "SESSION", nullptr, max_execution_time_name, limit); +} + } // namespace // ----- Event tracking ----------------------------------------------------------------- @@ -315,9 +452,18 @@ bool Event_tracking_connection_implementation::callback( const bool mcp = thd_has_grant(thd, kMcpUser) && !thd_has_grant(thd, kMcpAdmin); if (mcp && !opt_enabled) { mark_session(thd, false); // refused: no disconnect event follows - return deny("mcp.enabled", statements_denied); + return deny("mcp.enabled", statements_denied, data->connection_id, "connection refused"); + } + if (mark_session(thd, mcp)) // could not mark an MCP session: fail closed + return deny("mcp_guard", statements_denied, data->connection_id, + "connection refused: session state unavailable"); + if (mcp) { + SessionState *state = session_state(thd); + // COM_CHANGE_USER resets session variables: apply the limit again. + if (state != nullptr && data->event_subclass == EVENT_TRACKING_CONNECTION_CHANGE_USER) + state->applied_time_limit = kTimeLimitNotApplied; + apply_time_limit(thd, state); } - mark_session(thd, mcp); return false; } case EVENT_TRACKING_CONNECTION_DISCONNECT: @@ -328,6 +474,23 @@ bool Event_tracking_connection_implementation::callback( } } +// Command events: COM_RESET_CONNECTION resets session variables (raising no connection +// event), so the time limit is applied again at the next statement. +mysql_event_tracking_command_subclass_t Event_tracking_command_implementation::filtered_sub_events = + EVENT_TRACKING_COMMAND_START; + +bool Event_tracking_command_implementation::callback( + const mysql_event_tracking_command_data *data) { + if (data->status != 0) return false; + static constexpr std::string_view kReset = "Reset Connection"; + const std::string_view command(data->command.str ? data->command.str : "", + data->command.str ? data->command.length : 0); + if (command != kReset) return false; // cheap check first: command events are frequent + SessionState *state = session_state(current_thd_or_null()); + if (state != nullptr) state->applied_time_limit = kTimeLimitNotApplied; + return false; +} + // Global variable events: SET GLOBAL / SET PERSIST of mcp.* needs MCP_ADMIN, on top of // the SYSTEM_VARIABLES_ADMIN the server requires. Only a client's SET statement // (sql_command "set_option") is checked: the server applies persisted values through the @@ -359,16 +522,23 @@ mysql_event_tracking_query_subclass_t Event_tracking_query_implementation::filte bool Event_tracking_query_implementation::callback(const mysql_event_tracking_query_data *data) { MYSQL_THD thd = current_thd_or_null(); - if (!is_mcp_session(thd)) return false; - if (data->event_subclass == EVENT_TRACKING_QUERY_START) ++statements_total; - if (!opt_enabled) return deny("mcp.enabled", statements_denied); + SessionState *state = session_state(thd); + if (state == nullptr) return false; + if (data->event_subclass == EVENT_TRACKING_QUERY_START) { + ++statements_total; + apply_time_limit(thd, state); // picks up a changed mcp.max_execution_time_ms + } + const char *cmd = data->sql_command ? data->sql_command : ""; + const char *query = data->query.str ? data->query.str : ""; + const size_t len = data->query.str ? data->query.length : 0; + const std::string_view text(query, len); + if (!opt_enabled) return deny("mcp.enabled", statements_denied, data->connection_id, text); if (opt_read_only) { - const char *cmd = data->sql_command ? data->sql_command : ""; - const char *query = data->query.str ? data->query.str : ""; - const size_t len = data->query.str ? data->query.length : 0; const char *why = mcp_guard::read_only_violation(cmd, query, len); - if (why != nullptr) return deny(why, statements_denied); + if (why != nullptr) return deny(why, statements_denied, data->connection_id, text); } + const char *why = mcp_guard::time_limit_violation(cmd, query, len, opt_max_execution_time_ms); + if (why != nullptr) return deny(why, statements_denied, data->connection_id, text); return false; } @@ -381,7 +551,18 @@ bool Event_tracking_table_access_implementation::callback( const mysql_event_tracking_table_access_data *data) { MYSQL_THD thd = current_thd_or_null(); if (!is_mcp_session(thd)) return false; - if (!opt_enabled) return deny("mcp.enabled", table_access_denied); + const std::string_view db(data->table_database.str ? data->table_database.str : "", + data->table_database.str ? data->table_database.length : 0); + const std::string_view table(data->table_name.str ? data->table_name.str : "", + data->table_name.str ? data->table_name.length : 0); + const bool write = data->event_subclass != EVENT_TRACKING_TABLE_ACCESS_READ; + // Built only for a refusal: table events are frequent. + const auto refuse = [&](const char *option) { + const std::string what = + std::string(write ? "write " : "read ") + std::string(db) + "." + std::string(table); + return deny(option, table_access_denied, data->connection_id, what); + }; + if (!opt_enabled) return refuse("mcp.enabled"); const auto allowed = current_allowed_schemas(); mcp_guard::TablePolicy policy; policy.read_only = opt_read_only; @@ -389,11 +570,8 @@ bool Event_tracking_table_access_implementation::callback( policy.restrict_schemas = allowed->restrict; policy.allowed_schemas = &allowed->names; policy.case_insensitive_names = case_insensitive_names; - const std::string_view db(data->table_database.str ? data->table_database.str : "", - data->table_database.str ? data->table_database.length : 0); - const bool write = data->event_subclass != EVENT_TRACKING_TABLE_ACCESS_READ; const char *why = mcp_guard::table_violation(policy, db, write); - if (why != nullptr) return deny(why, table_access_denied); + if (why != nullptr) return refuse(why); return false; } @@ -422,10 +600,20 @@ mysql_service_status_t mcp_guard_deinit() { session_slot = nullptr; } if (unregister_privileges()) failed = true; + if (max_execution_time_name != nullptr) { + mysql_service_mysql_string_factory->destroy(max_execution_time_name); + max_execution_time_name = nullptr; + } return failed ? 1 : 0; } mysql_service_status_t mcp_guard_init() { + log_bi = mysql_service_log_builtins; + log_bs = mysql_service_log_builtins_string; + static const char kName[] = "max_execution_time"; + if (mysql_service_mysql_string_converter->convert_from_buffer( + &max_execution_time_name, kName, sizeof(kName) - 1, "latin1")) + max_execution_time_name = nullptr; if (register_privileges() || mysql_service_mysql_thd_store->register_slot("component_mcp_guard", free_session_slot, &session_slot) || @@ -441,13 +629,15 @@ mysql_service_status_t mcp_guard_init() { } // namespace +IMPLEMENTS_SERVICE_EVENT_TRACKING_COMMAND(mcp_guard); IMPLEMENTS_SERVICE_EVENT_TRACKING_CONNECTION(mcp_guard); IMPLEMENTS_SERVICE_EVENT_TRACKING_GLOBAL_VARIABLE(mcp_guard); IMPLEMENTS_SERVICE_EVENT_TRACKING_QUERY(mcp_guard); IMPLEMENTS_SERVICE_EVENT_TRACKING_TABLE_ACCESS(mcp_guard); BEGIN_COMPONENT_PROVIDES(mcp_guard) -PROVIDES_SERVICE_EVENT_TRACKING_CONNECTION(mcp_guard), +PROVIDES_SERVICE_EVENT_TRACKING_COMMAND(mcp_guard), + PROVIDES_SERVICE_EVENT_TRACKING_CONNECTION(mcp_guard), PROVIDES_SERVICE_EVENT_TRACKING_GLOBAL_VARIABLE(mcp_guard), PROVIDES_SERVICE_EVENT_TRACKING_QUERY(mcp_guard), PROVIDES_SERVICE_EVENT_TRACKING_TABLE_ACCESS(mcp_guard), END_COMPONENT_PROVIDES(); @@ -458,6 +648,9 @@ REQUIRES_SERVICE(dynamic_privilege_register), REQUIRES_SERVICE(global_grants_che REQUIRES_SERVICE(mysql_thd_store), REQUIRES_SERVICE(component_sys_variable_register), REQUIRES_SERVICE(component_sys_variable_unregister), REQUIRES_SERVICE(status_variable_registration), REQUIRES_SERVICE(mysql_runtime_error), + REQUIRES_SERVICE(mysql_system_variable_update_integer), + REQUIRES_SERVICE(mysql_string_factory), REQUIRES_SERVICE(mysql_string_converter), + REQUIRES_SERVICE(log_builtins), REQUIRES_SERVICE(log_builtins_string), END_COMPONENT_REQUIRES(); BEGIN_COMPONENT_METADATA(mcp_guard) diff --git a/src/mcp/guard/mcp_guard_policy.h b/src/mcp/guard/mcp_guard_policy.h index 18f931e..9e6b05b 100644 --- a/src/mcp/guard/mcp_guard_policy.h +++ b/src/mcp/guard/mcp_guard_policy.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -214,6 +215,135 @@ inline const char *read_only_violation(std::string_view cmd, const char *query, return nullptr; } +// ----- mcp.max_execution_time_ms ------------------------------------------------------ + +inline bool all_digits(std::string_view s) { + if (s.empty() || s.size() > 18) return false; + for (char c : s) + if (!std::isdigit(static_cast(c))) return false; + return true; +} + +// An explicit limit is acceptable when it is a positive literal no larger than the floor. +inline bool within_floor(std::string_view value, unsigned long long floor) { + if (!all_digits(value)) return false; + const unsigned long long v = std::stoull(std::string(value)); + return v > 0 && v <= floor; +} + +// A SET of the session's max_execution_time that would lift the floor: to 0, above it, or +// to anything but a literal (DEFAULT, an expression, a variable). +inline bool set_raises_time_limit(const std::vector &t, unsigned long long floor) { + for (size_t i = 0; i < t.size(); ++i) { + if (t[i].kind != Token::kWord) continue; + if (t[i].text != "MAX_EXECUTION_TIME" && t[i].text != "@@MAX_EXECUTION_TIME") continue; + size_t j = i + 1; + if (j < t.size() && t[j].kind == Token::kPunct && t[j].text == ":") ++j; + if (!(j < t.size() && t[j].kind == Token::kPunct && t[j].text == "=")) continue; // not assigned + ++j; + const bool literal = j < t.size() && t[j].kind == Token::kWord && + (j + 1 == t.size() || (t[j + 1].kind == Token::kPunct && + (t[j + 1].text == "," || t[j + 1].text == ";"))); + if (!literal || !within_floor(t[j].text, floor)) return true; + } + return false; +} + +// An optimizer hint (/*+ ... */) that sets the statement's time limit, either +// MAX_EXECUTION_TIME(n) or SET_VAR(max_execution_time = n), to 0, above the floor, or to +// something that is not a literal. Searched in the raw text, so a hint-like string literal +// is also refused; that only errs on the safe side. +inline bool hint_raises_time_limit(const char *s, size_t n, unsigned long long floor) { + std::string upper(s, n); + for (char &c : upper) c = static_cast(std::toupper(static_cast(c))); + size_t pos = 0; + while ((pos = upper.find("/*+", pos)) != std::string::npos) { + const size_t end = upper.find("*/", pos + 3); + const std::string body = upper.substr(pos + 3, end == std::string::npos ? std::string::npos + : end - pos - 3); + size_t k = 0; + while ((k = body.find("MAX_EXECUTION_TIME", k)) != std::string::npos) { + size_t v = k + std::strlen("MAX_EXECUTION_TIME"); + while (v < body.size() && std::isspace(static_cast(body[v]))) ++v; + if (v < body.size() && (body[v] == '(' || body[v] == '=')) ++v; + while (v < body.size() && std::isspace(static_cast(body[v]))) ++v; + size_t e = v; + while (e < body.size() && std::isdigit(static_cast(body[e]))) ++e; + if (!within_floor(std::string_view(body).substr(v, e - v), floor)) return true; + // The whole operand must be the number: MySQL reads 1G or 1K as a scaled value. + size_t after = e; + while (after < body.size() && std::isspace(static_cast(body[after]))) ++after; + if (after >= body.size() || (body[after] != ')' && body[after] != ',')) return true; + k = e; + } + if (end == std::string::npos) break; + pos = end + 2; + } + return false; +} + +// Returns "mcp.max_execution_time_ms" when the statement would lift the floor, or nullptr. +// floor == 0 disables the check. Text is read in both quoting modes, as for read_only. +inline const char *time_limit_violation(std::string_view cmd, const char *query, size_t len, + unsigned long long floor) { + if (floor == 0) return nullptr; + if (hint_raises_time_limit(query, len, floor)) return "mcp.max_execution_time_ms"; + if (cmd == "set_option") { + for (bool escapes : {true, false}) + if (set_raises_time_limit(tokenize(query, len, escapes), floor)) + return "mcp.max_execution_time_ms"; + } + return nullptr; +} + +// ----- Error log ----------------------------------------------------------------------- + +// One reading of redact_for_log(): backslash_escapes as in tokenize(). +inline std::string redact_literals(const char *s, size_t n, size_t max_len, + bool backslash_escapes) { + std::string out; + size_t i = 0; + while (i < n && out.size() < max_len) { + const char c = s[i]; + if (c == '\'' || c == '"') { + out.push_back(c); + out.push_back('?'); + out.push_back(c); + ++i; + while (i < n) { + if (backslash_escapes && s[i] == '\\' && i + 1 < n) { + i += 2; + } else if (s[i] == c && i + 1 < n && s[i + 1] == c) { + i += 2; + } else if (s[i] == c) { + ++i; + break; + } else { + ++i; + } + } + continue; + } + out.push_back(c == '\n' || c == '\r' || c == '\t' ? ' ' : c); + ++i; + } + if (i < n) out += "..."; + return out; +} + +// The statement text for the error log: string literals replaced by '?' (they can hold +// secrets), line breaks and tabs by spaces, at most max_len bytes. Quoted identifiers stay. +// Where a literal ends depends on sql_mode (NO_BACKSLASH_ESCAPES), so both readings are +// made; if they differ, only the text before the first quote is kept. +inline std::string redact_for_log(const char *s, size_t n, size_t max_len = 256) { + std::string with = redact_literals(s, n, max_len, true); + if (with == redact_literals(s, n, max_len, false)) return with; + size_t q = 0; + while (q < n && q < max_len && s[q] != '\'' && s[q] != '"') ++q; + std::string out = redact_literals(s, q, max_len, true); + return out + "'?'..."; +} + // Parses mcp.allowed_schemas: comma-separated schema names, spaces trimmed, empty // entries dropped. An empty list means every schema. inline std::vector parse_schema_list(std::string_view v) { diff --git a/src/mcp/guard/test_mcp_guard_policy.cc b/src/mcp/guard/test_mcp_guard_policy.cc index 1210261..ff2f16c 100644 --- a/src/mcp/guard/test_mcp_guard_policy.cc +++ b/src/mcp/guard/test_mcp_guard_policy.cc @@ -96,6 +96,76 @@ int main() { expect_ro("select", "SELECT \"for\" FROM t", false); expect_ro("call", "CALL mysql_mcp.list_tables('\\') ", false); + // mcp.max_execution_time_ms + struct TimeCase { + const char *cmd, *sql; + bool denied; + }; + const TimeCase time_cases[] = { + {"set_option", "SET SESSION max_execution_time = 500", false}, + {"set_option", "SET max_execution_time=1000", false}, + {"set_option", "SET @@session.max_execution_time = 999, sql_mode = ''", false}, + {"set_option", "SET @max_execution_time = 0", false}, // a user variable + {"set_option", "SET sql_mode = ''", false}, + {"set_option", "SET SESSION max_execution_time = 0", true}, + {"set_option", "SET max_execution_time = 5000", true}, + {"set_option", "SET @@max_execution_time := 2000", true}, + {"set_option", "SET max_execution_time = DEFAULT", true}, + {"set_option", "SET max_execution_time = 500 * 100", true}, + {"set_option", "SET max_execution_time = @x", true}, + {"set_option", "SET a = 1, max_execution_time = 0", true}, + {"select", "SELECT /*+ MAX_EXECUTION_TIME(500) */ * FROM t", false}, + {"select", "SELECT /*+ MAX_EXECUTION_TIME(0) */ * FROM t", true}, + {"select", "SELECT /*+ max_execution_time( 5000 ) */ * FROM t", true}, + {"select", "SELECT /*+ SET_VAR(max_execution_time=0) */ * FROM t", true}, + {"select", "SELECT /*+ SET_VAR(max_execution_time = 800) */ * FROM t", false}, + {"select", "SELECT /*+ BKA(t) */ * FROM t", false}, + {"select", "SELECT 'max_execution_time = 0' FROM t", false}, + {"select", "SELECT /*+ SET_VAR(max_execution_time=1G) */ SLEEP(5)", true}, + {"select", "SELECT /*+ MAX_EXECUTION_TIME(1K) */ SLEEP(5)", true}, + {"select", "SELECT /*+ SET_VAR(max_execution_time=5.5) */ 1", true}, + {"select", "SELECT /*+ MAX_EXECUTION_TIME(900) BKA(t) */ * FROM t", false}, + {"set_option", "SET max_execution_time = 1G", true}, + }; + for (const TimeCase &c : time_cases) { + const char *why = time_limit_violation(c.cmd, c.sql, std::strlen(c.sql), 1000); + if ((why != nullptr) != c.denied) { + std::printf("FAIL time limit %-60s want %s\n", c.sql, c.denied ? "deny" : "allow"); + ++failures; + } + } + if (time_limit_violation("set_option", "SET max_execution_time = 0", 26, 0) != nullptr) { + std::printf("FAIL time limit floor 0 must not check\n"); + ++failures; + } + + // Error log redaction. + { + const char *q = "SELECT 'token=secret', \"pw\" FROM `t`\nWHERE a = 'it''s'"; + const std::string r = redact_for_log(q, std::strlen(q)); + if (r != "SELECT '?', \"?\" FROM `t` WHERE a = '?'") { + std::printf("FAIL redact_for_log: %s\n", r.c_str()); + ++failures; + } + // NO_BACKSLASH_ESCAPES: 'a\' is a whole literal there; the readings differ, so only + // the text before the first quote is kept. + const char *nbe = "SELECT 'a\\', 'token=hunter2' FROM t FOR UPDATE"; + const std::string rn = redact_for_log(nbe, std::strlen(nbe)); + if (rn.find("hunter2") != std::string::npos || rn != "SELECT '?'...") { + std::printf("FAIL redact_for_log NO_BACKSLASH_ESCAPES: %s\n", rn.c_str()); + ++failures; + } + const char *esc = "SELECT 'it\\'s secret' FROM t"; + if (redact_for_log(esc, std::strlen(esc)).find("secret") != std::string::npos) { + std::printf("FAIL redact_for_log escaped quote: %s\n", redact_for_log(esc, std::strlen(esc)).c_str()); + ++failures; + } + if (redact_for_log(q, std::strlen(q), 10) != "SELECT '?'...") { + std::printf("FAIL redact_for_log truncation: %s\n", redact_for_log(q, std::strlen(q), 10).c_str()); + ++failures; + } + } + // Tokenizer details. { const char *q = "SELECT 'it''s' , \"a\\\"b\" FROM `we``ird`";