From 46d25ed922914fc24c4f6c8fc6436428505578c6 Mon Sep 17 00:00:00 2001
From: Alexandre Pujol <alexandre@pujol.io>
Date: Fri, 20 Oct 2023 23:11:11 +0100
Subject: [PATCH] feat(aa-log): improve error formating on rules.

---
 pkg/aa/data_test.go         |  4 +++-
 pkg/aa/profile.go           |  7 ++++---
 pkg/aa/profile_test.go      |  1 -
 pkg/aa/rules.go             | 34 +++++++++++++++++++++++++++++-----
 pkg/aa/template.go          |  4 ++--
 pkg/aa/templates/comment.j2 |  8 +++++++-
 6 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/pkg/aa/data_test.go b/pkg/aa/data_test.go
index 016aa272..9ec559ba 100644
--- a/pkg/aa/data_test.go
+++ b/pkg/aa/data_test.go
@@ -62,7 +62,7 @@ var (
 		"apparmor":  "ALLOWED",
 		"class":     "mount",
 		"operation": "mount",
-		"info":      "failed perms check", // TODO: not attach_disconnected
+		"info":      "failed perms check",
 		"error":     "-13",
 		"profile":   "dockerd",
 		"name":      "/var/lib/docker/overlay2/metacopy-check906831159/merged/",
@@ -71,11 +71,13 @@ var (
 		"srcname":   "overlay",
 	}
 	mount1 = &Mount{
+		Qualifier:       Qualifier{Comment: "failed perms check"},
 		MountConditions: MountConditions{FsType: "overlay", Options: []string{}},
 		Source:          "overlay",
 		MountPoint:      "/var/lib/docker/overlay2/opaque-bug-check1209538631/merged/",
 	}
 	mount2 = &Mount{
+		Qualifier:       Qualifier{Comment: "failed perms check"},
 		MountConditions: MountConditions{FsType: "overlay", Options: []string{}},
 		Source:          "overlay",
 		MountPoint:      "/var/lib/docker/overlay2/metacopy-check906831159/merged/",
diff --git a/pkg/aa/profile.go b/pkg/aa/profile.go
index 1aa881a2..0af3dede 100644
--- a/pkg/aa/profile.go
+++ b/pkg/aa/profile.go
@@ -66,15 +66,16 @@ func (p *AppArmorProfile) String() string {
 
 // AddRule adds a new rule to the profile from a log map
 func (p *AppArmorProfile) AddRule(log map[string]string) {
+	// Generate profile flags and extra rules
 	switch log["error"] {
 	case "-2":
 		if !slices.Contains(p.Flags, "mediate_deleted") {
 			p.Flags = append(p.Flags, "mediate_deleted")
 		}
 	case "-13":
-		// FIXME: -13 can be a lot of things, not only attach_disconnected
-		// Eg: info="User namespace creation restricted"
-		if !slices.Contains(p.Flags, "attach_disconnected") {
+		if strings.Contains(log["info"], "namespace creation restricted") {
+			p.Rules = append(p.Rules, UsernsFromLog(log))
+		} else if strings.Contains(log["info"], "disconnected path") && !slices.Contains(p.Flags, "attach_disconnected") {
 			p.Flags = append(p.Flags, "attach_disconnected")
 		}
 	default:
diff --git a/pkg/aa/profile_test.go b/pkg/aa/profile_test.go
index 062bd4eb..64da482e 100644
--- a/pkg/aa/profile_test.go
+++ b/pkg/aa/profile_test.go
@@ -153,7 +153,6 @@ func TestAppArmorProfile_AddRule(t *testing.T) {
 			log:  mount2Log,
 			want: &AppArmorProfile{
 				Profile: Profile{
-					Flags: []string{"attach_disconnected"},
 					Rules: []ApparmorRule{mount2},
 				},
 			},
diff --git a/pkg/aa/rules.go b/pkg/aa/rules.go
index b083054c..56793910 100644
--- a/pkg/aa/rules.go
+++ b/pkg/aa/rules.go
@@ -4,7 +4,9 @@
 
 package aa
 
-import "strings"
+import (
+	"strings"
+)
 
 type Rule struct {
 	Comment     string
@@ -12,7 +14,6 @@ type Rule struct {
 	FileInherit bool
 }
 
-
 func (r *Rule) Less(other any) bool {
 	return false
 }
@@ -28,6 +29,8 @@ type Qualifier struct {
 	Owner       bool
 	NoNewPrivs  bool
 	FileInherit bool
+	Optional    bool
+	Comment     string
 	Prefix      string
 	Padding     string
 }
@@ -46,20 +49,41 @@ func NewQualifierFromLog(log map[string]string) Qualifier {
 	if log["apparmor"] == "AUDIT" {
 		audit = true
 	}
+
 	fileInherit := false
 	if log["operation"] == "file_inherit" {
 		fileInherit = true
 	}
+
 	noNewPrivs := false
-	if log["error"] == "-1" {
-		noNewPrivs = true
+	optional := false
+	msg := ""
+	switch log["error"] {
+	case "-1":
+		if strings.Contains(log["info"], "optional:") {
+			optional = true
+			msg = strings.Replace(log["info"], "optional: ", "", 1)
+		} else {
+			noNewPrivs = true
+		}
+	case "-13":
+		ignoreProfileInfo := []string{"namespace", "disconnected path"}
+		for _, info := range ignoreProfileInfo {
+			if strings.Contains(log["info"], info) {
+				break
+			}
+		}
+		msg = log["info"]
+	default:
 	}
+
 	return Qualifier{
 		Audit:       audit,
-		AccessType:  "",
 		Owner:       owner,
 		NoNewPrivs:  noNewPrivs,
 		FileInherit: fileInherit,
+		Optional:    optional,
+		Comment:     msg,
 	}
 }
 
diff --git a/pkg/aa/template.go b/pkg/aa/template.go
index f11900c7..e80ef52d 100644
--- a/pkg/aa/template.go
+++ b/pkg/aa/template.go
@@ -30,12 +30,12 @@ var (
 	tmplAppArmorProfile = generateTemplate()
 
 	// convert apparmor requested mask to apparmor access mode
-	// TODO: Should be a map of slice, not exhausive yet
+	// TODO: Should be a map of slice, not exhaustive yet
 	maskToAccess = map[string]string{
 		"a":            "w",
 		"c":            "w",
 		"d":            "w",
-		"k":            "rk",
+		"k":            "k",
 		"l":            "l",
 		"m":            "rm",
 		"r":            "r",
diff --git a/pkg/aa/templates/comment.j2 b/pkg/aa/templates/comment.j2
index bcc799b8..ce7c30b9 100644
--- a/pkg/aa/templates/comment.j2
+++ b/pkg/aa/templates/comment.j2
@@ -1,5 +1,5 @@
 {{- define "comment" -}}
-    {{- if or .FileInherit .NoNewPrivs -}}
+    {{- if or .FileInherit .NoNewPrivs .Optional .Comment -}}
         {{- "  #" -}}
     {{- end -}}
     {{- if .FileInherit -}}
@@ -8,4 +8,10 @@
     {{- if .NoNewPrivs -}}
         {{- " no new privs" -}}
     {{- end -}}
+    {{- if .Optional -}}
+        {{- " optional:" -}}
+    {{- end -}}
+    {{- with .Comment -}}
+        {{ " " }}{{ . }}
+    {{- end -}}
 {{- end -}}