Merge pull request #14108 from mxpv/labels

cri: allocate the pod SELinux label in the CRI layer instead of podsandbox

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

internal/cri/server/helpers_selinux_linux_test.go

@@ -14,7 +14,7 @@
    limitations under the License.
 */
 
-package podsandbox
+package server
 
 import (
 	"testing"

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()
 }

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

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,

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"

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)...)
 }
 

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)

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),

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)

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
 }

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(),

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),

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")
+}