From ed20174ce4eb9bea5dd7532095b5f8cbec3902a8 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Fri, 30 Mar 2018 22:26:07 +0000 Subject: [PATCH] Add RunAsGroup support. Signed-off-by: Lantao Liu --- pkg/server/container_create.go | 39 +++++- pkg/server/sandbox_run.go | 12 +- vendor.conf | 2 +- .../containerd/archive/tar_windows.go | 9 +- .../contrib/seccomp/seccomp_default.go | 22 +-- .../github.com/containerd/containerd/image.go | 21 ++- .../containerd/linux/shim/client/client.go | 2 +- .../containerd/oci/spec_opts_unix.go | 128 +++++++++++++----- .../containerd/containerd/sys/socket_unix.go | 5 + .../containerd/containerd/vendor.conf | 4 +- 10 files changed, 174 insertions(+), 70 deletions(-) diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index be9a79890..c4f730d0d 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -19,6 +19,7 @@ package server import ( "os" "path/filepath" + "strconv" "strings" "time" @@ -219,11 +220,16 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta // Set container username. This could only be done by containerd, because it needs // access to the container rootfs. Pass user name to containerd, and let it overwrite // the spec for us. - if uid := securityContext.GetRunAsUser(); uid != nil { - specOpts = append(specOpts, oci.WithUserID(uint32(uid.GetValue()))) + userstr, err := generateUserString( + securityContext.GetRunAsUsername(), + securityContext.GetRunAsUser(), + securityContext.GetRunAsGroup(), + ) + if err != nil { + return nil, errors.Wrap(err, "failed to generate user string") } - if username := securityContext.GetRunAsUsername(); username != "" { - specOpts = append(specOpts, oci.WithUsername(username)) + if userstr != "" { + specOpts = append(specOpts, oci.WithUser(userstr)) } apparmorSpecOpts, err := generateApparmorSpecOpts( @@ -884,3 +890,28 @@ func ensureSharedOrSlave(path string, lookupMount func(string) (mount.Info, erro } return errors.Errorf("path %q is mounted on %q but it is not a shared or slave mount", path, mountInfo.Mountpoint) } + +// generateUserString generates valid user string based on OCI Image Spec v1.0.0. +// TODO(random-liu): Add group name support in CRI. +func generateUserString(username string, uid, gid *runtime.Int64Value) (string, error) { + var userstr, groupstr string + if uid != nil { + userstr = strconv.FormatInt(uid.GetValue(), 10) + } + if username != "" { + userstr = username + } + if gid != nil { + groupstr = strconv.FormatInt(gid.GetValue(), 10) + } + if userstr == "" { + if groupstr != "" { + return "", errors.Errorf("user group %q is specified without user", groupstr) + } + return "", nil + } + if groupstr != "" { + userstr = userstr + ":" + groupstr + } + return userstr, nil +} diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index e27ec8bc1..cfa085835 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -145,8 +145,16 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox logrus.Debugf("Sandbox container spec: %+v", spec) var specOpts []oci.SpecOpts - if uid := securityContext.GetRunAsUser(); uid != nil { - specOpts = append(specOpts, oci.WithUserID(uint32(uid.GetValue()))) + userstr, err := generateUserString( + "", + securityContext.GetRunAsUser(), + securityContext.GetRunAsGroup(), + ) + if err != nil { + return nil, errors.Wrap(err, "failed to generate user string") + } + if userstr != "" { + specOpts = append(specOpts, oci.WithUser(userstr)) } seccompSpecOpts, err := generateSeccompSpecOpts( diff --git a/vendor.conf b/vendor.conf index 931c034d6..e71d7a1dd 100644 --- a/vendor.conf +++ b/vendor.conf @@ -4,7 +4,7 @@ github.com/boltdb/bolt e9cf4fae01b5a8ff89d0ec6b32f0d9c9f79aefdd github.com/BurntSushi/toml a368813c5e648fee92e5f6c30e3944ff9d5e8895 github.com/containerd/cgroups fe281dd265766145e943a034aa41086474ea6130 github.com/containerd/console cb7008ab3d8359b78c5f464cb7cf160107ad5925 -github.com/containerd/containerd v1.1.0-rc.0 +github.com/containerd/containerd c0f7fcd910a02cd388c089525d7ea17f9f229a43 github.com/containerd/continuity 3e8f2ea4b190484acb976a5b378d373429639a1a github.com/containerd/fifo 3d5202aec260678c48179c56f40e6f38a095738c github.com/containerd/go-runc bcb223a061a3dd7de1a89c0b402a60f4dd9bd307 diff --git a/vendor/github.com/containerd/containerd/archive/tar_windows.go b/vendor/github.com/containerd/containerd/archive/tar_windows.go index 80edcbbe0..025796a7b 100644 --- a/vendor/github.com/containerd/containerd/archive/tar_windows.go +++ b/vendor/github.com/containerd/containerd/archive/tar_windows.go @@ -35,7 +35,6 @@ import ( "github.com/Microsoft/go-winio" "github.com/Microsoft/hcsshim" - "github.com/containerd/containerd/log" "github.com/containerd/containerd/sys" ) @@ -180,8 +179,12 @@ func applyWindowsLayer(ctx context.Context, root string, tr *tar.Reader, options return 0, err } defer func() { - if err := w.Close(); err != nil { - log.G(ctx).Errorf("failed to close layer writer: %v", err) + if err2 := w.Close(); err2 != nil { + // This error should not be discarded as a failure here + // could result in an invalid layer on disk + if err == nil { + err = err2 + } } }() diff --git a/vendor/github.com/containerd/containerd/contrib/seccomp/seccomp_default.go b/vendor/github.com/containerd/containerd/contrib/seccomp/seccomp_default.go index fe5f43273..11b446a6e 100644 --- a/vendor/github.com/containerd/containerd/contrib/seccomp/seccomp_default.go +++ b/vendor/github.com/containerd/containerd/contrib/seccomp/seccomp_default.go @@ -444,25 +444,8 @@ func DefaultProfile(sp *specs.Spec) *specs.LinuxSeccomp { }) } - // make a map of enabled capabilities - caps := make(map[string]bool) + admin := false for _, c := range sp.Process.Capabilities.Bounding { - caps[c] = true - } - for _, c := range sp.Process.Capabilities.Effective { - caps[c] = true - } - for _, c := range sp.Process.Capabilities.Inheritable { - caps[c] = true - } - for _, c := range sp.Process.Capabilities.Permitted { - caps[c] = true - } - for _, c := range sp.Process.Capabilities.Ambient { - caps[c] = true - } - - for c := range caps { switch c { case "CAP_DAC_READ_SEARCH": s.Syscalls = append(s.Syscalls, specs.LinuxSyscall{ @@ -471,6 +454,7 @@ func DefaultProfile(sp *specs.Spec) *specs.LinuxSeccomp { Args: []specs.LinuxSeccompArg{}, }) case "CAP_SYS_ADMIN": + admin = true s.Syscalls = append(s.Syscalls, specs.LinuxSyscall{ Names: []string{ "bpf", @@ -558,7 +542,7 @@ func DefaultProfile(sp *specs.Spec) *specs.LinuxSeccomp { } } - if !caps["CAP_SYS_ADMIN"] { + if !admin { switch runtime.GOARCH { case "s390", "s390x": s.Syscalls = append(s.Syscalls, specs.LinuxSyscall{ diff --git a/vendor/github.com/containerd/containerd/image.go b/vendor/github.com/containerd/containerd/image.go index 1af706c7f..e2f10a903 100644 --- a/vendor/github.com/containerd/containerd/image.go +++ b/vendor/github.com/containerd/containerd/image.go @@ -25,7 +25,6 @@ import ( "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/rootfs" - "github.com/containerd/containerd/snapshots" digest "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -124,15 +123,25 @@ func (i *image) Unpack(ctx context.Context, snapshotterName string) error { unpacked bool ) for _, layer := range layers { - labels := map[string]string{ - "containerd.io/uncompressed": layer.Diff.Digest.String(), - } - - unpacked, err = rootfs.ApplyLayer(ctx, layer, chain, sn, a, snapshots.WithLabels(labels)) + unpacked, err = rootfs.ApplyLayer(ctx, layer, chain, sn, a) if err != nil { return err } + if unpacked { + // Set the uncompressed label after the uncompressed + // digest has been verified through apply. + cinfo := content.Info{ + Digest: layer.Blob.Digest, + Labels: map[string]string{ + "containerd.io/uncompressed": layer.Diff.Digest.String(), + }, + } + if _, err := cs.Update(ctx, cinfo, "labels.containerd.io/uncompressed"); err != nil { + return err + } + } + chain = append(chain, layer.Diff.Digest) } diff --git a/vendor/github.com/containerd/containerd/linux/shim/client/client.go b/vendor/github.com/containerd/containerd/linux/shim/client/client.go index 37881a36f..f779d071f 100644 --- a/vendor/github.com/containerd/containerd/linux/shim/client/client.go +++ b/vendor/github.com/containerd/containerd/linux/shim/client/client.go @@ -145,7 +145,7 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so func newSocket(address string) (*net.UnixListener, error) { if len(address) > 106 { - return nil, errors.Errorf("%q: unix socket path too long (limit 106)", address) + return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) } l, err := net.Listen("unix", "\x00"+address) if err != nil { diff --git a/vendor/github.com/containerd/containerd/oci/spec_opts_unix.go b/vendor/github.com/containerd/containerd/oci/spec_opts_unix.go index 87d313add..e8ce21d52 100644 --- a/vendor/github.com/containerd/containerd/oci/spec_opts_unix.go +++ b/vendor/github.com/containerd/containerd/oci/spec_opts_unix.go @@ -118,32 +118,7 @@ func WithImageConfig(image Image) SpecOpts { } s.Process.Cwd = cwd if config.User != "" { - // According to OCI Image Spec v1.0.0, the following are valid for Linux: - // user, uid, user:group, uid:gid, uid:group, user:gid - parts := strings.Split(config.User, ":") - switch len(parts) { - case 1: - v, err := strconv.Atoi(parts[0]) - if err != nil { - // if we cannot parse as a uint they try to see if it is a username - return WithUsername(config.User)(ctx, client, c, s) - } - return WithUserID(uint32(v))(ctx, client, c, s) - case 2: - // TODO: support username and groupname - v, err := strconv.Atoi(parts[0]) - if err != nil { - return errors.Wrapf(err, "parse uid %s", parts[0]) - } - uid := uint32(v) - if v, err = strconv.Atoi(parts[1]); err != nil { - return errors.Wrapf(err, "parse gid %s", parts[1]) - } - gid := uint32(v) - s.Process.User.UID, s.Process.User.GID = uid, gid - default: - return fmt.Errorf("invalid USER value %s", config.User) - } + return WithUser(config.User)(ctx, client, c, s) } return nil } @@ -259,6 +234,82 @@ func WithNamespacedCgroup() SpecOpts { } } +// WithUser accepts a valid user string in OCI Image Spec v1.0.0: +// user, uid, user:group, uid:gid, uid:group, user:gid +// and set the correct UID and GID for container. +func WithUser(userstr string) SpecOpts { + return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) error { + parts := strings.Split(userstr, ":") + switch len(parts) { + case 1: + v, err := strconv.Atoi(parts[0]) + if err != nil { + // if we cannot parse as a uint they try to see if it is a username + return WithUsername(userstr)(ctx, client, c, s) + } + return WithUserID(uint32(v))(ctx, client, c, s) + case 2: + var username, groupname string + var uid, gid uint32 + v, err := strconv.Atoi(parts[0]) + if err != nil { + username = parts[0] + } else { + uid = uint32(v) + } + if v, err = strconv.Atoi(parts[1]); err != nil { + groupname = parts[1] + } else { + gid = uint32(v) + } + if username == "" && groupname == "" { + s.Process.User.UID, s.Process.User.GID = uid, gid + return nil + } + f := func(root string) error { + if username != "" { + uid, _, err = getUIDGIDFromPath(root, func(u user.User) bool { + return u.Name == username + }) + if err != nil { + return err + } + } + if groupname != "" { + gid, err = getGIDFromPath(root, func(g user.Group) bool { + return g.Name == groupname + }) + if err != nil { + return err + } + } + s.Process.User.UID, s.Process.User.GID = uid, gid + return nil + } + if c.Snapshotter == "" && c.SnapshotKey == "" { + if !isRootfsAbs(s.Root.Path) { + return errors.New("rootfs absolute path is required") + } + return f(s.Root.Path) + } + if c.Snapshotter == "" { + return errors.New("no snapshotter set for container") + } + if c.SnapshotKey == "" { + return errors.New("rootfs snapshot not created for container") + } + snapshotter := client.SnapshotService(c.Snapshotter) + mounts, err := snapshotter.Mounts(ctx, c.SnapshotKey) + if err != nil { + return err + } + return mount.WithTempMount(ctx, mounts, f) + default: + return fmt.Errorf("invalid USER value %s", userstr) + } + } +} + // WithUIDGID allows the UID and GID for the Process to be set func WithUIDGID(uid, gid uint32) SpecOpts { return func(_ context.Context, _ Client, _ *containers.Container, s *specs.Spec) error { @@ -398,12 +449,7 @@ func getUIDGIDFromPath(root string, filter func(user.User) bool) (uid, gid uint3 if err != nil { return 0, 0, err } - f, err := os.Open(ppath) - if err != nil { - return 0, 0, err - } - defer f.Close() - users, err := user.ParsePasswdFilter(f, filter) + users, err := user.ParsePasswdFileFilter(ppath, filter) if err != nil { return 0, 0, err } @@ -414,6 +460,24 @@ func getUIDGIDFromPath(root string, filter func(user.User) bool) (uid, gid uint3 return uint32(u.Uid), uint32(u.Gid), nil } +var errNoGroupsFound = errors.New("no groups found") + +func getGIDFromPath(root string, filter func(user.Group) bool) (gid uint32, err error) { + gpath, err := fs.RootPath(root, "/etc/group") + if err != nil { + return 0, err + } + groups, err := user.ParseGroupFileFilter(gpath, filter) + if err != nil { + return 0, err + } + if len(groups) == 0 { + return 0, errNoGroupsFound + } + g := groups[0] + return uint32(g.Gid), nil +} + func isRootfsAbs(root string) bool { return filepath.IsAbs(root) } diff --git a/vendor/github.com/containerd/containerd/sys/socket_unix.go b/vendor/github.com/containerd/containerd/sys/socket_unix.go index 4d71c709a..cde68e2fc 100644 --- a/vendor/github.com/containerd/containerd/sys/socket_unix.go +++ b/vendor/github.com/containerd/containerd/sys/socket_unix.go @@ -23,11 +23,16 @@ import ( "os" "path/filepath" + "github.com/pkg/errors" "golang.org/x/sys/unix" ) // CreateUnixSocket creates a unix socket and returns the listener func CreateUnixSocket(path string) (net.Listener, error) { + // BSDs have a 104 limit + if len(path) > 104 { + return nil, errors.Errorf("%q: unix socket path too long (> 106)", path) + } if err := os.MkdirAll(filepath.Dir(path), 0660); err != nil { return nil, err } diff --git a/vendor/github.com/containerd/containerd/vendor.conf b/vendor/github.com/containerd/containerd/vendor.conf index cf11546ae..16ce52a21 100644 --- a/vendor/github.com/containerd/containerd/vendor.conf +++ b/vendor/github.com/containerd/containerd/vendor.conf @@ -76,9 +76,9 @@ k8s.io/kubernetes v1.10.0-rc.1 k8s.io/utils 258e2a2fa64568210fbd6267cf1d8fd87c3cb86e # zfs dependencies -github.com/containerd/zfs 2e6f60521b5690bf2f265c416a42b251c2a3ec8e +github.com/containerd/zfs 9a0b8b8b5982014b729cd34eb7cd7a11062aa6ec github.com/mistifyio/go-zfs 166add352731e515512690329794ee593f1aaff2 github.com/pborman/uuid c65b2f87fee37d1c7854c9164a450713c28d50cd # aufs dependencies -github.com/containerd/aufs 049ef88d84c1f49e52479d9f5f10d6756dd03a8b +github.com/containerd/aufs a7fbd554da7a9eafbe5a460a421313a9fd18d988