parser: convert xmatch to use out of band transitions

xattrs can contain NULL characters in their values which means we can
not user regular NULL transitions to separate values. To fix this
use out of band transition instead.

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

parser/libapparmor_re/aare_rules.cc

@@ -140,13 +140,14 @@ bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit,
 
 /*
  * append_rule is like add_rule, but appends the rule to any existing rules
- * with a null transition. The appended rule matches with the same permissions
+ * with a separating transition. The appended rule matches with the same
+ * permissions
  * as the rule it's appended to.
  *
  * This is used by xattrs matching where, after matching the path, the DFA is
  * advanced by a null character for each xattr.
  */
-bool aare_rules::append_rule(const char *rule, dfaflags_t flags)
+bool aare_rules::append_rule(const char *rule, bool oob, dfaflags_t flags)
 {
 	Node *tree = NULL;
 	if (regex_parse(&tree, rule))
@@ -162,13 +163,13 @@ bool aare_rules::append_rule(const char *rule, dfaflags_t flags)
 
 	/*
 	 * For each matching state, we want to create an optional path
-	 * separated by a null character.
+	 * separated by a separating character.
 	 *
 	 * When matching xattrs, the DFA must end up in an accepting state for
 	 * the path, then each value of the xattrs. Using an optional node
 	 * lets each rule end up in an accepting state.
 	 */
-	tree = new OptionalNode(new CatNode(new CharNode(0), tree));
+	tree = new OptionalNode(new CatNode(oob ? new CharNode(transchar(-1, true)) : new CharNode(0), tree));
 	PermExprMap::iterator it;
 	for (it = expr_map.begin(); it != expr_map.end(); it++) {
 		expr_map[it->first] = new CatNode(it->second, tree);

parser/libapparmor_re/aare_rules.h

@@ -104,7 +104,7 @@ class aare_rules {
 		      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, bool oob);
-	bool append_rule(const char *rule, dfaflags_t flags);
+	bool append_rule(const char *rule, bool oob, dfaflags_t flags);
 	void *create_dfa(size_t *size, int *min_match_len, dfaflags_t flags);
 };
 

parser/libapparmor_re/chfa.cc

@@ -406,7 +406,7 @@ void CHFA::flex_table(ostream &os, const char *name)
 	}
 
 	/* Write the actual flex parser table. */
-
+	/* TODO: add max_oob */
 	size_t hsize = pad64(sizeof(th) + sizeof(th_version) + strlen(name) + 1);
 	th.th_magic = htonl(YYTH_REGEX_MAGIC);
 	th.th_flags = htons(chfaflags);

parser/libapparmor_re/expr-tree.h

@@ -279,11 +279,11 @@ public:
 	 */
 	virtual int min_match_len() { return 0; }
 	/*
-	 * contains_null returns if the expression tree contains a null character.
-	 * Null characters indicate that the rest of the DFA matches the xattrs and
-	 * not the path. This is used to compute min_match_len.
+	 * contains_oob returns if the expression tree contains a oob character.
+	 * oob characters indicate that the rest of the DFA matches has an
+	 * out of band transition. This is used to compute min_match_len.
 	 */
-	virtual bool contains_null() { return false; }
+	virtual bool contains_oob() { return false; }
 
 	virtual int eq(Node *other) = 0;
 	virtual ostream &dump(ostream &os) = 0;
@@ -421,14 +421,17 @@ public:
 
 	int min_match_len()
 	{
-		if (c == 0) {
-			// Null character indicates end of string.
+		if (c < 0) {
+			// oob characters indicates end of string.
+			// note: does NOT currently calc match len
+			// base on NULL char separator transitions
+			// which some match rules use.
 			return 0;
 		}
 		return 1;
 	}
 
-	bool contains_null() { return c == 0; }
+	bool contains_oob() { return c < 0; }
 
 	transchar c;
 };
@@ -473,16 +476,16 @@ public:
 
 	int min_match_len()
 	{
-		if (contains_null()) {
+		if (contains_oob()) {
 			return 0;
 		}
 		return 1;
 	}
 
-	bool contains_null()
+	bool contains_oob()
 	{
 		for (Chars::iterator i = chars.begin(); i != chars.end(); i++) {
-			if (*i == 0) {
+			if (*i < 0) {
 				return true;
 			}
 		}
@@ -540,16 +543,16 @@ public:
 
 	int min_match_len()
 	{
-		if (contains_null()) {
+		if (contains_oob()) {
 			return 0;
 		}
 		return 1;
 	}
 
-	bool contains_null()
+	bool contains_oob()
 	{
 		for (Chars::iterator i = chars.begin(); i != chars.end(); i++) {
-			if (*i == 0) {
+			if (*i < 0) {
 				return false;
 			}
 		}
@@ -581,8 +584,6 @@ public:
 		return 0;
 	}
 	ostream &dump(ostream &os) { return os << "."; }
-
-	bool contains_null() { return true; }
 };
 
 /* Match a node zero or more times. (This is a unary operator.) */
@@ -611,7 +612,7 @@ public:
 		return os << ")*";
 	}
 
-	bool contains_null() { return child[0]->contains_null(); }
+	bool contains_oob() { return child[0]->contains_oob(); }
 };
 
 /* Match a node zero or one times. */
@@ -660,7 +661,7 @@ public:
 		return os << ")+";
 	}
 	int min_match_len() { return child[0]->min_match_len(); }
-	bool contains_null() { return child[0]->contains_null(); }
+	bool contains_oob() { return child[0]->contains_oob(); }
 };
 
 /* Match a pair of consecutive nodes. */
@@ -711,18 +712,18 @@ public:
 	int min_match_len()
 	{
 		int len = child[0]->min_match_len();
-		if (child[0]->contains_null()) {
-			// Null characters are used to indicate when the DFA transitions
+		if (child[0]->contains_oob()) {
+			// oob characters are used to indicate when the DFA transitions
 			// from matching the path to matching the xattrs. If the left child
-			// contains a null character, the right side doesn't contribute to
+			// contains an oob character, the right side doesn't contribute to
 			// the path match.
 			return len;
 		}
 		return len + child[1]->min_match_len();
 	}
-	bool contains_null()
+	bool contains_oob()
 	{
-		return child[0]->contains_null() || child[1]->contains_null();
+		return child[0]->contains_oob() || child[1]->contains_oob();
 	}
 };
 
@@ -771,9 +772,9 @@ public:
 		}
 		return m2;
 	}
-	bool contains_null()
+	bool contains_oob()
 	{
-		return child[0]->contains_null() || child[1]->contains_null();
+		return child[0]->contains_oob() || child[1]->contains_oob();
 	}
 };
 

parser/parser_regex.c

@@ -512,7 +512,7 @@ static int process_profile_name_xmatch(Profile *prof)
 				convert_aaregex_to_pcre(xattr_value, 0,
 							glob_default, tbuf,
 							&len);
-				if (!rules->append_rule(tbuf.c_str(), dfaflags)) {
+				if (!rules->append_rule(tbuf.c_str(), true, dfaflags)) {
 					delete rules;
 					return FALSE;
 				}