From 29f66c3828de3247494a22e6fb8c5346ec72b5e6 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Tue, 24 Dec 2024 01:13:45 -0800
Subject: [PATCH] parser: drop priority from state permissions

The priority field is only used during state construction, and can
even prevent later optimizations like minimization. The parser already
explcitily clears the states priority field as part of the last thing
done during construction so it doesn't prevent minimization
optimizations.

This means the state priority not only wastes storage because it is
unused post construction but if used it could introduce regressions,
or other issues.

The change to the minimization tests just removes looking for the
priority field that is no longer reported.

Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit cc31a0da223321e18324cc58aa9cdc9e025c6ea8)
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 parser/libapparmor_re/hfa.cc | 23 ++++++-----------------
 parser/libapparmor_re/hfa.h  | 20 +++-----------------
 parser/tst/minimize.sh       | 20 ++++++++++----------
 3 files changed, 19 insertions(+), 44 deletions(-)

diff --git a/parser/libapparmor_re/hfa.cc b/parser/libapparmor_re/hfa.cc
index da7230914..fa05c26bb 100644
--- a/parser/libapparmor_re/hfa.cc
+++ b/parser/libapparmor_re/hfa.cc
@@ -508,11 +508,6 @@ DFA::DFA(Node *root, optflags const &opts, bool buildfiledfa): root(root), filed
 	 */
 	nnodes_cache.clear();
 	node_map.clear();
-	/* once created the priority information is no longer needed and
-	 * can prevent sets with the same perms and different priorities
-	 * from being merged during minimization
-	 */
-	clear_priorities();
 }
 
 DFA::~DFA()
@@ -666,13 +661,6 @@ int DFA::apply_and_clear_deny(void)
 	return c;
 }
 
-void DFA::clear_priorities(void)
-{
-	for (Partition::iterator i = states.begin(); i != states.end(); i++)
-		(*i)->perms.priority = 0;
-}
-
-
 
 /* minimize the number of dfa states */
 void DFA::minimize(optflags const &opts)
