[apparmor] [patch 10/26] Convert aare_rules into a class

john.johansen at canonical.com john.johansen at canonical.com
Thu Mar 27 15:45:23 UTC 2014


This cleans things up a bit and fixes a bug where not all rules are
getting properly counted so that the addition of policy_mediation
rules fails to generate the policy dfa in some cases.

Because the policy dfa is being generated correctly now we need to
fix some tests to use the new -M flag to specify the expected features
set of the test.

Signed-off-by: John Johansen <john.johansen at canonical.com>


---
 parser/dbus.c                       |   24 ++++------
 parser/libapparmor_re/aare_rules.cc |   80 +++++++++++-------------------------
 parser/libapparmor_re/aare_rules.h  |   34 +++++++--------
 parser/mount.c                      |   47 +++++++++------------
 parser/parser_regex.c               |   79 +++++++++++++++--------------------
 parser/profile.cc                   |    4 -
 parser/profile.h                    |    6 +-
 7 files changed, 114 insertions(+), 160 deletions(-)

--- 2.9-test.orig/parser/dbus.c
+++ 2.9-test/parser/dbus.c
@@ -301,28 +301,26 @@
 	}
 
 	if (mode & AA_DBUS_BIND) {
-		if (!aare_add_rule_vec(prof.policy.rules, deny,
-				       mode & AA_DBUS_BIND,
-				       audit & AA_DBUS_BIND,
-				       2, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, mode & AA_DBUS_BIND,
+						    audit & AA_DBUS_BIND,
+						    2, vec, dfaflags))
 			goto fail;
 	}
 	if (mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE)) {
-		if (!aare_add_rule_vec(prof.policy.rules, deny,
-				mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
-				audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
-				6, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny,
+				       mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
+				       audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE),
+				       6, vec, dfaflags))
 			goto fail;
 	}
 	if (mode & AA_DBUS_EAVESDROP) {
-		if (!aare_add_rule_vec(prof.policy.rules, deny,
-				mode & AA_DBUS_EAVESDROP,
-				audit & AA_DBUS_EAVESDROP,
-				1, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny,
+						    mode & AA_DBUS_EAVESDROP,
+						    audit & AA_DBUS_EAVESDROP,
+						    1, vec, dfaflags))
 			goto fail;
 	}
 
-	prof.policy.count++;
 	return RULE_OK;
 
 fail:
--- 2.9-test.orig/parser/libapparmor_re/aare_rules.cc
+++ 2.9-test/parser/libapparmor_re/aare_rules.cc
@@ -34,38 +34,20 @@
 #include "chfa.h"
 #include "../immunix.h"
 
-struct aare_ruleset {
-	int reverse;
-	Node *root;
-};
 
-aare_ruleset_t *aare_new_ruleset(int reverse)
-{
-	aare_ruleset_t *container = (aare_ruleset_t *) malloc(sizeof(aare_ruleset_t));
-	if (!container)
-		return NULL;
-
-	container->root = NULL;
-	container->reverse = reverse;
-
-	return container;
-}
 
-void aare_delete_ruleset(aare_ruleset_t *rules)
+aare_rules::~aare_rules(void)
 {
-	if (rules) {
-		if (rules->root)
-			rules->root->release();
-		free(rules);
-	}
+	if (root)
+		root->release();
 
 	aare_reset_matchflags();
 }
 
-int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny,
-			     uint32_t perms, uint32_t audit, dfaflags_t flags)
+bool aare_rules::add_rule(const char *rule, int deny, uint32_t perms,
+			  uint32_t audit, dfaflags_t flags)
 {
-	return aare_add_rule_vec(rules, deny, perms, audit, 1, &rule, flags);
+	return add_rule_vec(deny, perms, audit, 1, &rule, flags);
 }
 
 #define FLAGS_WIDTH 2
@@ -94,9 +76,8 @@
 #undef RESET_FLAGS
 }
 
