Merge pull request #33500 from Random-Liu/apparmor-seccomp-cri

Automatic merge from submit-queue

Add seccomp and apparmor support.

This PR adds seccomp and apparmor support in new CRI.

This a WIP because I'm still adding unit test for some of the functions. Sent this PR here for design discussion.

This PR is similar with https://github.com/kubernetes/kubernetes/pull/33450.
The differences are:
* This PR passes seccomp and apparmor configuration via annotations;
* This PR keeps the seccomp handling logic in docker shim because current seccomp implementation is very docker specific, and @timstclair told me that even the json seccomp profile file is defined by docker.

Notice that this PR almost passes related annotations in `api.Pod` to the runtime directly instead of introducing new CRI annotation.

@yujuhong @feiskyer @timstclair

pkg/kubelet/dockershim/docker_container.go

@@ -166,7 +166,11 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeApi
 		// Note: ShmSize is handled in kube_docker_client.go
 	}
 
-	hc.SecurityOpt = []string{getSeccompOpts()}
+	var err error
+	hc.SecurityOpt, err = getContainerSecurityOpts(config.Metadata.GetName(), sandboxConfig, ds.seccompProfileRoot)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate container security options for container %q: %v", config.Metadata.GetName(), err)
+	}
 	// TODO: Add or drop capabilities.
 
 	createConfig.HostConfig = hc

pkg/kubelet/dockershim/docker_container_test.go

