fix(client): honor VAULT_CACERT and TLS env vars when connecting to Vault#114
Open
linuxus wants to merge 1 commit into
Open
fix(client): honor VAULT_CACERT and TLS env vars when connecting to Vault#114linuxus wants to merge 1 commit into
linuxus wants to merge 1 commit into
Conversation
…ault NewVaultClient replaced the Vault API client's HTTP transport with a fresh tls.Config that set only InsecureSkipVerify, discarding the RootCAs (and client cert / server name) that api.DefaultConfig() derives from the standard Vault TLS environment variables via ReadEnvironment(). As a result a private or self-signed CA (e.g. `vault server -dev-tls`, or Vault behind internal PKI) could not be trusted: the only options were adding the CA to the host trust store or disabling verification with VAULT_SKIP_VERIFY=true. Configure TLS through config.ConfigureTLS, which updates the existing transport's TLSClientConfig in place (RootCAs, client certificate, ServerName) instead of replacing it. This honors the standard Vault env vars VAULT_CACERT, VAULT_CAPATH, VAULT_CLIENT_CERT, VAULT_CLIENT_KEY and VAULT_TLS_SERVER_NAME. Skip-verify is then applied explicitly from the caller-resolved value: ConfigureTLS (and ReadEnvironment) only ever set InsecureSkipVerify to true and never clear it, so setting it directly preserves the existing context-over-env precedence in both directions. No public signature change. Add a test verifying that VAULT_CACERT lets the client verify a server presenting a self-signed certificate, and that verification still fails without it.
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
1 similar comment
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
NewVaultClient(pkg/client/client.go) overwrites the Vault API client's HTTP transport with a freshtls.Configthat sets onlyInsecureSkipVerify:This discards the
RootCAs(and client certificate / server name) thatapi.DefaultConfig()configures from the standard Vault TLS environment variables viaReadEnvironment(). As a result there is no way to trust a private or self-signed CA — for example a Vault started withvault server -dev-tls, or any Vault behind an internal PKI. The only available options are adding the CA to the host trust store or disabling verification entirely withVAULT_SKIP_VERIFY=true.Fix
Configure TLS via
config.ConfigureTLS(...), which updates the existing transport'sTLSClientConfigin place (RootCAs, client certificate, ServerName) instead of replacing it. This honors the standard Vault env vars, consistent with the Vault CLI and othervault/apiconsumers:VAULT_CACERTVAULT_CAPATHVAULT_CLIENT_CERT/VAULT_CLIENT_KEYVAULT_TLS_SERVER_NAMESkip-verify is then applied explicitly from the caller-resolved value.
ConfigureTLS(andReadEnvironment) only ever setInsecureSkipVerifytotrueand never clear it, so the flag is set directly to preserve the existing context-over-env precedence in both directions (this keepsTestCreateVaultClientForSession_SkipTLSVerifypassing, including "context false wins over env true").No public signature change;
CreateVaultClientForSessionis unaffected.Testing
TestNewVaultClientHonorsCACert: spins up anhttptest.NewTLSServer, writes its cert as a CA bundle, and asserts that a request succeeds withVAULT_CACERTset and fails without it (i.e. verification is not silently disabled).go build ./...,go vet ./...,gofmtclean.go test ./...(e2e,pkg/client,pkg/tools/kv), including the existing skip-verify precedence tests.Manual verification