fix(snap): require mTLS for the snap gateway - #3726
Merged
Merged
Conversation
Replace the installer opt-in with an authenticated snap gateway. The wrapper no longer forces plaintext, so the gateway serves TLS from the bundle it already generates in $SNAP_COMMON/tls. The install hook writes a config that enables mTLS user auth instead of unauthenticated access, and a new post-refresh hook migrates the exact legacy default on existing installs. install.sh waits for the gateway, detects whether it serves TLS, copies the client bundle into the target user's snap state directory, and registers the gateway over HTTPS. Older plaintext snap revisions still register over HTTP with a warning. The release canary asserts mTLS auth and HTTPS registration. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
drew
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
September 25, 2026 23:26
6 tasks
An explicit [openshell.gateway.mtls_auth] table fails config preflight, which validates mTLS auth before the local TLS bundle supplies the client CA. Write a default that pins the Docker driver instead; with the wrapper's TLS bundle the gateway requires client certificates and enables mTLS user auth automatically, as the native packages do. The mTLS gateway rejects TLS handshakes without a client certificate, and it still answers plaintext loopback HTTP for sandbox service routing, so the installer could misdetect it as a legacy plaintext gateway. Probe HTTPS with the root-owned client bundle, and treat a gateway as legacy only when a plaintext gRPC Health call succeeds. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-3726.docs.buildwithfern.com/openshell |
Detect the mTLS snap from the installed revision's post-refresh hook instead of probing plaintext gRPC, and drop the scheme global. Remove the installer's pre-hook config fallback, which is dead now that every channel ships the install hook and which wrote the insecure default. Give the install hook a single write path with a simple backup name, and shorten the manual client certificate steps. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Stop selecting the OpenShell snap just because the snap command exists. Linux installs default to the Debian or RPM package; OPENSHELL_INSTALL_METHOD=snap (or deb, rpm) selects the package explicitly. Hosts that already have the OpenShell snap keep refreshing it rather than gaining a second gateway on the same port. The release canary and snap repro script opt in explicitly. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Published revisions use refresh-mode: endure, and snapd honors the old revision's setting during a refresh, so the plaintext gateway kept running with the migrated config unused until a manual restart. Restart the gateway from the post-refresh hook so the mTLS config takes effect immediately. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
pimlock
approved these changes
Sep 26, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Require mTLS on snap install
Related Issue
No issue required: localized hardening of the Snap packaging default.
Changes
OPENSHELL_DISABLE_TLS=true, so the gateway serves TLS from the bundle it generates in$SNAP_COMMON/tlsand requires a client certificate. The default config (snap/hooks/install) no longer setsallow_unauthenticated_users = trueor pins a compute driver; with the local TLS bundle and an auto-detected local driver, mTLS user authentication turns on automatically.post-refreshhook, replaces any regular gateway config that explicitly setsallow_unauthenticated_users = trueordisable_tls = truewith the secure default, without keeping a copy. Secure custom configs, symlinks, and non-regular files are left alone.refresh-modechanges fromenduretorestart, and thepost-refreshhook runssnapctl restarton the gateway. Published revisions useendure, which snapd honors when refreshing away from them, so without the explicit restart the plaintext gateway kept running after an upgrade. Refreshes interrupt active sandbox sessions.install.shregisters the snap over mTLS. It detects an mTLS snap revision by the presence of itspost-refreshhook, waits for the gateway by probing HTTPS with the root-owned client bundle, copies the bundle into the target user's snap state (directories 0700, files 0600, written by the user), and registershttps://127.0.0.1:17670 --local, replacing an existing registration. Older plaintext revisions still register over HTTP with a warning.install.sh. Linux installs default to the Debian or RPM package.OPENSHELL_INSTALL_METHOD=snap|deb|rpmselects the package explicitly; hosts that already have the OpenShell snap keep refreshing it. The release canary Snap jobs and the Snap repro script opt in explicitly.snap install),architecture/build.md, the cluster debugging skill, the release canary (asserts mTLS auth and an HTTPS registration), and the install-script, hook, wrapper, packaging, and release-formula tests.Testing
Automated
mise run pre-commitpasses on the final commit.tasks/scripts/test-install-sh.sh(install-method selection, mTLS detection, HTTPS/HTTP registration, listener probe, client bundle permissions),test-snap-install-hook.sh(fresh default, insecure default and edited configs replaced with no copy kept, secure custom configs/symlinks/directories untouched,post-refreshmigrates and runssnapctl restart), andtest-packaging-assets.sh(includes the wrapper tests).release_formula_test.pypasses.mise run testpassed on an earlier revision of this branch, before the later hook and installer changes.VM (tmachine
ubuntu-docker-rootful, real snapd and Docker)Snaps: the published edge snap (rev 1605, plaintext) and the same snap repacked with this PR's wrapper,
installandpost-refreshhooks, andrefresh-mode: restart. Gateway and CLI binaries are unchanged by this PR.install.shregistered the installing user over HTTPS withAuthenticated (mTLS transport); a sandbox was created and ran a command. A second local user without the bundle was rejected over plaintext gRPC (404), HTTPS without a certificate, and the plaintext CLI, and could not read either copy of the client key.install.shwithOPENSHELL_INSTALL_METHOD=snapagainst the storelatest/stablesnap (rev 1606, plaintext): installed, warned about unauthenticated access, and registered over HTTP.post-refresh); rerunninginstall.shreplaced the registration with HTTPS and authenticated with mTLS. This scenario found and verified thepost-refreshrestart fix. The store install/refresh insideinstall.shwas stubbed for this rerun because the patched snap only exists locally.Not covered before merge
Checklist