Repository navigation
Verify TLS with the OS trust store and add tls-check diagnostics - #7
Conversation
reqwest's rustls-tls feature only trusts the compiled-in webpki-roots, so a TLS-inspecting proxy whose CA is installed in the OS failed with "invalid peer certificate: UnknownIssuer". Every HTTP client now gets its rustls config from tls::client_builder(), built with rustls-platform-verifier: the macOS keychains, the Windows certificate store, or the Linux system bundle (SSL_CERT_FILE/SSL_CERT_DIR override it). `cve-intel tls-check [host...]` shows the proxy used (credentials masked), the CA store read, the chain each host presents and the verifier's decision, with a hint. The global `--debug` flag logs rustls, verifier and connection details to stderr.
client_builder() reported a failed platform verifier through log::warn!, but no logger is installed unless --debug is passed, so a machine with no usable CA store silently fell back to the built-in roots. Print the reason once per process on stderr instead, pointing at tls-check. That was cve-core's only log call, so drop the log dependency there and the cve_core target from the --debug filter. Also fix the Cargo.toml comment to name tls::client_builder.
Fazmin
left a comment
There was a problem hiding this comment.
Thanks for this, it's a really solid piece of work. Trusting the OS store is the right call for people behind TLS-inspecting proxies, and tls-check is going to save a lot of back-and-forth on support issues. I like that the recording verifier only lives inside diagnose() and hands the platform verifier's decision back unchanged. Routing every client through client_builder() is a nice cleanup too.
A few small things I'd like fixed before merging, each only a line or two:
1. client_config() should pick ring explicitly (tls.rs ~L22). ClientConfig::builder() only works while exactly one rustls provider is compiled in. That's true today, but rustls' default features (and hyper-rustls, tokio-rustls, or reqwest 0.13's rustls feature) bring in aws-lc-rs. If a future dependency or Tauri plugin pulls one of those in, this panics inside Clients::new and the app crashes at start-up. client_config_recording() already uses builder_with_provider(ring), so using the same thing here makes the "exactly one provider" guarantee hold in the code and not just in Cargo.lock.
2. redact_userinfo can leak a proxy password (tls.rs ~L245). It cuts the authority at the first / before looking for @, so with HTTPS_PROXY=http://alice:pa/ss@proxy:3128 the password comes out unmasked in both the text and the --json output. That's exactly the kind of output people paste into issues. Splitting on the last @ across the whole remainder (or parsing with url and masking everything if parsing fails) would fix it. A test for it would be great.
3. The hint is wrong when the verifier can't be set up (tls.rs ~L100 and ~L355). On a Linux box with no CA bundle, the fallback warning tells people to run tls-check. diagnose then fails with "could not set up the platform verifier…", the chain is empty, and the hint says the connection "failed before TLS (DNS, firewall…)". That sends them the wrong way. A specific hint for this case ("no CA certificates were found; install ca-certificates or set SSL_CERT_FILE") would close the loop.
And some smaller notes, not blocking:
--debugnever shows anything fromrustls: it's built without itsloggingfeature, so it has nologdependency. Either add"logging"to the features or drop rustls from the README and PR description.- The UnknownIssuer hint prints the Linux commands (
update-ca-trust,SSL_CERT_FILE) on macOS and Windows too, where they don't apply. I'm also not sure macOS reports a proxy's untrusted root asUnknownIssuerat all, rather than as a generic "not trusted" error. If you have a Mac handy it might be worth a quick check. - Certificate subject and issuer are printed raw in the text output, so a hostile server could slip terminal escape sequences into it. Stripping control characters would be enough.
diagnoseispubbut only safe as a CLI tool, sincevalidate_domaindoesn't resolve names or block private addresses. A doc comment saying it shouldn't be exposed over MCP or REST as it is would help whoever touches it next.trust_store_paths()has no branch for targets other than Linux, macOS and Windows, so it won't build on Android. That isn't a target for us, so this is just an FYI.
Once those three are in, I'm happy to merge this. Then #8 can be rebased on top. Thanks again!
- Name the ring provider in client_config(), shared with tls-check, so
a second rustls provider pulled in by a dependency cannot panic at
start-up.
- Mask everything before the last '@' in proxy URLs, so a password
with an unescaped '/', '?' or '#' is not printed.
- Give a specific hint when the platform verifier cannot start (no CA
certificates) instead of blaming DNS or the firewall.
- Make the "add the CA" advice match the OS, treat macOS's
errSecNotTrusted like UnknownIssuer, and hint on other rejections.
- Show control and bidi characters in server-supplied text as \u{..}.
- Enable rustls' logging feature so --debug shows handshake details.
- Document that diagnose() must stay a local CLI tool; fix the Android
cfg in trust_store_paths().
|
Thanks for the careful review! Everything is addressed in 8240bcb. Blocking
Notes
I'll rebase #8 once this is merged. |
|
Thanks so much, Paul! This all looks great. I really appreciate how quickly you turned it around, and the extra care on the macOS hint and the terminal escapes. |
|
Hey Paul, one tiny thing before I merge, sorry! CI is all green apart from the dependency audit. Could you run |
rustls-platform-verifier pulls it in for wasm builds only, so native builds were not affected, but cargo audit scans the whole lockfile.
|
Hi Fazmin, Thanks! |
|
Merged! Thanks so much, Paul. And thank you for the kind words, that really made my day :) Looking forward to the rebased #8 and the OAuth PR next week! |
Why
reqwest's
rustls-tlsfeature only trusts the compiled-inwebpki-roots. Behind a TLS-inspecting proxy, every lookup fails withinvalid peer certificate: UnknownIssuer, even when the proxy's CA is installed in the operating system and curl and browsers work. There was also no way to tell from the app which CA was missing or whether a proxy was in the path.Approach
Platform verifier. Certificates are now checked by
rustls-platform-verifier, so the app trusts the same store as the rest of the machine:SSL_CERT_FILE/SSL_CERT_DIRas overrides.This avoids shipping our own root list or a CA setting that users would have to keep in sync with the OS.
One crypto provider.
rustlsis pulled in withringonly, matching reqwest, so there is exactly one crypto provider andClientConfig::builder()cannot panic over an ambiguous default.Fallback. If the platform verifier cannot be set up (e.g. a Linux box with no CA bundle at all),
client_builder()falls back to reqwest's built-in roots instead of failing every request, and prints the reason once on stderr (pointing attls-check) so it is visible without--debug.How it works
cve_core::tls(new module)client_builder()is now the single place areqwest::ClientBuilderis made. These clients now use it:Clients(all upstream sources)llm.rsreqwest::Client::new()is the loopbackhttp://127.0.0.1health poll for the local model server. It involves no TLS.diagnose(host)makes aHEAD /request with the same proxy and verifier rules. A recording verifier captures the chain the server presented and the platform verifier's decision, without changing that decision. The report gives a hint for the common failures:cve-intel tls-check [host…] [--json]validators::validate_domain.--debugflagrustls,rustls_platform_verifier,reqwestandhyper_utiltargets. Stdout stays clean for--jsonand MCP stdio..env.exampledocumentsSSL_CERT_FILE,SSL_CERT_DIR,HTTPS_PROXYandNO_PROXY.Type of change
rustls0.23 withring,rustls-platform-verifier0.6,log0.4)Testing
tls.rs: the platform verifier and client build, proxy credential redaction,NO_PROXYmatching, hint selection, and X.509 subject/issuer parsing from a fixed test certificate.cargo fmt --all --check,cargo clippy -p cve-core -p cve-server --all-targets -- -D warnings,cargo test -p cve-core -p cve-server(266 passed) andcargo check -p cve-intel-desktopall pass locally on Linux.cve-intel tls-check services.nvd.nist.gov api.osv.devverifies both chains against the system store and exits 0.--debug tls-check api.first.orglogs the verifier loading the system roots and the connection setup to stderr.cve-intel cvss …still prints 9.8.tls-checkis a CLI-only diagnostic the local user runs. It is not exposed over MCP, REST or the agent.Checklist
cve-core(tls.rs); the CLI only formats the report.validate_domain).--debugis limited to connection-setup targets.