Merge pull request #4700 from mikebrow/cri-security-profile-update

CRI security profile update for CRI graduation
containerd · Jan 12, 2021 · 550b494 · 550b494
2 parents 3b6a386 + 6467c33
commit 550b494
Show file tree

Hide file tree

Showing 8 changed files with 1,300 additions and 532 deletions.
diff --git a/go.mod b/go.mod
@@ -62,7 +62,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 v0.19.4
+	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
@@ -722,8 +722,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
@@ -306,8 +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.GetApparmorProfile(),
+		asp,
 		securityContext.GetPrivileged(),
 		c.apparmorEnabled())
 	if err != nil {
@@ -317,8 +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.GetSeccompProfilePath(),
+		ssp,
 		securityContext.GetPrivileged(),
 		c.seccompEnabled())
 	if err != nil {
@@ -330,77 +346,128 @@ 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(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 == "" {
-		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")
+			}
 		}
 		return nil, nil
 	}
-	switch seccompProf {
-	case "", unconfinedProfile:
+
+	if sp == nil {
+		return nil, 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 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(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")
+			}
 		}
 		return nil, nil
 	}
-	switch apparmorProf {
-	// Based on kubernetes#51746, default apparmor profile should be applied
-	// for when apparmor is not specified.
-	case runtimeDefault, "":
+
+	if sp == nil {
+		// Based on kubernetes#51746, default apparmor profile should be applied
+		// for when apparmor is not specified.
+		sp, _ = generateSecurityProfile("")
+	}
+
+	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")
 			}
 			return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile)
 		}
 		return apparmor.WithProfile(appArmorProfile), nil
+	default:
+		return nil, errors.New("apparmor unknown ProfileType")
 	}
 }