diff options
-rw-r--r-- | libpod/container.go | 12 | ||||
-rw-r--r-- | libpod/container_internal.go | 47 | ||||
-rw-r--r-- | libpod/container_internal_linux.go | 25 | ||||
-rw-r--r-- | libpod/oci_linux.go | 91 | ||||
-rw-r--r-- | libpod/options.go | 22 | ||||
-rw-r--r-- | libpod/runtime_ctr.go | 11 | ||||
-rw-r--r-- | libpod/runtime_volume_linux.go | 14 | ||||
-rw-r--r-- | libpod/volume.go | 2 | ||||
-rw-r--r-- | test/system/400-unprivileged-access.bats | 91 |
9 files changed, 171 insertions, 144 deletions
diff --git a/libpod/container.go b/libpod/container.go index 6d80a9bf4..739406e42 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -146,18 +146,12 @@ type ContainerState struct { ConfigPath string `json:"configPath,omitempty"` // RunDir is a per-boot directory for container content RunDir string `json:"runDir,omitempty"` - // DestinationRunDir is where the files in RunDir will be accessible for the container. - // It is different than RunDir when using userNS - DestinationRunDir string `json:"destinationRunDir,omitempty"` // Mounted indicates whether the container's storage has been mounted // for use Mounted bool `json:"mounted,omitempty"` // Mountpoint contains the path to the container's mounted storage as given - // by containers/storage. It can be different than RealMountpoint when - // usernamespaces are used + // by containers/storage. Mountpoint string `json:"mountPoint,omitempty"` - // RealMountpoint contains the path to the container's mounted storage - RealMountpoint string `json:"realMountPoint,omitempty"` // StartedTime is the time the container was started StartedTime time.Time `json:"startedTime,omitempty"` // FinishedTime is the time the container finished executing @@ -186,10 +180,6 @@ type ContainerState struct { // the path of the file on disk outside the container BindMounts map[string]string `json:"bindMounts,omitempty"` - // UserNSRoot is the directory used as root for the container when using - // user namespaces. - UserNSRoot string `json:"userNSRoot,omitempty"` - // ExtensionStageHooks holds hooks which will be executed by libpod // and not delegated to the OCI runtime. ExtensionStageHooks map[string][]spec.Hook `json:"extensionStageHooks,omitempty"` diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 7a90bc7d4..daa32007a 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -310,23 +310,12 @@ func (c *Container) setupStorage(ctx context.Context) error { } if !rootless.IsRootless() && (len(c.config.IDMappings.UIDMap) != 0 || len(c.config.IDMappings.GIDMap) != 0) { - info, err := os.Stat(c.runtime.config.TmpDir) - if err != nil { - return errors.Wrapf(err, "cannot stat `%s`", c.runtime.config.TmpDir) - } - if err := os.Chmod(c.runtime.config.TmpDir, info.Mode()|0111); err != nil { - return errors.Wrapf(err, "cannot chmod `%s`", c.runtime.config.TmpDir) - } - root := filepath.Join(c.runtime.config.TmpDir, "containers-root", c.ID()) - if err := os.MkdirAll(root, 0755); err != nil { - return errors.Wrapf(err, "error creating userNS tmpdir for container %s", c.ID()) - } - if err := os.Chown(root, c.RootUID(), c.RootGID()); err != nil { + if err := os.Chown(containerInfo.RunDir, c.RootUID(), c.RootGID()); err != nil { return err } - c.state.UserNSRoot, err = filepath.EvalSymlinks(root) - if err != nil { - return errors.Wrapf(err, "failed to eval symlinks for %s", root) + + if err := os.Chown(containerInfo.Dir, c.RootUID(), c.RootGID()); err != nil { + return err } } @@ -334,10 +323,6 @@ func (c *Container) setupStorage(ctx context.Context) error { c.config.MountLabel = containerInfo.MountLabel c.config.StaticDir = containerInfo.Dir c.state.RunDir = containerInfo.RunDir - c.state.DestinationRunDir = c.state.RunDir - if c.state.UserNSRoot != "" { - c.state.DestinationRunDir = filepath.Join(c.state.UserNSRoot, "rundir") - } // Set the default Entrypoint and Command if containerInfo.Config != nil { @@ -372,12 +357,6 @@ func (c *Container) teardownStorage() error { return errors.Wrapf(err, "failed to cleanup container %s storage", c.ID()) } - if c.state.UserNSRoot != "" { - if err := os.RemoveAll(c.state.UserNSRoot); err != nil { - return errors.Wrapf(err, "error removing userns root %q", c.state.UserNSRoot) - } - } - if err := c.runtime.storageService.DeleteContainer(c.ID()); err != nil { // If the container has already been removed, warn but do not // error - we wanted it gone, it is already gone. @@ -432,6 +411,7 @@ func (c *Container) refresh() error { if err != nil { return errors.Wrapf(err, "error retrieving temporary directory for container %s", c.ID()) } + c.state.RunDir = dir if len(c.config.IDMappings.UIDMap) != 0 || len(c.config.IDMappings.GIDMap) != 0 { info, err := os.Stat(c.runtime.config.TmpDir) @@ -448,16 +428,6 @@ func (c *Container) refresh() error { if err := os.Chown(root, c.RootUID(), c.RootGID()); err != nil { return err } - c.state.UserNSRoot, err = filepath.EvalSymlinks(root) - if err != nil { - return errors.Wrapf(err, "failed to eval symlinks for %s", root) - } - } - - c.state.RunDir = dir - c.state.DestinationRunDir = c.state.RunDir - if c.state.UserNSRoot != "" { - c.state.DestinationRunDir = filepath.Join(c.state.UserNSRoot, "rundir") } // We need to pick up a new lock @@ -1260,7 +1230,7 @@ func (c *Container) writeStringToRundir(destFile, output string) (string, error) return "", err } - return filepath.Join(c.state.DestinationRunDir, destFile), nil + return filepath.Join(c.state.RunDir, destFile), nil } // appendStringToRundir appends the provided string to the runtimedir file @@ -1277,7 +1247,7 @@ func (c *Container) appendStringToRundir(destFile, output string) (string, error return "", errors.Wrapf(err, "unable to write %s", destFileName) } - return filepath.Join(c.state.DestinationRunDir, destFile), nil + return filepath.Join(c.state.RunDir, destFile), nil } // Save OCI spec to disk, replacing any existing specs for the container @@ -1410,6 +1380,9 @@ func (c *Container) mount() (string, error) { if err != nil { return "", errors.Wrapf(err, "error resolving storage path for container %s", c.ID()) } + if err := os.Chown(mountPoint, c.RootUID(), c.RootGID()); err != nil { + return "", errors.Wrapf(err, "cannot chown %s to %d:%d", mountPoint, c.RootUID(), c.RootGID()) + } return mountPoint, nil } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 23de1aa01..504d6c135 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -25,7 +25,6 @@ import ( "github.com/containers/libpod/pkg/lookup" "github.com/containers/libpod/pkg/resolvconf" "github.com/containers/libpod/pkg/rootless" - "github.com/containers/storage/pkg/idtools" "github.com/cyphar/filepath-securejoin" "github.com/opencontainers/runc/libcontainer/user" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -99,11 +98,6 @@ func (c *Container) prepare() (err error) { // Finish up mountStorage c.state.Mounted = true c.state.Mountpoint = mountPoint - if c.state.UserNSRoot == "" { - c.state.RealMountpoint = c.state.Mountpoint - } else { - c.state.RealMountpoint = filepath.Join(c.state.UserNSRoot, "mountpoint") - } logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint) }() @@ -220,13 +214,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } m.Options = options - - // If we are using a user namespace, we will use an intermediate - // directory to bind mount volumes - if c.state.UserNSRoot != "" && strings.HasPrefix(m.Source, c.runtime.config.VolumePath) { - newSourceDir := filepath.Join(c.state.UserNSRoot, "volumes") - m.Source = strings.Replace(m.Source, c.runtime.config.VolumePath, newSourceDir, 1) - } } g.SetProcessSelinuxLabel(c.ProcessLabel()) @@ -313,13 +300,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { } } - if c.config.Rootfs == "" { - if err := idtools.MkdirAllAs(c.state.RealMountpoint, 0700, c.RootUID(), c.RootGID()); err != nil { - return nil, err - } - } - - g.SetRootPath(c.state.RealMountpoint) + g.SetRootPath(c.state.Mountpoint) g.AddAnnotation(crioAnnotations.Created, c.config.CreatedTime.Format(time.RFC3339Nano)) g.AddAnnotation("org.opencontainers.image.stopSignal", fmt.Sprintf("%d", c.config.StopSignal)) @@ -820,7 +801,7 @@ func (c *Container) makeBindMounts() error { } // Add Secret Mounts - secretMounts := secrets.SecretMountsWithUIDGID(c.config.MountLabel, c.state.RunDir, c.runtime.config.DefaultMountsFile, c.state.DestinationRunDir, c.RootUID(), c.RootGID(), rootless.IsRootless()) + secretMounts := secrets.SecretMountsWithUIDGID(c.config.MountLabel, c.state.RunDir, c.runtime.config.DefaultMountsFile, c.state.RunDir, c.RootUID(), c.RootGID(), rootless.IsRootless()) for _, mount := range secretMounts { if _, ok := c.state.BindMounts[mount.Destination]; !ok { c.state.BindMounts[mount.Destination] = mount.Source @@ -907,7 +888,7 @@ func (c *Container) generateResolvConf() (string, error) { return "", err } - return filepath.Join(c.state.DestinationRunDir, "resolv.conf"), nil + return filepath.Join(c.state.RunDir, "resolv.conf"), nil } // generateHosts creates a containers hosts file diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go index f85c5ee62..8c0abad80 100644 --- a/libpod/oci_linux.go +++ b/libpod/oci_linux.go @@ -3,17 +3,14 @@ package libpod import ( - "fmt" "os" "os/exec" "path/filepath" - "runtime" "strings" - "sync" + "syscall" "github.com/containerd/cgroups" "github.com/containers/libpod/utils" - "github.com/containers/storage/pkg/idtools" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -62,72 +59,40 @@ func newPipe() (parent *os.File, child *os.File, err error) { return os.NewFile(uintptr(fds[1]), "parent"), os.NewFile(uintptr(fds[0]), "child"), nil } -// CreateContainer creates a container in the OCI runtime -// TODO terminal support for container -// Presently just ignoring conmon opts related to it -func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreOptions *ContainerCheckpointOptions) (err error) { - if ctr.state.UserNSRoot == "" { - // no need of an intermediate mount ns - return r.createOCIContainer(ctr, cgroupParent, restoreOptions) - } - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - runtime.LockOSThread() - - var fd *os.File - fd, err = os.Open(fmt.Sprintf("/proc/%d/task/%d/ns/mnt", os.Getpid(), unix.Gettid())) +// makeAccessible changes the path permission and each parent directory to have --x--x--x +func makeAccessible(path string, uid, gid int) error { + for ; path != "/"; path = filepath.Dir(path) { + st, err := os.Stat(path) if err != nil { - return - } - defer fd.Close() - - // create a new mountns on the current thread - if err = unix.Unshare(unix.CLONE_NEWNS); err != nil { - return - } - defer unix.Setns(int(fd.Fd()), unix.CLONE_NEWNS) - - // don't spread our mounts around - err = unix.Mount("/", "/", "none", unix.MS_REC|unix.MS_SLAVE, "") - if err != nil { - return - } - err = unix.Mount(ctr.state.Mountpoint, ctr.state.RealMountpoint, "none", unix.MS_BIND, "") - if err != nil { - return + if os.IsNotExist(err) { + return nil + } + return err } - if err := idtools.MkdirAllAs(ctr.state.DestinationRunDir, 0700, ctr.RootUID(), ctr.RootGID()); err != nil { - return + if int(st.Sys().(*syscall.Stat_t).Uid) == uid && int(st.Sys().(*syscall.Stat_t).Gid) == gid { + continue } - - err = unix.Mount(ctr.state.RunDir, ctr.state.DestinationRunDir, "none", unix.MS_BIND, "") - if err != nil { - return + if st.Mode()&0111 != 0111 { + if err := os.Chmod(path, os.FileMode(st.Mode()|0111)); err != nil { + return err + } } + } + return nil +} - if ctr.state.UserNSRoot != "" { - _, err := os.Stat(ctr.runtime.config.VolumePath) - if err != nil && !os.IsNotExist(err) { - return - } - if err == nil { - volumesTarget := filepath.Join(ctr.state.UserNSRoot, "volumes") - if err := idtools.MkdirAs(volumesTarget, 0700, ctr.RootUID(), ctr.RootGID()); err != nil { - return - } - if err = unix.Mount(ctr.runtime.config.VolumePath, volumesTarget, "none", unix.MS_BIND, ""); err != nil { - return - } +// CreateContainer creates a container in the OCI runtime +// TODO terminal support for container +// Presently just ignoring conmon opts related to it +func (r *OCIRuntime) createContainer(ctr *Container, cgroupParent string, restoreOptions *ContainerCheckpointOptions) (err error) { + if len(ctr.config.IDMappings.UIDMap) != 0 || len(ctr.config.IDMappings.GIDMap) != 0 { + for _, i := range []string{ctr.state.RunDir, ctr.runtime.config.TmpDir, ctr.config.StaticDir, ctr.state.Mountpoint, ctr.runtime.config.VolumePath} { + if err := makeAccessible(i, ctr.RootUID(), ctr.RootGID()); err != nil { + return err } } - - err = r.createOCIContainer(ctr, cgroupParent, restoreOptions) - }() - wg.Wait() - - return err + } + return r.createOCIContainer(ctr, cgroupParent, restoreOptions) } func rpmVersion(path string) string { diff --git a/libpod/options.go b/libpod/options.go index 55a0c0999..24f126e66 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1274,6 +1274,28 @@ func WithVolumeName(name string) VolumeCreateOption { } } +// WithVolumeUID sets the uid of the owner. +func WithVolumeUID(uid int) VolumeCreateOption { + return func(volume *Volume) error { + if volume.valid { + return ErrVolumeFinalized + } + volume.config.UID = uid + return nil + } +} + +// WithVolumeGID sets the gid of the owner. +func WithVolumeGID(gid int) VolumeCreateOption { + return func(volume *Volume) error { + if volume.valid { + return ErrVolumeFinalized + } + volume.config.GID = gid + return nil + } +} + // WithVolumeLabels sets the labels of the volume. func WithVolumeLabels(labels map[string]string) VolumeCreateOption { return func(volume *Volume) error { diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 7c39d8ced..506aee477 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -182,14 +182,11 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. if vol.Source[0] != '/' && isNamedVolume(vol.Source) { volInfo, err := r.state.Volume(vol.Source) if err != nil { - newVol, err := r.newVolume(ctx, WithVolumeName(vol.Source), withSetCtrSpecific()) + newVol, err := r.newVolume(ctx, WithVolumeName(vol.Source), withSetCtrSpecific(), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())) if err != nil { return nil, errors.Wrapf(err, "error creating named volume %q", vol.Source) } ctr.config.Spec.Mounts[i].Source = newVol.MountPoint() - if err := os.Chown(ctr.config.Spec.Mounts[i].Source, ctr.RootUID(), ctr.RootGID()); err != nil { - return nil, errors.Wrapf(err, "cannot chown %q to %d:%d", ctr.config.Spec.Mounts[i].Source, ctr.RootUID(), ctr.RootGID()) - } if err := ctr.copyWithTarFromImage(ctr.config.Spec.Mounts[i].Destination, ctr.config.Spec.Mounts[i].Source); err != nil && !os.IsNotExist(err) { return nil, errors.Wrapf(err, "failed to copy content into new volume mount %q", vol.Source) } @@ -204,11 +201,7 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. } if !MountExists(ctr.config.Spec.Mounts, "/dev/shm") && ctr.config.ShmDir == "" { - if ctr.state.UserNSRoot == "" { - ctr.config.ShmDir = filepath.Join(ctr.bundlePath(), "shm") - } else { - ctr.config.ShmDir = filepath.Join(ctr.state.UserNSRoot, "shm") - } + ctr.config.ShmDir = filepath.Join(ctr.bundlePath(), "shm") if err := os.MkdirAll(ctr.config.ShmDir, 0700); err != nil { if !os.IsExist(err) { return nil, errors.Wrapf(err, "unable to create shm %q dir", ctr.config.ShmDir) diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index 5767a99e7..db5c29242 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -51,10 +51,20 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) } // Create the mountpoint of this volume - fullVolPath := filepath.Join(r.config.VolumePath, volume.config.Name, "_data") - if err := os.MkdirAll(fullVolPath, 0755); err != nil { + volPathRoot := filepath.Join(r.config.VolumePath, volume.config.Name) + if err := os.MkdirAll(volPathRoot, 0700); err != nil { + return nil, errors.Wrapf(err, "error creating volume directory %q", volPathRoot) + } + if err := os.Chown(volPathRoot, volume.config.UID, volume.config.GID); err != nil { + return nil, errors.Wrapf(err, "error chowning volume directory %q to %d:%d", volPathRoot, volume.config.UID, volume.config.GID) + } + fullVolPath := filepath.Join(volPathRoot, "_data") + if err := os.Mkdir(fullVolPath, 0755); err != nil { return nil, errors.Wrapf(err, "error creating volume directory %q", fullVolPath) } + if err := os.Chown(fullVolPath, volume.config.UID, volume.config.GID); err != nil { + return nil, errors.Wrapf(err, "error chowning volume directory %q to %d:%d", fullVolPath, volume.config.UID, volume.config.GID) + } if err := LabelVolumePath(fullVolPath, true); err != nil { return nil, err } diff --git a/libpod/volume.go b/libpod/volume.go index 0c7618841..0b37d44ef 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -21,6 +21,8 @@ type VolumeConfig struct { Options map[string]string `json:"options"` Scope string `json:"scope"` IsCtrSpecific bool `json:"ctrSpecific"` + UID int `json:"uid"` + GID int `json:"gid"` } // Name retrieves the volume's name diff --git a/test/system/400-unprivileged-access.bats b/test/system/400-unprivileged-access.bats new file mode 100644 index 000000000..c195d71eb --- /dev/null +++ b/test/system/400-unprivileged-access.bats @@ -0,0 +1,91 @@ +#!/usr/bin/env bats -*- bats -*- +# +# Tests #2730 - regular users are not able to read/write container storage +# + +load helpers + +@test "podman container storage is not accessible by unprivileged users" { + skip_if_rootless "test meaningless without suid" + + run_podman run --name c_uidmap --uidmap 0:10000:10000 $IMAGE true + run_podman run --name c_uidmap_v --uidmap 0:10000:10000 -v foo:/foo $IMAGE true + + run_podman run --name c_mount $IMAGE \ + sh -c "echo hi > /myfile;mkdir -p /mydir/mysubdir; chmod 777 /myfile /mydir /mydir/mysubdir" + + run_podman mount c_mount + mount_path=$output + + # Do all the work from within a test script. Since we'll be invoking it + # as a user, the parent directory must be world-readable. + test_script=$PODMAN_TMPDIR/fail-if-writable + cat >$test_script <<"EOF" +#!/bin/sh + +path="$1" + +die() { + echo "#/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv" >&2 + echo "#| FAIL: $*" >&2 + echo "#\\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^" >&2 + + exit 1 +} + +parent=$(dirname "$path") +if chmod +w $parent; then + die "Able to chmod $parent" +fi +if chmod +w "$path"; then + die "Able to chmod $path" +fi + +if [ -d "$path" ]; then + if ls "$path" >/dev/null; then + die "Able to run 'ls $path' without error" + fi + if echo hi >"$path"/test; then + die "Able to write to file under $path" + fi +else + # Plain file + if cat "$path" >/dev/null; then + die "Able to read $path" + fi + if echo hi >"$path"; then + die "Able to write to $path" + fi +fi + +exit 0 +EOF + chmod 755 $PODMAN_TMPDIR $test_script + + # get podman image and container storage directories + run_podman info --format '{{.store.GraphRoot}}' + GRAPH_ROOT="$output" + run_podman info --format '{{.store.RunRoot}}' + RUN_ROOT="$output" + + # The main test: find all world-writable files or directories underneath + # container storage, run the test script as a nonroot user, and try to + # access each path. + find $GRAPH_ROOT $RUN_ROOT \! -type l -perm -o+w -print | while read i; do + dprint " o+w: $i" + + # use chroot because su fails if uid/gid don't exist or have no shell + # For development: test all this by removing the "--userspec x:x" + chroot --userspec 1000:1000 / $test_script "$i" + done + + # Done. Clean up. + rm -f $test_script + + run_podman umount c_mount + run_podman rm c_mount + + run_podman rm c_uidmap c_uidmap_v +} + +# vim: filetype=sh |