From 88fb149cf5647db8649ab9a60e81bb719d41d496 Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Mon, 26 Sep 2016 00:46:29 -0700 Subject: [PATCH 1/2] Add seccomp and apparmor support. --- pkg/kubelet/dockershim/docker_container.go | 6 ++++- pkg/kubelet/dockershim/docker_sandbox.go | 16 +++++++---- pkg/kubelet/dockershim/docker_service.go | 11 +++++--- pkg/kubelet/dockershim/helpers.go | 31 +++++++++++++++++----- pkg/kubelet/dockertools/docker_manager.go | 25 ++++++++++++++--- pkg/kubelet/kubelet.go | 2 +- pkg/security/apparmor/helpers.go | 8 +++++- 7 files changed, 78 insertions(+), 21 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index c479b880f11..00020406a6b 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/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 diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index c3574a537ca..bb5911618bb 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/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. @@ -252,9 +255,12 @@ func makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, image string) *dock setSandboxResources(hc) // Set security options. - hc.SecurityOpt = []string{getSeccompOpts()} - - return createConfig + var err error + hc.SecurityOpt, err = getSandboxSecurityOpts(c, ds.seccompProfileRoot) + 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) { diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index d35ffcac90a..54aaefba4b5 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -53,9 +53,11 @@ 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{ - client: dockertools.NewInstrumentedDockerInterface(client), + seccompProfileRoot: seccompProfileRoot, + client: dockertools.NewInstrumentedDockerInterface(client), } } @@ -76,7 +78,10 @@ type DockerLegacyService interface { } type dockerService struct { - client dockertools.DockerInterface + // TODO: Current seccomp implementation is very docker specific. Move this somewhere else + // after we define more general seccomp api. + seccompProfileRoot string + client dockertools.DockerInterface } // Version returns the runtime name, runtime version and runtime API version diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index 61a5cd3f2aa..1a444b3aa4a 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/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 -// through the runtime API (annotations?).See dockerManager.getSecurityOpts() -// for the details. Always set the default seccomp profile for now. -// Also need to support syntax for different docker versions. -func getSeccompOpts() string { - return fmt.Sprintf("%s=%s", "seccomp", defaultSeccompProfile) +// getContainerSecurityOpt gets container security options from container and sandbox config, currently from sandbox +// annotations. +// It is an experimental feature and may be promoted to official runtime api in the future. +func getContainerSecurityOpts(containerName string, sandboxConfig *runtimeApi.PodSandboxConfig, seccompProfileRoot string) ([]string, error) { + appArmorOpts, err := dockertools.GetAppArmorOpts(sandboxConfig.GetAnnotations(), containerName) + 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 { diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index ce098d54829..53f38b65453 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/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 diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index adc372c32f7..d107eb526b4 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -535,7 +535,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, diff --git a/pkg/security/apparmor/helpers.go b/pkg/security/apparmor/helpers.go index fa440d0104a..eb576bf3824 100644 --- a/pkg/security/apparmor/helpers.go +++ b/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. From 0771e64ab8e21ed006d31def3a75e2de9c4e4418 Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Tue, 27 Sep 2016 15:13:27 -0700 Subject: [PATCH 2/2] Add unit test for get security option functions. --- .../dockershim/docker_container_test.go | 4 +- pkg/kubelet/dockershim/docker_sandbox_test.go | 4 +- pkg/kubelet/dockershim/docker_service.go | 2 - pkg/kubelet/dockershim/docker_service_test.go | 2 +- pkg/kubelet/dockershim/helpers_test.go | 115 ++++++++++++++++++ 5 files changed, 120 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/dockershim/docker_container_test.go b/pkg/kubelet/dockershim/docker_container_test.go index 3e5f6e82dd4..21d296b1c7d 100644 --- a/pkg/kubelet/dockershim/docker_container_test.go +++ b/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" @@ -91,7 +91,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"} diff --git a/pkg/kubelet/dockershim/docker_sandbox_test.go b/pkg/kubelet/dockershim/docker_sandbox_test.go index 0e91c1b69cc..8fcf1f0c6d0 100644 --- a/pkg/kubelet/dockershim/docker_sandbox_test.go +++ b/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) diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index 54aaefba4b5..3db96741ba9 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -78,8 +78,6 @@ type DockerLegacyService interface { } type dockerService struct { - // TODO: Current seccomp implementation is very docker specific. Move this somewhere else - // after we define more general seccomp api. seccompProfileRoot string client dockertools.DockerInterface } diff --git a/pkg/kubelet/dockershim/docker_service_test.go b/pkg/kubelet/dockershim/docker_service_test.go index 3a59592d36d..deba2de337d 100644 --- a/pkg/kubelet/dockershim/docker_service_test.go +++ b/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 diff --git a/pkg/kubelet/dockershim/helpers_test.go b/pkg/kubelet/dockershim/helpers_test.go index 742ab80a572..f2793627c23 100644 --- a/pkg/kubelet/dockershim/helpers_test.go +++ b/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) + } + } +}