From c0bc903101f3012e39cc8a2a5ff761d1be8b9789 Mon Sep 17 00:00:00 2001
From: Alexandre Pujol <alexandre@pujol.io>
Date: Sat, 30 Sep 2023 13:54:04 +0100
Subject: [PATCH] feat(aa): use profile guideline to sort file rules.

---
 pkg/aa/data_test.go  |  4 ++--
 pkg/aa/file.go       | 29 +++++++++++++++++++++++------
 pkg/aa/rules_test.go | 30 ++++++++++++++++++++++++++++++
 pkg/aa/template.go   |  5 ++---
 4 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/pkg/aa/data_test.go b/pkg/aa/data_test.go
index 95b7a2c6..8e6ad857 100644
--- a/pkg/aa/data_test.go
+++ b/pkg/aa/data_test.go
@@ -219,7 +219,7 @@ var (
 		"operation":      "open",
 		"class":          "file",
 		"profile":        "gsd-print-notifications",
-		"name":           "/proc/4163/cgroup",
+		"name":           "@{PROC}/4163/cgroup",
 		"comm":           "gsd-print-notif",
 		"requested_mask": "r",
 		"denied_mask":    "r",
@@ -236,7 +236,7 @@ var (
 	}
 	file2 = &File{
 		Qualifier: Qualifier{Owner: true, NoNewPrivs: true},
-		Path:      "/proc/4163/cgroup",
+		Path:      "@{PROC}/4163/cgroup",
 		Access:    "r",
 		Target:    "",
 	}
diff --git a/pkg/aa/file.go b/pkg/aa/file.go
index e14674cf..e304d822 100644
--- a/pkg/aa/file.go
+++ b/pkg/aa/file.go
@@ -4,6 +4,10 @@
 
 package aa
 
+import (
+	"strings"
+)
+
 type File struct {
 	Qualifier
 	Path   string
@@ -22,16 +26,30 @@ func FileFromLog(log map[string]string) ApparmorRule {
 
 func (r *File) Less(other any) bool {
 	o, _ := other.(*File)
-	if r.Qualifier.Equals(o.Qualifier) {
+	letterR := ""
+	letterO := ""
+	for _, letter := range fileAlphabet {
+		if strings.HasPrefix(r.Path, letter) {
+			letterR = letter
+		}
+		if strings.HasPrefix(o.Path, letter) {
+			letterO = letter
+		}
+	}
+
+	if fileWeights[letterR] == fileWeights[letterO] || letterR == "" || letterO == "" {
 		if r.Path == o.Path {
-			if r.Access == o.Access {
-				return r.Target < o.Target
+			if r.Qualifier.Equals(o.Qualifier) {
+				if r.Access == o.Access {
+					return r.Target < o.Target
+				}
+				return r.Access < o.Access
 			}
-			return r.Access < o.Access
+			return r.Qualifier.Less(o.Qualifier)
 		}
 		return r.Path < o.Path
 	}
-	return r.Qualifier.Less(o.Qualifier)
+	return fileWeights[letterR] < fileWeights[letterO]
 }
 
 func (r *File) Equals(other any) bool {
@@ -39,4 +57,3 @@ func (r *File) Equals(other any) bool {
 	return r.Path == o.Path && r.Access == o.Access &&
 		r.Target == o.Target && r.Qualifier.Equals(o.Qualifier)
 }
-
diff --git a/pkg/aa/rules_test.go b/pkg/aa/rules_test.go
index a55f36e7..70a13a1e 100644
--- a/pkg/aa/rules_test.go
+++ b/pkg/aa/rules_test.go
@@ -139,8 +139,38 @@ func TestRule_Less(t *testing.T) {
 			name:  "file",
 			rule:  file1,
 			other: file2,
+			want:  true,
+		},
+		{
+			name:  "file/empty",
+			rule:  &File{},
+			other: &File{},
 			want:  false,
 		},
+		{
+			name:  "file/equal",
+			rule:  &File{Path: "/usr/share/poppler/cMap/Identity-H"},
+			other: &File{Path: "/usr/share/poppler/cMap/Identity-H"},
+			want:  false,
+		},
+		{
+			name:  "file/owner",
+			rule:  &File{Path: "/usr/share/poppler/cMap/Identity-H", Qualifier: Qualifier{Owner: true}},
+			other: &File{Path: "/usr/share/poppler/cMap/Identity-H"},
+			want:  true,
+		},
+		{
+			name:  "file/access",
+			rule:  &File{Path: "/usr/share/poppler/cMap/Identity-H", Access: "r"},
+			other: &File{Path: "/usr/share/poppler/cMap/Identity-H", Access: "w"},
+			want:  true,
+		},
+		{
+			name:  "file/close",
+			rule:  &File{Path: "/usr/share/poppler/cMap/"},
+			other: &File{Path: "/usr/share/poppler/cMap/Identity-H"},
+			want:  true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/aa/template.go b/pkg/aa/template.go
index 808fe532..cbac0461 100644
--- a/pkg/aa/template.go
+++ b/pkg/aa/template.go
@@ -90,9 +90,8 @@ var (
 		"/opt",                // 2.3 opt binaries & libraries
 		"/usr/share",          // 3. shared data
 		"/etc",                // 4. system configuration
-		"/",                   // 5.1 system data
-		"/var",                // 5.2 system data read/write data
-		"/boot",               // 5.3 boot files
+		"/var",                // 5.1 system read/write data
+		"/boot",               // 5.2 boot files
 		"/home",               // 6.1 user data
 		"@{HOME}",             // 6.2 home files
 		"@{user_cache_dirs}",  // 7.1 user caches