Skip to content

feat(build): add system CA root mode - #2324

Merged
SDAChess merged 10 commits into
NVIDIA:mainfrom
politerealism:openshell-unbundle-certs
Aug 4, 2026
Merged

feat(build): add system CA root mode#2324
SDAChess merged 10 commits into
NVIDIA:mainfrom
politerealism:openshell-unbundle-certs

Conversation

@politerealism

@politerealismpoliterealism commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a bundled-ca-roots Cargo feature (default on) to openshell-supervisor-network and openshell-sandbox, allowing Linux distribution builds to use the platform trust store instead of bundled Mozilla roots by building without this feature
  • Add a system-ca-roots convenience feature alias on openshell-sandbox that includes all other defaults (telemetry) except bundled CA roots, so distro builds can use --no-default-features --features system-ca-roots without manually re-adding unrelated defaults
  • Make rustls-native-certs a regular (non-optional) dependency — when bundled-ca-roots is absent, the platform trust store is used automatically
  • Switch CLI and gateway unconditionally to native CA roots (rustls-tls-native-roots, tls-rustls-ring-native-roots) so RPM-built binaries respect the system trust store without feature flags
  • Fix rust:verify:telemetry-off CI task to explicitly re-enable bundled-ca-roots when disabling default features

Related Issue

