feat(policy): accept numeric UIDs for sandbox process identity#1973
Open
sjenning wants to merge 4 commits into
Open
feat(policy): accept numeric UIDs for sandbox process identity#1973sjenning wants to merge 4 commits into
sjenning wants to merge 4 commits into
Conversation
Allow run_as_user and run_as_group to be either the literal 'sandbox' or a numeric UID/GID within [1000, 2_000_000_000]. This removes the hard dependency on a baked-in 'sandbox' user in container images, enabling compute drivers to inject resolved UIDs at sandbox creation. Phase 1 of NVIDIA#1959. Signed-off-by: Seth Jennings <sjenning@redhat.com>
Allow run_as_user and run_as_group to be numeric UIDs/GIDs, removing
the hard dependency on a baked-in 'sandbox' user in container images.
Changes:
- validate_sandbox_user(): accepts numeric UIDs without passwd lookup
(logs OCSF event); keeps passwd check for "sandbox" name; rejects
non-numeric non-sandbox strings that fail passwd lookup
- prepare_filesystem(): passes numeric UIDs/GIDs directly to chown()
instead of requiring a passwd entry
- drop_privileges(): resolves numeric UIDs/GIDs directly via UID::from_raw
/ Gid::from_raw; skips initgroups when target uid matches current euid;
uses guard conditions before setgid/setuid calls
- session_user_and_home(): falls back to ("{uid}", "/sandbox") for
numeric UIDs, avoiding a passwd lookup that will fail
Re-exports MIN_SANDBOX_UID and MAX_SANDBOX_UID from openshell-policy
so callers have consistent range constants.
Phase 2 of NVIDIA#1959.
Signed-off-by: Seth Jennings <sjenning@redhat.com>
…hift SCC annotations Phase 3 of the numeric-UID plan: allow operators to specify explicit sandbox_uid/sandbox_gid in Kubernetes driver config, auto-detect from OpenShift SCC namespace annotations, and propagate resolved values to supervisor container env vars and PVC init container securityContext. Changes: - Add sandbox_uid/sandbox_gid fields to KubernetesComputeConfig - Add SANDBOX_UID/SANDBOX_GID env var constants to openshell-core - Implement resolve_sandbox_identity() to fetch namespace annotations and auto-detect OpenShift SCC UID ranges (sa.scc.uid-range) - Pass resolved UID/GID through SandboxPodParams to pod spec builder - Inject SANDBOX_UID/SANDBOX_GID env vars into supervisor container - Update PVC init container securityContext with resolved UID/GID instead of hard-coded root - Add comprehensive unit tests for resolution logic and annotation parsing (resolve_sandbox_uid, resolve_sandbox_gid, OpenShift SCC annotation parsing) Signed-off-by: Seth Jennings <sjenning@redhat.com>
…mples Phase 4 of the numeric-UID plan: replace hardcoded SANDBOX_UID (10001) in VM rootfs preparation with configurable sandbox_uid/sandbox_gid fields. Changes: - Add sandbox_uid/sandbox_gid to VmDriverConfig with serde derives - Pass resolved UID/GID through prepare_sandbox_rootfs_from_image_root to ensure_sandbox_guest_user which writes /etc/passwd/group/gshadow - Update BYOC Dockerfile: remove groupadd/useradd, document runtime UID injection and the ability to skip baked-in sandbox user - Update gateway-config.mdx: document sandbox_uid/sandbox_gid for both Kubernetes (with OpenShift SCC autodetection) and VM drivers - Update sandbox-compute-drivers.mdx: add Sandbox User Identity section explaining numeric UID support across all compute drivers - Update rootfs tests to use non-default UIDs, verify config passthrough Signed-off-by: Seth Jennings <sjenning@redhat.com>
sjenning
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
elezar
reviewed
Jun 23, 2026
|
|
||
| /// The annotation key for the OpenShift ServiceAccount UID range. | ||
| /// Format: `<start>/<size>` (e.g. `1000000000/10000`). | ||
| pub const ANNOTATION_SCC_UID_RANGE: &str = "openshift.io/sa.scc.uid-range"; |
Member
There was a problem hiding this comment.
Question: Do we want to leak openshift-specifics into the general k8s driver? What is the alternative? Does it make sense to make the annotation(s) configurable?
Contributor
Author
There was a problem hiding this comment.
Do we really don't. Ideally, a restricted pod in OpenShift will have runAsUser set to some high UID automatically. However, because the supervisor currently needs to run as root and then setuid to the unprivileged user, we can't allow the platform to assign runAsUser.
elezar
reviewed
Jun 23, 2026
| .parse::<u32>() | ||
| .ok() | ||
| .filter(|&gid| gid >= openshell_policy::MIN_SANDBOX_UID) | ||
| } |
Member
There was a problem hiding this comment.
Is there any reason that this is a separate implementation if the two annotations have exactly the same format?
Contributor
Author
There was a problem hiding this comment.
No, they can use the same parsing function
elezar
reviewed
Jun 23, 2026
Comment on lines
+471
to
+475
| annotations: if ns_annotations.is_empty() { | ||
| None | ||
| } else { | ||
| Some(ns_annotations) | ||
| }, |
Member
There was a problem hiding this comment.
What type of object are we creating here? Is it required to have the same annotations as the namespace?
Contributor
Author
There was a problem hiding this comment.
We are creating the Sandbox CR, and no it does not need the SCC namespace annotations
sjenning
commented
Jun 25, 2026
| "runAsUser": sandbox_uid, | ||
| "runAsGroup": sandbox_gid, | ||
| "fsGroup": sandbox_gid, | ||
| }, |
Contributor
Author
There was a problem hiding this comment.
This is wrong. The supervisor switches to this user at runtime.
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
3 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
WIP: STILL TESTING
Extend OpenShell to accept numeric UIDs/GID values for sandbox process identity (
run_as_userandrun_as_group) in addition to the literal"sandbox"name. This enables finer-grained control over which non-root user identity sandboxes run as, with validation that rejects UID 0 and values outside the acceptable range[1000, 2_000_000_000].Implements #1959
Changes
is_valid_sandbox_identity()function andMIN_SANDBOX_UID/MAX_SANDBOX_UIDconstants; accept numeric UIDs in addition to"sandbox"setuid/setgid/setgroups) instead of only the "sandbox" namerun_as_userandrun_as_groupfrom config values or fall back to OpenShift SCC annotations, supporting numeric IDsTesting
mise run pre-commitpassesChecklist