Skip to content

Commit c6b0d99

Browse files
Trust the OS store and SSL_CERT_FILE for HTTPS (#1107) (#1326)
* Trust platform and SSL_CERT_FILE roots (WIP) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Trust the OS store and SSL_CERT_FILE for HTTPS Every HTTPS call (patch API, public proxy, blob and registry fetches, telemetry, self-update) trusted only the bundled Mozilla roots. Behind a TLS-inspecting corporate proxy, whose private CA no setting could add, every request failed with UnknownIssuer. All production reqwest clients are now built by one constructor, utils::http::client_builder(), which adds the platform trust store to the bundled roots. rustls-native-certs reads the macOS keychain, the Windows store or the Linux bundle, and honors SSL_CERT_FILE and SSL_CERT_DIR. The store is read once per process and unusable roots are skipped. Verification is never relaxed. Tests run a local TLS server signed by a private CA: the handshake succeeds only when SSL_CERT_FILE names that CA. A text ratchet keeps new reqwest::Client constructors out of production code. Fixes #1107 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Read platform roots only on an unknown issuer CI's scan benchmark showed 5-10 ms more CPU per run: every process read the whole platform store (hundreds of PEM files on Linux) and every client re-parsed those roots. client_builder() now hands reqwest one shared rustls config, built once per process. Its verifier checks the bundled webpki roots first and reads the platform store (or SSL_CERT_FILE / SSL_CERT_DIR) only after an UnknownIssuer, then once per process. Other verification errors are returned unchanged, so trust is still the union of both sets and verification is never relaxed. Building a client no longer parses any roots. The load-error log line flagged by CodeQL is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent bc3f920 commit c6b0d99

14 files changed

Lines changed: 545 additions & 8 deletions

File tree

‎Cargo.lock‎

Lines changed: 70 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ sha2 = "=0.10.9"
2828
sha1 = "=0.10.6"
2929
hex = "=0.4.3"
3030
reqwest = { version = "=0.12.28", features = ["rustls-tls", "json"], default-features = false }
31+
# Platform trust store (+ SSL_CERT_FILE / SSL_CERT_DIR) for every HTTPS
32+
# client, alongside the bundled webpki roots (utils::http::client_builder).
33+
# `rustls` and `webpki-roots` are the exact builds reqwest's `rustls-tls`
34+
# already pulls in (ring provider); the shared client config is built
35+
# from them directly.
36+
rustls-native-certs = "=0.8.4"
37+
rustls = { version = "=0.23.45", default-features = false, features = ["std", "ring", "tls12"] }
38+
webpki-roots = "=1.0.6"
39+
tokio-rustls = { version = "=0.26.4", default-features = false, features = ["ring", "tls12"] }
3140
tokio = { version = "=1.50.0", features = ["full"] }
3241
tokio-util = "=0.7.18"
3342
futures-util = { version = "=0.3.32", default-features = false, features = ["std"] }

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,7 @@ Exactly three keys are honored, each slotting **below** the env var and **above*
11281128
Contract properties:
11291129

11301130
- **Read-only pledge**: socket-patch never creates, modifies, or deletes this file; socket-cli owns it. There is no `socket-patch login`/`config` subcommand — use `socket login`.
1131-
- Other socket-cli keys (`apiProxy`, `enforcedOrgs`, `skipAskToPersistDefaultOrg`) and unknown keys are ignored. Non-string or empty values for the three honored keys count as unset. For an HTTP forward proxy use the standard `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` vars, which the HTTP client honors; socket-cli's `apiProxy` is deliberately not mapped (and is unrelated to `--proxy-url`, which is the public patch *endpoint*).
1131+
- Other socket-cli keys (`apiProxy`, `enforcedOrgs`, `skipAskToPersistDefaultOrg`) and unknown keys are ignored. Non-string or empty values for the three honored keys count as unset. For an HTTP forward proxy use the standard `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` vars, which the HTTP client honors. Every HTTPS client (patch API, public proxy, blob/artifact and registry fetches, telemetry, self-update) trusts the bundled webpki (Mozilla) roots plus the platform trust store; `SSL_CERT_FILE` / `SSL_CERT_DIR`, when set, replace the platform store with that bundle/directory, so a TLS-inspecting proxy's private CA can be trusted. Verification is never disabled. socket-cli's `apiProxy` is deliberately not mapped (and is unrelated to `--proxy-url`, which is the public patch *endpoint*).
11321132
- Missing file / unresolvable data dir: silent (the normal case). Present but unreadable or undecodable (not base64(JSON), with a plain-JSON leniency fallback): a one-shot stderr warning naming the path, then treated as absent — never fatal, and `--json` stdout stays clean (all diagnostics are stderr-only).
11331133
- The file is read lazily at most once per process, only when a key is still unresolved after flag + env.
11341134
- The telemetry endpoint resolver shares the same `apiBaseUrl` chain as API-client construction (`resolve_api_base_url`), so telemetry can never target a different host than the client.

‎crates/socket-patch-core/Cargo.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ sha2 = { workspace = true }
2626
sha1 = { workspace = true }
2727
hex = { workspace = true }
2828
reqwest = { workspace = true }
29+
rustls-native-certs = { workspace = true }
30+
rustls = { workspace = true }
31+
webpki-roots = { workspace = true }
2932
tokio = { workspace = true }
3033
# CancellationToken for the in-memory hosted engine (`hosted::memory`).
3134
tokio-util = { workspace = true }
@@ -83,3 +86,4 @@ tempfile = { workspace = true }
8386
tokio = { workspace = true, features = ["full", "test-util"] }
8487
serial_test = { workspace = true }
8588
wiremock = { workspace = true }
89+
tokio-rustls = { workspace = true }

‎crates/socket-patch-core/src/api/client.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1991,7 +1991,7 @@ fn api_client(api_token: Option<&str>, timeouts: &ApiTimeouts) -> reqwest::Clien
19911991
}
19921992

