diff --git a/internal/cri/server/helpers.go b/internal/cri/server/helpers.go index f1b21187ec..9fe38931c6 100644 --- a/internal/cri/server/helpers.go +++ b/internal/cri/server/helpers.go @@ -29,6 +29,7 @@ import ( "github.com/containerd/typeurl/v2" runtimespec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/selinux/go-selinux/label" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" containerd "github.com/containerd/containerd/v2/client" @@ -479,6 +480,18 @@ func toLabel(selinuxOptions *runtime.SELinuxOption) ([]string, error) { return labels, nil } +// initLabelsFromOpt allocates the SELinux process and mount labels described by +// the given CRI SELinux options. On success the MCS level of the returned process +// label is reserved and the caller owns it until it is either released with +// selinux.ReleaseLabel or handed over to the label store. +func initLabelsFromOpt(selinuxOpts *runtime.SELinuxOption) (string, string, error) { + labels, err := toLabel(selinuxOpts) + if err != nil { + return "", "", err + } + return label.InitLabels(labels) +} + func checkSelinuxLevel(level string) error { if len(level) == 0 { return nil diff --git a/internal/cri/server/podsandbox/helpers_selinux_linux_test.go b/internal/cri/server/helpers_selinux_linux_test.go similarity index 99% rename from internal/cri/server/podsandbox/helpers_selinux_linux_test.go rename to internal/cri/server/helpers_selinux_linux_test.go index db311320db..242c17ceed 100644 --- a/internal/cri/server/podsandbox/helpers_selinux_linux_test.go +++ b/internal/cri/server/helpers_selinux_linux_test.go @@ -14,7 +14,7 @@ limitations under the License. */ -package podsandbox +package server import ( "testing" diff --git a/internal/cri/server/podsandbox/helpers_linux.go b/internal/cri/server/podsandbox/helpers_linux.go index 55d9df2472..09dca9a50d 100644 --- a/internal/cri/server/podsandbox/helpers_linux.go +++ b/internal/cri/server/podsandbox/helpers_linux.go @@ -22,7 +22,6 @@ import ( "os" "path" "path/filepath" - "regexp" "sort" "strings" "syscall" @@ -32,7 +31,6 @@ import ( "github.com/moby/sys/mountinfo" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" - "github.com/opencontainers/selinux/go-selinux/label" "golang.org/x/sys/unix" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -133,54 +131,6 @@ func (c *Controller) pinUserNamespace(sandboxID string, netnsPath string) error return nil } -func toLabel(selinuxOptions *runtime.SELinuxOption) ([]string, error) { - var labels []string - - if selinuxOptions == nil { - return nil, nil - } - if err := checkSelinuxLevel(selinuxOptions.Level); err != nil { - return nil, err - } - if selinuxOptions.User != "" { - labels = append(labels, "user:"+selinuxOptions.User) - } - if selinuxOptions.Role != "" { - labels = append(labels, "role:"+selinuxOptions.Role) - } - if selinuxOptions.Type != "" { - labels = append(labels, "type:"+selinuxOptions.Type) - } - if selinuxOptions.Level != "" { - labels = append(labels, "level:"+selinuxOptions.Level) - } - - return labels, nil -} - -func initLabelsFromOpt(selinuxOpts *runtime.SELinuxOption) (string, string, error) { - labels, err := toLabel(selinuxOpts) - if err != nil { - return "", "", err - } - return label.InitLabels(labels) -} - -func checkSelinuxLevel(level string) error { - if len(level) == 0 { - return nil - } - - matched, err := regexp.MatchString(`^s\d(-s\d)??(:c\d{1,4}(\.c\d{1,4})?(,c\d{1,4}(\.c\d{1,4})?)*)?$`, level) - if err != nil { - return fmt.Errorf("the format of 'level' %q is not correct: %w", level, err) - } - if !matched { - return fmt.Errorf("the format of 'level' %q is not correct", level) - } - return nil -} - func (c *Controller) seccompEnabled() bool { return seccomp.IsEnabled() } diff --git a/internal/cri/server/podsandbox/sandbox_run.go b/internal/cri/server/podsandbox/sandbox_run.go index 5aae14021c..275cea01da 100644 --- a/internal/cri/server/podsandbox/sandbox_run.go +++ b/internal/cri/server/podsandbox/sandbox_run.go @@ -28,7 +28,6 @@ import ( "github.com/containerd/nri" v1 "github.com/containerd/nri/types/v1" "github.com/containerd/typeurl/v2" - "github.com/opencontainers/selinux/go-selinux" containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/sandbox" @@ -138,7 +137,7 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll // NOTE: sandboxContainerSpec SHOULD NOT have side // effect, e.g. accessing/creating files, so that we can test // it safely. - spec, err := c.sandboxContainerSpec(id, config, &imageSpec.Config, metadata.NetNSPath, ociRuntime.PodAnnotations) + spec, err := c.sandboxContainerSpec(id, config, &imageSpec.Config, metadata.NetNSPath, metadata.ProcessLabel, ociRuntime.PodAnnotations) if err != nil { return cin, fmt.Errorf("failed to generate sandbox container spec: %w", err) } @@ -151,14 +150,6 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll }).Debug("sandbox container spec") } - metadata.ProcessLabel = spec.Process.SelinuxLabel - defer func() { - if retErr != nil { - selinux.ReleaseLabel(metadata.ProcessLabel) - } - }() - labels["selinux_label"] = metadata.ProcessLabel - // handle any KVM based runtime if err := modifyProcessLabel(ociRuntime.Type, spec); err != nil { return cin, err diff --git a/internal/cri/server/podsandbox/sandbox_run_linux.go b/internal/cri/server/podsandbox/sandbox_run_linux.go index 997c6f87f1..c163fb0848 100644 --- a/internal/cri/server/podsandbox/sandbox_run_linux.go +++ b/internal/cri/server/podsandbox/sandbox_run_linux.go @@ -27,6 +27,7 @@ import ( imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" + "github.com/opencontainers/selinux/go-selinux/label" "golang.org/x/sys/unix" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -38,7 +39,7 @@ import ( ) func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, - imageConfig *imagespec.ImageConfig, nsPath string, runtimePodAnnotations []string) (_ *runtimespec.Spec, retErr error) { + imageConfig *imagespec.ImageConfig, nsPath string, podProcessLabel string, runtimePodAnnotations []string) (*runtimespec.Spec, error) { // Creates a spec Generator with the default spec. // TODO(random-liu): [P1] Compare the default settings with docker and containerd default. specOpts := []oci.SpecOpts{ @@ -157,15 +158,17 @@ func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxC specOpts = append(specOpts, oci.WithMounts(mounts)) - processLabel, mountLabel, err := initLabelsFromOpt(securityContext.GetSelinuxOptions()) + // The pod SELinux label is allocated and owned by the CRI server (it lives in + // Metadata.ProcessLabel). Re-derive the process and mount labels from it rather than + // minting a new one, so that the reservation has a single owner. + labelOptions, err := selinux.DupSecOpt(podProcessLabel) if err != nil { - return nil, fmt.Errorf("failed to init selinux options %+v: %w", securityContext.GetSelinuxOptions(), err) + return nil, fmt.Errorf("failed to parse pod selinux label %q: %w", podProcessLabel, err) + } + processLabel, mountLabel, err := label.InitLabels(labelOptions) + if err != nil { + return nil, fmt.Errorf("failed to init selinux labels from pod selinux label %q: %w", podProcessLabel, err) } - defer func() { - if retErr != nil { - selinux.ReleaseLabel(processLabel) - } - }() supplementalGroups := securityContext.GetSupplementalGroups() specOpts = append(specOpts, diff --git a/internal/cri/server/podsandbox/sandbox_run_linux_test.go b/internal/cri/server/podsandbox/sandbox_run_linux_test.go index 72f54bf4ee..77b7358408 100644 --- a/internal/cri/server/podsandbox/sandbox_run_linux_test.go +++ b/internal/cri/server/podsandbox/sandbox_run_linux_test.go @@ -28,6 +28,7 @@ import ( imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" + "github.com/opencontainers/selinux/go-selinux/label" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" runtime "k8s.io/cri-api/pkg/apis/runtime/v1" @@ -516,7 +517,7 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { if test.configChange != nil { test.configChange(config) } - spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, nil) + spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, "", nil) if test.expectErr { assert.Error(t, err) assert.Nil(t, spec) @@ -532,6 +533,36 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { } } +// TestSandboxContainerSpecPodProcessLabel verifies that the pause spec reuses the pod +// SELinux label allocated by the CRI server instead of minting a new one. +func TestSandboxContainerSpecPodProcessLabel(t *testing.T) { + if !selinux.GetEnabled() { + t.Skip("selinux is not enabled") + } + + // Let InitLabels allocate a unique MCS level rather than hardcoding one that a + // container running on this host may already hold. + podProcessLabel, podMountLabel, err := label.InitLabels(nil) + require.NoError(t, err) + require.NotEmpty(t, podProcessLabel) + defer selinux.ReleaseLabel(podProcessLabel) + + c := newControllerService() + c.config.RootDir = t.TempDir() + c.config.StateDir = t.TempDir() + defer func() { + assert.NoError(t, unmountRecursive(context.Background(), c.config.StateDir)) + }() + + config, imageConfig, _ := getRunPodSandboxTestData(c.config) + spec, err := c.sandboxContainerSpec("test-id", config, imageConfig, "test-cni", podProcessLabel, nil) + require.NoError(t, err) + require.NotNil(t, spec.Linux) + + assert.Equal(t, podProcessLabel, spec.Process.SelinuxLabel) + assert.Equal(t, podMountLabel, spec.Linux.MountLabel) +} + func TestSetupSandboxFiles(t *testing.T) { const ( testID = "test-id" diff --git a/internal/cri/server/podsandbox/sandbox_run_other.go b/internal/cri/server/podsandbox/sandbox_run_other.go index b9069a59fa..30445dffdc 100644 --- a/internal/cri/server/podsandbox/sandbox_run_other.go +++ b/internal/cri/server/podsandbox/sandbox_run_other.go @@ -27,7 +27,7 @@ import ( runtime "k8s.io/cri-api/pkg/apis/runtime/v1" ) -func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, _ *imagespec.ImageConfig, _ string, _ []string) (_ *runtimespec.Spec, _ error) { +func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, _ *imagespec.ImageConfig, _ string, _ string, _ []string) (_ *runtimespec.Spec, _ error) { return c.runtimeSpec(id, annotations.DefaultCRIAnnotations(id, "", c.getSandboxImageName(), config, true)...) } diff --git a/internal/cri/server/podsandbox/sandbox_run_test.go b/internal/cri/server/podsandbox/sandbox_run_test.go index 99b19036a5..198a69fce0 100644 --- a/internal/cri/server/podsandbox/sandbox_run_test.go +++ b/internal/cri/server/podsandbox/sandbox_run_test.go @@ -110,7 +110,7 @@ func TestSandboxContainerSpec(t *testing.T) { if test.imageConfigChange != nil { test.imageConfigChange(imageConfig) } - spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, + spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, "", test.podAnnotations) if test.expectErr { assert.Error(t, err) diff --git a/internal/cri/server/podsandbox/sandbox_run_windows.go b/internal/cri/server/podsandbox/sandbox_run_windows.go index a3bd579833..238400256c 100644 --- a/internal/cri/server/podsandbox/sandbox_run_windows.go +++ b/internal/cri/server/podsandbox/sandbox_run_windows.go @@ -32,7 +32,7 @@ import ( ) func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxConfig, - imageConfig *imagespec.ImageConfig, nsPath string, runtimePodAnnotations []string) (*runtimespec.Spec, error) { + imageConfig *imagespec.ImageConfig, nsPath string, _ string, runtimePodAnnotations []string) (*runtimespec.Spec, error) { // Creates a spec Generator with the default spec. specOpts := []oci.SpecOpts{ oci.WithEnv(imageConfig.Env), diff --git a/internal/cri/server/podsandbox/sandbox_run_windows_test.go b/internal/cri/server/podsandbox/sandbox_run_windows_test.go index 18e86d3955..246fd45689 100644 --- a/internal/cri/server/podsandbox/sandbox_run_windows_test.go +++ b/internal/cri/server/podsandbox/sandbox_run_windows_test.go @@ -102,7 +102,7 @@ func TestSandboxWindowsNetworkNamespace(t *testing.T) { c := newControllerService() config, imageConfig, specCheck := getRunPodSandboxTestData(c.config) - spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, nil) + spec, err := c.sandboxContainerSpec(testID, config, imageConfig, nsPath, "", nil) assert.NoError(t, err) assert.NotNil(t, spec) specCheck(t, testID, spec) diff --git a/internal/cri/server/sandbox_run.go b/internal/cri/server/sandbox_run.go index 831c2e82a9..1972888a7f 100644 --- a/internal/cri/server/sandbox_run.go +++ b/internal/cri/server/sandbox_run.go @@ -139,6 +139,31 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox // Save sandbox name sandboxInfo.AddLabel("name", name) + // Allocate the pod SELinux labels before the sandbox record is created, so that the + // process label is part of the very first metadata write and is restored on recovery + // without an extra store update. The sandbox implementation derives the labels it + // needs from Metadata.ProcessLabel instead of minting its own. + selinuxOpts := config.GetLinux().GetSecurityContext().GetSelinuxOptions() + processLabel, _, err := initLabelsFromOpt(selinuxOpts) + if err != nil { + return nil, fmt.Errorf("failed to init selinux options %+v: %w", selinuxOpts, err) + } + // Hold a reservation on the MCS level of processLabel for the duration of this + // call, so that a concurrent RunPodSandbox cannot be handed the same level while + // this sandbox is still being set up. + // + // The reservation is refcounted per level by the shared label store, so it is + // dropped unconditionally on return: by then sandboxStore.Add has either taken the + // store's own reservation (which lives until the sandbox is deleted) or the sandbox + // never made it into the store and the level is free again. Releasing through the + // store rather than calling selinux.ReleaseLabel directly matters when the pod + // pinned an explicit SELinux level that another live pod already uses: the level is + // only really freed once the last holder lets go of it. + if err := c.labels.Reserve(processLabel); err != nil { + return nil, fmt.Errorf("failed to reserve selinux label %q: %w", processLabel, err) + } + defer c.labels.Release(processLabel) + // Create initial internal sandbox object. sandbox := sandboxstore.NewSandbox( sandboxstore.Metadata{ @@ -146,6 +171,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox Name: name, Config: config, RuntimeHandler: r.GetRuntimeHandler(), + ProcessLabel: processLabel, }, sandboxstore.Status{ State: sandboxstore.StateUnknown, @@ -336,6 +362,15 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox } } + // CRI owns the pod SELinux label; a label from the controller is ignored. + if l := ctrl.Labels["selinux_label"]; l != "" && l != sandbox.ProcessLabel { + log.G(ctx).WithFields(log.Fields{ + "sandbox": id, + "controller_label": l, + "selinux_label": sandbox.ProcessLabel, + }).Warn("ignoring the SELinux label returned by the sandbox controller, using the label allocated by CRI") + } + sandboxInfo.Labels = ctrl.Labels sandboxInfo.Spec = ctrl.Spec @@ -343,13 +378,6 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox return nil, fmt.Errorf("unable to save sandbox %q to sandbox store: %w", id, err) } - labels := ctrl.Labels - if labels == nil { - labels = map[string]string{} - } - - sandbox.ProcessLabel = labels["selinux_label"] - if err := sandbox.Status.Update(func(status sandboxstore.Status) (sandboxstore.Status, error) { status.Pid = ctrl.Pid // NRI reads the pid from status during RunPodSandbox hook status.CreatedAt = ctrl.CreatedAt @@ -408,7 +436,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox // Send CONTAINER_STARTED event with ContainerId equal to SandboxId. c.generateAndSendContainerEvent(ctx, id, id, runtime.ContainerEventType_CONTAINER_STARTED_EVENT) - sandboxRuntimeCreateTimer.WithValues(labels["oci_runtime_type"]).UpdateSince(runtimeStart) + sandboxRuntimeCreateTimer.WithValues(ociRuntime.Type).UpdateSince(runtimeStart) return &runtime.RunPodSandboxResponse{PodSandboxId: id}, nil } diff --git a/internal/cri/server/service.go b/internal/cri/server/service.go index adb711ca54..4fc3bb089d 100644 --- a/internal/cri/server/service.go +++ b/internal/cri/server/service.go @@ -132,6 +132,11 @@ type criService struct { imageFSPaths map[string]string // os is an interface for all required os operations. os osinterface.OS + // labels reserves the MCS levels of the SELinux process labels held by the + // sandbox and container stores. It is shared with both, so reservations are + // refcounted per level across pods and containers that use the same one. + labels *label.Store + // sandboxStore stores all resources associated with sandboxes. sandboxStore *sandboxstore.Store // sandboxNameIndex stores all sandbox names and make sure each name @@ -222,6 +227,7 @@ func NewCRIService(options *CRIServiceOptions) (CRIService, runtime.RuntimeServi client: options.Client, imageFSPaths: options.ImageService.ImageFSPaths(), os: osinterface.RealOS{}, + labels: labels, sandboxStore: sandboxstore.NewStore(labels, statsCollector), containerStore: containerstore.NewStore(labels, statsCollector), sandboxNameIndex: registrar.NewRegistrar(), diff --git a/internal/cri/server/service_test.go b/internal/cri/server/service_test.go index 21efe3c7ab..fd6ef0363c 100644 --- a/internal/cri/server/service_test.go +++ b/internal/cri/server/service_test.go @@ -147,6 +147,7 @@ func newTestCRIService(opts ...testOpt) *criService { service := &criService{ config: testConfig, os: ostesting.NewFakeOS(), + labels: labels, sandboxStore: sandboxstore.NewStore(labels, nil), sandboxNameIndex: registrar.NewRegistrar(), containerStore: containerstore.NewStore(labels, nil), diff --git a/internal/cri/store/sandbox/sandbox_linux_test.go b/internal/cri/store/sandbox/sandbox_linux_test.go new file mode 100644 index 0000000000..61fc4f158f --- /dev/null +++ b/internal/cri/store/sandbox/sandbox_linux_test.go @@ -0,0 +1,61 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package sandbox + +import ( + "strings" + "testing" + + "github.com/containerd/containerd/v2/internal/cri/store/label" + + assertlib "github.com/stretchr/testify/assert" +) + +// TestSandboxStoreProcessLabel verifies that adding a sandbox reserves the MCS level of +// its process label and that deleting it releases the level again. Recovery restores +// Metadata.ProcessLabel and goes through Add, so this is what re-reserves the label of a +// sandbox that survived a containerd restart. +func TestSandboxStoreProcessLabel(t *testing.T) { + assert := assertlib.New(t) + + const processLabel = "system_u:system_r:container_t:s0:c4,c5" + + s := NewStore(label.NewStore(), nil) + reserved := map[string]bool{} + s.labels.Reserver = func(l string) error { + reserved[strings.SplitN(l, ":", 4)[3]] = true + return nil + } + s.labels.Releaser = func(l string) { + reserved[strings.SplitN(l, ":", 4)[3]] = false + } + + sb := NewSandbox( + Metadata{ + ID: "1", + Name: "Sandbox-1", + ProcessLabel: processLabel, + }, + Status{State: StateReady}, + ) + + assert.NoError(s.Add(sb)) + assert.True(reserved["s0:c4,c5"], "process label should be reserved on add") + + s.Delete(sb.ID) + assert.False(reserved["s0:c4,c5"], "process label should be released on delete") +}