fix(tls): trust the OS certificate store on relay WebSocket connections - #3455
fix(tls): trust the OS certificate store on relay WebSocket connections#3455benjaminkitt wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d285851cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # WebSocket client (test client) | ||
| tokio-tungstenite = { version = "0.29", features = ["rustls-tls-webpki-roots"] } | ||
| tokio-tungstenite = { version = "0.29", features = ["rustls-tls-webpki-roots", "rustls-tls-native-roots"] } |
There was a problem hiding this comment.
The reviewed commit has no Signed-off-by trailer, so the repository's required DCO Check will reject it before these manifest changes can merge; recreate or rebase the commit with --signoff.
AGENTS.md reference: AGENTS.md:L111-L111
Useful? React with 👍 / 👎.
04001ac to
0f38243
Compare
|
Ran into this exact issue independently — same Heads up on why CI is red here: the diff only edits |
tokio-tungstenite is built with only rustls-tls-webpki-roots, and native_websocket.rs calls bare connect_async() with no custom Connector, so the relay WebSocket trusts a compiled-in Mozilla root list and nothing else. Behind a TLS-inspecting proxy the re-signed certificate is rejected with 'invalid peer certificate: UnknownIssuer', and no keychain entry, SSL_CERT_FILE or NODE_EXTRA_CA_CERTS can influence it. This is easy to misdiagnose because HTTPS works: reqwest uses rustls-platform-verifier, which does read the OS trust store. Only the WebSocket path is affected, so the app looks healthy until the relay never connects. The two root-store features are additive (tls.rs pushes both into one RootCertStore, and the webpki branch is not gated on not(rustls-tls-native-roots)), so this preserves current behaviour and additionally honours the platform store. Both Cargo.lock files are regenerated so `cargo build --locked` succeeds in CI. rustls-native-certs was already vendored in each lockfile via the reqwest/rustls-platform-verifier path, so the only change is the new dependency edge from tokio-tungstenite 0.29.0 — no new packages and no version bumps. Signed-off-by: Benjamin Kitt <ben@thepuddle.com>
0f38243 to
a0b8b12
Compare
Problem
The desktop app's relay WebSocket trusts only a compiled-in Mozilla root list and ignores the OS trust store, so Buzz cannot connect to a relay from a machine behind a TLS-inspecting proxy (Netskope, Zscaler, Palo Alto, …). The connection fails with:
There is no user- or admin-side workaround: installing the corporate CA into the system keychain has no effect, and neither
SSL_CERT_FILEnorNODE_EXTRA_CA_CERTSis consulted on this path.This is easy to misdiagnose, because everything else in the app works. HTTPS requests go through
reqwest, which usesrustls-platform-verifierand does read the OS trust store. Only the WebSocket path is affected, so the app looks healthy right up until the relay never connects.Root cause
desktop/src-tauri/Cargo.tomland the rootCargo.tomlbuildtokio-tungstenitewith only therustls-tls-webpki-rootsfeature.rustls-tls-native-rootsis not enabled anywhere in the workspace, so feature unification does not compensate.desktop/src-tauri/src/native_websocket.rscalls bareconnect_async(url)with no customConnector, so tokio-tungstenite builds its default root store — which under that feature set is the bundled Mozilla roots only.desktop/src/shared/api/relayClientSession.tsandreadOnlyRelayClient.tsreach the relay viainvoke("plugin:websocket|connect"), so relay traffic goes through the Rust path above rather than the webview. (The webview would have used the OS trust store and would not have this problem.)The same applies to the workspace dependency, so
buzz,buzz-acpandbuzz-dev-mcpare affected too.buzz-agentusesreqwestonly and is unaffected.Confirmed against a v0.5.0 release build: the binary contains the Mozilla root names (
ISRG Root X1,DigiCert Global Root G2, …) and containsSecTrustEvaluateWithError(rustls-platform-verifier, via reqwest) but norustls_native_certs/load_native_certssymbols.Fix
Enable
rustls-tls-native-rootsalongside the existing feature, in both manifests.The two root-store features are additive — tokio-tungstenite's
src/tls.rspushes both sets into the sameRootCertStore, and the webpki branch is not gated onnot(rustls-tls-native-roots). So this preserves today's behaviour for everyone (the bundled Mozilla roots are still there) and additionally honours the platform trust store.Verification
Reproduced with a standalone tokio-tungstenite 0.29 probe against a real proxied relay, connecting with each root store in turn:
I then built the full app from this branch and confirmed the relay connects normally on the affected machine, and that the rebuilt
buzz/buzz-acp/buzz-dev-mcpsidecars pick up the change through the workspace dependency.Alternative worth considering
Since the app already depends on
rustls-platform-verifierthrough reqwest, an arguably tidier fix is to build aConnector::Rustlsfrom the platform verifier and pass it toconnect_async_tls_with_config. That would make WebSocket trust behaviour match HTTP trust behaviour instead of having the two paths use different trust stores. Happy to rework this PR that way if you'd prefer it.