-int aare_add_rule_vec(aare_ruleset_t *rules, int deny,
-				 uint32_t perms, uint32_t audit,
-				 int count, const char **rulev, dfaflags_t flags)
+bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit,
+			      int count, const char **rulev, dfaflags_t flags)
 {
 	Node *tree = NULL, *accept;
 	int exact_match;
@@ -105,15 +86,15 @@
 	assert(perms != 0);
 
 	if (regex_parse(&tree, rulev[0]))
-		return 0;
+		return false;
 	for (int i = 1; i < count; i++) {
 		Node *subtree = NULL;
 		Node *node = new CharNode(0);
 		if (!node)
-			return 0;
+			return false;
 		tree = new CatNode(tree, node);
 		if (regex_parse(&subtree, rulev[i]))
-			return 0;
+			return false;
 		tree = new CatNode(tree, subtree);
 	}
 
@@ -132,26 +113,15 @@
 			exact_match = 0;
 	}
 
-	if (rules->reverse)
+	if (reverse)
 		flip_tree(tree);
 
 /* 0x7f == 4 bits x mods + 1 bit unsafe mask + 1 bit ix, + 1 pux after shift */
 #define EXTRACT_X_INDEX(perm, shift) (((perm) >> (shift + 7)) & 0x7f)
 
-//if (perms & ALL_AA_EXEC_TYPE && (!perms & AA_EXEC_BITS))
-//      fprintf(stderr, "adding X rule without MAY_EXEC: 0x%x %s\n", perms, rulev[0]);
-
-//if (perms & ALL_EXEC_TYPE)
-//    fprintf(stderr, "adding X rule %s 0x%x\n", rulev[0], perms);
-
-//if (audit)
-//fprintf(stderr, "adding rule with audit bits set: 0x%x %s\n", audit, rulev[0]);
-
-//if (perms & AA_CHANGE_HAT)
-//    fprintf(stderr, "adding change_hat rule %s\n", rulev[0]);
 
-/* the permissions set is assumed to be non-empty if any audit
- * bits are specified */
+	/* the permissions set is assumed to be non-empty if any audit
+	 * bits are specified */
 	accept = NULL;
 	for (unsigned int n = 0; perms && n < (sizeof(perms) * 8); n++) {
 		uint32_t mask = 1 << n;
@@ -230,44 +200,44 @@
  		cerr << "\n\n";
 	}
 
-	if (rules->root)
-		rules->root = new AltNode(rules->root, new CatNode(tree, accept));
+	if (root)
+		root = new AltNode(root, new CatNode(tree, accept));
 	else
-		rules->root = new CatNode(tree, accept);
+		root = new CatNode(tree, accept);
 
-	return 1;
+	rule_count++;
 
+	return true;
 }
 
 /* create a dfa from the ruleset
  * returns: buffer contain dfa tables, @size set to the size of the tables
  *          else NULL on failure
  */
-void *aare_create_dfa(aare_ruleset_t *rules, size_t *size,
-				 dfaflags_t flags)
+void *aare_rules::create_dfa(size_t *size, dfaflags_t flags)
 {
 	char *buffer = NULL;
 
-	label_nodes(rules->root);
+	label_nodes(root);
 	if (flags & DFA_DUMP_TREE) {
 		cerr << "\nDFA: Expression Tree\n";
-		rules->root->dump(cerr);
+		root->dump(cerr);
 		cerr << "\n\n";
 	}
 
 	if (flags & DFA_CONTROL_TREE_SIMPLE) {
-		rules->root = simplify_tree(rules->root, flags);
+		root = simplify_tree(root, flags);
 
 		if (flags & DFA_DUMP_SIMPLE_TREE) {
 			cerr << "\nDFA: Simplified Expression Tree\n";
-			rules->root->dump(cerr);
+			root->dump(cerr);
 			cerr << "\n\n";
 		}
 	}
 
 	stringstream stream;
 	try {
-		DFA dfa(rules->root, flags);
+		DFA dfa(root, flags);
 		if (flags & DFA_DUMP_UNIQ_PERMS)
 			dfa.dump_uniq_perms("dfa");
 
--- 2.9-test.orig/parser/libapparmor_re/aare_rules.h
+++ 2.9-test/parser/libapparmor_re/aare_rules.h
@@ -24,26 +24,24 @@
 #include <stdint.h>
 
 #include "apparmor_re.h"
+#include "expr-tree.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+class aare_rules {
+	Node *root;
+public:
+	int reverse;
+	int rule_count;
+	aare_rules(): root(NULL), reverse(0), rule_count(0) { };
+	aare_rules(int reverse): root(NULL), reverse(reverse), rule_count(0) { };
+	~aare_rules();
+
+	bool add_rule(const char *rule, int deny, uint32_t perms,
+		      uint32_t audit, dfaflags_t flags);
+	bool add_rule_vec(int deny, uint32_t perms, uint32_t audit, int count,
+			  const char **rulev, dfaflags_t flags);
+	void *create_dfa(size_t *size, dfaflags_t flags);
+};
 
-struct aare_ruleset;
-
-typedef struct aare_ruleset aare_ruleset_t;
-
-aare_ruleset_t *aare_new_ruleset(int reverse);
-void aare_delete_ruleset(aare_ruleset_t *rules);
-int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny, uint32_t perms,
-		  uint32_t audit, dfaflags_t flags);
-int aare_add_rule_vec(aare_ruleset_t *rules, int deny, uint32_t perms,
-		      uint32_t audit, int count, const char **rulev,
-		      dfaflags_t flags);
-void *aare_create_dfa(aare_ruleset_t *rules, size_t *size, dfaflags_t flags);
 void aare_reset_matchflags(void);
 
