From e4781e763ae05229cf77ae498ea432b021a1a652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Fri, 14 Aug 2026 12:54:29 +0800 Subject: [PATCH] fix(kms): apply the configured skip-TLS-verify to every Vault client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VaultConnectionSettings carried no TLS state, so both backend constructors dropped VaultConfig::tls on the floor and build_client never called VaultClientSettingsBuilder::verify. vaultrs 0.8.0 then fell back to its own default, leaving verification on: with RUSTFS_KMS_VAULT_SKIP_TLS_VERIFY=true and RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true against a self-signed Vault, startup still failed the handshake with UnknownIssuer. Carry skip_tls_verify on the connection settings and set verify explicitly on every client generation, authenticated and login alike. Setting it unconditionally also closes a bypass in the other direction: left unset, vaultrs derives verify from its own VAULT_SKIP_VERIFY variable, so a stray value in the environment disabled certificate verification without passing the KMS insecure-defaults gate. The restore path pins verification on: VaultRestoreTarget carries no TLS settings, and recovery is the last path that should accept an unauthenticated Vault. The remaining TlsConfig fields (ca_cert_path, client_cert_path, client_key_path) are still unused, but no supported input can set them — every constructor leaves them None. --- crates/kms/src/backends/vault.rs | 1 + crates/kms/src/backends/vault_credentials.rs | 45 ++++++++++++++++++++ crates/kms/src/backends/vault_transit.rs | 1 + crates/kms/src/backup/vault_restore.rs | 4 ++ 4 files changed, 51 insertions(+) diff --git a/crates/kms/src/backends/vault.rs b/crates/kms/src/backends/vault.rs index 84ba8e6fa..56d2a856e 100644 --- a/crates/kms/src/backends/vault.rs +++ b/crates/kms/src/backends/vault.rs @@ -550,6 +550,7 @@ impl VaultKmsClient { address: config.address.clone(), namespace: config.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + skip_tls_verify: config.tls.as_ref().is_some_and(|tls| tls.skip_verify), }; let source = token_source_for(&config.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config( diff --git a/crates/kms/src/backends/vault_credentials.rs b/crates/kms/src/backends/vault_credentials.rs index 8a93e61cb..c50bc0eb5 100644 --- a/crates/kms/src/backends/vault_credentials.rs +++ b/crates/kms/src/backends/vault_credentials.rs @@ -580,6 +580,9 @@ pub(crate) struct VaultConnectionSettings { pub(crate) namespace: Option, /// Per-attempt HTTP timeout applied to the underlying reqwest client. pub(crate) attempt_timeout: Duration, + /// Whether to accept an unverified Vault server certificate. Gated on + /// `allow_insecure_dev_defaults` by `KmsConfig::validate`. + pub(crate) skip_tls_verify: bool, } impl VaultConnectionSettings { @@ -593,6 +596,11 @@ impl VaultConnectionSettings { // operation-level retry policy. settings_builder.timeout(Some(self.attempt_timeout)); settings_builder.token(token); + // Always set explicitly: left unset, vaultrs derives this from its own + // VAULT_SKIP_VERIFY variable, so a stray value in the environment would + // disable certificate verification behind the KMS configuration and its + // insecure-defaults gate. + settings_builder.verify(!self.skip_tls_verify); if let Some(namespace) = &self.namespace { settings_builder.namespace(Some(namespace.clone())); @@ -969,6 +977,7 @@ mod tests { address: "http://127.0.0.1:8200".to_string(), namespace: Some("team-namespace".to_string()), attempt_timeout: Duration::from_secs(30), + skip_tls_verify: false, } } @@ -1164,6 +1173,42 @@ mod tests { assert!(format!("{source:?}").contains("KubernetesLogin")); } + /// The configured flag has to reach the HTTP client, not just the config + /// struct: every generation (authenticated and login) builds its own client, + /// and a Vault with a self-signed certificate fails the handshake unless + /// each one carries the setting. + #[test] + fn test_skip_tls_verify_reaches_every_vault_client_generation() { + for skip_tls_verify in [false, true] { + let settings = VaultConnectionSettings { + address: "https://vault.example.com:8200".to_string(), + namespace: None, + attempt_timeout: Duration::from_secs(30), + skip_tls_verify, + }; + + let authenticated = settings.build_client(TEST_TOKEN).expect("authenticated client must build"); + assert_eq!(authenticated.settings.verify, !skip_tls_verify); + + let login = settings.build_login_client().expect("login client must build"); + assert_eq!(login.settings.verify, !skip_tls_verify); + } + } + + /// vaultrs derives `verify` from its own VAULT_SKIP_VERIFY variable when the + /// builder leaves it unset, which would disable certificate verification + /// without passing the KMS insecure-defaults gate. + #[test] + fn test_vaultrs_skip_verify_env_cannot_override_the_configured_setting() { + temp_env::with_var("VAULT_SKIP_VERIFY", Some("true"), || { + let client = test_settings().build_client(TEST_TOKEN).expect("client must build"); + assert!( + client.settings.verify, + "a stray VAULT_SKIP_VERIFY must not disable verification behind the KMS configuration" + ); + }); + } + /// The projected token is read fresh per login attempt and trimmed, so a /// kubelet rotation is picked up without a restart and a trailing newline /// does not corrupt the assertion sent to Vault. diff --git a/crates/kms/src/backends/vault_transit.rs b/crates/kms/src/backends/vault_transit.rs index 095fcf455..602c53154 100644 --- a/crates/kms/src/backends/vault_transit.rs +++ b/crates/kms/src/backends/vault_transit.rs @@ -415,6 +415,7 @@ impl VaultTransitKmsClient { address: config.address.clone(), namespace: config.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + skip_tls_verify: config.tls.as_ref().is_some_and(|tls| tls.skip_verify), }; let source = token_source_for(&config.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config( diff --git a/crates/kms/src/backup/vault_restore.rs b/crates/kms/src/backup/vault_restore.rs index b7d4884cc..b033c4d5f 100644 --- a/crates/kms/src/backup/vault_restore.rs +++ b/crates/kms/src/backup/vault_restore.rs @@ -450,6 +450,10 @@ impl VaultRestoreClient { address: target.address.clone(), namespace: target.namespace.clone(), attempt_timeout: kms_config.effective_timeout(), + // A restore target carries no TLS settings, so certificates are + // always verified: recovery is the last path that should accept an + // unauthenticated Vault. + skip_tls_verify: false, }; let source = token_source_for(&target.auth_method, &settings)?; let policy = VaultCredentialPolicy::from_kms_config(