oci/state: use crun's non-standard root state field to avoid bundle dir access - #1971
Open
henry118 wants to merge 1 commit into
Open
oci/state: use crun's non-standard root state field to avoid bundle dir access#1971henry118 wants to merge 1 commit into
henry118 wants to merge 1 commit into
Conversation
Signed-off-by: Henry Wang <henrwang@nvidia.com>
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
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.
Partially fixes #648.
The OCI runtime spec defines what a runtime passes to hooks on stdin: a state JSON object containing a container's bundle path. To find the container root, the toolkit currently opens
<bundle>/config.jsonand readsroot.pathfrom it. This requires read access to the bundle directory.When Podman runs a container with
--userns nomapor--userns auto, the invoking user's UID is intentionally excluded from the container's user namespace UID map. The hook process runs inside the container's user namespace with an unmapped UID, which has no permission to read the bundle directory owned by the host user. As a result,GetContainerRoot()fails openingconfig.jsonwith permission denied, and every CDI hook that calls it exits with an error.crunincludes a non-standardrootfield in the state JSON it sends to hooks, populated from config.json'sroot.path:https://github.com/containers/crun/blob/c3a49b822cc45c1ae9e0459eff534efe61c4a5be/src/libcrun/container.c#L746
This provides the
rootfspath directly, without requiring any access to the bundle directory.GetContainerRoot()can now checks.Rootfirst. If present, it can be returned directly without accessing the config JSON file.This fix is limited to
crunonly.runcdoes not include arootfield in its state JSON. And this fix requirescrun≥ 1.27 because 1.27 included a change which allows ignoringchdirpermission errors before container hook is executed (containers/crun#2035).Given #648 is reported on Podman and
crunis podman's preferred and default runtime, this fix should be suffice for majority of the use cases.