@@ -1405,7 +1393,7 @@ int accept_perms(NodeVec *state, perms_t &perms, bool filedfa)
 {
 	int error = 0;
 	perms_t exact;
-
+	int priority = MIN_INTERNAL_PRIORITY;
 	perms.clear();
 
 	if (!state)
@@ -1416,12 +1404,13 @@ int accept_perms(NodeVec *state, perms_t &perms, bool filedfa)
 			continue;
 
 		MatchFlag *match = static_cast<MatchFlag *>(*i);
-		if (perms.priority > match->priority)
+		if (priority > match->priority)
 			continue;
 
-		if (perms.priority < match->priority) {
-			perms.clear(match->priority);
-			exact.clear(match->priority);
+		if (priority < match->priority) {
+			priority = match->priority;
+			perms.clear();
+			exact.clear();
 		}
 		if (match->is_type(NODE_TYPE_EXACTMATCHFLAG)) {
 			/* exact match only ever happens with x */
diff --git a/parser/libapparmor_re/hfa.h b/parser/libapparmor_re/hfa.h
index 89a55b1d7..c705304ea 100644
--- a/parser/libapparmor_re/hfa.h
+++ b/parser/libapparmor_re/hfa.h
@@ -52,37 +52,26 @@ ostream &operator<<(ostream &os, State &state);
 
 class perms_t {
 public:
-	perms_t(void): priority(MIN_INTERNAL_PRIORITY), allow(0), deny(0), prompt(0), audit(0), quiet(0), exact(0) { };
+	perms_t(void): allow(0), deny(0), prompt(0), audit(0), quiet(0), exact(0) { };
 
 	bool is_accept(void) { return (allow | deny | prompt | audit | quiet); }
 
 	void dump_header(ostream &os)
 	{
-		os << "priority (allow/deny/prompt/audit/quiet)";
+		os << "(allow/deny/prompt/audit/quiet)";
 	}
 	void dump(ostream &os)
 	{
-		os << " " << priority << " (0x " << hex
+		os << "(0x " << hex
 		   << allow << "/" << deny << "/" << "/" << prompt << "/" << audit << "/" << quiet
 		   << ')' << dec;
 	}
 
 	void clear(void) {
-		priority = MIN_INTERNAL_PRIORITY;
-		allow = deny = prompt = audit = quiet = exact = 0;
-	}
-	void clear(int p) {
-		priority = p;
 		allow = deny = prompt = audit = quiet = exact = 0;
 	}
 	void add(perms_t &rhs, bool filedfa)
 	{
-		if (priority > rhs.priority)
-			return;
-		if (priority < rhs.priority) {
-			*this = rhs;
-			return;
-		} //else if (rhs.priority == priority) {
 		deny |= rhs.deny;
 
 		if (filedfa && !is_merged_x_consistent(allow & ALL_USER_EXEC,
@@ -156,8 +145,6 @@ public:
 
 	bool operator<(perms_t const &rhs)const
 	{
-		if (priority < rhs.priority)
-			return priority < rhs.priority;
 		if (allow < rhs.allow)
 			return allow < rhs.allow;
 		if (deny < rhs.deny)
@@ -169,7 +156,6 @@ public:
 		return quiet < rhs.quiet;
 	}
 
-	int priority;
 	perm32_t allow, deny, prompt, audit, quiet, exact;
 };
 
diff --git a/parser/tst/minimize.sh b/parser/tst/minimize.sh
index 054831fe8..8b3c4850c 100755
--- a/parser/tst/minimize.sh
+++ b/parser/tst/minimize.sh
@@ -78,7 +78,7 @@ APPARMOR_PARSER="${APPARMOR_PARSER:-../apparmor_parser}"
 # {a} (0x 40030/0/0/0)
 
 echo -n "Minimize profiles basic perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 6 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 6 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -93,7 +93,7 @@ echo "ok"
 # {9} (0x 12804a/0/2800a/0)
 # {c} (0x 40030/0/0/0)
 echo -n "Minimize profiles audit perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 6 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 6 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -112,7 +112,7 @@ echo "ok"
 # {c} (0x 40030/0/0/0)
 
 echo -n "Minimize profiles deny perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 6 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 6 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -130,7 +130,7 @@ echo "ok"
 # {c} (0x 40030/0/0/0)
 
 echo -n "Minimize profiles audit deny perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -O filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 5 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -O filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 5 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -155,7 +155,7 @@ echo "ok"
 ## NOTE: change count from 6 to 7 when extend perms is not dependent on
 ## prompt rules being present
 echo -n "Minimize profiles extended no-filter audit deny perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.extended-perms-no-policydb -QT -O minimize -O no-filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 7 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.extended-perms-no-policydb -QT -O minimize -O no-filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 7 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -173,7 +173,7 @@ echo "ok"
 # {2} (0x 4/0//0/0/0)   <- from policydb still showing up bug
 
 echo -n "Minimize profiles extended filter audit deny perms "
-if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.extended-perms-no-policydb -QT -O minimize -O filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 6 ] ; then
+if [ "$(echo "/t { /a r, /b w, /c a, /d l, /e k, /f m, audit deny /** w, }" | ${APPARMOR_PARSER} -M features_files/features.extended-perms-no-policydb -QT -O minimize -O filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 6 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -208,7 +208,7 @@ echo "ok"
 #
 
 echo -n "Minimize profiles xtrans "
-if [ "$(echo "/t { /b px, /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 3 ] ; then
+if [ "$(echo "/t { /b px, /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 3 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -216,7 +216,7 @@ echo "ok"
 
 # same test as above + audit
 echo -n "Minimize profiles audit xtrans "
-if [ "$(echo "/t { /b px, audit /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 3 ] ; then
+if [ "$(echo "/t { /b px, audit /* Pixr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 3 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -229,7 +229,7 @@ echo "ok"
 # {3} (0x 0/fe17f85/0/14005)
 
 echo -n "Minimize profiles deny xtrans "
-if [ "$(echo "/t { /b px, deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 1 ] ; then
+if [ "$(echo "/t { /b px, deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 1 ] ; then
     echo "failed"
     exit 1;
 fi
@@ -241,7 +241,7 @@ echo "ok"
 # {3} (0x 0/fe17f85/0/0)
 
 echo -n "Minimize profiles audit deny xtrans "
-if [ "$(echo "/t { /b px, audit deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -O no-filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*} 0 (.*)$')" -ne 0 ] ; then
+if [ "$(echo "/t { /b px, audit deny /* xr, /a Cx -> foo, }" | ${APPARMOR_PARSER} -M features_files/features.nopolicydb -QT -O minimize -O no-filter-deny -D dfa-states 2>&1 | grep -v '<==' | grep -c '^{.*}(.*)$')" -ne 0 ] ; then
     echo "failed"
     exit 1;
 fi