oxidecomputer / oxidecomputer/omicron

Tracking issue: reqwest `default-tls` feature

Open
#7,624 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

Although no uses of reqwest within Omicron use the default-tls feature, because of the use of a workspace-hack crate all of our reqwest clients are using native-tls (thus OpenSSL) by default. I don't think we've ever written down our decision of whether to use native-tls or rustls but all of our Dropshot services use rustls (the default), and it is generally speaking our preference to have one TLS implementation, not two.

├── reqwest feature "__tls"
│   ├── reqwest feature "__rustls" (*)
│   └── reqwest feature "default-tls"
│       └── reqwest feature "default"
│           ├── crucible-agent-client v0.0.1 (https://github.com/oxidecomputer/crucible?rev=d2d8f8ad449df7e2befb7ee2723a442dd74b9b72#d2d8f8ad)
│           │   └── crucible-agent-client feature "default"
│           │       ├── nexus-test-utils v0.1.0 (/home/iliana/git/omicron/nexus/test-utils) (*)
│           │       ├── omicron-nexus v0.1.0 (/home/iliana/git/omicron/nexus) (*)
│           │       ├── omicron-omdb v0.1.0 (/home/iliana/git/omicron/dev-tools/omdb) (*)
│           │       └── omicron-sled-agent v0.1.0 (/home/iliana/git/omicron/sled-agent) (*)
│           ├── crucible-pantry-client v0.0.1 (https://github.com/oxidecomputer/crucible?rev=d2d8f8ad449df7e2befb7ee2723a442dd74b9b72#d2d8f8ad)
│           │   └── crucible-pantry-client feature "default"
│           │       └── omicron-nexus v0.1.0 (/home/iliana/git/omicron/nexus) (*)
│           ├── ddm-admin-client v0.1.0 (https://github.com/oxidecomputer/maghemite?rev=cb2b592e890ca9e93d8193e9765e2a62459d5fa8#cb2b592e) (*)
│           ├── libfalcon v0.1.0 (https://github.com/oxidecomputer/falcon?branch=main#f3fe0542) (*)
│           ├── lldpd-client v0.1.0 (https://github.com/oxidecomputer/lldp#ce952e61)
│           │   └── lldpd-client feature "default"
│           │       └── omicron-nexus v0.1.0 (/home/iliana/git/omicron/nexus) (*)
│           ├── mg-admin-client v0.1.0 (https://github.com/oxidecomputer/maghemite?rev=cb2b592e890ca9e93d8193e9765e2a62459d5fa8#cb2b592e) (*)
│           └── omicron-workspace-hack v0.1.0 (/home/iliana/git/omicron/workspace-hack) (*)

This issue tracks dropping the default-tls feature (at least by default) from:

And then:

  • Set up cargo-deny in CI and deny native-tls to prevent this accidentally happening again

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the workspace-hack dependency tree and the linked Crucible, Maghemite, FALCON, and lldp changes to identify which upstream features still enable reqwest default-tls. Then review how cargo-deny would be added to CI. Done means the remaining upstream checkboxes are resolved and CI denies native-tls from being reintroduced.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
ci-cd, security
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.