Enables downstream RPM packaging (https://gitlab.com/redhat/hummingbird/rpms/-/merge_requests/2503) to build against system CA roots.

Design

bundled-ca-roots is a Cargo feature. "System CA roots" is a build mode selected by building without bundled-ca-roots. There is no system-ca-roots feature on openshell-supervisor-network — the #[cfg(not(feature = "bundled-ca-roots"))] code path uses rustls-native-certs to read from the platform trust store.

The system-ca-roots alias on openshell-sandbox is a convenience feature that includes telemetry (and any future non-CA defaults) so distro package recipes don't need to track unrelated feature additions:

# Distro build (RPM, etc.)
cargo build -p openshell-sandbox --no-default-features --features system-ca-roots
# Default build (bundled Mozilla roots + telemetry)
cargo build -p openshell-sandbox

Compile-time decision rather than runtime because: (1) a distro auditor needs cargo tree proof that bundled roots aren't linked, (2) the supervisor binary is injected into containers with no config surface, and (3) the compile-time guard caught webpki-roots leaking back through transitive dependencies that a runtime flag never would have.

Changes

  • crates/openshell-supervisor-network/Cargo.toml: Make webpki-roots optional behind bundled-ca-roots (default); make rustls-native-certs a regular dependency
  • crates/openshell-supervisor-network/src/l7/tls.rs: Use #[cfg(feature = "bundled-ca-roots")] / #[cfg(not(feature = "bundled-ca-roots"))] to select CA root source
  • crates/openshell-sandbox/Cargo.toml: Forward bundled-ca-roots to supervisor-network; add system-ca-roots convenience alias; set default-features = false on the dependency
  • tasks/rust.toml: Update rust:verify:system-ca-roots to use the alias; fix rust:verify:telemetry-off to pass --features bundled-ca-roots
  • Cargo.toml: Add rustls-native-certs to workspace dependencies; switch reqwest, sqlx, kube to native root variants
  • crates/openshell-sdk/Cargo.toml: Disable oauth2 default features to prevent webpki-roots leaking back through oauth2's rustls-tls default feature
  • .github/workflows/branch-checks.yml: Update step name to use "build mode" terminology
  • architecture/build.md: Document the single-toggle model and alias

Testing

  • mise run rust:verify:telemetry-off passes (telemetry compiled out, CA root guard satisfied)
  • mise run rust:verify:system-ca-roots passes (system CA roots mode compiles, no webpki-roots in dep tree)
  • cargo test -p openshell-supervisor-network passes under default features
  • cargo check --all-features passes (no mutual exclusion conflict)
  • Downstream RPM spec (cargo build --release --bin openshell --bin openshell-gateway) builds without feature flags

Checklist

  • Conventional commit format
  • No bundled Mozilla roots leak into system CA roots builds
  • CLI and gateway use native roots unconditionally
  • CI telemetry-off verification updated
  • cargo --all-features works

maxamillionand others added 2 commits June 30, 2026 17:03
Allow distro builds to use native trust stores for supervisor upstream TLS while keeping bundled Mozilla roots as the default. Avoid bundled root crates in system-ca-roots builds by using native-root TLS features and z3 0.20.
Signed-off-by: Adam Miller <admiller@redhat.com>
The telemetry-off task uses --no-default-features which now disables
bundled-ca-roots in addition to telemetry, triggering the compile_error
guard. Re-enable bundled-ca-roots explicitly so the task verifies only
telemetry compilation.
Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@copy-pr-bot

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @politerealism.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions

github-actionsBot commented Jul 16, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@politerealism

Copy link
Copy Markdown
ContributorAuthor

I have read the DCO document and I hereby sign the DCO.

@politerealism

Copy link
Copy Markdown
ContributorAuthor

recheck

@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test 00c9b14

Resolve conflicts in Cargo.lock and crates/openshell-sandbox/Cargo.toml.
Keep default-features = false on openshell-supervisor-network while
adding new middleware dependencies from main.
Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism

Copy link
Copy Markdown
ContributorAuthor

@TaylorMutch@derekwaynecarr or others, can I get an Okay to test and review? I'm working on getting the MR for Hummingbird to work with Openshell and I think this PR will get it working.

@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test b454e32

The bare `oauth2 = "5"` dependency re-enabled default features
(rustls-tls → reqwest/rustls-tls → webpki-roots), defeating the
system-ca-roots feature gate. Mirror the CLI fix: disable defaults
and enable only the `reqwest` feature.
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test dea4d85

@elezar

Copy link
Copy Markdown
Member

@politerealism could you comment on why we elected for a compile-time decision instead of runtime selection?

@politerealism

politerealism commented Jul 22, 2026

Copy link
Copy Markdown
ContributorAuthor

Yes! @elezar , there are two reasons why it made sense to use compile-time decision instead of runtime selection. If it were runtime, webpki-roots would still be compiled in and linked. A distro packager can't certify a binary that ships it's own trust store alongside the system. The compile_error! guard makes this auditable. Cargo tree proves the dependency is physically absent.
The other reason is the supervisor binary is injected into containers. There's no natural place to put a "use system roots" toggle at runtime without adding confi surface to a security critical component that meant to be minimal.
One more thought is the SDK pulled webpki-roots back in through requests defaults. The completime guard and CI check caught it, but a runtime flag never would have.

Comment threadcrates/openshell-supervisor-network/src/l7/tls.rs
Comment threadarchitecture/build.md Outdated
Replace mutually exclusive bundled-ca-roots / system-ca-roots features
with a single bundled-ca-roots toggle. Disabling it implies system roots
via rustls-native-certs, which is now a regular (non-optional) dependency.
This fixes cargo --all-features and simplifies the distro build interface
from --no-default-features --features system-ca-roots to just
--no-default-features.
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Comment threadtasks/rust.toml Outdated
Comment threadtasks/rust.toml Outdated
@elezar

Copy link
Copy Markdown
Member

One concern with the current shape is that "system CA mode" is expressed as bare --no-default-features. That makes the CA-root choice entangled with unrelated defaults like telemetry, so package recipes and CI have to remember to manually re-enable every other default feature.

Two options I see here:

Firstly, we introduce named feature-set aliases instead, e.g. default-bundled-ca-roots and default-system-ca-roots, where both include the normal default feature set except for the CA-root provider? Then distro builds can use something like:

cargo build -p openshell-sandbox --no-default-features --features default-system-ca-roots

That still preserves the core guarantee that the system-root artifact is compiled without bundled roots, but makes the build intent explicit and less fragile.

An alternative would be to flip the default. That is to say, by default we build binaries WITHTOUT the bundled-ca-roots feature. We would then explicitly add --features bundled-ca-roots when building a portable sandbox binary. This does require some
initial changes to the CI and build tooling, but the diff doesn't look too large. The bigger concern is whether we're ok with making this breaking change to downstream users that are building openshell from source?

In the longer term, if we want upstream packages to ship both variants, the gateway could select between two prebuilt supervisor artifacts at startup. That should remain artifact selection, not a runtime switch inside one binary, so distro packages that prohibit bundled roots can still ship only the system-root artifact.

@elezar

Copy link
Copy Markdown
Member

One wording/design note: the PR currently describes this as adding bundled-ca-roots / system-ca-roots feature flags, but as implemented only bundled-ca-roots is a Cargo feature. The system-root path is selected by building without bundled-ca-roots.

Could we make that terminology consistent across the PR description/docs/CI labels? I think the clean framing is:

  • bundled-ca-roots: Cargo feature
  • system CA roots: build mode / default when bundled-ca-roots is not enabled

I do not think a feature alias is a blocker here as long as every --no-default-features recipe that wants the normal sandbox behavior consistently re-adds the required default features, especially telemetry. If we see more default features being added to openshell-sandbox, we can revisit adding a named alias such as default-system-ca-roots to avoid that becoming fragile.

Add a system-ca-roots feature alias on openshell-sandbox that includes
all other defaults (telemetry) except bundled-ca-roots, so distro
builds can use --no-default-features --features system-ca-roots without
manually re-adding unrelated defaults. Update the verify CI task to use
the alias and scope checks to the sandbox package. Fix task description
to use "build mode" terminology instead of implying a Cargo feature.
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Comment threadcrates/openshell-sandbox/Cargo.toml
Comment threadcrates/openshell-sdk/Cargo.toml
@elezar

Copy link
Copy Markdown
Member

/ok-to-test fe2171d

…ar default
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@maxamillion

Copy link
Copy Markdown
Collaborator

/ok-to-test e2129cf

Signed-off-by: politerealism <burdcat17@gmail.com>
# Conflicts:
#	Cargo.lock
SDAChess
SDAChess previously approved these changes Aug 3, 2026
@SDAChess
SDAChess enabled auto-merge August 3, 2026 16:53
@SDAChess

Copy link
Copy Markdown
Collaborator

/ok-to-test ea5eaf4

The function signature changed to return Result but the test call sites
were not updated, causing type mismatch compilation errors in CI.
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
auto-merge was automatically disabled August 3, 2026 18:12

Head branch was pushed to by a user without write access

@SDAChess

Copy link
Copy Markdown
Collaborator

/ok-to-test dbcae15

@SDAChess
SDAChess added this pull request to the merge queueAug 4, 2026
Merged via the queue into NVIDIA:main with commit 0e9a44cAug 4, 2026
70 of 77 checks passed
Sign up for freeto 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.

5 participants

@politerealism@maxamillion@elezar@SDAChess@matthewgrossman