19931993
timeouts
1994-
.apply(reqwest::Client::builder().default_headers(default_headers))
1994+
.apply(crate::utils::http::client_builder().default_headers(default_headers))
19951995
.build()
19961996
.expect("failed to build reqwest client")
19971997
}
@@ -2008,7 +2008,7 @@ fn plain_client(timeouts: &ApiTimeouts) -> reqwest::Client {
20082008
HeaderValue::from_static(USER_AGENT_VALUE),
20092009
);
20102010
timeouts
2011-
.apply(reqwest::Client::builder().default_headers(headers))
2011+
.apply(crate::utils::http::client_builder().default_headers(headers))
20122012
.build()
20132013
.expect("failed to build plain reqwest client")
20142014
}

‎crates/socket-patch-core/src/telemetry.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ fn prepare_send(event: PatchTelemetryEvent, auth: &TelemetryAuth) -> PreparedSen
327327
async fn send_telemetry_event(prepared: PreparedSend) {
328328
let PreparedSend { event, url, bearer } = prepared;
329329

330-
let client = match reqwest::Client::builder()
330+
let client = match crate::utils::http::client_builder()
331331
.connect_timeout(std::time::Duration::from_secs(2))
332332
.timeout(std::time::Duration::from_secs(5))
333333
.build()

‎crates/socket-patch-core/src/update/download.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ fn download_client(
6060
endpoints: &UpdateEndpoints,
6161
timeouts: &UpdateTimeouts,
6262
) -> Result<reqwest::Client, UpdateError> {
63-
reqwest::Client::builder()
63+
crate::utils::http::client_builder()
6464
.user_agent(crate::constants::USER_AGENT)
6565
.connect_timeout(timeouts.connect)
6666
.timeout(timeouts.download)

‎crates/socket-patch-core/src/update/release.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ fn metadata_client(
264264
timeouts: &UpdateTimeouts,
265265
redirects: reqwest::redirect::Policy,
266266
) -> Result<reqwest::Client, UpdateError> {
267-
reqwest::Client::builder()
267+
crate::utils::http::client_builder()
268268
.user_agent(crate::constants::USER_AGENT)
269269
.connect_timeout(timeouts.connect)
270270
.timeout(timeouts.metadata)

0 commit comments

Comments
 (0)