From 66307d0b4eb9b0b2c890592dbd425274d97e1bd7 Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Wed, 10 May 2023 20:22:17 -0700 Subject: [PATCH] CRI: Support Linux usernames for !linux platforms The oci.WithUser option was being applied in container_create_linux.go instead of the cross plat buildLinuxSpec method. There's been recent work to try and make every spec option that can be applied on any platform able to do so, and this falls under that. However, WithUser on linux platforms relies on the containers SnapshotKey being filled out, which means the spec option needs to be applied during container creation. To make this a little more generic, I've created a new platformSpecOpts method that handles any spec opts that rely on runtime state (rootfs mounted for example) for some platforms, or just platform options that we still don't have workarounds for to be able to specify them for other platforms (apparmor, seccomp etc.) by internally calling the already existing containerSpecOpts method. Signed-off-by: Danny Canter --- pkg/cri/sbserver/container_create.go | 89 ++++++++++++++++++- pkg/cri/sbserver/container_create_linux.go | 64 ++----------- .../sbserver/container_create_linux_test.go | 3 +- 3 files changed, 95 insertions(+), 61 deletions(-) diff --git a/pkg/cri/sbserver/container_create.go b/pkg/cri/sbserver/container_create.go index 004886afe..f274e671f 100644 --- a/pkg/cri/sbserver/container_create.go +++ b/pkg/cri/sbserver/container_create.go @@ -251,7 +251,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta } }() - specOpts, err := c.containerSpecOpts(config, &image.ImageSpec.Config) + specOpts, err := c.platformSpecOpts(platform, config, &image.ImageSpec.Config) if err != nil { return nil, fmt.Errorf("failed to get container spec opts: %w", err) } @@ -404,6 +404,93 @@ const ( hostnameEnv = "HOSTNAME" ) +// generateUserString generates valid user string based on OCI Image Spec +// v1.0.0. +// +// CRI defines that the following combinations are valid: +// +// (none) -> "" +// username -> username +// username, uid -> username +// username, uid, gid -> username:gid +// username, gid -> username:gid +// uid -> uid +// uid, gid -> uid:gid +// gid -> error +// +// 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 "", fmt.Errorf("user group %q is specified without user", groupstr) + } + return "", nil + } + if groupstr != "" { + userstr = userstr + ":" + groupstr + } + return userstr, nil +} + +// platformSpecOpts adds additional runtime spec options that may rely on +// runtime information (rootfs mounted), or platform specific checks with +// no defined workaround (yet) to specify for other platforms. +func (c *criService) platformSpecOpts( + platform platforms.Platform, + config *runtime.ContainerConfig, + imageConfig *imagespec.ImageConfig, +) ([]oci.SpecOpts, error) { + var specOpts []oci.SpecOpts + + // First deal with the set of options we can use across platforms currently. + // Linux user strings have workarounds on other platforms to avoid needing to + // mount the rootfs, but on Linux hosts it must be mounted + // + // TODO(dcantah): I think the seccomp package can be made to compile on + // !linux and used here as well. + if platform.OS == "linux" { + // 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. + securityContext := config.GetLinux().GetSecurityContext() + userstr, err := generateUserString( + securityContext.GetRunAsUsername(), + securityContext.GetRunAsUser(), + securityContext.GetRunAsGroup()) + if err != nil { + return nil, fmt.Errorf("failed to generate user string: %w", err) + } + if userstr == "" { + // Lastly, since no user override was passed via CRI try to set via OCI + // Image + userstr = imageConfig.User + } + if userstr != "" { + specOpts = append(specOpts, oci.WithUser(userstr)) + } + } + + // Now grab the truly platform specific options (seccomp, apparmor etc. for linux + // for example). + ctrSpecOpts, err := c.containerSpecOpts(config, imageConfig) + if err != nil { + return nil, err + } + specOpts = append(specOpts, ctrSpecOpts...) + + return specOpts, nil +} + // buildContainerSpec build container's OCI spec depending on controller's target platform OS. func (c *criService) buildContainerSpec( platform platforms.Platform, diff --git a/pkg/cri/sbserver/container_create_linux.go b/pkg/cri/sbserver/container_create_linux.go index 49b5e79bf..f33438d65 100644 --- a/pkg/cri/sbserver/container_create_linux.go +++ b/pkg/cri/sbserver/container_create_linux.go @@ -52,28 +52,12 @@ const ( ) func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) ([]oci.SpecOpts, error) { - var specOpts []oci.SpecOpts + var ( + specOpts []oci.SpecOpts + err error + ) securityContext := config.GetLinux().GetSecurityContext() - // 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. - userstr, err := generateUserString( - securityContext.GetRunAsUsername(), - securityContext.GetRunAsUser(), - securityContext.GetRunAsGroup()) - if err != nil { - return nil, fmt.Errorf("failed to generate user string: %w", err) - } - if userstr == "" { - // Lastly, since no user override was passed via CRI try to set via OCI - // Image - userstr = imageConfig.User - } - if userstr != "" { - specOpts = append(specOpts, oci.WithUser(userstr)) - } - - userstr = "0" // runtime default + userstr := "0" // runtime default if securityContext.GetRunAsUsername() != "" { userstr = securityContext.GetRunAsUsername() } else if securityContext.GetRunAsUser() != nil { @@ -279,44 +263,6 @@ func appArmorProfileExists(profile string) (bool, error) { } } -// generateUserString generates valid user string based on OCI Image Spec -// v1.0.0. -// -// CRI defines that the following combinations are valid: -// -// (none) -> "" -// username -> username -// username, uid -> username -// username, uid, gid -> username:gid -// username, gid -> username:gid -// uid -> uid -// uid, gid -> uid:gid -// gid -> error -// -// 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 "", fmt.Errorf("user group %q is specified without user", groupstr) - } - return "", nil - } - if groupstr != "" { - userstr = userstr + ":" + groupstr - } - return userstr, nil -} - // snapshotterOpts returns any Linux specific snapshotter options for the rootfs snapshot func snapshotterOpts(snapshotterName string, config *runtime.ContainerConfig) []snapshots.Opt { return []snapshots.Opt{} diff --git a/pkg/cri/sbserver/container_create_linux_test.go b/pkg/cri/sbserver/container_create_linux_test.go index 745b2c0c0..6be91dc00 100644 --- a/pkg/cri/sbserver/container_create_linux_test.go +++ b/pkg/cri/sbserver/container_create_linux_test.go @@ -31,6 +31,7 @@ import ( "github.com/containerd/containerd/contrib/seccomp" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/oci" + "github.com/containerd/containerd/platforms" imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" @@ -1506,7 +1507,7 @@ additional-group-for-root:x:22222:root require.NoError(t, err) spec.Root.Path = tempRootDir // simulating /etc/{passwd, group} - opts, err := c.containerSpecOpts(containerConfig, imageConfig) + opts, err := c.platformSpecOpts(platforms.DefaultSpec(), containerConfig, imageConfig) require.NoError(t, err) oci.ApplyOpts(ctx, nil, testContainer, spec, opts...)