feat(mysql): support cleartext auth plugin for Warpgate bastions (#336)#337
feat(mysql): support cleartext auth plugin for Warpgate bastions (#336)#337pokertour wants to merge 5 commits into
Conversation
…ularisDB#336) Add an opt-in "cleartext password plugin" toggle for MySQL/MariaDB so connections can authenticate through bastions like Warpgate, which require the mysql_clear_password auth plugin. The option is gated on a TLS mode being enabled — cleartext credentials are refused over an unencrypted link. Bastions like Warpgate proxy MySQL but do not implement the prepared-statement protocol (COM_STMT_PREPARE), so any sqlx::query() — which always prepares — failed with server error 1047 ("Not implemented"). When the cleartext plugin is enabled, the whole MySQL driver now routes statements through the text protocol (COM_QUERY via sqlx::raw_sql): introspection, SHOW CREATE, query execution/streaming, view/trigger DDL, inserts/updates/ deletes (values inlined as escaped literals), EXPLAIN, and export. Also fix a pool cache-key collision: the ad-hoc key (used for unsaved connections, e.g. the New Connection modal) omitted the username. Warpgate multiplexes many targets behind one host:port and selects the backend by username, so two different targets shared a pool and served each other's databases. The key now includes the username and the cleartext flag. Tests: pure literal/escaping helpers and pool-key regressions.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (18 files)
Previous Review Summary (commit c1731f9)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit c1731f9)Status: No Issues Found | Recommendation: Merge Files Reviewed (18 files)
Reviewed by kimi-k2.6-20260420 · Input: 117.3K · Output: 23.7K · Cached: 1.3M |
NewtTheWolf
left a comment
NewtTheWolf
left a comment
There was a problem hiding this comment.
Thanks — nice work, and good catch on the pool-key/username multiplexing bug. 🙏 Text-protocol routing is clean and the pool fix is verified. Two security points before merge, plus one nit. Tests green on the branch (mysql 26, pool 27).
| // bastions like Warpgate. Cleartext credentials must never be sent over an | ||
| // unencrypted link, so refuse to enable it when TLS is disabled. | ||
| if params.enable_cleartext_plugin.unwrap_or(false) { | ||
| if matches!(ssl_mode, MySqlSslMode::Disabled) { |
There was a problem hiding this comment.
blocker — Preferred only tries TLS and falls back to plaintext, so cleartext creds can still cross an unencrypted link — the exact thing this guards against. Gate on Required/VerifyCa/VerifyIdentity only (and drop "Preferred" from the error text).
There was a problem hiding this comment.
fixed in 01a67ec. build_mysql_options now only enables the cleartext plugin when the SSL mode is enforced (Required / VerifyCa / VerifyIdentity); Preferred is rejected alongside Disabled since it can silently fall back to plaintext. Dropped "Preferred" from the error text too. The UI is aligned: the checkbox in NewConnectionModal is now gated on the same enforced modes (and the hint string updated across all 8 locales). Added cleartext_plugin_rejected_with_preferred_tls + cleartext_plugin_allowed_with_enforced_tls.
| /// value can no longer travel as a bind parameter, so it is inlined as an | ||
| /// escaped literal instead. Mirrors `mysql_real_escape_string` for the | ||
| /// default `sql_mode` (backslash escapes enabled). | ||
| pub(super) fn mysql_string_literal(s: &str) -> String { |
There was a problem hiding this comment.
blocker — hardcoded backslash-escaping is unsound under NO_BACKSLASH_ESCAPES (ANSI mode / some bastion targets): 'o\'brien' parses as o'brien by default but breaks there. User cell values flow through here on the text-protocol UPDATE/DELETE path, so this is an injection vector. Check the server's @@sql_mode and double the quote ('') when backslash escaping is off.
There was a problem hiding this comment.
Fixed in 01a67ec. mysql_string_literal is now sql_mode-aware: under NO_BACKSLASH_ESCAPES the backslash is treated as literal and the single quote is escaped by doubling ('') instead of ', so o'brien / trailing backslashes no longer break out of the literal. The server setting is read once per operation via SELECT @@sql_mode and threaded through a small TextProto { enabled, no_backslash_escapes } that replaces the old text: bool across the driver (pk_literal and inline_str_placeholders included). The verbatim query path (exec_on_mysql_conn) uses TextProto::protocol_only, so it skips the extra roundtrip since it never inlines literals. Added mysql_string_literal_no_backslash_escapes_mode and an injection-payload case under both modes.
| /// | ||
| /// Note: this is only safe for the driver's own queries, whose `?` chars are | ||
| /// exclusively bind placeholders (never literal question marks in strings). | ||
| pub(super) fn inline_str_placeholders(sql: &str, binds: &[&str]) -> String { |
There was a problem hiding this comment.
nit — fine for the driver's own queries, but it assumes every ? is a placeholder; it would break if ever reused for user SQL containing a ? inside a string literal. Worth an assert or a name that signals "internal queries only".
There was a problem hiding this comment.
Done in 01a67ec — added a # Safety section to inline_str_placeholders spelling out that it treats every ? as a bind placeholder and is therefore only sound for the driver's own hand-written introspection queries, never arbitrary user SQL.
…de-aware escaping
Resolve the two blockers and the nit from NewtTheWolf's review of the
cleartext/Warpgate feature.
- Gate the cleartext password plugin on an *enforced* TLS mode only
(Required/VerifyCa/VerifyIdentity). `Preferred` merely attempts TLS and can
silently fall back to plaintext, so it is now rejected alongside `Disabled`.
The UI checkbox in NewConnectionModal and the i18n hint (8 locales) are gated
to match the backend.
- Make text-protocol string literals sql_mode-aware. Under NO_BACKSLASH_ESCAPES
(ANSI mode / some bastion targets) the backslash is literal, so `\'` is
mis-parsed and user cell values become an injection vector on the
UPDATE/DELETE/INSERT path. `mysql_string_literal` now doubles the quote (`''`)
in that mode. The setting is read once per op via `SELECT @@sql_mode` and
threaded through a new `TextProto { enabled, no_backslash_escapes }` that
replaces the old `text: bool`; `exec_on_mysql_conn` uses `protocol_only`
(runs user SQL verbatim, no roundtrip).
- Document `inline_str_placeholders` as internal-queries-only (# Safety).
Adds tests for NO_BACKSLASH_ESCAPES escaping and for rejecting cleartext with
Preferred TLS.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Hi @pokertour if you can solve conflicts, will merge it in upstream. |
# Conflicts: # src-tauri/src/drivers/mysql/mod.rs # src-tauri/src/models.rs # src-tauri/src/pool_manager.rs # src-tauri/src/pool_manager_tests.rs # src/components/modals/NewConnectionModal.tsx
3 participants
Add an opt-in "cleartext password plugin" toggle for MySQL/MariaDB so connections can authenticate through bastions like Warpgate, which require the mysql_clear_password auth plugin. The option is gated on a TLS mode being enabled — cleartext credentials are refused over an unencrypted link.
Bastions like Warpgate proxy MySQL but do not implement the prepared-statement protocol (COM_STMT_PREPARE), so any sqlx::query() — which always prepares — failed with server error 1047 ("Not implemented"). When the cleartext plugin is enabled, the whole MySQL driver now routes statements through the text protocol (COM_QUERY via sqlx::raw_sql): introspection, SHOW CREATE, query execution/streaming, view/trigger DDL, inserts/updates/ deletes (values inlined as escaped literals), EXPLAIN, and export.
Also fix a pool cache-key collision: the ad-hoc key (used for unsaved connections, e.g. the New Connection modal) omitted the username. Warpgate multiplexes many targets behind one host:port and selects the backend by username, so two different targets shared a pool and served each other's databases. The key now includes the username and the cleartext flag.
Tests: pure literal/escaping helpers and pool-key regressions.
Closes #336