Skip to content

feat(security): wip#1267

Draft
pophilpo wants to merge 6 commits into
mainfrom
pophilpo/https-compliance
Draft

feat(security): wip#1267
pophilpo wants to merge 6 commits into
mainfrom
pophilpo/https-compliance

Conversation

@pophilpo

@pophilpo pophilpo commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

wip

Shared client builder (security-utils)

  • Renamed http_client_builder() → client_builder() (leaving the old name as possible to use, but with deprecated warning)

  • TLS 1.3 minimum enforced

  • https_only(true) in production, enforced by clippy ( see below)

  • CA pinning: 9 specific CAs (Amazon Root CA 1–4, Starfield G2, Google Trust Services R1–R4), system trust store disabled

  • SHA256 hash verification of embedded cert bytes at runtime

  • allow-http = []in security-utils feature for crates with local HTTP test servers, only to be used as dev-dependency feature flag.

All production reqwest clients migrated to security_utils :: client_builder()

  • orb-backend-status

  • orb-connd

  • orb-speed-test

  • orb-attest

  • orb-se050-reprovision

Explicit exceptions

  • update-agent + update-agent-loader: system certs retained to keep update path alive after extended offline periods. Both now have TLS 1.3 minimum and https_only(true) set manually.

Clippy enforcement

  • clippy.toml at workspace root bans reqwest::Client::builder, reqwest::Client::new, and blocking equivalents with reason messages pointing to client_builder()from security utils

Pending

  • Test orb-software changes on the orb

  • priv-orb-core: bump orb-security-utils rev, rename http_client_builder() → client_builder(), add clippy.toml — deferred until orb-software changes are tested

  • Check orb-relay-messages

@worldcoin worldcoin deleted a comment from chatgpt-codex-connector Bot Jun 18, 2026
@pophilpo pophilpo marked this pull request as ready for review June 18, 2026 12:21
@pophilpo pophilpo requested a review from a team as a code owner June 18, 2026 12:21
@github-actions

Copy link
Copy Markdown

Found two concrete issues:

  • security-utils/src/reqwest.rs:154: This changes the shared client from TLS 1.2 minimum to TLS 1.3 minimum. That disables TLS 1.2 fallback entirely, so any pinned-CA endpoint that still only supports TLS 1.2 will fail even though the previous helper allowed it and TLS negotiation would still prefer 1.3 when available. Use TLS_1_2 unless the intended policy is explicitly TLS 1.3-only.

  • security-utils/Cargo.toml:18 / security-utils/src/reqwest.rs:156: allow-http can be enabled in any build, including release, and then the shared helper silently disables https_only for all consumers in that Cargo build graph. Since this crate is meant to enforce HTTPS-only, this should have a hard guard like compile_error! for feature = "allow-http" outside debug/test builds, or be split into test-only construction that cannot be selected for production artifacts.

I could not run cargo check in this sandbox because rustup tried to write under a read-only home directory.

@pophilpo pophilpo marked this pull request as draft June 18, 2026 12:51

@vmenge vmenge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vmenge

vmenge commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Found two concrete issues:

  • security-utils/src/reqwest.rs:154: This changes the shared client from TLS 1.2 minimum to TLS 1.3 minimum. That disables TLS 1.2 fallback entirely, so any pinned-CA endpoint that still only supports TLS 1.2 will fail even though the previous helper allowed it and TLS negotiation would still prefer 1.3 when available. Use TLS_1_2 unless the intended policy is explicitly TLS 1.3-only.
  • security-utils/Cargo.toml:18 / security-utils/src/reqwest.rs:156: allow-http can be enabled in any build, including release, and then the shared helper silently disables https_only for all consumers in that Cargo build graph. Since this crate is meant to enforce HTTPS-only, this should have a hard guard like compile_error! for feature = "allow-http" outside debug/test builds, or be split into test-only construction that cannot be selected for production artifacts.

I could not run cargo check in this sandbox because rustup tried to write under a read-only home directory.

you should admit urself in a scrapyard

@pophilpo pophilpo requested a review from eth44 June 19, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants