diff --git a/go.mod b/go.mod index 63ac9fe77..bce2db81f 100644 --- a/go.mod +++ b/go.mod @@ -61,7 +61,7 @@ require ( k8s.io/apiserver v0.19.4 k8s.io/client-go v0.19.4 k8s.io/component-base v0.19.4 - k8s.io/cri-api 3990421b69a024ba452db5f6e4ae7d45d93a019a + k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0 k8s.io/klog/v2 v2.2.0 k8s.io/utils v0.0.0-20200729134348-d5654de09c73 ) diff --git a/go.sum b/go.sum index 93a68cbe7..bc7d01eb7 100644 --- a/go.sum +++ b/go.sum @@ -714,8 +714,8 @@ k8s.io/client-go v0.19.4/go.mod h1:ZrEy7+wj9PjH5VMBCuu/BDlvtUAku0oVFk4MmnW9mWA= k8s.io/component-base v0.19.4 h1:HobPRToQ8KJ9ubRju6PUAk9I5V1GNMJZ4PyWbiWA0uI= k8s.io/component-base v0.19.4/go.mod h1:ZzuSLlsWhajIDEkKF73j64Gz/5o0AgON08FgRbEPI70= k8s.io/cri-api v0.17.3/go.mod h1:X1sbHmuXhwaHs9xxYffLqJogVsnI+f6cPRcgPel7ywM= -k8s.io/cri-api v0.19.4 h1:Vc00x5LSSbLBgvj7UAi4kjsv276n4SGX0XlI/pWhG2E= -k8s.io/cri-api v0.19.4/go.mod h1:UN/iU9Ua0iYdDREBXNE9vqCJ7MIh/FW3VIL0d8pw7Fw= +k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0 h1:/AO/xlTAHFP+ZLhfYMjSUzMsZe9of1fwh1TupXAKzKE= +k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0/go.mod h1:UN/iU9Ua0iYdDREBXNE9vqCJ7MIh/FW3VIL0d8pw7Fw= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A= diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index d5f6d038e..61208145a 100644 --- a/pkg/cri/server/container_create_linux.go +++ b/pkg/cri/server/container_create_linux.go @@ -306,9 +306,15 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon } specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr)) + asp := securityContext.GetApparmor() + if asp == nil { + asp, err = generateApparmorSecurityProfile(securityContext.GetApparmorProfile()) // nolint:staticcheck deprecated but we don't want to remove yet + if err != nil { + return nil, errors.Wrap(err, "failed to generate apparmor spec opts") + } + } apparmorSpecOpts, err := generateApparmorSpecOpts( - securityContext.GetApparmor(), - securityContext.GetApparmorProfile(), // nolint:staticcheck deprecated but we don't want to remove yet + asp, securityContext.GetPrivileged(), c.apparmorEnabled()) if err != nil { @@ -318,9 +324,17 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, apparmorSpecOpts) } + ssp := securityContext.GetSeccomp() + if ssp == nil { + ssp, err = generateSeccompSecurityProfile( + securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet + c.config.UnsetSeccompProfile) + if err != nil { + return nil, errors.Wrap(err, "failed to generate seccomp spec opts") + } + } seccompSpecOpts, err := c.generateSeccompSpecOpts( - securityContext.GetSeccomp(), - securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet + ssp, securityContext.GetPrivileged(), c.seccompEnabled()) if err != nil { @@ -332,24 +346,51 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon return specOpts, nil } +func generateSeccompSecurityProfile(profilePath string, unsetProfilePath string) (*runtime.SecurityProfile, error) { + if profilePath != "" { + return generateSecurityProfile(profilePath) + } + if unsetProfilePath != "" { + return generateSecurityProfile(unsetProfilePath) + } + return nil, nil +} +func generateApparmorSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) { + if profilePath != "" { + return generateSecurityProfile(profilePath) + } + return nil, nil +} + +func generateSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) { + switch profilePath { + case runtimeDefault, dockerDefault, "": + return &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_RuntimeDefault, + }, nil + case unconfinedProfile: + return &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Unconfined, + }, nil + default: + // Require and Trim default profile name prefix + if !strings.HasPrefix(profilePath, profileNamePrefix) { + return nil, errors.Errorf("invalid profile %q", profilePath) + } + return &runtime.SecurityProfile{ + ProfileType: runtime.SecurityProfile_Localhost, + LocalhostRef: strings.TrimPrefix(profilePath, profileNamePrefix), + }, nil + } +} + // generateSeccompSpecOpts generates containerd SpecOpts for seccomp. -func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) { +func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, privileged, seccompEnabled bool) (oci.SpecOpts, error) { if privileged { // Do not set seccomp profile when container is privileged return nil, nil } - if seccompProf == "" && sp == nil { - seccompProf = c.config.UnsetSeccompProfile - } - // Set seccomp profile - if seccompProf == runtimeDefault || seccompProf == dockerDefault { - // use correct default profile (Eg. if not configured otherwise, the default is docker/default) - seccompProf = seccompDefaultProfile - } if !seccompEnabled { - if seccompProf != "" && seccompProf != unconfinedProfile { - return nil, errors.New("seccomp is not supported") - } if sp != nil { if sp.ProfileType != runtime.SecurityProfile_Unconfined { return nil, errors.New("seccomp is not supported") @@ -358,49 +399,33 @@ func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, seccom return nil, nil } - if sp != nil { - if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { - return nil, errors.New("seccomp config invalid LocalhostRef must only be set if ProfileType is Localhost") - } - switch sp.ProfileType { - case runtime.SecurityProfile_Unconfined: - // Do not set seccomp profile. - return nil, nil - case runtime.SecurityProfile_RuntimeDefault: - return seccomp.WithDefaultProfile(), nil - case runtime.SecurityProfile_Localhost: - // trimming the localhost/ prefix just in case even through it should not - // be necessary with the new SecurityProfile struct - return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil - default: - return nil, errors.New("seccomp unknown ProfileType") - } + if sp == nil { + return nil, nil } - switch seccompProf { - case "", unconfinedProfile: + if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { + return nil, errors.New("seccomp config invalid LocalhostRef must only be set if ProfileType is Localhost") + } + switch sp.ProfileType { + case runtime.SecurityProfile_Unconfined: // Do not set seccomp profile. return nil, nil - case dockerDefault: - // Note: WithDefaultProfile specOpts must be added after capabilities + case runtime.SecurityProfile_RuntimeDefault: return seccomp.WithDefaultProfile(), nil + case runtime.SecurityProfile_Localhost: + // trimming the localhost/ prefix just in case even though it should not + // be necessary with the new SecurityProfile struct + return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil default: - // Require and Trim default profile name prefix - if !strings.HasPrefix(seccompProf, profileNamePrefix) { - return nil, errors.Errorf("invalid seccomp profile %q", seccompProf) - } - return seccomp.WithProfile(strings.TrimPrefix(seccompProf, profileNamePrefix)), nil + return nil, errors.New("seccomp unknown ProfileType") } } // generateApparmorSpecOpts generates containerd SpecOpts for apparmor. -func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string, privileged, apparmorEnabled bool) (oci.SpecOpts, error) { +func generateApparmorSpecOpts(sp *runtime.SecurityProfile, privileged, apparmorEnabled bool) (oci.SpecOpts, error) { if !apparmorEnabled { // Should fail loudly if user try to specify apparmor profile // but we don't support it. - if apparmorProf != "" && apparmorProf != unconfinedProfile { - return nil, errors.New("apparmor is not supported") - } if sp != nil { if sp.ProfileType != runtime.SecurityProfile_Unconfined { return nil, errors.New("apparmor is not supported") @@ -409,55 +434,31 @@ func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string, return nil, nil } - if sp != nil { - if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { - return nil, errors.New("apparmor config invalid LocalhostRef must only be set if ProfileType is Localhost") - } - - switch sp.ProfileType { - case runtime.SecurityProfile_Unconfined: - // Do not set apparmor profile. - return nil, nil - case runtime.SecurityProfile_RuntimeDefault: - if privileged { - // Do not set apparmor profile when container is privileged - return nil, nil - } - return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil - case runtime.SecurityProfile_Localhost: - // trimming the localhost/ prefix just in case even through it should not - // be necessary with the new SecurityProfile struct - appArmorProfile := strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix) - if profileExists, err := appArmorProfileExists(appArmorProfile); !profileExists { - if err != nil { - return nil, errors.Wrap(err, "failed to generate apparmor spec opts") - } - return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile) - } - return apparmor.WithProfile(appArmorProfile), nil - default: - return nil, errors.New("apparmor unknown ProfileType") - } + if sp == nil { + // Based on kubernetes#51746, default apparmor profile should be applied + // for when apparmor is not specified. + sp, _ = generateSecurityProfile("") } - switch apparmorProf { - // Based on kubernetes#51746, default apparmor profile should be applied - // for when apparmor is not specified. - case runtimeDefault, "": + if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" { + return nil, errors.New("apparmor config invalid LocalhostRef must only be set if ProfileType is Localhost") + } + + switch sp.ProfileType { + case runtime.SecurityProfile_Unconfined: + // Do not set apparmor profile. + return nil, nil + case runtime.SecurityProfile_RuntimeDefault: if privileged { // Do not set apparmor profile when container is privileged return nil, nil } // TODO (mikebrow): delete created apparmor default profile return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil - case unconfinedProfile: - return nil, nil - default: - // Require and Trim default profile name prefix - if !strings.HasPrefix(apparmorProf, profileNamePrefix) { - return nil, errors.Errorf("invalid apparmor profile %q", apparmorProf) - } - appArmorProfile := strings.TrimPrefix(apparmorProf, profileNamePrefix) + case runtime.SecurityProfile_Localhost: + // trimming the localhost/ prefix just in case even through it should not + // be necessary with the new SecurityProfile struct + appArmorProfile := strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix) if profileExists, err := appArmorProfileExists(appArmorProfile); !profileExists { if err != nil { return nil, errors.Wrap(err, "failed to generate apparmor spec opts") @@ -465,6 +466,8 @@ func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string, return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile) } return apparmor.WithProfile(appArmorProfile), nil + default: + return nil, errors.New("apparmor unknown ProfileType") } } diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index c5cc4fcfd..e1dfc8d9e 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -832,10 +832,6 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { profile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), }, - "should return error if specified profile is invalid": { - profile: "test-profile", - expectErr: true, - }, "should use default profile when seccomp is empty": { defaultProfile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), @@ -903,14 +899,29 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) { cri := &criService{} cri.config.UnsetSeccompProfile = test.defaultProfile - specOpts, err := cri.generateSeccompSpecOpts(test.sp, test.profile, test.privileged, !test.disable) - assert.Equal(t, - reflect.ValueOf(test.specOpts).Pointer(), - reflect.ValueOf(specOpts).Pointer()) - if test.expectErr { - assert.Error(t, err) + ssp := test.sp + csp, err := generateSeccompSecurityProfile( + test.profile, + test.defaultProfile) + if err != nil { + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } else { - assert.NoError(t, err) + if ssp == nil { + ssp = csp + } + specOpts, err := cri.generateSeccompSpecOpts(ssp, test.privileged, !test.disable) + assert.Equal(t, + reflect.ValueOf(test.specOpts).Pointer(), + reflect.ValueOf(specOpts).Pointer()) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } }) } @@ -1039,14 +1050,27 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { }, } { t.Logf("TestCase %q", desc) - specOpts, err := generateApparmorSpecOpts(test.sp, test.profile, test.privileged, !test.disable) - assert.Equal(t, - reflect.ValueOf(test.specOpts).Pointer(), - reflect.ValueOf(specOpts).Pointer()) - if test.expectErr { - assert.Error(t, err) + asp := test.sp + csp, err := generateApparmorSecurityProfile(test.profile) + if err != nil { + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } else { - assert.NoError(t, err) + if asp == nil { + asp = csp + } + specOpts, err := generateApparmorSpecOpts(asp, test.privileged, !test.disable) + assert.Equal(t, + reflect.ValueOf(test.specOpts).Pointer(), + reflect.ValueOf(specOpts).Pointer()) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } } } } diff --git a/pkg/cri/server/sandbox_run_linux.go b/pkg/cri/server/sandbox_run_linux.go index 575f85c45..04c70ccd2 100644 --- a/pkg/cri/server/sandbox_run_linux.go +++ b/pkg/cri/server/sandbox_run_linux.go @@ -163,10 +163,19 @@ func (c *criService) sandboxContainerSpecOpts(config *runtime.PodSandboxConfig, var ( securityContext = config.GetLinux().GetSecurityContext() specOpts []oci.SpecOpts + err error ) + ssp := securityContext.GetSeccomp() + if ssp == nil { + ssp, err = generateSeccompSecurityProfile( + securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet + c.config.UnsetSeccompProfile) + if err != nil { + return nil, errors.Wrap(err, "failed to generate seccomp spec opts") + } + } seccompSpecOpts, err := c.generateSeccompSpecOpts( - securityContext.GetSeccomp(), - securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet + ssp, securityContext.GetPrivileged(), c.seccompEnabled()) if err != nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index 82f2596d8..ebcc75829 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -501,7 +501,7 @@ k8s.io/client-go/util/workqueue # k8s.io/component-base v0.19.4 ## explicit k8s.io/component-base/logs/logreduction -# k8s.io/cri-api v0.19.4 +# k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0 ## explicit k8s.io/cri-api/pkg/apis k8s.io/cri-api/pkg/apis/runtime/v1alpha2