-#ifdef __cplusplus
-}
-#endif
 #endif				/* __LIBAA_RE_RULES_H */
--- 2.9-test.orig/parser/mount.c
+++ 2.9-test/parser/mount.c
@@ -631,9 +631,9 @@
 			tmpallow = allow;
 
 		/* rule for match without required data || data MATCH_CONT */
-		if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow,
-				       audit | AA_AUDIT_MNT_DATA, 4,
-				       vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, tmpallow,
+					      audit | AA_AUDIT_MNT_DATA, 4,
+					      vec, dfaflags))
 			goto fail;
 		count++;
 
@@ -643,10 +643,9 @@
 			if (!build_mnt_opts(optsbuf, opts))
 				goto fail;
 			vec[4] = optsbuf.c_str();
-			if (!aare_add_rule_vec(prof.policy.rules, deny,
-					       allow,
-					       audit | AA_AUDIT_MNT_DATA,
-					       5, vec, dfaflags))
+			if (!prof.policy.rules->add_rule_vec(deny, allow,
+						      audit | AA_AUDIT_MNT_DATA,
+						      5, vec, dfaflags))
 				goto fail;
 			count++;
 		}
@@ -674,8 +673,8 @@
 		if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags))
 			goto fail;
 		vec[3] = flagsbuf;
-		if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
-				       audit, 4, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+						    dfaflags))
 			goto fail;
 		count++;
 	}
@@ -703,8 +702,8 @@
 		if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags))
 			goto fail;
 		vec[3] = flagsbuf;
-		if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
-				       audit, 4, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+						    dfaflags))
 			goto fail;
 		count++;
 	}
@@ -733,8 +732,8 @@
 		if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags))
 			goto fail;
 		vec[3] = flagsbuf;
-		if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
-				       audit, 4, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec,
+						    dfaflags))
 			goto fail;
 		count++;
 	}
@@ -773,9 +772,9 @@
 			tmpallow = allow;
 
 		/* rule for match without required data || data MATCH_CONT */
-		if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow,
-				       audit | AA_AUDIT_MNT_DATA, 4,
-				       vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, tmpallow,
+					      audit | AA_AUDIT_MNT_DATA, 4,
+					      vec, dfaflags))
 			goto fail;
 		count++;
 
@@ -785,10 +784,9 @@
 			if (!build_mnt_opts(optsbuf, opts))
 				goto fail;
 			vec[4] = optsbuf.c_str();
-			if (!aare_add_rule_vec(prof.policy.rules, deny,
-					       allow,
-					       audit | AA_AUDIT_MNT_DATA,
-					       5, vec, dfaflags))
+			if (!prof.policy.rules->add_rule_vec(deny, allow,
+						      audit | AA_AUDIT_MNT_DATA,
+						      5, vec, dfaflags))
 				goto fail;
 			count++;
 		}
@@ -799,8 +797,8 @@
 		if (!convert_entry(mntbuf, mnt_point))
 			goto fail;
 		vec[0] = mntbuf.c_str();
-		if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
-				       audit, 1, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 1, vec,
+						    dfaflags))
 			goto fail;
 		count++;
 	}
@@ -813,8 +811,8 @@
 		if (!clear_and_convert_entry(devbuf, device))
 			goto fail;
 		vec[1] = devbuf.c_str();
-		if (!aare_add_rule_vec(prof.policy.rules, deny, allow,
-				       audit, 2, vec, dfaflags))
+		if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 2, vec,
+						    dfaflags))
 			goto fail;
 		count++;
 	}
