Merge pull request #52395 from dixudx/fix_apparmor_annotation_unconfined

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

enable to specific unconfined AppArmor profile

**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #52370

**Special notes for your reviewer**:
/assign @tallclair @liggitt 

**Release note**:

```release-note
enable to specific unconfined AppArmor profile
```

pkg/kubelet/apis/cri/v1alpha1/runtime/api.pb.go

@@ -1397,6 +1397,7 @@ type LinuxContainerSecurityContext struct {
 	SupplementalGroups []int64 `protobuf:"varint,8,rep,packed,name=supplemental_groups,json=supplementalGroups" json:"supplemental_groups,omitempty"`
 	// AppArmor profile for the container, candidate values are:
 	// * runtime/default: equivalent to not specifying a profile.
+	// * unconfined: no profiles are loaded
 	// * localhost/<profile_name>: profile loaded on the node
 	//    (localhost) by name. The possible profile names are detailed at
 	//    http://wiki.apparmor.net/index.php/AppArmor_Core_Policy_Reference

pkg/kubelet/apis/cri/v1alpha1/runtime/api.proto

@@ -523,6 +523,7 @@ message LinuxContainerSecurityContext {
     repeated int64 supplemental_groups = 8;
     // AppArmor profile for the container, candidate values are:
     // * runtime/default: equivalent to not specifying a profile.
+    // * unconfined: no profiles are loaded
     // * localhost/<profile_name>: profile loaded on the node
     //    (localhost) by name. The possible profile names are detailed at
     //    http://wiki.apparmor.net/index.php/AppArmor_Core_Policy_Reference

pkg/kubelet/dockershim/helpers.go

@@ -393,6 +393,11 @@ func getAppArmorOpts(profile string) ([]dockerOpt, error) {
 		return nil, nil
 	}
 
+	// Return unconfined profile explicitly
+	if profile == apparmor.ProfileNameUnconfined {
+		return []dockerOpt{{"apparmor", apparmor.ProfileNameUnconfined, ""}}, nil
+	}
+
 	// Assume validation has already happened.
 	profileName := strings.TrimPrefix(profile, apparmor.ProfileNamePrefix)
 	return []dockerOpt{{"apparmor", profileName, ""}}, nil

pkg/security/apparmor/helpers.go

@@ -35,13 +35,16 @@ const (
 	ProfileRuntimeDefault = "runtime/default"
 	// The prefix for specifying profiles loaded on the node.
 	ProfileNamePrefix = "localhost/"
+
+	// Unconfined profile
+	ProfileNameUnconfined = "unconfined"
 )
 
 // Checks whether app armor is required for pod to be run.
 func isRequired(pod *v1.Pod) bool {
-	for key := range pod.Annotations {
+	for key, value := range pod.Annotations {
 		if strings.HasPrefix(key, ContainerAnnotationKeyPrefix) {
-			return true
+			return value != ProfileNameUnconfined
 		}
 	}
 	return false

pkg/security/apparmor/validate.go

@@ -136,7 +136,7 @@ func validateProfile(profile string, loadedProfiles map[string]bool) error {
 }
 
 func ValidateProfileFormat(profile string) error {
-	if profile == "" || profile == ProfileRuntimeDefault {
+	if profile == "" || profile == ProfileRuntimeDefault || profile == ProfileNameUnconfined {
 		return nil
 	}
 	if !strings.HasPrefix(profile, ProfileNamePrefix) {

pkg/security/apparmor/validate_test.go

@@ -63,6 +63,7 @@ func TestValidateProfile(t *testing.T) {
 	}{
 		{"", true},
 		{ProfileRuntimeDefault, true},
+		{ProfileNameUnconfined, true},
 		{"baz", false}, // Missing local prefix.
 		{ProfileNamePrefix + "/usr/sbin/ntpd", true},
 		{ProfileNamePrefix + "foo-bar", true},

test/e2e/common/apparmor.go

@@ -52,7 +52,7 @@ func LoadAppArmorProfiles(f *framework.Framework) {
 // CreateAppArmorTestPod creates a pod that tests apparmor profile enforcement. The pod exits with
 // an error code if the profile is incorrectly enforced. If runOnce is true the pod will exit after
 // a single test, otherwise it will repeat the test every 1 second until failure.
-func CreateAppArmorTestPod(f *framework.Framework, runOnce bool) *api.Pod {
+func CreateAppArmorTestPod(f *framework.Framework, unconfined bool, runOnce bool) *api.Pod {
 	profile := "localhost/" + appArmorProfilePrefix + f.Namespace.Name
 	testCmd := fmt.Sprintf(`
 if touch %[1]s; then
@@ -61,12 +61,24 @@ if touch %[1]s; then
 elif ! touch %[2]s; then
   echo "FAILURE: write to %[2]s should be allowed"
   exit 2
-elif ! grep "%[3]s" /proc/self/attr/current; then
+elif [[ $(< /proc/self/attr/current) != "%[3]s" ]]; then
   echo "FAILURE: not running with expected profile %[3]s"
   echo "found: $(cat /proc/self/attr/current)"
   exit 3
 fi`, appArmorDeniedPath, appArmorAllowedPath, appArmorProfilePrefix+f.Namespace.Name)
 
+	if unconfined {
+		profile = apparmor.ProfileNameUnconfined
+		testCmd = `
+if cat /proc/sysrq-trigger 2>&1 | grep 'Permission denied'; then
+  echo 'FAILURE: reading /proc/sysrq-trigger should be allowed'
+  exit 1
+elif [[ $(< /proc/self/attr/current) != "unconfined" ]]; then
+  echo 'FAILURE: not running with expected profile unconfined'
+  exit 2
+fi`
+	}
+
 	if !runOnce {
 		testCmd = fmt.Sprintf(`while true; do
 %s

test/e2e/node/apparmor.go

@@ -39,7 +39,11 @@ var _ = SIGDescribe("AppArmor", func() {
 		})
 
 		It("should enforce an AppArmor profile", func() {
-			common.CreateAppArmorTestPod(f, true)
+			common.CreateAppArmorTestPod(f, false, true)
+		})
+
+		It("can disable an AppArmor profile, using unconfined", func() {
+			common.CreateAppArmorTestPod(f, true, true)
 		})
 	})
 })

test/e2e/upgrades/apparmor.go

@@ -55,7 +55,7 @@ func (t *AppArmorUpgradeTest) Setup(f *framework.Framework) {
 
 	// Create the initial test pod.
 	By("Creating a long-running AppArmor enabled pod.")
-	t.pod = common.CreateAppArmorTestPod(f, false)
+	t.pod = common.CreateAppArmorTestPod(f, false, false)
 
 	// Verify initial state.
 	t.verifyNodesAppArmorEnabled(f)
@@ -91,7 +91,10 @@ func (t *AppArmorUpgradeTest) verifyPodStillUp(f *framework.Framework) {
 
 func (t *AppArmorUpgradeTest) verifyNewPodSucceeds(f *framework.Framework) {
 	By("Verifying an AppArmor profile is enforced for a new pod")
-	common.CreateAppArmorTestPod(f, true)
+	common.CreateAppArmorTestPod(f, false, true)
+
+	By("Verifying an unconfined AppArmor profile is enforced for a new pod")
+	common.CreateAppArmorTestPod(f, true, true)
 }
 
 func (t *AppArmorUpgradeTest) verifyNodesAppArmorEnabled(f *framework.Framework) {