Merge pull request #73 from bytedance/update-profile-template

feat: Merge the same child profiles for AppArmor enforcer.
bytedance · May 20, 2024 · c8cad5a · c8cad5a
2 parents 4b536dd + 338a419
commit c8cad5a
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 40 deletions.
diff --git a/internal/profile/apparmor/apparmor.go b/internal/profile/apparmor/apparmor.go
@@ -346,48 +346,50 @@ func GenerateEnhanceProtectProfile(enhanceProtect *varmor.EnhanceProtect, profil
 	}
 
 	// Attack Protection
+	index := 0
 	for _, attackProtectionRule := range enhanceProtect.AttackProtectionRules {
 		if len(attackProtectionRule.Targets) == 0 {
 			for _, rule := range attackProtectionRule.Rules {
 				baseRules += generateAttackProtectionRules(rule)
 			}
-		}
-	}
+		} else {
+			// build a child profile for certain binaries
+			childProfileName := fmt.Sprintf("child_%d", index)
+			childProfilePath := fmt.Sprintf("%s//%s", profileName, childProfileName)
+			childProfileRules := baseRules
+			index += 1
 
-	// childName(target): childRules
-	childRulesMap := make(map[string]string)
-	for _, attackProtectionRule := range enhanceProtect.AttackProtectionRules {
-		if len(attackProtectionRule.Targets) != 0 {
-			var childRules string
+			for _, rule := range attackProtectionRule.Rules {
+				childProfileRules += generateAttackProtectionRules(rule)
+			}
 
-			for _, childName := range attackProtectionRule.Targets {
-				if _, ok := childRulesMap[childName]; !ok {
-					childRules = baseRules
-				} else {
-					childRules = childRulesMap[childName]
-				}
+			targetsCx := ""
+			for _, target := range attackProtectionRule.Targets {
+				targetsCx += fmt.Sprintf("%s cx -> %s,\n", target, childProfileName)
+			}
 
-				for _, rule := range attackProtectionRule.Rules {
-					childRules += generateAttackProtectionRules(rule)
-					childRulesMap[childName] = childRules
-				}
+			targetsRix := ""
+			for _, target := range attackProtectionRule.Targets {
+				targetsRix += fmt.Sprintf("%s rix,\n", target)
 			}
-		}
-	}
 
-	for childName, childRules := range childRulesMap {
-		if enhanceProtect.Privileged {
-			// Create the child profile for privileged container based on the AlwaysAllow child template
-			baseRules += fmt.Sprintf(alwaysAllowChildTemplate, childName, childName, childName, childRules)
-		} else {
-			// Create the child profile for unprivileged container based on the RuntimeDefault child template
-			childProfileName := fmt.Sprintf("%s//%s", profileName, childName)
-			baseRules += fmt.Sprintf(runtimeDefaultChildTemplate,
-				childProfileName,                // signal
-				childName, childName, childName, // target
-				profileName, childProfileName, // signal
-				profileName, childProfileName, // ptrace
-				childRules)
+			if enhanceProtect.Privileged {
+				baseRules += fmt.Sprintf(alwaysAllowChildTemplate,
+					targetsCx,
+					childProfileName,
+					targetsRix,
+					childProfileRules)
+			} else {
+				baseRules += fmt.Sprintf(runtimeDefaultChildTemplate,
+					childProfilePath, // parent may send signal to child
+					childProfilePath, // parent may ptrace child
+					targetsCx,
+					childProfileName,
+					targetsRix,
+					profileName, childProfilePath, // signal
+					profileName, childProfilePath, // ptrace
+					childProfileRules)
+			}
 		}
 	}
 

diff --git a/internal/profile/apparmor/builder.go b/internal/profile/apparmor/builder.go
@@ -167,7 +167,7 @@ func buildSignalRules(dynamicResult *varmor.DynamicResult, profileName string, d
 	return ruleSet
 }
 
-func buildDefaultAllowRules(dynamicResult *varmor.DynamicResult) string {
+func buildDefaultAllowRules() string {
 	// From docker-default profile
 	//   https://github.com/moby/moby/blob/master/profiles/apparmor/template.go
 	//   https://github.com/containerd/containerd/blob/main/contrib/apparmor/template.go
@@ -188,7 +188,7 @@ func GenerateProfileWithBehaviorModel(dynamicResult *varmor.DynamicResult, debug
 		ruleSet += buildNetworkRules(dynamicResult, debug)
 		ruleSet += buildPtraceRules(dynamicResult, profileName, debug)
 		ruleSet += buildSignalRules(dynamicResult, profileName, debug)
-		ruleSet += buildDefaultAllowRules(dynamicResult)
+		ruleSet += buildDefaultAllowRules()
 
 		profile := fmt.Sprintf(defenseInDepthTemplate, profileName, ruleSet)
 		return base64.StdEncoding.EncodeToString([]byte(profile)), nil

diff --git a/internal/profile/apparmor/template.go b/internal/profile/apparmor/template.go
@@ -41,10 +41,10 @@ profile %s flags=(attach_disconnected,mediate_deleted) {
 `
 
 const alwaysAllowChildTemplate = `
-%s cx,
-profile %s flags=(attach_disconnected,mediate_deleted) {
-  %s rix,
 
+%s
+profile %s flags=(attach_disconnected,mediate_deleted) {
+  %s
   #include <abstractions/base>
 
   file,
@@ -123,10 +123,12 @@ const runtimeDefaultChildTemplate = `
 
 # processes with parent profile may send signal to processes with child profile
 signal (send) peer=%s,
-%s cx,
-profile %s flags=(attach_disconnected,mediate_deleted) {
-  %s rix,
+# processes with parent profile may ptrace processes with child profile, but not vice versa.
+ptrace (trace,read) peer=%s,
 
+%s
+profile %s flags=(attach_disconnected,mediate_deleted) {
+  %s
   #include <abstractions/base>
 
   network,
@@ -167,6 +169,7 @@ profile %s flags=(attach_disconnected,mediate_deleted) {
 
   # processes with parent profile may ptrace processes with child profile, but not vice versa.
   ptrace (tracedby,readby) peer=%s,
+  # processes with child profile may ptrace processes amongst themselves.
   ptrace (trace,read,tracedby,readby) peer=%s,
 
 %s