reload default apparmor profile if it is unloaded

Signed-off-by: Xianglin Gao <xlgao@zju.edu.cn>

server/apparmor/apparmor.go

@@ -17,8 +17,8 @@ import (
 )
 
 const (
-	// defaultApparmorProfile is the name of default apparmor profile name.
+	// DefaultApparmorProfile is the name of default apparmor profile name.
-	defaultApparmorProfile = "ocid-default"
+	DefaultApparmorProfile = "ocid-default"
 
 	// profileDirectory is the file store for apparmor profiles and macros.
 	profileDirectory = "/etc/apparmor.d"
@@ -47,13 +47,11 @@ type profileData struct {
 	Version int
 }
 
-// InstallDefaultAppArmorProfile installs default apparmor profile.
+// LoadDefaultAppArmorProfile loads default apparmor profile, if it is not loaded.
-func InstallDefaultAppArmorProfile() {
+func LoadDefaultAppArmorProfile() {
-	if err := InstallDefault(defaultApparmorProfile); err != nil {
+	if !IsLoaded(DefaultApparmorProfile) {
-		// Allow daemon to run if loading failed, but are active
+		if err := InstallDefault(DefaultApparmorProfile); err != nil {
-		// (possibly through another run, manually, or via system startup)
+			logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded:%v", DefaultApparmorProfile, err)
-		if !IsLoaded(defaultApparmorProfile) {
-			logrus.Errorf("AppArmor enabled on system but the %s profile could not be loaded.", defaultApparmorProfile)
 		}
 	}
 }

server/apparmor/apparmor_unsupported.go

@@ -3,6 +3,9 @@
 package apparmor
 
 const (
+	// DefaultApparmorProfile is the name of default apparmor profile name.
+	DefaultApparmorProfile = "ocid-default"
+
 	// ContainerAnnotationKeyPrefix is the prefix to an annotation key specifying a container profile.
 	ContainerAnnotationKeyPrefix = "container.apparmor.security.beta.kubernetes.io/"
 
@@ -17,8 +20,8 @@ func IsEnabled() bool {
 	return false
 }
 
-// InstallDefaultAppArmorProfile dose nothing, when build without apparmor build tag.
+// LoadDefaultAppArmorProfile dose nothing, when build without apparmor build tag.
-func InstallDefaultAppArmorProfile() {
+func LoadDefaultAppArmorProfile() {
 }
 
 // GetProfileNameFromPodAnnotations dose nothing, when build without apparmor build tag.

server/container_create.go

@@ -393,6 +393,11 @@ func (s *Server) getAppArmorProfileName(annotations map[string]string, ctrName s
 	}
 
 	if profile == apparmor.ProfileRuntimeDefault {
+		// reload default apparmor profile if it is unloaded.
+		if s.appArmorProfile == apparmor.DefaultApparmorProfile {
+			apparmor.LoadDefaultAppArmorProfile()
+		}
+
 		// If the value is runtime/default, then return default profile.
 		return s.appArmorProfile
 	}

server/server.go

@@ -299,7 +299,7 @@ func New(config *Config) (*Server, error) {
 	s.seccompProfile = seccompConfig
 
 	if s.appArmorEnabled {
-		apparmor.InstallDefaultAppArmorProfile()
+		apparmor.LoadDefaultAppArmorProfile()
 	}
 	s.appArmorProfile = config.ApparmorProfile
 

test/apparmor.bats

@@ -15,8 +15,8 @@ function teardown() {
     fi
 
     # this test requires apparmor, so skip this test if apparmor is not enabled.
-    enabled=is_apparmor_enabled
+    enabled=$(is_apparmor_enabled)
-    if [[ "$enabled" = "0" ]]; then
+    if [[ "$enabled" -eq 0 ]]; then
         skip "skip this test since apparmor is not enabled."
     fi
 
@@ -52,12 +52,12 @@ function teardown() {
     fi
 
     # this test requires apparmor, so skip this test if apparmor is not enabled.
-    enabled=is_apparmor_enabled
+    enabled=$(is_apparmor_enabled)
-    if [[ "$enabled" -eq "0" ]]; then
+    if [[ "$enabled" -eq 0 ]]; then
         skip "skip this test since apparmor is not enabled."
     fi
 
-    load_apparmor_test_profile
+    load_apparmor_profile "$APPARMOR_TEST_PROFILE_PATH"
     start_ocid "" "$APPARMOR_TEST_PROFILE_NAME"
 
     sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname2": "apparmor-test-deny-write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor2.json
@@ -79,7 +79,7 @@ function teardown() {
     cleanup_ctrs
     cleanup_pods
     stop_ocid
-    remove_apparmor_test_profile
+    remove_apparmor_profile "$APPARMOR_TEST_PROFILE_PATH"
 }
 
 # 3. test running with loading a specific apparmor profile but not as ocid default apparmor profile.
@@ -91,12 +91,12 @@ function teardown() {
     fi
 
     # this test requires apparmor, so skip this test if apparmor is not enabled.
-    enabled=is_apparmor_enabled
+    enabled=$(is_apparmor_enabled)
-    if [[ "$enabled" -eq "0" ]]; then
+    if [[ "$enabled" -eq 0 ]]; then
         skip "skip this test since apparmor is not enabled."
     fi
 
-    load_apparmor_test_profile
+    load_apparmor_profile "$APPARMOR_TEST_PROFILE_PATH"
     start_ocid
 
     sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname3": "apparmor-test-deny-write"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor3.json
@@ -118,7 +118,7 @@ function teardown() {
     cleanup_ctrs
     cleanup_pods
     stop_ocid
-    remove_apparmor_test_profile
+    remove_apparmor_profile "$APPARMOR_TEST_PROFILE_PATH"
 }
 
 # 4. test running with wrong apparmor profile name.
@@ -130,8 +130,8 @@ function teardown() {
     fi
 
     # this test requires apparmor, so skip this test if apparmor is not enabled.
-    enabled=is_apparmor_enabled
+    enabled=$(is_apparmor_enabled)
-    if [[ "$enabled" -eq "0" ]]; then
+    if [[ "$enabled" -eq 0 ]]; then
         skip "skip this test since apparmor is not enabled."
     fi
 
@@ -149,6 +149,44 @@ function teardown() {
     [[ "$output" =~ "Creating container failed" ]]
 
 
+    cleanup_ctrs
+    cleanup_pods
+    stop_ocid
+}
+
+# 5. test running with default apparmor profile unloaded.
+# test that we can will fail when running a ctr with rong apparmor profile name.
+@test "run a container after unloading default apparmor profile" {
+    # this test requires docker, thus it can't yet be run in a container
+    if [ "$TRAVIS" = "true" ]; then # instead of $TRAVIS, add a function is_containerized to skip here
+        skip "cannot yet run this test in a container, use sudo make localintegration"
+    fi
+
+    # this test requires apparmor, so skip this test if apparmor is not enabled.
+    enabled=$(is_apparmor_enabled)
+    if [[ "$enabled" -eq 0 ]]; then
+        skip "skip this test since apparmor is not enabled."
+    fi
+
+    start_ocid
+    remove_apparmor_profile "$FAKE_OCID_DEFAULT_PROFILE_PATH"
+
+    sed -e 's/%VALUE%/,"container\.apparmor\.security\.beta\.kubernetes\.io\/testname5": "runtime\/default"/g' "$TESTDATA"/sandbox_config_seccomp.json > "$TESTDIR"/apparmor5.json
+
+    run ocic pod create --name apparmor5 --config "$TESTDIR"/apparmor5.json
+    echo "$output"
+    [ "$status" -eq 0 ]
+    pod_id="$output"
+    run ocic ctr create --name testname5 --config "$TESTDATA"/container_redis.json --pod "$pod_id"
+    echo "$output"
+    [ "$status" -eq 0 ]
+    ctr_id="$output"
+    [ "$status" -eq 0 ]
+    run ocic ctr execsync --id "$ctr_id" touch test.txt
+    echo "$output"
+    [ "$status" -eq 0 ]
+
+
     cleanup_ctrs
     cleanup_pods
     stop_ocid

test/helpers.bash

@@ -28,6 +28,8 @@ RUNC_BINARY=${RUNC_PATH:-/usr/local/sbin/runc}
 APPARMOR_PARSER_BINARY=${APPARMOR_PARSER_BINARY:-/sbin/apparmor_parser}
 # Path of the apparmor profile for test.
 APPARMOR_TEST_PROFILE_PATH=${APPARMOR_TEST_PROFILE_PATH:-${TESTDATA}/apparmor_test_deny_write}
+# Path of the apparmor profile for unloading ocid-default.
+FAKE_OCID_DEFAULT_PROFILE_PATH=${FAKE_OCID_DEFAULT_PROFILE_PATH:-${TESTDATA}/fake_ocid_default}
 # Name of the apparmor profile for test.
 APPARMOR_TEST_PROFILE_NAME=${APPARMOR_TEST_PROFILE_NAME:-apparmor-test-deny-write}
 # Path of boot config.
@@ -148,12 +150,12 @@ function cleanup_test() {
 }
 
 
-function load_apparmor_test_profile() {
+function load_apparmor_profile() {
-	"$APPARMOR_PARSER_BINARY" -r "$APPARMOR_TEST_PROFILE_PATH"
+	"$APPARMOR_PARSER_BINARY" -r "$1"
 }
 
-function remove_apparmor_test_profile() {
+function remove_apparmor_profile() {
-	"$APPARMOR_PARSER_BINARY" -R "$APPARMOR_TEST_PROFILE_PATH"
+	"$APPARMOR_PARSER_BINARY" -R "$1"
 }
 
 function is_seccomp_enabled() {

test/seccomp.bats

@@ -15,8 +15,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-    if [[ "$enabled" =~ "0" ]]; then
+    if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -56,8 +56,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -97,8 +97,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -133,8 +133,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -158,8 +158,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -202,8 +202,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -243,8 +243,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -284,8 +284,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -325,8 +325,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 
@@ -362,8 +362,8 @@ function teardown() {
 	fi
 
 	# this test requires seccomp, so skip this test if seccomp is not enabled.
-	enabled=is_seccomp_enabled
+	enabled=$(is_seccomp_enabled)
-	if [[ "$enabled" =~ "0" ]]; then
+	if [[ "$enabled" -eq 0 ]]; then
 		skip "skip this test since seccomp is not enabled."
 	fi
 

test/testdata/fake_ocid_default

@@ -0,0 +1 @@
+profile ocid-default flags=(attach_disconnected) {}