diff options
author | OpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com> | 2021-02-23 10:11:28 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-23 10:11:28 -0500 |
commit | 1702cbc6917f431bcc65d6c5bdc6fcf99231977a (patch) | |
tree | 2b138dda345970e5898593162c38e10d4909fabd | |
parent | 96fc9d983e0fc5bae48c3cec3acce86cdb6e1059 (diff) | |
parent | 874f2327e6ca963edda7cc46819d51048d3d19a8 (diff) | |
download | podman-1702cbc6917f431bcc65d6c5bdc6fcf99231977a.tar.gz podman-1702cbc6917f431bcc65d6c5bdc6fcf99231977a.tar.bz2 podman-1702cbc6917f431bcc65d6c5bdc6fcf99231977a.zip |
Merge pull request #8349 from EduardoVega/7778-chowning-based-on-uid
Add U volume flag to chown source volumes
-rw-r--r-- | docs/source/markdown/podman-create.1.md | 9 | ||||
-rw-r--r-- | docs/source/markdown/podman-run.1.md | 11 | ||||
-rw-r--r-- | libpod/container.go | 2 | ||||
-rw-r--r-- | libpod/container_internal_linux.go | 36 | ||||
-rw-r--r-- | libpod/options.go | 5 | ||||
-rw-r--r-- | pkg/specgen/generate/container_create.go | 5 | ||||
-rw-r--r-- | pkg/specgen/volumes.go | 13 | ||||
-rw-r--r-- | pkg/util/mountOpts.go | 7 | ||||
-rw-r--r-- | pkg/util/utils.go | 14 | ||||
-rw-r--r-- | test/e2e/run_volume_test.go | 53 | ||||
-rw-r--r-- | vendor/github.com/containers/common/pkg/chown/chown.go | 122 | ||||
-rw-r--r-- | vendor/modules.txt | 1 |
12 files changed, 270 insertions, 8 deletions
diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 7782949a9..30cadf703 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -1036,6 +1036,7 @@ The _options_ is a comma delimited list and can be: * [**no**]**dev** * [**no**]**suid** * [**O**] +* [**U**] The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume will be mounted into the container at this directory. @@ -1065,6 +1066,14 @@ You can add `:ro` or `:rw` suffix to a volume to mount it read-only or read-write mode, respectively. By default, the volumes are mounted read-write. See examples. + `Chowning Volume Mounts` + +By default, Podman does not change the owner and group of source volume directories mounted into containers. If a container is created in a new user namespace, the UID and GID in the container may correspond to another UID and GID on the host. + +The `:U` suffix tells Podman to use the correct host UID and GID based on the UID and GID within the container, to change recursively the owner and group of the source volume. + +**Warning** use with caution since this will modify the host filesystem. + `Labeling Volume Mounts` Labeling systems like SELinux require that proper labels are placed on volume diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 8c0b12e90..a633df94e 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -1111,6 +1111,7 @@ The _options_ is a comma delimited list and can be: <sup>[[1]](#Footnote1)</sup> * [**no**]**dev** * [**no**]**suid** * [**O**] +* [**U**] The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume will be mounted into the container at this directory. @@ -1139,6 +1140,14 @@ container. You can add **:ro** or **:rw** option to mount a volume in read-only or read-write mode, respectively. By default, the volumes are mounted read-write. + `Chowning Volume Mounts` + +By default, Podman does not change the owner and group of source volume directories mounted into containers. If a container is created in a new user namespace, the UID and GID in the container may correspond to another UID and GID on the host. + +The `:U` suffix tells Podman to use the correct host UID and GID based on the UID and GID within the container, to change recursively the owner and group of the source volume. + +**Warning** use with caution since this will modify the host filesystem. + `Labeling Volume Mounts` Labeling systems like SELinux require that proper labels are placed on volume @@ -1450,6 +1459,8 @@ $ podman run -v /var/db:/data1 -i -t fedora bash $ podman run -v data:/data2 -i -t fedora bash $ podman run -v /var/cache/dnf:/var/cache/dnf:O -ti fedora dnf -y update + +$ podman run -d -e MYSQL_ROOT_PASSWORD=root --user mysql --userns=keep-id -v ~/data:/var/lib/mysql:z,U mariadb ``` Using **--mount** flags to mount a host directory as a container folder, specify diff --git a/libpod/container.go b/libpod/container.go index 9841bddf7..ee6e243ac 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -236,6 +236,8 @@ type ContainerOverlayVolume struct { Dest string `json:"dest"` // Source specifies the source path of the mount. Source string `json:"source,omitempty"` + // Options holds overlay volume options. + Options []string `json:"options,omitempty"` } // ContainerImageVolume is a volume based on a container image. The container diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 43a345ea9..9c3e91a97 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -23,7 +23,9 @@ import ( "github.com/containernetworking/plugins/pkg/ns" "github.com/containers/buildah/pkg/chrootuser" "github.com/containers/buildah/pkg/overlay" + butil "github.com/containers/buildah/util" "github.com/containers/common/pkg/apparmor" + "github.com/containers/common/pkg/chown" "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/subscriptions" "github.com/containers/common/pkg/umask" @@ -356,13 +358,28 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { return nil, err } - // Check if the spec file mounts contain the label Relabel flags z or Z. - // If they do, relabel the source directory and then remove the option. + // Get host UID and GID based on the container process UID and GID. + hostUID, hostGID, err := butil.GetHostIDs(util.IDtoolsToRuntimeSpec(c.config.IDMappings.UIDMap), util.IDtoolsToRuntimeSpec(c.config.IDMappings.GIDMap), uint32(execUser.Uid), uint32(execUser.Gid)) + if err != nil { + return nil, err + } + + // Check if the spec file mounts contain the options z, Z or U. + // If they have z or Z, relabel the source directory and then remove the option. + // If they have U, chown the source directory and them remove the option. for i := range g.Config.Mounts { m := &g.Config.Mounts[i] var options []string for _, o := range m.Options { switch o { + case "U": + if m.Type == "tmpfs" { + options = append(options, []string{fmt.Sprintf("uid=%d", execUser.Uid), fmt.Sprintf("gid=%d", execUser.Gid)}...) + } else { + if err := chown.ChangeHostPathOwnership(m.Source, true, int(hostUID), int(hostGID)); err != nil { + return nil, err + } + } case "z": fallthrough case "Z": @@ -427,6 +444,21 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { if err != nil { return nil, errors.Wrapf(err, "mounting overlay failed %q", overlayVol.Source) } + + // Check overlay volume options + for _, o := range overlayVol.Options { + switch o { + case "U": + if err := chown.ChangeHostPathOwnership(overlayVol.Source, true, int(hostUID), int(hostGID)); err != nil { + return nil, err + } + + if err := chown.ChangeHostPathOwnership(contentDir, true, int(hostUID), int(hostGID)); err != nil { + return nil, err + } + } + } + g.AddMount(overlayMount) } diff --git a/libpod/options.go b/libpod/options.go index 627ea8c57..6344e1acc 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1429,8 +1429,9 @@ func WithOverlayVolumes(volumes []*ContainerOverlayVolume) CtrCreateOption { for _, vol := range volumes { ctr.config.OverlayVolumes = append(ctr.config.OverlayVolumes, &ContainerOverlayVolume{ - Dest: vol.Dest, - Source: vol.Source, + Dest: vol.Dest, + Source: vol.Source, + Options: vol.Options, }) } diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 3b7112959..03697b353 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -247,8 +247,9 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. var vols []*libpod.ContainerOverlayVolume for _, v := range overlays { vols = append(vols, &libpod.ContainerOverlayVolume{ - Dest: v.Destination, - Source: v.Source, + Dest: v.Destination, + Source: v.Source, + Options: v.Options, }) } options = append(options, libpod.WithOverlayVolumes(vols)) diff --git a/pkg/specgen/volumes.go b/pkg/specgen/volumes.go index 83634b4ef..d85d2bdd1 100644 --- a/pkg/specgen/volumes.go +++ b/pkg/specgen/volumes.go @@ -31,6 +31,8 @@ type OverlayVolume struct { Destination string `json:"destination"` // Source specifies the source path of the mount. Source string `json:"source,omitempty"` + // Options holds overlay volume options. + Options []string `json:"options,omitempty"` } // ImageVolume is a volume based on a container image. The container image is @@ -100,10 +102,17 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na if strings.HasPrefix(src, "/") || strings.HasPrefix(src, ".") { // This is not a named volume overlayFlag := false + chownFlag := false for _, o := range options { if o == "O" { overlayFlag = true - if len(options) > 1 { + + joinedOpts := strings.Join(options, "") + if strings.Contains(joinedOpts, "U") { + chownFlag = true + } + + if len(options) > 2 || (len(options) == 2 && !chownFlag) { return nil, nil, nil, errors.New("can't use 'O' with other options") } } @@ -113,6 +122,8 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na newOverlayVol := new(OverlayVolume) newOverlayVol.Destination = cleanDest newOverlayVol.Source = src + newOverlayVol.Options = options + if _, ok := overlayVolumes[newOverlayVol.Destination]; ok { return nil, nil, nil, errors.Wrapf(errDuplicateDest, newOverlayVol.Destination) } diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go index b3a38f286..f13dc94ec 100644 --- a/pkg/util/mountOpts.go +++ b/pkg/util/mountOpts.go @@ -25,7 +25,7 @@ type defaultMountOptions struct { // The sourcePath variable, if not empty, contains a bind mount source. func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string, error) { var ( - foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ bool + foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ, foundU bool ) newOptions := make([]string, 0, len(options)) @@ -116,6 +116,11 @@ func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'z' and 'Z' can be used") } foundZ = true + case "U": + if foundU { + return nil, errors.Wrapf(ErrDupeMntOption, "the 'U' option can only be set once") + } + foundU = true default: return nil, errors.Wrapf(ErrBadMntOption, "unknown mount option %q", opt) } diff --git a/pkg/util/utils.go b/pkg/util/utils.go index c6ecd91f6..a4c8f3a64 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -23,6 +23,7 @@ import ( "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" v1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/crypto/ssh/terminal" @@ -692,3 +693,16 @@ func CoresToPeriodAndQuota(cores float64) (uint64, int64) { func PeriodAndQuotaToCores(period uint64, quota int64) float64 { return float64(quota) / float64(period) } + +// IDtoolsToRuntimeSpec converts idtools ID mapping to the one of the runtime spec. +func IDtoolsToRuntimeSpec(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxIDMapping) { + for _, idmap := range idMaps { + tempIDMap := specs.LinuxIDMapping{ + ContainerID: uint32(idmap.ContainerID), + HostID: uint32(idmap.HostID), + Size: uint32(idmap.Size), + } + convertedIDMap = append(convertedIDMap, tempIDMap) + } + return convertedIDMap +} diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 20c43bf4a..454dfdc83 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -2,8 +2,10 @@ package integration import ( "fmt" + "io/ioutil" "os" "os/exec" + "os/user" "path/filepath" "strings" @@ -590,4 +592,55 @@ VOLUME /test/` Expect(session.ExitCode()).To(Equal(0)) Expect(len(session.OutputToStringArray())).To(Equal(2)) }) + + It("podman run with U volume flag", func() { + SkipIfRemote("Overlay volumes only work locally") + + u, err := user.Current() + Expect(err).To(BeNil()) + name := u.Username + if name == "root" { + name = "containers" + } + + content, err := ioutil.ReadFile("/etc/subuid") + if err != nil { + Skip("cannot read /etc/subuid") + } + if !strings.Contains(string(content), name) { + Skip("cannot find mappings for the current user") + } + + if os.Getenv("container") != "" { + Skip("Overlay mounts not supported when running in a container") + } + if rootless.IsRootless() { + if _, err := exec.LookPath("fuse_overlay"); err != nil { + Skip("Fuse-Overlayfs required for rootless overlay mount test") + } + } + + mountPath := filepath.Join(podmanTest.TempDir, "secrets") + os.Mkdir(mountPath, 0755) + vol := mountPath + ":" + dest + ":U" + + session := podmanTest.Podman([]string{"run", "--rm", "--user", "888:888", "-v", vol, ALPINE, "stat", "-c", "%u:%g", dest}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + found, _ := session.GrepString("888:888") + Expect(found).Should(BeTrue()) + + session = podmanTest.Podman([]string{"run", "--rm", "--user", "888:888", "--userns", "auto", "-v", vol, ALPINE, "stat", "-c", "%u:%g", dest}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + found, _ = session.GrepString("888:888") + Expect(found).Should(BeTrue()) + + vol = 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.ExitCode()).To(Equal(0)) + found, _ = session.GrepString("888:888") + Expect(found).Should(BeTrue()) + }) }) diff --git a/vendor/github.com/containers/common/pkg/chown/chown.go b/vendor/github.com/containers/common/pkg/chown/chown.go new file mode 100644 index 000000000..fe794304e --- /dev/null +++ b/vendor/github.com/containers/common/pkg/chown/chown.go @@ -0,0 +1,122 @@ +package chown + +import ( + "os" + "os/user" + "path/filepath" + "syscall" + + "github.com/containers/storage/pkg/homedir" + "github.com/pkg/errors" +) + +// DangerousHostPath validates if a host path is dangerous and should not be modified +func DangerousHostPath(path string) (bool, error) { + excludePaths := map[string]bool{ + "/": true, + "/bin": true, + "/boot": true, + "/dev": true, + "/etc": true, + "/etc/passwd": true, + "/etc/pki": true, + "/etc/shadow": true, + "/home": true, + "/lib": true, + "/lib64": true, + "/media": true, + "/opt": true, + "/proc": true, + "/root": true, + "/run": true, + "/sbin": true, + "/srv": true, + "/sys": true, + "/tmp": true, + "/usr": true, + "/var": true, + "/var/lib": true, + "/var/log": true, + } + + if home := homedir.Get(); home != "" { + excludePaths[home] = true + } + + if sudoUser := os.Getenv("SUDO_USER"); sudoUser != "" { + if usr, err := user.Lookup(sudoUser); err == nil { + excludePaths[usr.HomeDir] = true + } + } + + absPath, err := filepath.Abs(path) + if err != nil { + return true, err + } + + realPath, err := filepath.EvalSymlinks(absPath) + if err != nil { + return true, err + } + + if excludePaths[realPath] { + return true, nil + } + + return false, nil +} + +// ChangeHostPathOwnership changes the uid and gid ownership of a directory or file within the host. +// This is used by the volume U flag to change source volumes ownership +func ChangeHostPathOwnership(path string, recursive bool, uid, gid int) error { + // Validate if host path can be chowned + isDangerous, err := DangerousHostPath(path) + if err != nil { + return errors.Wrapf(err, "failed to validate if host path is dangerous") + } + + if isDangerous { + return errors.Errorf("chowning host path %q is not allowed. You can manually `chown -R %d:%d %s`", path, uid, gid, path) + } + + // Chown host path + if recursive { + err := filepath.Walk(path, func(filePath string, f os.FileInfo, err error) error { + if err != nil { + return err + } + + // Get current ownership + currentUID := int(f.Sys().(*syscall.Stat_t).Uid) + currentGID := int(f.Sys().(*syscall.Stat_t).Gid) + + if uid != currentUID || gid != currentGID { + return os.Lchown(filePath, uid, gid) + } + + return nil + }) + + if err != nil { + return errors.Wrapf(err, "failed to chown recursively host path") + } + } else { + // Get host path info + f, err := os.Lstat(path) + if err != nil { + return errors.Wrapf(err, "failed to get host path information") + } + + // Get current ownership + currentUID := int(f.Sys().(*syscall.Stat_t).Uid) + currentGID := int(f.Sys().(*syscall.Stat_t).Gid) + + if uid != currentUID || gid != currentGID { + if err := os.Lchown(path, uid, gid); err != nil { + return errors.Wrapf(err, "failed to chown host path") + } + } + } + + return nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 8fb3197de..39190656c 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -95,6 +95,7 @@ github.com/containers/common/pkg/apparmor/internal/supported github.com/containers/common/pkg/auth github.com/containers/common/pkg/capabilities github.com/containers/common/pkg/cgroupv2 +github.com/containers/common/pkg/chown github.com/containers/common/pkg/completion github.com/containers/common/pkg/config github.com/containers/common/pkg/parse |