Skip to content

Commit

Permalink
Do not chown newly created volume sources
Browse files Browse the repository at this point in the history
When running containers with

podman run --userns=keep-id --userns $UID:$GID -v test:/tmp/test ...

The volumes directory ends up being owned by root within the user
namespace, which is not root of the general namespace nor the users
uid.

If we just allow podman to create these directories with the same UID
that is running podman, IE don't chown, they end up with the correct
UID in all cases.

The actual volume will be chowned to the UID of the container.

Fixes: containers#16741

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
  • Loading branch information
rhatdan committed Dec 9, 2022
1 parent 859f40a commit a77fa03
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
3 changes: 0 additions & 3 deletions libpod/runtime_volume_common.go
Expand Up @@ -151,9 +151,6 @@ func (r *Runtime) newVolume(ctx context.Context, noCreatePluginVolume bool, opti
if err := os.MkdirAll(volPathRoot, 0700); err != nil {
return nil, fmt.Errorf("creating volume directory %q: %w", volPathRoot, err)
}
if err := idtools.SafeChown(volPathRoot, volume.config.UID, volume.config.GID); err != nil {
return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", volPathRoot, volume.config.UID, volume.config.GID, err)
}
fullVolPath := filepath.Join(volPathRoot, "_data")
if err := os.MkdirAll(fullVolPath, 0755); err != nil {
return nil, fmt.Errorf("creating volume directory %q: %w", fullVolPath, err)
Expand Down
12 changes: 7 additions & 5 deletions test/e2e/run_volume_test.go
Expand Up @@ -799,11 +799,13 @@ VOLUME /test/`, ALPINE)
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(ContainSubstring("888:888"))

vol += ",O"
session = podmanTest.Podman([]string{"run", "--rm", "--user", "888:888", "--userns", "keep-id", "-v", vol, ALPINE, "stat", "-c", "%u:%g", dest})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(ContainSubstring("888:888"))
if rootless.IsRootless() {
vol += ",O"
session = podmanTest.Podman([]string{"run", "--rm", "--user", "888:888", "--userns", "keep-id", "-v", vol, ALPINE, "stat", "-c", "%u:%g", dest})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(ContainSubstring("888:888"))
}
})

It("podman run with --mount and U flag", func() {
Expand Down
22 changes: 22 additions & 0 deletions test/system/160-volumes.bats
Expand Up @@ -507,4 +507,26 @@ EOF
is "$output" "" "Should print no output"
}

@test "podman default volume with --keep-id and --user " {
skip_if_not_rootless "--userns=keep-id only works in rootless mode"
myvolume=myvol$(random_string)
myvolume2=myvol$(random_string)
myfile=myfile$(random_string)
mytext=$(random_string)
run_podman run --userns=keep-id --user=${UID} -v ${myvolume}:/vol:z $IMAGE sh -c "echo $mytext > /vol/$myfile"
run_podman volume inspect $myvolume --format '{{.Mountpoint}}'
mount=$output
run stat -c %u $(dirname ${output})
is "$output" "$UID" "Volume directory should be created with current UID"
run stat -c %u $mount/$myfile
is "$output" "$UID" "Volume data should be created with current UID"
run_podman run --userns=keep-id --user=1:2 -v ${myvolume2}:/vol:z $IMAGE sh -c "echo $mytext > /vol/$myfile"
run_podman volume inspect $myvolume2 --format '{{.Mountpoint}}'
mount=$(dirname ${output})
run stat -c %u $mount
is "$output" "$UID" "Volumes should be created with current UID"
run stat -c %u $mount/$myfile
assert "$output" != "$UID" "Content should not be created with current UID when user run with different UID"
}

# vim: filetype=sh

0 comments on commit a77fa03

Please sign in to comment.