diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go index 67a652e91..c5ec3dfdd 100644 --- a/pkg/cri/opts/spec_linux.go +++ b/pkg/cri/opts/spec_linux.go @@ -593,16 +593,16 @@ func WithSupplementalGroups(groups []int64) oci.SpecOpts { } // WithPodNamespaces sets the pod namespaces for the container -func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, pid uint32) oci.SpecOpts { +func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, sandboxPid uint32, targetPid uint32) oci.SpecOpts { namespaces := config.GetNamespaceOptions() opts := []oci.SpecOpts{ - oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(pid)}), - oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(pid)}), - oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(pid)}), + oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(sandboxPid)}), + oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(sandboxPid)}), + oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(sandboxPid)}), } if namespaces.GetPid() != runtime.NamespaceMode_CONTAINER { - opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(pid)})) + opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(targetPid)})) } return oci.Compose(opts...) } diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index 7d9c4b536..26386e991 100644 --- a/pkg/cri/server/container_create_linux.go +++ b/pkg/cri/server/container_create_linux.go @@ -270,9 +270,24 @@ func (c *criService) containerSpec( specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue)) } + // Default target PID namespace is the sandbox PID. + targetPid := sandboxPid + // If the container targets another container's PID namespace, + // set targetPid to the PID of that container. + nsOpts := securityContext.GetNamespaceOptions() + if nsOpts.GetPid() == runtime.NamespaceMode_TARGET { + targetContainer, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId) + if err != nil { + return nil, errors.Wrapf(err, "invalid target container") + } + + status := targetContainer.Status.Get() + targetPid = status.Pid + } + specOpts = append(specOpts, customopts.WithOOMScoreAdj(config, c.config.RestrictOOMScoreAdj), - customopts.WithPodNamespaces(securityContext, sandboxPid), + customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid), customopts.WithSupplementalGroups(supplementalGroups), customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer), customopts.WithAnnotation(annotations.SandboxID, sandboxID), diff --git a/pkg/cri/server/container_start.go b/pkg/cri/server/container_start.go index 90c807d7f..11390f1f6 100644 --- a/pkg/cri/server/container_start.go +++ b/pkg/cri/server/container_start.go @@ -84,6 +84,17 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain return nil, errors.Errorf("sandbox container %q is not running", sandboxID) } + // Recheck target container validity in Linux namespace options. + if linux := config.GetLinux(); linux != nil { + nsOpts := linux.GetSecurityContext().GetNamespaceOptions() + if nsOpts.GetPid() == runtime.NamespaceMode_TARGET { + _, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId) + if err != nil { + return nil, errors.Wrap(err, "invalid target container") + } + } + } + ioCreation := func(id string) (_ containerdio.IO, err error) { stdoutWC, stderrWC, err := c.createContainerLoggers(meta.LogPath, config.GetTty()) if err != nil { diff --git a/pkg/cri/server/helpers.go b/pkg/cri/server/helpers.go index 1d5f49f11..8c9a4ac41 100644 --- a/pkg/cri/server/helpers.go +++ b/pkg/cri/server/helpers.go @@ -242,6 +242,30 @@ func (c *criService) ensureImageExists(ctx context.Context, ref string, config * return &newImage, nil } +// validateTargetContainer checks that a container is a valid +// target for a container using PID NamespaceMode_TARGET. +// The target container must be in the same sandbox and must be running. +// Returns the target container for convenience. +func (c *criService) validateTargetContainer(sandboxID, targetContainerID string) (containerstore.Container, error) { + targetContainer, err := c.containerStore.Get(targetContainerID) + if err != nil { + return containerstore.Container{}, errors.Wrapf(err, "container %q does not exist", targetContainerID) + } + + targetSandboxID := targetContainer.Metadata.SandboxID + if targetSandboxID != sandboxID { + return containerstore.Container{}, + errors.Errorf("container %q (sandbox %s) does not belong to sandbox %s", targetContainerID, targetSandboxID, sandboxID) + } + + status := targetContainer.Status.Get() + if state := status.State(); state != runtime.ContainerState_CONTAINER_RUNNING { + return containerstore.Container{}, errors.Errorf("container %q is not running - in state %s", targetContainerID, state) + } + + return targetContainer, nil +} + // isInCRIMounts checks whether a destination is in CRI mount list. func isInCRIMounts(dst string, mounts []*runtime.Mount) bool { for _, m := range mounts { diff --git a/pkg/cri/server/helpers_test.go b/pkg/cri/server/helpers_test.go index 4605ac9b2..7cd9314e1 100644 --- a/pkg/cri/server/helpers_test.go +++ b/pkg/cri/server/helpers_test.go @@ -20,6 +20,7 @@ import ( "context" "io/ioutil" "testing" + "time" "github.com/containerd/containerd/oci" "github.com/containerd/containerd/plugin" @@ -34,6 +35,7 @@ import ( criconfig "github.com/containerd/containerd/pkg/cri/config" "github.com/containerd/containerd/pkg/cri/store" + containerstore "github.com/containerd/containerd/pkg/cri/store/container" imagestore "github.com/containerd/containerd/pkg/cri/store/image" ) @@ -501,3 +503,94 @@ func TestEnsureRemoveAllWithFile(t *testing.T) { t.Fatal(err) } } + +// Helper function for setting up an environment to test PID namespace targeting. +func addContainer(c *criService, containerID, sandboxID string, PID uint32, createdAt, startedAt, finishedAt int64) error { + meta := containerstore.Metadata{ + ID: containerID, + SandboxID: sandboxID, + } + status := containerstore.Status{ + Pid: PID, + CreatedAt: createdAt, + StartedAt: startedAt, + FinishedAt: finishedAt, + } + container, err := containerstore.NewContainer(meta, + containerstore.WithFakeStatus(status), + ) + if err != nil { + return err + } + return c.containerStore.Add(container) +} + +func TestValidateTargetContainer(t *testing.T) { + testSandboxID := "test-sandbox-uid" + + // The existing container that will be targeted. + testTargetContainerID := "test-target-container" + testTargetContainerPID := uint32(4567) + + // A container that has finished running and cannot be targeted. + testStoppedContainerID := "stopped-target-container" + testStoppedContainerPID := uint32(6789) + + // A container from another pod. + testOtherContainerSandboxID := "other-sandbox-uid" + testOtherContainerID := "other-target-container" + testOtherContainerPID := uint32(7890) + + // Container create/start/stop times. + createdAt := time.Now().Add(-15 * time.Second).UnixNano() + startedAt := time.Now().Add(-10 * time.Second).UnixNano() + finishedAt := time.Now().Add(-5 * time.Second).UnixNano() + + c := newTestCRIService() + + // Create a target container. + err := addContainer(c, testTargetContainerID, testSandboxID, testTargetContainerPID, createdAt, startedAt, 0) + require.NoError(t, err, "error creating test target container") + + // Create a stopped container. + err = addContainer(c, testStoppedContainerID, testSandboxID, testStoppedContainerPID, createdAt, startedAt, finishedAt) + require.NoError(t, err, "error creating test stopped container") + + // Create a container in another pod. + err = addContainer(c, testOtherContainerID, testOtherContainerSandboxID, testOtherContainerPID, createdAt, startedAt, 0) + require.NoError(t, err, "error creating test container in other pod") + + for desc, test := range map[string]struct { + targetContainerID string + expectError bool + }{ + "target container in pod": { + targetContainerID: testTargetContainerID, + expectError: false, + }, + "target stopped container in pod": { + targetContainerID: testStoppedContainerID, + expectError: true, + }, + "target container does not exist": { + targetContainerID: "no-container-with-this-id", + expectError: true, + }, + "target container in other pod": { + targetContainerID: testOtherContainerID, + expectError: true, + }, + } { + t.Run(desc, func(t *testing.T) { + targetContainer, err := c.validateTargetContainer(testSandboxID, test.targetContainerID) + if test.expectError { + require.Error(t, err, "target should have been invalid but no error") + return + } + require.NoErrorf(t, err, "target should have been valid but got error") + + assert.Equal(t, test.targetContainerID, targetContainer.ID, "returned target container does not have expected ID") + }) + } + +}