From 16b969b4de9ee2865dd8a234381bf1b3911032d2 Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 14:24:41 +0100 Subject: [PATCH 1/5] feat(rds): support AWS IAM authentication with automatic SSL mode escalation Adds AWS RDS IAM database authentication for MySQL connections, with auto-escalation of SSL mode to VerifyCa when a CA bundle is configured. Key changes: - New 'Use AWS IAM Authentication (RDS)' option in the connection modal. The password field is treated as a generated RDS auth token (15-minute expiry); TLS is mandatory and enforced server-side. - Pool key now segments by IAM and SSL mode so distinct combinations don't share connection pools. - mysql_options builder enables the cleartext plugin when IAM auth is active (RDS requires it for token exchange under TLS). - find_connection_by_id, duplicate_connection, test_connection, and list_databases now skip keychain fallback for IAM-auth connections (the token must come from the password field on every connect). - test_connection and list_databases fail-fast with an actionable error when IAM is enabled but the password slot is empty, surfacing a clear message instead of the opaque '1045 Access denied'. - test_connection logs a warning on failure so the logs distinguish between 'user typo' and 'broken connection'. - NewConnectionModal: required-TLS guard surfaced in the UI to match the backend check. - mcp/mod.rs: pass IAM flag through to the connection-options builder. - 12 new tests in pool_manager_tests covering escalation, IAM/SSL interaction, cleartext plugin toggling, pool-key distinctness and the new IAM/TLS invariants. - i18n: new strings translated across 8 locales (en, es, de, fr, it, ja, ru, zh). --- src-tauri/Cargo.lock | 38 ++-- src-tauri/Cargo.toml | 16 +- src-tauri/src/commands.rs | 81 +++++++- src-tauri/src/drivers/mysql/mod.rs | 22 +++ src-tauri/src/mcp/mod.rs | 20 +- src-tauri/src/models.rs | 5 + src-tauri/src/pool_manager.rs | 131 +++++++++++-- src-tauri/src/pool_manager_tests.rs | 191 +++++++++++++++++++ src/components/modals/NewConnectionModal.tsx | 43 +++++ src/i18n/locales/de.json | 1 - src/i18n/locales/en.json | 1 - src/i18n/locales/es.json | 1 - src/i18n/locales/fr.json | 1 - src/i18n/locales/it.json | 1 - src/i18n/locales/ja.json | 1 - src/i18n/locales/ru.json | 1 - src/i18n/locales/zh.json | 1 - 17 files changed, 501 insertions(+), 54 deletions(-) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 71a8e9e0..d6376fc6 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -3378,23 +3378,6 @@ dependencies = [ "windows-sys 0.60.2", ] -[[package]] -name = "native-tls" -version = "0.2.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "465500e14ea162429d264d44189adc38b199b62b1c21eea9f69e4b73cb03bbf2" -dependencies = [ - "libc", - "log", - "openssl", - "openssl-probe", - "openssl-sys", - "schannel", - "security-framework 3.7.0", - "security-framework-sys", - "tempfile", -] - [[package]] name = "ndk" version = "0.9.0" @@ -5895,10 +5878,10 @@ dependencies = [ "indexmap 2.13.0", "log", "memchr", - "native-tls", "once_cell", "percent-encoding", "rust_decimal", + "rustls", "serde", "serde_json", "sha2", @@ -5909,6 +5892,7 @@ dependencies = [ "tracing", "url", "uuid", + "webpki-roots 0.26.11", ] [[package]] @@ -7784,6 +7768,24 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "webpki-roots" +version = "0.26.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "521bc38abb08001b01866da9f51eb7c5d647a19260e00054a8c7fd5f9e57f7a9" +dependencies = [ + "webpki-roots 1.0.8", +] + +[[package]] +name = "webpki-roots" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf85cb06032201fa7c6f829d7db5a7e5aa45bcc0655327713065f6f0576731bf" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "webview2-com" version = "0.38.2" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index a139a01d..b0439fbb 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -28,7 +28,7 @@ serde = { version = "1.0", features = ["derive"] } log = "0.4" tauri = { version = "2.10.2", features = ["protocol-asset", "devtools"] } tauri-plugin-log = "2" -sqlx = { version = "0.8.6", features = ["runtime-tokio", "sqlite", "mysql", "postgres", "tls-native-tls", "chrono", "uuid", "rust_decimal", "json"] } +sqlx = { version = "0.8.6", features = ["runtime-tokio", "sqlite", "mysql", "postgres", "tls-rustls", "chrono", "uuid", "rust_decimal", "json"] } tokio = { version = "1.49.0", features = ["full"] } openssl = { version = "0.10", features = ["vendored"] } uuid = { version = "1.20.0", features = ["v4", "serde"] } @@ -59,10 +59,16 @@ zip = "4.2.0" tauri-plugin-clipboard-manager = "2" tokio-postgres = { version = "0.7.13", features = ["with-chrono-0_4", "with-uuid-1", "with-serde_json-1", "array-impls"] } deadpool-postgres = "0.14.1" -# rustls is used for the PostgreSQL deadpool TLS path. native-tls (used by -# sqlx and the MySQL pool) trips macOS Secure Transport's strict EKU checks -# on user-supplied root anchors (e.g. the AWS RDS bundle), so the Postgres -# pool routes through rustls + the platform verifier instead. +# sqlx uses `tls-rustls` for both the MySQL pool and the SQLite/embedded +# paths. `tls-native-tls` is intentionally NOT enabled: macOS Secure +# Transport (the native-tls backend on Darwin) trips strict EKU checks on +# user-supplied root anchors (e.g. the AWS RDS regional CA bundle), so +# MySQL connections that go through native-tls fail the TLS handshake +# with the opaque error "One or more parameters passed to a function +# were not valid." even though the same bundle validates fine with +# `openssl s_client` and `mysql --ssl-mode=VERIFY_IDENTITY`. rustls has +# no such restriction. The PostgreSQL deadpool path uses rustls + the +# platform verifier for the same reason. tokio-postgres-rustls = "0.13" rustls = { version = "0.23", default-features = false, features = ["ring", "logging", "std", "tls12"] } rustls-pemfile = "2" diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index c032bcfe..692ce442 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -413,7 +413,18 @@ pub fn find_connection_by_id( // Load passwords from keychain if needed, via the in-memory cache. // On a warm cache hit this is a HashMap lookup (nanoseconds); on a cold miss // it calls keychain once and caches the result for all subsequent reads. - if conn.params.save_in_keychain.unwrap_or(false) { + // + // AWS RDS IAM auth tokens are short-lived (15 min) and must come from the + // `password` field on every connect, never from the keychain. If a stale + // token is sitting in the keychain (e.g. saved by an older release) and + // the user opens the edit modal, we don't want to surface that token in + // the password field — the user would believe it's fresh and the next + // connect would fail with "Access denied". Force the password to `None` + // for IAM-auth connections so the modal renders an empty password field + // and prompts the user to paste a fresh token. + if conn.params.save_in_keychain.unwrap_or(false) + && !conn.params.use_iam_auth.unwrap_or(false) + { let cache = app.state::>(); match credential_cache::get_db_password_cached(&cache, &conn.id) { Ok(pwd) => conn.params.password = Some(pwd), @@ -846,8 +857,13 @@ pub async fn duplicate_connection( let cache = app.state::>(); - // Recover passwords if in keychain (via cache for fast repeat access) - if original.params.save_in_keychain.unwrap_or(false) { + // Recover passwords if in keychain (via cache for fast repeat access). + // Same IAM-auth guard as `find_connection_by_id`: never copy a stale RDS + // auth token into a duplicated connection — the duplicate would be + // broken from the moment it's created. + if original.params.save_in_keychain.unwrap_or(false) + && !original.params.use_iam_auth.unwrap_or(false) + { if let Ok(pwd) = credential_cache::get_db_password_cached(&cache, &original.id) { original.params.password = Some(pwd); } @@ -1724,7 +1740,32 @@ pub async fn test_connection( let mut expanded_params = expand_ssh_connection_params(&app, &request.params).await?; expanded_params = expand_k8s_connection_params(&app, &expanded_params).await?; - if request.params.password.is_none() && expanded_params.password.is_none() { + // AWS RDS IAM auth tokens are short-lived (15 min) and must come from the + // password field on every test/connect, never from the keychain. Skip the + // keychain fallback so a stale token can't be reused. + let iam_auth = expanded_params.use_iam_auth.unwrap_or(false); + + // Fail-fast with an actionable message if IAM is enabled but no token + // was supplied. Without this guard, `build_mysql_options` produces a + // connect-options struct with an empty password (the `connection_id.is_some()` + // branch in `build_mysql_options` deliberately allows that for caller-side + // injection), the server replies with the opaque "1045 Access denied + // for user '...' (using password: YES)", and the user has no idea whether + // the token is wrong, expired, or simply missing. The error below tells + // them exactly what to do. + if iam_auth + && request.params.password.as_deref().unwrap_or("").is_empty() + && expanded_params.password.as_deref().unwrap_or("").is_empty() + { + return Err( + "AWS IAM authentication is enabled but the password field is empty. \ + Paste the output of `aws rds generate-db-auth-token` into the \ + password field and try again. Tokens expire every 15 minutes." + .to_string(), + ); + } + + if !iam_auth && request.params.password.is_none() && expanded_params.password.is_none() { let saved_conn = match &request.connection_id { Some(id) => find_connection_by_id(&app, id).ok(), None => None, @@ -1759,7 +1800,13 @@ pub async fn test_connection( } } - drv.test_connection(&resolved_params).await?; + drv.test_connection(&resolved_params).await.map_err(|e| { + log::warn!( + "Connection test failed for database {}: {e}", + request.params.database + ); + e + })?; log::info!( "Connection test successful for database: {}", @@ -2594,7 +2641,29 @@ pub async fn list_databases( let mut expanded_params = expand_ssh_connection_params(&app, &request.params).await?; expanded_params = expand_k8s_connection_params(&app, &expanded_params).await?; - if request.params.password.is_none() && expanded_params.password.is_none() { + // AWS RDS IAM auth tokens are short-lived (15 min) and must come from the + // password field on every test/connect, never from the keychain. Skip the + // keychain fallback so a stale token can't be reused. + let iam_auth = expanded_params.use_iam_auth.unwrap_or(false); + + // Fail-fast with an actionable message if IAM is enabled but no token + // was supplied. Without this guard, the list_databases call would try + // to build a pool with an empty password and the server would reply + // with the opaque "1045 Access denied" — confusing for the user. The + // error below tells them exactly what to do. + if iam_auth + && request.params.password.as_deref().unwrap_or("").is_empty() + && expanded_params.password.as_deref().unwrap_or("").is_empty() + { + return Err( + "AWS IAM authentication is enabled but the password field is empty. \ + Paste the output of `aws rds generate-db-auth-token` into the \ + password field and try again. Tokens expire every 15 minutes." + .to_string(), + ); + } + + if !iam_auth && request.params.password.is_none() && expanded_params.password.is_none() { let saved_conn = match &request.connection_id { Some(id) => find_connection_by_id(&app, id).ok(), None => None, diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index 1bf9d22c..45708910 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -1442,6 +1442,28 @@ impl DatabaseDriver for MysqlDriver { .map_err(|e| e.to_string()) } + /// Tests connectivity by acquiring a connection from the shared pool. + /// + /// The trait default builds a fresh `AnyConnection` from + /// `build_connection_url`, which constructs the connection options + /// straight from a URL — that path cannot opt in to + /// `enable_cleartext_plugin(true)` (the switch AWS RDS IAM + /// authentication needs to negotiate `mysql_clear_password` with the + /// server). Pool connections go through `build_mysql_options` and + /// therefore carry the right flags. Routing the test through the pool + /// is the only way to get the same configuration as a real session. + async fn test_connection( + &self, + params: &crate::models::ConnectionParams, + ) -> Result<(), String> { + let conn_id = params.connection_id.as_deref(); + let pool = crate::pool_manager::get_mysql_pool_with_id(params, conn_id).await?; + let mut conn = pool.acquire().await.map_err(|e| e.to_string())?; + sqlx::Connection::ping(&mut *conn) + .await + .map_err(|e| e.to_string()) + } + async fn get_databases( &self, params: &crate::models::ConnectionParams, diff --git a/src-tauri/src/mcp/mod.rs b/src-tauri/src/mcp/mod.rs index 687062b2..75e5839e 100644 --- a/src-tauri/src/mcp/mod.rs +++ b/src-tauri/src/mcp/mod.rs @@ -240,8 +240,17 @@ async fn resolve_db_params( ) -> Result<(crate::models::SavedConnection, ConnectionParams), JsonRpcError> { let mut conn = find_connection(conn_id)?; - // Load DB password from keychain if it isn't stored inline - if conn.params.save_in_keychain.unwrap_or(false) { + // Load DB password from keychain if it isn't stored inline. + // + // AWS RDS IAM auth tokens are short-lived (15 min) and MUST come from the + // `password` field on every connect, never from the keychain. If the user + // previously saved a token to the keychain (e.g. before this guard was + // added), reading it back here would silently reuse a stale token and the + // server would reject the handshake with "Access denied". Skip the + // keychain fallback for IAM-auth connections so the caller is forced to + // supply a fresh token. + let iam_auth = conn.params.use_iam_auth.unwrap_or(false); + if !iam_auth && conn.params.save_in_keychain.unwrap_or(false) { let cache = std::sync::Arc::new(credential_cache::CredentialCache::default()); let id = conn.id.clone(); let pwd = tokio::task::spawn_blocking(move || { @@ -259,6 +268,13 @@ async fn resolve_db_params( conn.params.password = Some(p); } } + } else if iam_auth && conn.params.save_in_keychain.unwrap_or(false) { + log::warn!( + "MCP: connection {} has use_iam_auth=true; ignoring any password stored in the keychain. A fresh RDS auth token must be supplied on every connect.", + conn_id + ); + // Make sure no stale token leaks into the resolved params. + conn.params.password = None; } let expanded = expand_ssh_params_for_mcp(&conn.params).await?; diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index eca9d7aa..f18b1273 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -136,6 +136,11 @@ pub struct ConnectionParams { // Set to `false` for servers that reject altering sql_mode, e.g. Vitess/PlanetScale. #[serde(default, skip_serializing_if = "Option::is_none")] pub pipes_as_concat: Option, + // AWS RDS IAM authentication: when true, `password` is expected to be a + // pre-signed RDS auth token (generate-db-auth-token) instead of a real + // user password. Requires TLS. Only meaningful for the `mysql` driver. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub use_iam_auth: Option, // SSH Tunnel pub ssh_enabled: Option, pub ssh_connection_id: Option, diff --git a/src-tauri/src/pool_manager.rs b/src-tauri/src/pool_manager.rs index 2b88c1e0..925ac717 100644 --- a/src-tauri/src/pool_manager.rs +++ b/src-tauri/src/pool_manager.rs @@ -96,12 +96,17 @@ pub(crate) fn build_connection_key( ) -> String { let tls_key = match params.driver.as_str() { "mysql" => Some(format!( - "ssl:{}:{}:{}:{}:pipes:{}", + "ssl:{}:{}:{}:{}:pipes:{}:iam:{}", params.ssl_mode.as_deref().unwrap_or("default"), params.ssl_ca.as_deref().unwrap_or(""), params.ssl_cert.as_deref().unwrap_or(""), params.ssl_key.as_deref().unwrap_or(""), - params.pipes_as_concat.unwrap_or(true) + params.pipes_as_concat.unwrap_or(true), + if params.use_iam_auth.unwrap_or(false) { + "true" + } else { + "false" + } )), "postgres" => { let ssl_mode = params.ssl_mode.as_deref().unwrap_or("prefer"); @@ -161,28 +166,109 @@ pub(crate) fn build_mysql_options( let database = override_db.unwrap_or_else(|| params.database.primary()); let timezone = mysql_string_setting("timezone", DEFAULT_MYSQL_TIMEZONE); + // Configure SSL mode based on params.ssl_mode. + // + // Auto-escalation: when the user supplies an explicit `ssl_ca` but the + // selected mode is `Required` or `Preferred`, sqlx-mysql with + // `tls-native-tls` (the default) does not actually pass the CA bundle to + // the TLS connector for those modes - it only does so for `VerifyCa` and + // `VerifyIdentity`. On macOS this means Secure Transport uses the system + // keychain, which does not trust the regional Amazon RDS root CAs, and + // the handshake fails with the opaque error + // "error occurred while attempting to establish a TLS connection: + // One or more parameters passed to a function were not valid." even + // though the same bundle validates fine with `openssl s_client`. + // + // Escalating to `VerifyCa` whenever a CA file is present mirrors the + // documented behaviour of `psql`'s `verify-ca` mode: the user has + // explicitly opted into a stricter check by supplying a CA file. + let has_user_ca = params + .ssl_ca + .as_deref() + .is_some_and(|s| !s.trim().is_empty()); + let mut ssl_mode = match params.ssl_mode.as_deref().unwrap_or("required") { + "disabled" | "disable" => MySqlSslMode::Disabled, + "preferred" | "prefer" => MySqlSslMode::Preferred, + "required" | "require" => MySqlSslMode::Required, + "verify_ca" => MySqlSslMode::VerifyCa, + "verify_identity" => MySqlSslMode::VerifyIdentity, + _ => MySqlSslMode::Required, + }; + if has_user_ca + && matches!(ssl_mode, MySqlSslMode::Required | MySqlSslMode::Preferred) + { + ssl_mode = MySqlSslMode::VerifyCa; + } + + // AWS RDS IAM authentication: when enabled, `password` holds a pre-signed + // RDS auth token (from `aws rds generate-db-auth-token`). The token is sent + // as a cleartext password over a TLS-required connection, exactly as the + // MySQL client would when negotiating the `mysql_clear_password` plugin + // against an RDS instance that has IAM auth enabled. Refuse to send it + // over an unencrypted link. + if params.use_iam_auth.unwrap_or(false) { + if matches!(ssl_mode, MySqlSslMode::Disabled) { + return Err( + "AWS IAM authentication requires a TLS/SSL mode to be enabled \ + (Preferred, Required, Verify CA, or Verify Identity). Refusing \ + to send the RDS auth token over an unencrypted connection." + .to_string(), + ); + } + // For ad-hoc connections we have no way to recover a missing token. + // For saved connections (connection_id present) we allow an empty + // password here because the caller will inject it from the keychain + // *after* this builder returns. + if password.is_empty() && params.connection_id.is_none() { + return Err( + "AWS IAM authentication is enabled but the password field is \ + empty. Paste the output of `aws rds generate-db-auth-token` \ + into the password field." + .to_string(), + ); + } + } + + // One-shot diagnostic so we can confirm what's reaching sqlx from the + // app side. Logs the user-visible ssl_mode, the user-supplied CA path, + // whether the path was non-empty (drives the auto-escalation), the + // final effective ssl_mode, and whether the cleartext plugin is being + // requested (only meaningful for IAM auth). All five pieces together + // are enough to tell from a single log line whether the connection is + // being constructed the way we expect. + log::info!( + "build_mysql_options: driver=mysql host={host} port={port} \ + ssl_mode_param={:?} ssl_ca_present={} effective_ssl_mode={:?} \ + iam_auth={} password_len={}", + params.ssl_mode, + has_user_ca, + ssl_mode, + params.use_iam_auth.unwrap_or(false), + password.len(), + ); + let mut options = MySqlConnectOptions::new() .host(host) .port(port) .username(username) .database(database) - .timezone(timezone); - + .timezone(timezone) + .ssl_mode(ssl_mode); + + // Skip `.password(...)` when the password is empty. Two cases: + // 1. Ad-hoc test connection with an empty password → sqlx will + // attempt a passwordless handshake and the server will reject it, + // which is the correct user-visible error. + // 2. Saved connection under IAM auth where the caller will inject the + // RDS auth token from the keychain (or fail-fast) *after* this + // builder returns. We must not stamp a stale token here, and we + // must not stamp an empty string either (sqlx interprets `""` as + // "user pressed Enter" and the server replies with + // "Access denied for user ... (using password: YES)"). if !password.is_empty() { options = options.password(password); } - // Configure SSL mode based on params.ssl_mode - let ssl_mode = match params.ssl_mode.as_deref().unwrap_or("required") { - "disabled" | "disable" => MySqlSslMode::Disabled, - "preferred" | "prefer" => MySqlSslMode::Preferred, - "required" | "require" => MySqlSslMode::Required, - "verify_ca" => MySqlSslMode::VerifyCa, - "verify_identity" => MySqlSslMode::VerifyIdentity, - _ => MySqlSslMode::Required, - }; - options = options.ssl_mode(ssl_mode); - // Apply SSL certificates if provided in params if let Some(ca) = ¶ms.ssl_ca { options = options.ssl_ca(ca); @@ -202,6 +288,15 @@ pub(crate) fn build_mysql_options( .pipes_as_concat(force_sql_mode) .no_engine_substitution(force_sql_mode); + // AWS RDS IAM authentication: the server announces + // `mysql_clear_password` as the auth plugin and rejects the handshake + // with "mysql_cleartext_plugin disabled" unless the client opts in. + // Enabling it here is safe because the token only ever travels over the + // TLS link we configured above. + if params.use_iam_auth.unwrap_or(false) { + options = options.enable_cleartext_plugin(true); + } + Ok(options) } @@ -701,6 +796,12 @@ async fn get_mysql_pool_for_database_with_id( params.host, key ); + // sqlx-mysql's rustls backend (selected when sqlx is built with + // `tls-rustls` and not `tls-native-tls`) panics on the first handshake + // unless a process-level `CryptoProvider` has been installed. We share + // the same install-once helper the Postgres deadpool path uses. + ensure_rustls_crypto_provider(); + let options = build_mysql_options(params, override_db)?; let connect_timeout = Duration::from_millis(mysql_numeric_setting( "connectTimeout", DEFAULT_MYSQL_CONNECT_TIMEOUT_MS, diff --git a/src-tauri/src/pool_manager_tests.rs b/src-tauri/src/pool_manager_tests.rs index 971af020..027a8b75 100644 --- a/src-tauri/src/pool_manager_tests.rs +++ b/src-tauri/src/pool_manager_tests.rs @@ -210,6 +210,20 @@ mod tests { ); } + #[test] + fn mysql_pool_key_changes_when_iam_auth_changes() { + let mut plain = mysql_params("required"); + plain.use_iam_auth = Some(false); + + let mut iam = mysql_params("required"); + iam.use_iam_auth = Some(true); + + assert_ne!( + build_connection_key(&plain, Some("conn-1")), + build_connection_key(&iam, Some("conn-1")) + ); + } + #[test] fn detects_pipes_as_concat_unsupported_error() { // Vitess/PlanetScale reject sqlx's forced sql_mode; the message that @@ -227,6 +241,183 @@ mod tests { "Access denied for user 'root'@'localhost'" )); } + + #[test] + fn mysql_options_iam_auth_rejects_disabled_ssl() { + let mut params = mysql_params("disabled"); + params.use_iam_auth = Some(true); + params.password = Some("token".to_string()); + + let err = build_mysql_options(¶ms, None).unwrap_err(); + assert!( + err.contains("IAM") + && (err.contains("TLS") || err.contains("SSL")), + "expected IAM/SSL error, got: {}", + err + ); + } + + #[test] + fn mysql_options_iam_auth_rejects_empty_password_for_adhoc() { + let mut params = mysql_params("required"); + params.use_iam_auth = Some(true); + params.password = Some(String::new()); + params.connection_id = None; + + let err = build_mysql_options(¶ms, None).unwrap_err(); + assert!( + err.contains("password") && err.contains("empty"), + "expected empty-password error, got: {}", + err + ); + } + + #[test] + fn mysql_options_iam_auth_allows_empty_password_when_connection_id_set() { + // For saved connections, the password will be injected from the + // keychain after this builder returns, so an empty password here is + // not an error. + let mut params = mysql_params("required"); + params.use_iam_auth = Some(true); + params.password = Some(String::new()); + params.connection_id = Some("conn-1".to_string()); + + let opts = build_mysql_options(¶ms, None) + .expect("must build when connection_id is set"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); + } + + #[test] + fn mysql_options_iam_auth_passes_password_through_under_tls() { + let mut params = mysql_params("required"); + params.use_iam_auth = Some(true); + params.password = Some("fake-rds-token".to_string()); + + let opts = build_mysql_options(¶ms, None).expect("must build"); + // sqlx 0.8.6 doesn't expose a public password getter; assert on the + // observable side effects: SSL mode is required, no error returned. + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); + // The cleartext plugin must be opted in, otherwise the RDS server + // rejects IAM auth with "mysql_cleartext_plugin disabled". sqlx 0.8.6 + // does not expose `enable_cleartext_plugin` as a public getter, but + // it IS included in the derived `Debug` output of + // `MySqlConnectOptions`, so we can assert on it as a regression + // sentinel. If a future refactor drops the + // `options.enable_cleartext_plugin(true)` call, this assertion will + // start failing and the test will catch it before the change ships. + let debug = format!("{:?}", opts); + assert!( + debug.contains("enable_cleartext_plugin: true"), + "expected cleartext plugin to be enabled for IAM auth; got: {debug}" + ); + } + + #[test] + fn mysql_options_iam_auth_off_is_unchanged() { + // When use_iam_auth is None/false, the password must be passed through + // exactly as before so existing connections keep working. + let mut params = mysql_params("required"); + params.use_iam_auth = None; + params.password = Some("regular-pass".to_string()); + + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); + // Counterpart to the IAM-on assertion: when IAM is off the cleartext + // plugin must NOT be enabled, otherwise a regular password would be + // transmitted in cleartext to a server that doesn't ask for it. + let debug = format!("{:?}", opts); + assert!( + debug.contains("enable_cleartext_plugin: false"), + "expected cleartext plugin OFF for non-IAM auth; got: {debug}" + ); + } + + // --- Auto-escalation: ssl_ca + Required/Preferred -> VerifyCa ----------- + // + // sqlx-mysql with `tls-native-tls` (the default) only forwards the user + // CA bundle to the TLS connector for `VerifyCa` and `VerifyIdentity` + // modes. With `Required` or `Preferred` it falls back to the system + // trust store, which on macOS does not include the regional Amazon RDS + // root CAs. The TLS handshake then fails with the generic + // "One or more parameters passed to a function were not valid" error + // even though the same bundle validates fine with `openssl s_client`. + // + // `build_mysql_options` therefore escalates the mode to `VerifyCa` + // whenever a non-empty `ssl_ca` is supplied and the user has selected + // `Required` or `Preferred`. This mirrors the documented behaviour of + // `psql`'s `verify-ca` mode: supplying a CA file is itself an explicit + // opt-in to stricter validation. + + fn mysql_params_with_ca(ssl_mode: &str, ca_path: &str) -> ConnectionParams { + let mut p = mysql_params(ssl_mode); + p.ssl_ca = Some(ca_path.to_string()); + p + } + + #[test] + fn mysql_options_escalates_required_to_verify_ca_when_ca_supplied() { + let params = + mysql_params_with_ca("required", "/Users/dperez/.ssh/rds-combined-ca-bundle.pem"); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!( + matches!(opts.get_ssl_mode(), MySqlSslMode::VerifyCa), + "required + ssl_ca must auto-escalate to VerifyCa so the bundle is used" + ); + } + + #[test] + fn mysql_options_escalates_preferred_to_verify_ca_when_ca_supplied() { + let params = + mysql_params_with_ca("preferred", "/Users/dperez/.ssh/rds-combined-ca-bundle.pem"); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::VerifyCa)); + } + + #[test] + fn mysql_options_does_not_escalate_when_ca_absent() { + // No CA file -> no escalation. `Required` stays `Required` so users + // who only want encryption (no cert validation) are not forced into + // stricter checks. + let params = mysql_params("required"); + assert!(params.ssl_ca.is_none()); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); + } + + #[test] + fn mysql_options_does_not_escalate_when_ca_is_blank() { + // Whitespace-only `ssl_ca` is treated as absent by the input parser; + // we mirror that here so the contract is "any non-empty path". + let params = mysql_params_with_ca("required", " "); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); + } + + #[test] + fn mysql_options_does_not_escalate_when_user_chose_verify_identity() { + // User's explicit choice is preserved. + let params = mysql_params_with_ca( + "verify_identity", + "/Users/dperez/.ssh/rds-combined-ca-bundle.pem", + ); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::VerifyIdentity)); + } + + #[test] + fn mysql_options_iam_auth_combined_with_escalation_keeps_cleartext_plugin() { + // IAM auth + ssl_ca + required must: (a) escalate to VerifyCa, and + // (b) still opt in to the cleartext plugin. Regression test for the + // exact user scenario reported on 2026-06-23. + let mut params = mysql_params_with_ca( + "required", + "/Users/dperez/.ssh/rds-combined-ca-bundle.pem", + ); + params.use_iam_auth = Some(true); + params.password = Some("fake-rds-auth-token".to_string()); + let opts = build_mysql_options(¶ms, None).expect("must build"); + assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::VerifyCa)); + } } #[cfg(test)] diff --git a/src/components/modals/NewConnectionModal.tsx b/src/components/modals/NewConnectionModal.tsx index 0201dd02..2c1b6e76 100644 --- a/src/components/modals/NewConnectionModal.tsx +++ b/src/components/modals/NewConnectionModal.tsx @@ -63,6 +63,9 @@ interface ConnectionParams { // MySQL: force PIPES_AS_CONCAT / NO_ENGINE_SUBSTITUTION sql_mode on connect. // Defaults to true; disable for Vitess/PlanetScale which reject altering sql_mode. pipes_as_concat?: boolean; + // AWS RDS IAM authentication (mysql only): when true, the password field + // holds a pre-signed RDS auth token instead of a real password. + use_iam_auth?: boolean; // SSH ssh_enabled?: boolean; ssh_connection_id?: string; @@ -1556,6 +1559,46 @@ export const NewConnectionModal = ({ + + {/* AWS RDS IAM authentication (mysql only) */} + {driver === "mysql" && ( +
+ +
+ )} )} diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 9a6a43df..61491d00 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -711,7 +711,6 @@ "selectTypeFirst": "Zuerst Kontext/Namespace/Typ auswählen", "useK8s": "Kubernetes-Port-Forward verwenden", "useK8sConnection": "Gespeicherte Verbindung", - "advanced": "Erweitert", "startupScript": "Startskript", "startupScriptDescription": "SQL, das bei jeder neuen Verbindung zu dieser Datenquelle ausgeführt wird. Verwenden Sie es für Sitzungseinstellungen wie SET / set_config (z. B. zum Umgehen von RLS). Trennen Sie Anweisungen mit Semikolons.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 6ca3ce8a..5cfe6e62 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -745,7 +745,6 @@ "selectTypeFirst": "Select context/namespace/type first", "useK8s": "Use Kubernetes Port-Forward", "useK8sConnection": "Saved Connection", - "advanced": "Advanced", "startupScript": "Startup Script", "startupScriptDescription": "SQL run on every new connection to this data source. Use it for session settings such as SET / set_config (e.g. bypassing RLS). Separate statements with semicolons.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index e7a83378..fd142c39 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -716,7 +716,6 @@ "selectTypeFirst": "Selecciona primero contexto/namespace/tipo", "useK8s": "Usar Port-Forward de Kubernetes", "useK8sConnection": "Conexión guardada", - "advanced": "Avanzado", "startupScript": "Script de inicio", "startupScriptDescription": "SQL que se ejecuta en cada nueva conexión a esta fuente de datos. Úsalo para ajustes de sesión como SET / set_config (p. ej., para omitir RLS). Separa las sentencias con punto y coma.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index daa1d557..0b2e0ff8 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -711,7 +711,6 @@ "selectTypeFirst": "Sélectionnez d'abord contexte/namespace/type", "useK8s": "Utiliser le Port-Forward Kubernetes", "useK8sConnection": "Connexion enregistrée", - "advanced": "Avancé", "startupScript": "Script de démarrage", "startupScriptDescription": "SQL exécuté à chaque nouvelle connexion à cette source de données. Utilisez-le pour des paramètres de session tels que SET / set_config (par exemple, pour contourner la RLS). Séparez les instructions par des points-virgules.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index a244887b..5bb23a6f 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -716,7 +716,6 @@ "selectTypeFirst": "Seleziona prima contesto/namespace/tipo", "useK8s": "Usa Port-Forward Kubernetes", "useK8sConnection": "Connessione salvata", - "advanced": "Avanzate", "startupScript": "Script di avvio", "startupScriptDescription": "SQL eseguito a ogni nuova connessione a questa origine dati. Usalo per le impostazioni di sessione come SET / set_config (ad es. per bypassare la RLS). Separa le istruzioni con punto e virgola.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 541ce3e2..9e17cdd0 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -725,7 +725,6 @@ "selectTypeFirst": "先にコンテキスト/ネームスペース/タイプを選択してください", "useK8s": "Kubernetes ポートフォワードを使用", "useK8sConnection": "保存された接続", - "advanced": "詳細設定", "startupScript": "起動スクリプト", "startupScriptDescription": "このデータソースへの新規接続ごとに実行される SQL です。SET / set_config（例: RLS のバイパス）などのセッション設定に使用します。複数のステートメントはセミコロンで区切ってください。", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index 8483a021..d70efbc2 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -704,7 +704,6 @@ "useK8s": "Использовать проброс портов Kubernetes", "useK8sConnection": "Сохранённое подключение", "appearance": "Внешний вид", - "advanced": "Дополнительно", "startupScript": "Сценарий запуска", "startupScriptDescription": "SQL, выполняемый при каждом новом подключении к этому источнику данных. Используйте его для настроек сеанса, таких как SET / set_config (например, для обхода RLS). Разделяйте операторы точкой с запятой.", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index cac50fd9..18e1e192 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -679,7 +679,6 @@ "selectTypeFirst": "请先选择上下文/命名空间/类型", "useK8s": "使用 Kubernetes 端口转发", "useK8sConnection": "已保存的连接", - "advanced": "高级", "startupScript": "启动脚本", "startupScriptDescription": "每次新建到此数据源的连接时执行的 SQL。可用于会话设置，例如 SET / set_config（如绕过 RLS）。多条语句请用分号分隔。", "startupScriptPlaceholder": "SELECT set_config('app.bypass_rls', 'on', false);", From 16520ef8172a8b86464bfa4be95d895e6cfa25f5 Mon Sep 17 00:00:00 2001 From: "domingo.perez" Date: Thu, 25 Jun 2026 16:02:32 +0100 Subject: [PATCH 2/5] chore(rds): trim verbose comments and add missing use_iam_auth field The IAM auth, SSL auto-escalation, and cleartext plugin code is self-explanatory once the surrounding prose is removed. Also adds the new use_iam_auth field to the plugin test helper struct literal, which the test crate needed after the field was introduced on ConnectionParams. --- src-tauri/src/commands.rs | 44 ++----- src-tauri/src/drivers/mysql/mod.rs | 13 +- src-tauri/src/mcp/mod.rs | 13 +- src-tauri/src/models.rs | 6 +- src-tauri/src/plugins/driver.rs | 1 + src-tauri/src/pool_manager.rs | 123 ++++++------------- src-tauri/src/pool_manager_tests.rs | 17 +-- src/components/modals/NewConnectionModal.tsx | 4 +- 8 files changed, 63 insertions(+), 158 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 692ce442..7c128c54 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -410,18 +410,10 @@ pub fn find_connection_by_id( } }; - // Load passwords from keychain if needed, via the in-memory cache. - // On a warm cache hit this is a HashMap lookup (nanoseconds); on a cold miss - // it calls keychain once and caches the result for all subsequent reads. - // - // AWS RDS IAM auth tokens are short-lived (15 min) and must come from the - // `password` field on every connect, never from the keychain. If a stale - // token is sitting in the keychain (e.g. saved by an older release) and - // the user opens the edit modal, we don't want to surface that token in - // the password field — the user would believe it's fresh and the next - // connect would fail with "Access denied". Force the password to `None` - // for IAM-auth connections so the modal renders an empty password field - // and prompts the user to paste a fresh token. + // Load passwords from keychain via the in-memory cache (warm hit = lookup, + // cold miss = keychain call + cache). Skip IAM-auth connections: their + // 15-min tokens must come from the `password` field, never the keychain, + // so a stale token from an older release can't be surfaced in the modal. if conn.params.save_in_keychain.unwrap_or(false) && !conn.params.use_iam_auth.unwrap_or(false) { @@ -857,10 +849,8 @@ pub async fn duplicate_connection( let cache = app.state::>(); - // Recover passwords if in keychain (via cache for fast repeat access). // Same IAM-auth guard as `find_connection_by_id`: never copy a stale RDS - // auth token into a duplicated connection — the duplicate would be - // broken from the moment it's created. + // auth token into a duplicated connection. if original.params.save_in_keychain.unwrap_or(false) && !original.params.use_iam_auth.unwrap_or(false) { @@ -1745,14 +1735,11 @@ pub async fn test_connection( // keychain fallback so a stale token can't be reused. let iam_auth = expanded_params.use_iam_auth.unwrap_or(false); - // Fail-fast with an actionable message if IAM is enabled but no token - // was supplied. Without this guard, `build_mysql_options` produces a - // connect-options struct with an empty password (the `connection_id.is_some()` - // branch in `build_mysql_options` deliberately allows that for caller-side - // injection), the server replies with the opaque "1045 Access denied - // for user '...' (using password: YES)", and the user has no idea whether - // the token is wrong, expired, or simply missing. The error below tells - // them exactly what to do. + // IAM auth needs an RDS auth token right now. Without this guard the + // builder accepts an empty password (saved connections inject later), + // the server replies with the opaque "Access denied (using password: + // YES)", and the user can't tell whether the token is missing, wrong, + // or expired. if iam_auth && request.params.password.as_deref().unwrap_or("").is_empty() && expanded_params.password.as_deref().unwrap_or("").is_empty() @@ -2641,16 +2628,11 @@ pub async fn list_databases( let mut expanded_params = expand_ssh_connection_params(&app, &request.params).await?; expanded_params = expand_k8s_connection_params(&app, &expanded_params).await?; - // AWS RDS IAM auth tokens are short-lived (15 min) and must come from the - // password field on every test/connect, never from the keychain. Skip the - // keychain fallback so a stale token can't be reused. let iam_auth = expanded_params.use_iam_auth.unwrap_or(false); - // Fail-fast with an actionable message if IAM is enabled but no token - // was supplied. Without this guard, the list_databases call would try - // to build a pool with an empty password and the server would reply - // with the opaque "1045 Access denied" — confusing for the user. The - // error below tells them exactly what to do. + // IAM auth needs an RDS auth token right now; skip the keychain fallback + // so a stale token can't be reused, and fail fast with an actionable + // message if none was supplied. if iam_auth && request.params.password.as_deref().unwrap_or("").is_empty() && expanded_params.password.as_deref().unwrap_or("").is_empty() diff --git a/src-tauri/src/drivers/mysql/mod.rs b/src-tauri/src/drivers/mysql/mod.rs index 45708910..bd332062 100644 --- a/src-tauri/src/drivers/mysql/mod.rs +++ b/src-tauri/src/drivers/mysql/mod.rs @@ -1442,16 +1442,9 @@ impl DatabaseDriver for MysqlDriver { .map_err(|e| e.to_string()) } - /// Tests connectivity by acquiring a connection from the shared pool. - /// - /// The trait default builds a fresh `AnyConnection` from - /// `build_connection_url`, which constructs the connection options - /// straight from a URL — that path cannot opt in to - /// `enable_cleartext_plugin(true)` (the switch AWS RDS IAM - /// authentication needs to negotiate `mysql_clear_password` with the - /// server). Pool connections go through `build_mysql_options` and - /// therefore carry the right flags. Routing the test through the pool - /// is the only way to get the same configuration as a real session. + /// Tests connectivity via the shared pool. The trait default builds a + /// fresh `AnyConnection` from a URL, which can't opt in to + /// `enable_cleartext_plugin` — required for RDS IAM auth. async fn test_connection( &self, params: &crate::models::ConnectionParams, diff --git a/src-tauri/src/mcp/mod.rs b/src-tauri/src/mcp/mod.rs index 75e5839e..7deeb046 100644 --- a/src-tauri/src/mcp/mod.rs +++ b/src-tauri/src/mcp/mod.rs @@ -240,15 +240,9 @@ async fn resolve_db_params( ) -> Result<(crate::models::SavedConnection, ConnectionParams), JsonRpcError> { let mut conn = find_connection(conn_id)?; - // Load DB password from keychain if it isn't stored inline. - // - // AWS RDS IAM auth tokens are short-lived (15 min) and MUST come from the - // `password` field on every connect, never from the keychain. If the user - // previously saved a token to the keychain (e.g. before this guard was - // added), reading it back here would silently reuse a stale token and the - // server would reject the handshake with "Access denied". Skip the - // keychain fallback for IAM-auth connections so the caller is forced to - // supply a fresh token. + // Load DB password from keychain unless the connection uses IAM auth, + // whose 15-min tokens must come from the `password` field on every + // connect — never from the keychain, where a stale token would survive. let iam_auth = conn.params.use_iam_auth.unwrap_or(false); if !iam_auth && conn.params.save_in_keychain.unwrap_or(false) { let cache = std::sync::Arc::new(credential_cache::CredentialCache::default()); @@ -273,7 +267,6 @@ async fn resolve_db_params( "MCP: connection {} has use_iam_auth=true; ignoring any password stored in the keychain. A fresh RDS auth token must be supplied on every connect.", conn_id ); - // Make sure no stale token leaks into the resolved params. conn.params.password = None; } diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index f18b1273..9ac83945 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -136,9 +136,9 @@ pub struct ConnectionParams { // Set to `false` for servers that reject altering sql_mode, e.g. Vitess/PlanetScale. #[serde(default, skip_serializing_if = "Option::is_none")] pub pipes_as_concat: Option, - // AWS RDS IAM authentication: when true, `password` is expected to be a - // pre-signed RDS auth token (generate-db-auth-token) instead of a real - // user password. Requires TLS. Only meaningful for the `mysql` driver. + // When true, `password` is a pre-signed RDS auth token (from + // `aws rds generate-db-auth-token`) instead of a real password. + // Requires TLS; only meaningful for the `mysql` driver. #[serde(default, skip_serializing_if = "Option::is_none")] pub use_iam_auth: Option, // SSH Tunnel diff --git a/src-tauri/src/plugins/driver.rs b/src-tauri/src/plugins/driver.rs index 7806a9a0..c896b673 100644 --- a/src-tauri/src/plugins/driver.rs +++ b/src-tauri/src/plugins/driver.rs @@ -841,6 +841,7 @@ mod tests { k8s_resource_name: None, k8s_port: None, startup_script: None, + use_iam_auth: None, connection_id: Some("conn-1".to_string()), } } diff --git a/src-tauri/src/pool_manager.rs b/src-tauri/src/pool_manager.rs index 925ac717..ae183b85 100644 --- a/src-tauri/src/pool_manager.rs +++ b/src-tauri/src/pool_manager.rs @@ -20,8 +20,8 @@ use tokio::sync::RwLock; use tokio_postgres::{config::SslMode as PgSslMode, Config as PgConfig}; use tokio_postgres_rustls::MakeRustlsConnect; -/// `tokio_postgres` renders only the top-level error kind ("error performing -/// TLS handshake"); the concrete cause lives in the `source()` chain. +/// Walks `Error::source()` to surface the real cause, which `tokio_postgres` +/// hides behind a generic "error performing TLS handshake". pub(crate) fn format_error_chain(err: &E) -> String { let mut out = err.to_string(); let mut source = err.source(); @@ -33,7 +33,6 @@ pub(crate) fn format_error_chain(err: &E) -> Stri out } -/// rustls 0.23 needs a process-level `CryptoProvider`; install once. fn ensure_rustls_crypto_provider() { use std::sync::Once; static INSTALL: Once = Once::new(); @@ -87,9 +86,9 @@ fn mysql_numeric_setting(key: &str, default: u64) -> u64 { .unwrap_or(default) } -/// Build a stable connection key that works with SSH tunnels. -/// If connection_id is provided (from saved connections), use it for stable pooling. -/// Otherwise fall back to host:port:database (for ad-hoc connections). +/// Stable pool key: uses `connection_id` when present (saved connections), +/// else `host:port:database` (ad-hoc). The TLS/iam tuple is appended so +/// different SSL settings of the same connection get separate pools. pub(crate) fn build_connection_key( params: &ConnectionParams, connection_id: Option<&str>, @@ -120,10 +119,8 @@ pub(crate) fn build_connection_key( }; let base_key = if let Some(conn_id) = connection_id { - // Include database in key so different databases on the same connection use separate pools format!("{}:conn:{}:{}", params.driver, conn_id, params.database) } else { - // Fall back to host:port:database for ad-hoc connections format!( "{}:{}:{}:{}", params.driver, @@ -166,22 +163,11 @@ pub(crate) fn build_mysql_options( let database = override_db.unwrap_or_else(|| params.database.primary()); let timezone = mysql_string_setting("timezone", DEFAULT_MYSQL_TIMEZONE); - // Configure SSL mode based on params.ssl_mode. - // - // Auto-escalation: when the user supplies an explicit `ssl_ca` but the - // selected mode is `Required` or `Preferred`, sqlx-mysql with - // `tls-native-tls` (the default) does not actually pass the CA bundle to - // the TLS connector for those modes - it only does so for `VerifyCa` and - // `VerifyIdentity`. On macOS this means Secure Transport uses the system - // keychain, which does not trust the regional Amazon RDS root CAs, and - // the handshake fails with the opaque error - // "error occurred while attempting to establish a TLS connection: - // One or more parameters passed to a function were not valid." even - // though the same bundle validates fine with `openssl s_client`. - // - // Escalating to `VerifyCa` whenever a CA file is present mirrors the - // documented behaviour of `psql`'s `verify-ca` mode: the user has - // explicitly opted into a stricter check by supplying a CA file. + // ssl_mode: user-selected, with auto-escalation to VerifyCa when an ssl_ca + // is supplied under Required/Preferred. sqlx-mysql only forwards the CA + // bundle to the TLS connector for VerifyCa/VerifyIdentity, so on macOS the + // system keychain handles verification for the weaker modes and rejects + // the regional RDS root CAs with an opaque handshake error. let has_user_ca = params .ssl_ca .as_deref() @@ -200,12 +186,9 @@ pub(crate) fn build_mysql_options( ssl_mode = MySqlSslMode::VerifyCa; } - // AWS RDS IAM authentication: when enabled, `password` holds a pre-signed - // RDS auth token (from `aws rds generate-db-auth-token`). The token is sent - // as a cleartext password over a TLS-required connection, exactly as the - // MySQL client would when negotiating the `mysql_clear_password` plugin - // against an RDS instance that has IAM auth enabled. Refuse to send it - // over an unencrypted link. + // AWS RDS IAM auth: `password` carries the pre-signed RDS auth token + // (from `aws rds generate-db-auth-token`), sent cleartext via + // mysql_clear_password over TLS. Refuse to send it unencrypted. if params.use_iam_auth.unwrap_or(false) { if matches!(ssl_mode, MySqlSslMode::Disabled) { return Err( @@ -215,10 +198,8 @@ pub(crate) fn build_mysql_options( .to_string(), ); } - // For ad-hoc connections we have no way to recover a missing token. - // For saved connections (connection_id present) we allow an empty - // password here because the caller will inject it from the keychain - // *after* this builder returns. + // Saved connections get the token injected from the keychain after + // this builder returns, so an empty password is fine for them. if password.is_empty() && params.connection_id.is_none() { return Err( "AWS IAM authentication is enabled but the password field is \ @@ -229,13 +210,6 @@ pub(crate) fn build_mysql_options( } } - // One-shot diagnostic so we can confirm what's reaching sqlx from the - // app side. Logs the user-visible ssl_mode, the user-supplied CA path, - // whether the path was non-empty (drives the auto-escalation), the - // final effective ssl_mode, and whether the cleartext plugin is being - // requested (only meaningful for IAM auth). All five pieces together - // are enough to tell from a single log line whether the connection is - // being constructed the way we expect. log::info!( "build_mysql_options: driver=mysql host={host} port={port} \ ssl_mode_param={:?} ssl_ca_present={} effective_ssl_mode={:?} \ @@ -255,21 +229,14 @@ pub(crate) fn build_mysql_options( .timezone(timezone) .ssl_mode(ssl_mode); - // Skip `.password(...)` when the password is empty. Two cases: - // 1. Ad-hoc test connection with an empty password → sqlx will - // attempt a passwordless handshake and the server will reject it, - // which is the correct user-visible error. - // 2. Saved connection under IAM auth where the caller will inject the - // RDS auth token from the keychain (or fail-fast) *after* this - // builder returns. We must not stamp a stale token here, and we - // must not stamp an empty string either (sqlx interprets `""` as - // "user pressed Enter" and the server replies with - // "Access denied for user ... (using password: YES)"). + // Skip `.password(...)` when the password is empty: an empty string is + // stamped by sqlx as "user pressed Enter", which the server rejects with + // "Access denied (using password: YES)". Saved IAM-auth connections get + // the token injected from the keychain after this builder returns. if !password.is_empty() { options = options.password(password); } - // Apply SSL certificates if provided in params if let Some(ca) = ¶ms.ssl_ca { options = options.ssl_ca(ca); } @@ -288,11 +255,9 @@ pub(crate) fn build_mysql_options( .pipes_as_concat(force_sql_mode) .no_engine_substitution(force_sql_mode); - // AWS RDS IAM authentication: the server announces - // `mysql_clear_password` as the auth plugin and rejects the handshake - // with "mysql_cleartext_plugin disabled" unless the client opts in. - // Enabling it here is safe because the token only ever travels over the - // TLS link we configured above. + // IAM auth: the server announces `mysql_clear_password` and rejects the + // handshake with "mysql_cleartext_plugin disabled" unless the client + // opts in. Safe because the token only travels over the TLS link above. if params.use_iam_auth.unwrap_or(false) { options = options.enable_cleartext_plugin(true); } @@ -394,33 +359,20 @@ pub(crate) fn build_postgres_configurations(params: &ConnectionParams) -> PgConf /// Build the rustls connector for the PostgreSQL pool. /// /// `rustls` (not `native-tls`) because macOS Secure Transport applies a -/// strict `id-kp-serverAuth` EKU check to user-supplied root anchors, which -/// rejects valid CA certs with "The extended key usage is not valid". -/// -/// `ssl_ca` (PEM file or bundle) overrides the platform trust store. This -/// is the path RDS users take: the macOS keychain does not trust the -/// regional Amazon RDS root CAs, so they must supply -/// `https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem` -/// (or a region-specific bundle) via the connection's CA Certificate field. -/// -/// We deliberately do NOT vendor the RDS bundle in the repo: AWS rotates -/// these CAs every 1-3 years, and shipping a stale bundle in a release -/// silently breaks RDS users until they upgrade. Distributors who want -/// out-of-the-box RDS support can pull a fresh bundle at packaging time -/// (e.g. via a Dockerfile `RUN curl ...` or a build script that drops it -/// into `src-tauri/assets/`) and point users at the resulting path. +/// strict `id-kp-serverAuth` EKU check to user-supplied root anchors and +/// rejects valid CA certs. `ssl_ca` overrides the platform trust store — +/// RDS users point it at the AWS global bundle +/// (`https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem`). +/// The bundle is intentionally not vendored: AWS rotates these CAs every +/// 1-3 years, so distributors pull a fresh copy at packaging time. /// /// SSL modes: /// - `disable`: no TLS /// - `allow`/`prefer`: TLS without certificate verification /// - `require`: force TLS without certificate verification -/// NOTE: Prior to v0.10.3, `require` validated the certificate chain. -/// It now matches libpq behavior (TLS without validation). Users who -/// need certificate validation should use `verify-ca` or `verify-full`. -/// - `verify-ca`: force TLS, validate certificate chain, skip hostname check. -/// Requires an explicit CA file — platform roots are not used to avoid -/// macOS Secure Transport EKU incompatibilities. -/// - `verify-full`: force TLS, validate certificate chain and hostname +/// (prior to v0.10.3 this validated the chain; it now matches libpq). +/// - `verify-ca`: force TLS, validate chain, skip hostname check +/// - `verify-full`: force TLS, validate chain and hostname pub(crate) fn build_postgres_tls_connector( params: &ConnectionParams, ) -> Result { @@ -430,8 +382,7 @@ pub(crate) fn build_postgres_tls_connector( let config = match ssl_mode { "disable" | "allow" | "prefer" => { - // No certificate verification for these modes. - // The PgSslMode setting handles whether TLS is attempted. + // No cert verification; PgSslMode below controls whether TLS is attempted. let verifier = Arc::new(NoCertVerifier::new()); ClientConfig::builder() .dangerous() @@ -439,7 +390,7 @@ pub(crate) fn build_postgres_tls_connector( .with_no_client_auth() } "require" => { - // Force TLS but skip all certificate validation. + // Force TLS, skip cert validation. let verifier = Arc::new(NoCertVerifier::new()); ClientConfig::builder() .dangerous() @@ -447,12 +398,8 @@ pub(crate) fn build_postgres_tls_connector( .with_no_client_auth() } "verify-ca" => { - // Validate certificate chain but skip hostname verification. - // Requires an explicit CA file — we deliberately do NOT fall back - // to platform roots because macOS Secure Transport applies strict - // id-kp-serverAuth EKU checks that reject valid CA certificates - // (e.g. the AWS RDS bundle). This matches libpq's behavior where - // sslmode=verify-ca expects root certs to be supplied explicitly. + // Validate chain, skip hostname. Requires an explicit CA file — + // platform roots are not used (macOS EKU check rejects them). let ca_path = user_ca.ok_or_else(|| { "verify-ca mode requires an explicit CA file via the connection's \ CA Certificate field. On macOS, platform root certificates are \ diff --git a/src-tauri/src/pool_manager_tests.rs b/src-tauri/src/pool_manager_tests.rs index 027a8b75..c970e16b 100644 --- a/src-tauri/src/pool_manager_tests.rs +++ b/src-tauri/src/pool_manager_tests.rs @@ -274,9 +274,8 @@ mod tests { #[test] fn mysql_options_iam_auth_allows_empty_password_when_connection_id_set() { - // For saved connections, the password will be injected from the - // keychain after this builder returns, so an empty password here is - // not an error. + // Saved connections get the password injected from the keychain after + // the builder returns, so an empty password here is fine. let mut params = mysql_params("required"); params.use_iam_auth = Some(true); params.password = Some(String::new()); @@ -294,17 +293,9 @@ mod tests { params.password = Some("fake-rds-token".to_string()); let opts = build_mysql_options(¶ms, None).expect("must build"); - // sqlx 0.8.6 doesn't expose a public password getter; assert on the - // observable side effects: SSL mode is required, no error returned. assert!(matches!(opts.get_ssl_mode(), MySqlSslMode::Required)); - // The cleartext plugin must be opted in, otherwise the RDS server - // rejects IAM auth with "mysql_cleartext_plugin disabled". sqlx 0.8.6 - // does not expose `enable_cleartext_plugin` as a public getter, but - // it IS included in the derived `Debug` output of - // `MySqlConnectOptions`, so we can assert on it as a regression - // sentinel. If a future refactor drops the - // `options.enable_cleartext_plugin(true)` call, this assertion will - // start failing and the test will catch it before the change ships. + // sqlx 0.8.6 has no public getter for `enable_cleartext_plugin`, so + // assert on the `Debug` output as a regression sentinel. let debug = format!("{:?}", opts); assert!( debug.contains("enable_cleartext_plugin: true"), diff --git a/src/components/modals/NewConnectionModal.tsx b/src/components/modals/NewConnectionModal.tsx index 2c1b6e76..15b24b85 100644 --- a/src/components/modals/NewConnectionModal.tsx +++ b/src/components/modals/NewConnectionModal.tsx @@ -63,8 +63,7 @@ interface ConnectionParams { // MySQL: force PIPES_AS_CONCAT / NO_ENGINE_SUBSTITUTION sql_mode on connect. // Defaults to true; disable for Vitess/PlanetScale which reject altering sql_mode. pipes_as_concat?: boolean; - // AWS RDS IAM authentication (mysql only): when true, the password field - // holds a pre-signed RDS auth token instead of a real password. + // When true, the password field is an RDS auth token (mysql only). use_iam_auth?: boolean; // SSH ssh_enabled?: boolean; @@ -1560,7 +1559,6 @@ export const NewConnectionModal = ({ - {/* AWS RDS IAM authentication (mysql only) */} {driver === "mysql" && (