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
rhatdan committed Mar 13, 2023
1 parent 3920799 commit bda8b5e
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 19 deletions.
4 changes: 2 additions & 2 deletions libpod/options.go
View file
Expand Up @@ -1700,7 +1700,7 @@ func WithVolumeUID(uid int) VolumeCreateOption {
return define.ErrVolumeFinalized
}

volume.config.UID = uid
volume.config.UID = &uid

return nil
}
Expand Down Expand Up @@ -1739,7 +1739,7 @@ func WithVolumeGID(gid int) VolumeCreateOption {
return define.ErrVolumeFinalized
}

volume.config.GID = gid
volume.config.GID = &gid

return nil
}
Expand Down
26 changes: 22 additions & 4 deletions libpod/runtime_volume_common.go
View file
Expand Up @@ -159,15 +159,33 @@ 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)
uid, gid := func() (int, int) {
uid := volume.uid()
gid := volume.gid()
switch {
case uid != -1 && gid != -1:
return uid, gid
case uid == -1:
return os.Getuid(), gid
case gid == -1:
return uid, os.Getgid()
}
return -1, -1
}()

if uid != -1 && gid != -1 {
if err := idtools.SafeChown(volPathRoot, uid, gid); err != nil {
return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", volPathRoot, uid, 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)
}
if err := idtools.SafeChown(fullVolPath, volume.config.UID, volume.config.GID); err != nil {
return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, volume.config.UID, volume.config.GID, err)
if uid != -1 && gid != -1 {
if err := idtools.SafeChown(fullVolPath, uid, gid); err != nil {
return nil, fmt.Errorf("chowning volume directory %q to %d:%d: %w", fullVolPath, uid, gid, err)
}
}
if err := LabelVolumePath(fullVolPath, volume.config.MountLabel); err != nil {
return nil, err
Expand Down
14 changes: 10 additions & 4 deletions libpod/volume.go
View file
Expand Up @@ -46,9 +46,9 @@ type VolumeConfig struct {
// Whether this volume is anonymous (will be removed on container exit)
IsAnon bool `json:"isAnon"`
// UID the volume will be created as.
UID int `json:"uid"`
UID *int `json:"uid"`
// GID the volume will be created as.
GID int `json:"gid"`
GID *int `json:"gid"`
// Size maximum of the volume.
Size uint64 `json:"size"`
// Inodes maximum of the volume.
Expand Down Expand Up @@ -200,7 +200,10 @@ func (v *Volume) uid() int {
if v.state.UIDChowned > 0 {
return v.state.UIDChowned
}
return v.config.UID
if v.config.UID == nil {
return -1
}
return *v.config.UID
}

// GID returns the GID the volume will be created as.
Expand All @@ -220,7 +223,10 @@ func (v *Volume) gid() int {
if v.state.GIDChowned > 0 {
return v.state.GIDChowned
}
return v.config.GID
if v.config.GID == nil {
return -1
}
return *v.config.GID
}

// CreatedTime returns the time the volume was created at. It was not tracked
Expand Down
20 changes: 11 additions & 9 deletions test/e2e/run_volume_test.go
View file
Expand Up @@ -798,11 +798,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 Expand Up @@ -939,24 +941,24 @@ USER testuser`, fedoraMinimal)

It("podman volume with uid and gid works", func() {
volName := "testVol"
volCreate := podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000", volName})
volCreate := podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=2000", volName})
volCreate.WaitWithDefaultTimeout()
Expect(volCreate).Should(Exit(0))

volMount := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%u", "/test"})
volMount.WaitWithDefaultTimeout()
Expect(volMount).Should(Exit(0))
Expect(volMount.OutputToString()).To(Equal("1000"))
Expect(volMount.OutputToString()).To(Equal("2000"))

volName = "testVol2"
volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=gid=1000", volName})
volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=gid=2001", volName})
volCreate.WaitWithDefaultTimeout()
Expect(volCreate).Should(Exit(0))

volMount = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%g", "/test"})
volMount.WaitWithDefaultTimeout()
Expect(volMount).Should(Exit(0))
Expect(volMount.OutputToString()).To(Equal("1000"))
Expect(volMount.OutputToString()).To(Equal("2001"))

volName = "testVol3"
volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000,gid=1000", volName})
Expand Down
21 changes: 21 additions & 0 deletions test/system/160-volumes.bats
View file
Expand Up @@ -529,5 +529,26 @@ EOF
run_podman volume rm myvol --force
}

@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 bda8b5e

Please sign in to comment.