@@ -823,7 +821,6 @@
 		/* didn't actually encode anything */
 		goto fail;
 
-	prof.policy.count++;
 	return RULE_OK;
 
 fail:
--- 2.9-test.orig/parser/parser_regex.c
+++ 2.9-test/parser/parser_regex.c
@@ -431,11 +431,11 @@
 		prof->xmatch_size = 0;
 	} else {
 		/* build a dfa */
-		aare_ruleset_t *rule = aare_new_ruleset(0);
-		if (!rule)
+		aare_rules *rules = new aare_rules();
+		if (!rules)
 			return FALSE;
-		if (!aare_add_rule(rule, tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) {
-			aare_delete_ruleset(rule);
+		if (!rules->add_rule(tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) {
+			delete rules;
 			return FALSE;
 		}
 		if (prof->altnames) {
@@ -450,15 +450,14 @@
 					len = strlen(alt->name);
 				if (len < prof->xmatch_len)
 					prof->xmatch_len = len;
-				if (!aare_add_rule(rule, tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) {
-					aare_delete_ruleset(rule);
+				if (!rules->add_rule(tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) {
+					delete rules;
 					return FALSE;
 				}
 			}
 		}
-		prof->xmatch = aare_create_dfa(rule, &prof->xmatch_size,
-					      dfaflags);
-		aare_delete_ruleset(rule);
+		prof->xmatch = rules->create_dfa(&prof->xmatch_size, dfaflags);
+		delete rules;
 		if (!prof->xmatch)
 			return FALSE;
 	}
@@ -466,7 +465,7 @@
 	return TRUE;
 }
 
-static int process_dfa_entry(aare_ruleset_t *dfarules, struct cod_entry *entry)
+static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry)
 {
 	std::string tbuf;
 	pattern_t ptype;
@@ -498,13 +497,13 @@
 	 * entry of the pair
 	 */
 	if (entry->deny && (entry->mode & AA_LINK_BITS)) {
-		if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny,
-				   entry->mode & ~AA_LINK_BITS,
-				   entry->audit & ~AA_LINK_BITS, dfaflags))
+		if (!dfarules->add_rule(tbuf.c_str(), entry->deny,
+					entry->mode & ~AA_LINK_BITS,
+					entry->audit & ~AA_LINK_BITS, dfaflags))
 			return FALSE;
 	} else if (entry->mode & ~AA_CHANGE_PROFILE) {
-		if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny, entry->mode,
-				   entry->audit, dfaflags))
+		if (!dfarules->add_rule(tbuf.c_str(), entry->deny, entry->mode,
+					entry->audit, dfaflags))
 			return FALSE;
 	}
 
@@ -526,7 +525,7 @@
 			perms |= LINK_TO_LINK_SUBSET(perms);
 			vec[1] = "/[^/].*";
 		}
-		if (!aare_add_rule_vec(dfarules, entry->deny, perms, entry->audit & AA_LINK_BITS, 2, vec, dfaflags))
+		if (!dfarules->add_rule_vec(entry->deny, perms, entry->audit & AA_LINK_BITS, 2, vec, dfaflags))
 			return FALSE;
 	}
 	if (entry->mode & AA_CHANGE_PROFILE) {
@@ -545,12 +544,12 @@
 		vec[index++] = tbuf.c_str();
 
 		/* regular change_profile rule */
-		if (!aare_add_rule_vec(dfarules, 0, AA_CHANGE_PROFILE | AA_ONEXEC, 0, index - 1, &vec[1], dfaflags))
+		if (!dfarules->add_rule_vec(0, AA_CHANGE_PROFILE | AA_ONEXEC, 0, index - 1, &vec[1], dfaflags))
 			return FALSE;
 		/* onexec rules - both rules are needed for onexec */
-		if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, 1, vec, dfaflags))
+		if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, 1, vec, dfaflags))
 			return FALSE;
-		if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, index, vec, dfaflags))
+		if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, index, vec, dfaflags))
 			return FALSE;
 	}
 	return TRUE;
@@ -560,15 +559,12 @@
 {
 	int ret = TRUE;
 	struct cod_entry *entry;
-	int count = 0;
 
 	list_for_each(prof->entries, entry) {
 		if (!process_dfa_entry(prof->dfa.rules, entry))
 			ret = FALSE;
-		count++;
 	}
 
-	prof->dfa.count = count;
 	return ret;
 }
 
