From f73de44a5b70c85458af955d74f45492ff07926a Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sat, 24 Dec 2022 20:09:04 +0900 Subject: [PATCH] oci: fix additional GIDs Test suite: ```yaml --- apiVersion: v1 kind: Pod metadata: name: test-no-option annotations: description: "Equivalent of `docker run` (no option)" spec: restartPolicy: Never containers: - name: main image: ghcr.io/containerd/busybox:1.28 args: ['sh', '-euxc', '[ "$(id)" = "uid=0(root) gid=0(root) groups=0(root),10(wheel)" ]'] --- apiVersion: v1 kind: Pod metadata: name: test-group-add-1-group-add-1234 annotations: description: "Equivalent of `docker run --group-add 1 --group-add 1234`" spec: restartPolicy: Never containers: - name: main image: ghcr.io/containerd/busybox:1.28 args: ['sh', '-euxc', '[ "$(id)" = "uid=0(root) gid=0(root) groups=0(root),1(daemon),10(wheel),1234" ]'] securityContext: supplementalGroups: [1, 1234] --- apiVersion: v1 kind: Pod metadata: name: test-user-1234 annotations: description: "Equivalent of `docker run --user 1234`" spec: restartPolicy: Never containers: - name: main image: ghcr.io/containerd/busybox:1.28 args: ['sh', '-euxc', '[ "$(id)" = "uid=1234 gid=0(root) groups=0(root)" ]'] securityContext: runAsUser: 1234 --- apiVersion: v1 kind: Pod metadata: name: test-user-1234-1234 annotations: description: "Equivalent of `docker run --user 1234:1234`" spec: restartPolicy: Never containers: - name: main image: ghcr.io/containerd/busybox:1.28 args: ['sh', '-euxc', '[ "$(id)" = "uid=1234 gid=1234 groups=1234" ]'] securityContext: runAsUser: 1234 runAsGroup: 1234 --- apiVersion: v1 kind: Pod metadata: name: test-user-1234-group-add-1234 annotations: description: "Equivalent of `docker run --user 1234 --group-add 1234`" spec: restartPolicy: Never containers: - name: main image: ghcr.io/containerd/busybox:1.28 args: ['sh', '-euxc', '[ "$(id)" = "uid=1234 gid=0(root) groups=0(root),1234" ]'] securityContext: runAsUser: 1234 supplementalGroups: [1234] ``` Signed-off-by: Akihiro Suda Signed-off-by: zhongjiawei --- oci/spec_opts.go | 33 +++++++++++++++++++ .../cri/pkg/server/container_create.go | 3 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/oci/spec_opts.go b/oci/spec_opts.go index 8b599f805..718c48246 100644 --- a/oci/spec_opts.go +++ b/oci/spec_opts.go @@ -84,6 +84,17 @@ func setCapabilities(s *Spec) { } } +// ensureAdditionalGids ensures that the primary GID is also included in the additional GID list. +func ensureAdditionalGids(s *Spec) { + setProcess(s) + for _, f := range s.Process.User.AdditionalGids { + if f == s.Process.User.GID { + return + } + } + s.Process.User.AdditionalGids = append([]uint32{s.Process.User.GID}, s.Process.User.AdditionalGids...) +} + // WithDefaultSpec returns a SpecOpts that will populate the spec with default // values. // @@ -459,7 +470,21 @@ func WithNamespacedCgroup() SpecOpts { // user, uid, user:group, uid:gid, uid:group, user:gid func WithUser(userstr string) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *Spec) error { + defer ensureAdditionalGids(s) setProcess(s) + s.Process.User.AdditionalGids = nil + + // For LCOW it's a bit harder to confirm that the user actually exists on the host as a rootfs isn't + // mounted on the host and shared into the guest, but rather the rootfs is constructed entirely in the + // guest itself. To accommodate this, a spot to place the user string provided by a client as-is is needed. + // The `Username` field on the runtime spec is marked by Platform as only for Windows, and in this case it + // *is* being set on a Windows host at least, but will be used as a temporary holding spot until the guest + // can use the string to perform these same operations to grab the uid:gid inside. + if s.Windows != nil && s.Linux != nil { + s.Process.User.Username = userstr + return nil + } + parts := strings.Split(userstr, ":") switch len(parts) { case 1: @@ -538,7 +563,9 @@ func WithUser(userstr string) SpecOpts { // 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 *Spec) error { + defer ensureAdditionalGids(s) setProcess(s) + s.Process.User.AdditionalGids = nil s.Process.User.UID = uid s.Process.User.GID = gid return nil @@ -551,7 +578,9 @@ func WithUIDGID(uid, gid uint32) SpecOpts { // additionally sets the gid to 0, and does not return an error. func WithUserID(uid uint32) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *Spec) (err error) { + defer ensureAdditionalGids(s) setProcess(s) + s.Process.User.AdditionalGids = nil if c.Snapshotter == "" && c.SnapshotKey == "" { if !isRootfsAbs(s.Root.Path) { return errors.Errorf("rootfs absolute path is required") @@ -604,7 +633,9 @@ func WithUserID(uid uint32) SpecOpts { // it returns error. func WithUsername(username string) SpecOpts { return func(ctx context.Context, client Client, c *containers.Container, s *Spec) (err error) { + defer ensureAdditionalGids(s) setProcess(s) + s.Process.User.AdditionalGids = nil if s.Linux != nil { if c.Snapshotter == "" && c.SnapshotKey == "" { if !isRootfsAbs(s.Root.Path) { @@ -659,7 +690,9 @@ func WithAdditionalGIDs(userstr string) SpecOpts { return nil } setProcess(s) + s.Process.User.AdditionalGids = nil setAdditionalGids := func(root string) error { + defer ensureAdditionalGids(s) var username string uid, err := strconv.Atoi(userstr) if err == nil { diff --git a/vendor/github.com/containerd/cri/pkg/server/container_create.go b/vendor/github.com/containerd/cri/pkg/server/container_create.go index e29cb40f8..ffa6cd614 100644 --- a/vendor/github.com/containerd/cri/pkg/server/container_create.go +++ b/vendor/github.com/containerd/cri/pkg/server/container_create.go @@ -230,7 +230,8 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta // Because it is still useful to get additional gids for uid 0. userstr = strconv.FormatInt(securityContext.GetRunAsUser().GetValue(), 10) } - specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr)) + specOpts = append(specOpts, customopts.WithDevices(c.os, config, c.config.DeviceOwnershipFromSecurityContext), + customopts.WithCapabilities(securityContext, c.allCaps)) apparmorSpecOpts, err := generateApparmorSpecOpts( securityContext.GetApparmorProfile(), -- 2.33.0