@@ -42,7 +42,7 @@ func makeContainerConfig(sConfig *runtimeApi.PodSandboxConfig, name, image strin
 // TestListContainers creates several containers and then list them to check
 // whether the correct metadatas, states, and labels are returned.
 func TestListContainers(t *testing.T) {
-	ds, _, _ := newTestDockerSevice()
+	ds, _, _ := newTestDockerService()
 	podName, namespace := "foo", "bar"
 	containerName, image := "sidecar", "logger"
 
@@ -92,7 +92,7 @@ func TestListContainers(t *testing.T) {
 // TestContainerStatus tests the basic lifecycle operations and verify that
 // the status returned reflects the operations performed.
 func TestContainerStatus(t *testing.T) {
-	ds, _, fClock := newTestDockerSevice()
+	ds, _, fClock := newTestDockerService()
 	sConfig := makeSandboxConfig("foo", "bar", "1", 0)
 	labels := map[string]string{"abc.xyz": "foo"}
 	annotations := map[string]string{"foo.bar.baz": "abc"}

pkg/kubelet/dockershim/docker_sandbox.go

@@ -53,7 +53,10 @@ func (ds *dockerService) RunPodSandbox(config *runtimeApi.PodSandboxConfig) (str
 	}
 
 	// Step 2: Create the sandbox container.
-	createConfig := makeSandboxDockerConfig(config, image)
+	createConfig, err := ds.makeSandboxDockerConfig(config, image)
+	if err != nil {
+		return "", fmt.Errorf("failed to make sandbox docker config for pod %q: %v", config.Metadata.GetName(), err)
+	}
 	createResp, err := ds.client.CreateContainer(*createConfig)
 	if err != nil || createResp == nil {
 		return "", fmt.Errorf("failed to create a sandbox for pod %q: %v", config.Metadata.GetName(), err)
@@ -194,7 +197,7 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([]
 	return result, nil
 }
 
-func makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, image string) *dockertypes.ContainerCreateConfig {
+func (ds *dockerService) makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, image string) (*dockertypes.ContainerCreateConfig, error) {
 	// Merge annotations and labels because docker supports only labels.
 	labels := makeLabels(c.GetLabels(), c.GetAnnotations())
 	// Apply a label to distinguish sandboxes from regular containers.
@@ -253,9 +256,12 @@ func makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, image string) *dock
 	setSandboxResources(hc)
 
 	// Set security options.
-	hc.SecurityOpt = []string{getSeccompOpts()}
+	var err error
-
+	hc.SecurityOpt, err = getSandboxSecurityOpts(c, ds.seccompProfileRoot)
-	return createConfig
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate sandbox security options for sandbox %q: %v", c.Metadata.GetName(), err)
+	}
+	return createConfig, nil
 }
 
 func setSandboxResources(hc *dockercontainer.HostConfig) {

pkg/kubelet/dockershim/docker_sandbox_test.go

@@ -48,7 +48,7 @@ func makeSandboxConfigWithLabelsAndAnnotations(name, namespace, uid string, atte
 // TestListSandboxes creates several sandboxes and then list them to check
 // whether the correct metadatas, states, and labels are returned.
 func TestListSandboxes(t *testing.T) {
-	ds, _, _ := newTestDockerSevice()
+	ds, _, _ := newTestDockerService()
 	name, namespace := "foo", "bar"
 	configs := []*runtimeApi.PodSandboxConfig{}
 	for i := 0; i < 3; i++ {
@@ -86,7 +86,7 @@ func TestListSandboxes(t *testing.T) {
 // TestSandboxStatus tests the basic lifecycle operations and verify that
 // the status returned reflects the operations performed.
 func TestSandboxStatus(t *testing.T) {
-	ds, _, fClock := newTestDockerSevice()
+	ds, _, fClock := newTestDockerService()
 	labels := map[string]string{"label": "foobar1"}
 	annotations := map[string]string{"annotation": "abc"}
 	config := makeSandboxConfigWithLabelsAndAnnotations("foo", "bar", "1", 0, labels, annotations)

pkg/kubelet/dockershim/docker_service.go

@@ -53,8 +53,10 @@ const (
 
 var internalLabelKeys []string = []string{containerTypeLabelKey, sandboxIDLabelKey}
 
-func NewDockerService(client dockertools.DockerInterface) DockerLegacyService {
+// NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process.
+func NewDockerService(client dockertools.DockerInterface, seccompProfileRoot string) DockerLegacyService {
 	return &dockerService{
+		seccompProfileRoot: seccompProfileRoot,
 		client:             dockertools.NewInstrumentedDockerInterface(client),
 	}
 }
@@ -76,6 +78,7 @@ type DockerLegacyService interface {
 }
 
 type dockerService struct {
+	seccompProfileRoot string
 	client             dockertools.DockerInterface
 }
 

pkg/kubelet/dockershim/docker_service_test.go

@@ -23,7 +23,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/clock"
 )
 
-func newTestDockerSevice() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) {
+func newTestDockerService() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) {
 	fakeClock := clock.NewFakeClock(time.Time{})
 	c := dockertools.NewFakeDockerClientWithClock(fakeClock)
 	return &dockerService{client: c}, c, fakeClock

pkg/kubelet/dockershim/helpers.go

@@ -28,6 +28,7 @@ import (
 	"github.com/golang/glog"
 
 	runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
+	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 )
 
 const (
@@ -179,12 +180,30 @@ func makePortsAndBindings(pm []*runtimeApi.PortMapping) (map[dockernat.Port]stru
 	return exposedPorts, portBindings
 }
 
-// TODO: Seccomp support. Need to figure out how to pass seccomp options
+// getContainerSecurityOpt gets container security options from container and sandbox config, currently from sandbox
-// through the runtime API (annotations?).See dockerManager.getSecurityOpts()
+// annotations.
-// for the details. Always set the default seccomp profile for now.
+// It is an experimental feature and may be promoted to official runtime api in the future.
-// Also need to support syntax for different docker versions.
+func getContainerSecurityOpts(containerName string, sandboxConfig *runtimeApi.PodSandboxConfig, seccompProfileRoot string) ([]string, error) {
-func getSeccompOpts() string {
+	appArmorOpts, err := dockertools.GetAppArmorOpts(sandboxConfig.GetAnnotations(), containerName)
-	return fmt.Sprintf("%s=%s", "seccomp", defaultSeccompProfile)
+	if err != nil {
+		return nil, err
+	}
+	seccompOpts, err := dockertools.GetSeccompOpts(sandboxConfig.GetAnnotations(), containerName, seccompProfileRoot)
+	if err != nil {
+		return nil, err
+	}
+	securityOpts := append(appArmorOpts, seccompOpts...)
+	var opts []string
+	for _, securityOpt := range securityOpts {
+		k, v := securityOpt.GetKV()
+		opts = append(opts, fmt.Sprintf("%s=%s", k, v))
+	}
+	return opts, nil
+}
+
+func getSandboxSecurityOpts(sandboxConfig *runtimeApi.PodSandboxConfig, seccompProfileRoot string) ([]string, error) {
+	// sandboxContainerName doesn't exist in the pod, so pod security options will be returned by default.
+	return getContainerSecurityOpts(sandboxContainerName, sandboxConfig, seccompProfileRoot)
 }
 
 func getNetworkNamespace(c *dockertypes.ContainerJSON) string {

pkg/kubelet/dockershim/helpers_test.go

@@ -20,6 +20,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"k8s.io/kubernetes/pkg/api"
+	runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
+	"k8s.io/kubernetes/pkg/security/apparmor"
 )
 
 func TestLabelsAndAnnotationsRoundTrip(t *testing.T) {
@@ -32,3 +36,114 @@ func TestLabelsAndAnnotationsRoundTrip(t *testing.T) {
 	assert.Equal(t, expectedLabels, actualLabels)
 	assert.Equal(t, expectedAnnotations, actualAnnotations)
 }
+
+// TestGetContainerSecurityOpts tests the logic of generating container security options from sandbox annotations.
+// The actual profile loading logic is tested in dockertools.
+// TODO: Migrate the corresponding test to dockershim.
+func TestGetContainerSecurityOpts(t *testing.T) {
+	containerName := "bar"
+	makeConfig := func(annotations map[string]string) *runtimeApi.PodSandboxConfig {
+		return makeSandboxConfigWithLabelsAndAnnotations("pod", "ns", "1234", 1, nil, annotations)
+	}
+
+	tests := []struct {
+		msg          string
+		config       *runtimeApi.PodSandboxConfig
+		expectedOpts []string
+	}{{
+		msg:          "No security annotations",
+		config:       makeConfig(nil),
+		expectedOpts: []string{"seccomp=unconfined"},
+	}, {
+		msg: "Seccomp unconfined",
+		config: makeConfig(map[string]string{
+			api.SeccompContainerAnnotationKeyPrefix + containerName: "unconfined",
+		}),
+		expectedOpts: []string{"seccomp=unconfined"},
+	}, {
+		msg: "Seccomp default",
+		config: makeConfig(map[string]string{
+			api.SeccompContainerAnnotationKeyPrefix + containerName: "docker/default",
+		}),
+		expectedOpts: nil,
+	}, {
+		msg: "Seccomp pod default",
+		config: makeConfig(map[string]string{
+			api.SeccompPodAnnotationKey: "docker/default",
+		}),
+		expectedOpts: nil,
+	}, {
+		msg: "AppArmor runtime/default",
+		config: makeConfig(map[string]string{
+			apparmor.ContainerAnnotationKeyPrefix + containerName: apparmor.ProfileRuntimeDefault,
+		}),
+		expectedOpts: []string{"seccomp=unconfined"},
+	}, {
+		msg: "AppArmor local profile",
+		config: makeConfig(map[string]string{
+			apparmor.ContainerAnnotationKeyPrefix + containerName: apparmor.ProfileNamePrefix + "foo",
+		}),
+		expectedOpts: []string{"seccomp=unconfined", "apparmor=foo"},
+	}, {
+		msg: "AppArmor and seccomp profile",
+		config: makeConfig(map[string]string{
+			api.SeccompContainerAnnotationKeyPrefix + containerName: "docker/default",
+			apparmor.ContainerAnnotationKeyPrefix + containerName:   apparmor.ProfileNamePrefix + "foo",
+		}),
+		expectedOpts: []string{"apparmor=foo"},
+	}}
+
+	for i, test := range tests {
+		opts, err := getContainerSecurityOpts(containerName, test.config, "test/seccomp/profile/root")
+		assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg)
+		assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg)
+		for _, opt := range test.expectedOpts {
+			assert.Contains(t, opts, opt, "TestCase[%d]: %s", i, test.msg)
+		}
+	}
+}
+
+// TestGetSandboxSecurityOpts tests the logic of generating sandbox security options from sandbox annotations.
+func TestGetSandboxSecurityOpts(t *testing.T) {
+	makeConfig := func(annotations map[string]string) *runtimeApi.PodSandboxConfig {
+		return makeSandboxConfigWithLabelsAndAnnotations("pod", "ns", "1234", 1, nil, annotations)
+	}
+
+	tests := []struct {
+		msg          string
+		config       *runtimeApi.PodSandboxConfig
+		expectedOpts []string
+	}{{
+		msg:          "No security annotations",
+		config:       makeConfig(nil),
+		expectedOpts: []string{"seccomp=unconfined"},
+	}, {
+		msg: "Seccomp default",
+		config: makeConfig(map[string]string{
+			api.SeccompPodAnnotationKey: "docker/default",
+		}),
+		expectedOpts: nil,
+	}, {
+		msg: "Seccomp unconfined",
+		config: makeConfig(map[string]string{
+			api.SeccompPodAnnotationKey: "unconfined",
+		}),
+		expectedOpts: []string{"seccomp=unconfined"},
+	}, {
+		msg: "Seccomp pod and container profile",
+		config: makeConfig(map[string]string{
+			api.SeccompContainerAnnotationKeyPrefix + "test-container": "unconfined",
+			api.SeccompPodAnnotationKey:                                "docker/default",
+		}),
+		expectedOpts: nil,
+	}}
+
+	for i, test := range tests {
+		opts, err := getSandboxSecurityOpts(test.config, "test/seccomp/profile/root")
+		assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg)
+		assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg)
+		for _, opt := range test.expectedOpts {
+			assert.Contains(t, opts, opt, "TestCase[%d]: %s", i, test.msg)
+		}
+	}
+}

pkg/kubelet/dockertools/docker_manager.go

@@ -1126,6 +1126,11 @@ type dockerOpt struct {
 	msg string
 }
 
+// Expose key/value from dockertools
+func (d dockerOpt) GetKV() (string, string) {
+	return d.key, d.value
+}
+
 // Get the docker security options for seccomp.
 func (dm *DockerManager) getSeccompOpts(pod *api.Pod, ctrName string) ([]dockerOpt, error) {
 	version, err := dm.APIVersion()
@@ -1140,10 +1145,16 @@ func (dm *DockerManager) getSeccompOpts(pod *api.Pod, ctrName string) ([]dockerO
 		return nil, nil // return early for Docker < 1.10
 	}
 
-	profile, profileOK := pod.ObjectMeta.Annotations[api.SeccompContainerAnnotationKeyPrefix+ctrName]
+	return GetSeccompOpts(pod.ObjectMeta.Annotations, ctrName, dm.seccompProfileRoot)
+}
+
+// Temporarily export this function to share with dockershim.
+// TODO: clean this up.
+func GetSeccompOpts(annotations map[string]string, ctrName, profileRoot string) ([]dockerOpt, error) {
+	profile, profileOK := annotations[api.SeccompContainerAnnotationKeyPrefix+ctrName]
 	if !profileOK {
 		// try the pod profile
-		profile, profileOK = pod.ObjectMeta.Annotations[api.SeccompPodAnnotationKey]
+		profile, profileOK = annotations[api.SeccompPodAnnotationKey]
 		if !profileOK {
 			// return early the default
 			return defaultSeccompOpt, nil
@@ -1165,7 +1176,7 @@ func (dm *DockerManager) getSeccompOpts(pod *api.Pod, ctrName string) ([]dockerO
 	}
 
 	name := strings.TrimPrefix(profile, "localhost/") // by pod annotation validation, name is a valid subpath
-	fname := filepath.Join(dm.seccompProfileRoot, filepath.FromSlash(name))
+	fname := filepath.Join(profileRoot, filepath.FromSlash(name))
 	file, err := ioutil.ReadFile(fname)
 	if err != nil {
 		return nil, fmt.Errorf("cannot load seccomp profile %q: %v", name, err)
@@ -1183,7 +1194,13 @@ func (dm *DockerManager) getSeccompOpts(pod *api.Pod, ctrName string) ([]dockerO
 
 // Get the docker security options for AppArmor.
 func (dm *DockerManager) getAppArmorOpts(pod *api.Pod, ctrName string) ([]dockerOpt, error) {
-	profile := apparmor.GetProfileName(pod, ctrName)
+	return GetAppArmorOpts(pod.Annotations, ctrName)
+}
+
+// Temporarily export this function to share with dockershim.
+// TODO: clean this up.
+func GetAppArmorOpts(annotations map[string]string, ctrName string) ([]dockerOpt, error) {
+	profile := apparmor.GetProfileNameFromPodAnnotations(annotations, ctrName)
 	if profile == "" || profile == apparmor.ProfileRuntimeDefault {
 		// The docker applies the default profile by default.
 		return nil, nil

pkg/kubelet/kubelet.go

@@ -507,7 +507,7 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration, kubeDeps *Kub
 		case "cri":
 			// Use the new CRI shim for docker. This is need for testing the
 			// docker integration through CRI, and may be removed in the future.
-			dockerService := dockershim.NewDockerService(klet.dockerClient)
+			dockerService := dockershim.NewDockerService(klet.dockerClient, kubeCfg.SeccompProfileRoot)
 			klet.containerRuntime, err = kuberuntime.NewKubeGenericRuntimeManager(
 				kubecontainer.FilterEventRecorder(kubeDeps.Recorder),
 				klet.livenessManager,

pkg/security/apparmor/helpers.go

@@ -49,7 +49,13 @@ func isRequired(pod *api.Pod) bool {
 
 // Returns the name of the profile to use with the container.
 func GetProfileName(pod *api.Pod, containerName string) string {
-	return pod.Annotations[ContainerAnnotationKeyPrefix+containerName]
+	return GetProfileNameFromPodAnnotations(pod.Annotations, containerName)
+}
+
+// GetProfileNameFromPodAnnotations gets the name of the profile to use with container from
+// pod annotations
+func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string {
+	return annotations[ContainerAnnotationKeyPrefix+containerName]
 }
 
 // Sets the name of the profile to use with the container.