@@ -579,17 +575,17 @@
 	if (!process_profile_name_xmatch(prof))
 		goto out;
 
-	prof->dfa.rules = aare_new_ruleset(0);
+	prof->dfa.rules = new aare_rules();
 	if (!prof->dfa.rules)
 		goto out;
 
 	if (!post_process_entries(prof))
 		goto out;
 
-	if (prof->dfa.count > 0) {
-		prof->dfa.dfa = aare_create_dfa(prof->dfa.rules, &prof->dfa.size,
-						dfaflags);
-		aare_delete_ruleset(prof->dfa.rules);
+	if (prof->dfa.rules->rule_count > 0) {
+		prof->dfa.dfa = prof->dfa.rules->create_dfa(&prof->dfa.size,
+							    dfaflags);
+		delete prof->dfa.rules;
 		prof->dfa.rules = NULL;
 		if (!prof->dfa.dfa)
 			goto out;
@@ -683,7 +679,7 @@
 {
 	int error = -1;
 
-	prof->policy.rules = aare_new_ruleset(0);
+	prof->policy.rules = new aare_rules();
 	if (!prof->policy.rules)
 		goto out;
 
@@ -694,26 +690,21 @@
 	 * to be supported
 	 */
 
-	if (kernel_supports_mount) {
-		if (!aare_add_rule(prof->policy.rules, mediates_mount, 0, AA_MAY_READ, 0, dfaflags))
+	if (kernel_supports_mount &&
+	    !prof->policy.rules->add_rule(mediates_mount, 0, AA_MAY_READ, 0, dfaflags))
 			goto out;
-		prof->policy.count++;
-	}
-	if (kernel_supports_dbus) {
-		if (!aare_add_rule(prof->policy.rules, mediates_dbus, 0, AA_MAY_READ, 0, dfaflags))
-			goto out;
-		prof->policy.count++;
-	}
-	if (prof->policy.count > 0) {
-		prof->policy.dfa = aare_create_dfa(prof->policy.rules,
-						  &prof->policy.size,
-						  dfaflags);
-		aare_delete_ruleset(prof->policy.rules);
+	if (kernel_supports_dbus &&
+	    !prof->policy.rules->add_rule(mediates_dbus, 0, AA_MAY_READ, 0, dfaflags))
+		goto out;
+	if (prof->policy.rules->rule_count > 0) {
+		prof->policy.dfa = prof->policy.rules->create_dfa(&prof->policy.size, dfaflags);
+		delete prof->policy.rules;
+
 		prof->policy.rules = NULL;
 		if (!prof->policy.dfa)
 			goto out;
 	} else {
-		aare_delete_ruleset(prof->policy.rules);
+		delete prof->policy.rules;
 		prof->policy.rules = NULL;
 	}
 
@@ -722,7 +713,7 @@
 	error = 0;
 
 out:
-	aare_delete_ruleset(prof->policy.rules);
+	delete prof->policy.rules;
 	prof->policy.rules = NULL;
 
 	return error;
--- 2.9-test.orig/parser/profile.cc
+++ 2.9-test/parser/profile.cc
@@ -67,11 +67,11 @@
 	for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end(); i++)
 		delete *i;
 	if (dfa.rules)
-		aare_delete_ruleset(dfa.rules);
+		delete dfa.rules;
 	if (dfa.dfa)
 		free(dfa.dfa);
 	if (policy.rules)
-		aare_delete_ruleset(policy.rules);
+		delete policy.rules;
 	if (policy.dfa)
 		free(policy.dfa);
 	if (xmatch)
--- 2.9-test.orig/parser/profile.h
+++ 2.9-test/parser/profile.h
@@ -18,6 +18,7 @@
 
 #include "parser.h"
 #include "rule.h"
+#include "libapparmor_re/aare_rules.h"
 
 class Profile;
 
@@ -120,12 +121,11 @@
 };
 
 struct dfa_stuff {
-	aare_ruleset_t *rules;
-	int count;
+	aare_rules *rules;
 	void *dfa;
 	size_t size;
 
-	dfa_stuff(void): rules(NULL), count(0), dfa(NULL), size(0) { }
+	dfa_stuff(void): rules(NULL), dfa(NULL), size(0) { }
 };
 
 class Profile {




More information about the AppArmor mailing list