parser: expand conditionals to allow comparisons

The apparmor parser supports if comparisons of boolean variables and
the definition status of set variables.

This commit expands the currently supported set to include comparisons
such as 'in', '>', '>=', '<', '<=', '==', and '!=' between
variables and/or text.

The comparison is done in lexicographical order, and since that can
cause issues comparing numbers, comparison between sets and numbers is
not allowed and the profile will fail to compile. Please refer to
apparmor.d.pod for example and details.

This commit also adds a file that generates test cases in the
parser. It is generated automatically with make check, but you can
generate them by running

make -C tst gen_conditionals

The generated tests will be under
tst/simple_tests/generated_conditional/

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>

parser/apparmor.d.pod

@@ -144,11 +144,15 @@ B<QUALIFIER BLOCK> = I<QUALIFIERS> I<BLOCK>
 
 B<INTEGER> = (+ | -)? [[:digit:]]+
 
-B<IF_BLOCK> = 'if' I<EXPR> I<BLOCK> [ 'else' I<BLOCK> ]
+B<IF_BLOCK> = 'if' I<EXPR> I<BLOCK> [ ( 'else' 'if'  I<EXPR> I<BLOCK> )* ] [ 'else' I<BLOCK> ]
 
 B<EXPR> = ( I<EXPR> ( 'and' | 'or' ) I<EXPR> | 'not' I<EXPR> | '(' I<EXPR> ')' | I<FACTOR> )
 
-B<FACTOR> = ( 'defined' I<VARIABLE> | [ 'defined' ] I<BOOL_VAR> )
+B<FACTOR> = ( 'defined' I<VARIABLE> | [ 'defined' ] I<BOOL_VAR> | ( I<TEXT> | I<VARIABLE> ) I<OPERATOR> ( I<TEXT> | I<VARIABLE> ) )
+
+B<OPERATOR> = ( 'in' | '==' | '!=' | '>' | '>=' | '<' | '<=' )
+
+B<BLOCK> = '{' ( I<RULES> )* '}'
 
 B<ACCESS TYPE> = ( 'allow' | 'deny' )
 
@@ -2226,17 +2230,53 @@ wrapped in an I<if> condition block with the condition expression
 being controlled by setting variable values.
 
 The condition expression can be composed of variables, boolean
-variables, a B<defined> check, open '(' and close ')' parentheses, and
+variables, a B<defined> check, the comparison operators B<in>, B<==>,
-the boolean operators B<and>, B<or>, and B<not>. Boolean operators are
+B<!=>, B<>>, B<>=>, B<<>, and B<<=>, open '(' and close ')' parentheses,
-evaluated left to write with priority in order of the following list.
+and the boolean operators B<and>, B<or>, and B<not>. Boolean operators
+are evaluated left to write with priority in order of the
+following list.
 
 =over 4
 
+=item B<in>	- tests whether the left text or variable is a subset of
+the text or variable on the right. The subset check is done by a full
+match, therefore partial string matches would not evaluate to true.
+
+=item B<==>	- tests whether both text or variable evaluate to the same
+value.
+
+=item B<!=>	- tests whether both text or variable evaluate to
+different values.
+
+=item B<>>	- tests whether the left text or variable is greater than
+the right text or variable.
+Comparisons between integers and set variables will fail to compile
+unless the variable contents are exactly one value, an integer. Other
+comparison will be strictly lexicographical.
+
+=item B<>=>	- tests whether the left text or variable is greater than
+or equal to the right text or variable.
+Comparisons between integers and set variables will fail to compile
+unless the variable contents are exactly one value, an integer. Other
+comparison will be strictly lexicographical.
+
+=item B<<>	- tests whether the left text or variable is lesser than
+the right text or variable.
+Comparisons between integers and set variables will fail to compile
+unless the variable contents are exactly one value, an integer. Other
+comparison will be strictly lexicographical.
+
+=item B<<=>	- tests whether the left text or variable is lesser than
+or equal to the right text or variable.
+Comparisons between integers and set variables will fail to compile
+unless the variable contents are exactly one value, an integer. Other
+comparison will be strictly lexicographical.
+
 =item B<defined> - tests whether the following variable or boolean
 variable has been defined/created.
 
 =item B<parentheses> - open '(' and close ')' paretheses are used to
-to group operations by priority.
+group operations by priority.
 
 =item B<not>	- boolean B<not> operator, negates the value of the
 following expression.
@@ -2249,9 +2289,6 @@ being combined with B<or> can be true for the result to be true.
 
 =back
 
-Currently set variables can only be included in the conditional
-expression when used with the defined check.
-
 An example of a profile conditional
 
 	if ${distro_mods} and defined @{HOME} {
@@ -2260,6 +2297,32 @@ An example of a profile conditional
 	  /home/*/.foo_file  rw,
 	}
 
+Since lexicographical comparisons using the B<>>, B<>=>, B<<>, B<<=>
+operators could lead to mistakes when comparing integers, comparisons
+between variables and integers will fail to compile unless the
+variable contains an integer.
+
+Eg.
+
+	@{BAR} = /home/user/ /home/user2/
+	@{BAR} += /home/user3/
+	@{TEST_VERSION} = 2
+	@{BAZ} = 10
+
+	/usr/bin/foo {
+	  if /home/user/ in @{BAR} {
+	    /** r,
+	  }
+	  if @{TEST_VERSION} >= @{BAZ} {
+	    /** w,
+	  } else if 10 > @{TEST_VERSION} {
+	    /** rw,
+	  }
+	  if @{BAZ} <= 10 {
+	    /** rw,
+	  }
+	}
+
 =head1 FILES
 
 =over 4

parser/cond_expr.cc

@@ -16,6 +16,8 @@
  *   Ltd.
  */
 
+#include <algorithm>
+
 #include "cond_expr.h"
 #include "parser.h"
 #include "symtab.h"
@@ -25,17 +27,16 @@ cond_expr::cond_expr(bool result):
 {
 }
 
-cond_expr::cond_expr(const char *var, bool defined)
+cond_expr::cond_expr(const char *var, cond_op op)
 {
 	variable *ref;
-	if (!defined) {
+	if (op == BOOLEAN_OP) {
 		ref = symtab::get_boolean_var(var);
 		if (!ref) {
-			/* FIXME check for set var */
 			yyerror(_("Unset boolean variable %s used in if-expression"), var);
 		}
 		result = ref->boolean;
-	} else {
+	} else if (op == DEFINED_OP) {
 		ref = symtab::get_set_var(var);
 		if (!ref) {
 			result = false;
@@ -43,5 +44,106 @@ cond_expr::cond_expr(const char *var, bool defined)
 			PDEBUG("Matched: defined set expr %s value %s\n", var, ref->expanded.begin()->c_str());
 			result = true;
 		}
+	} else
+		PERROR("Invalid operation for if-expression");
+}
+
+/* variables passed in conditionals can be variables or values.
+
+   if the string passed has the formatting of a variable (@{}), then
+   we should look for it in the symtab. if it's present in the symtab,
+   expand its values and return the expanded set. if it's not present
+   in the symtab, we should error out. if the string passed does not
+   have the formatting of a variable, we should treat it as if it was
+   a value. add it to a set and return it so comparisons can be made.
+*/
+std::set<std::string> cond_expr::get_set(const char *var)
+{
+	char *var_name = variable::process_var(var);
+	if (!var_name) {
+		/* not a variable */
+		return {var};
+	}
+	variable *ref = symtab::lookup_existing_symbol(var_name);
+	free(var_name);
+	if (!ref) {
+		yyerror(_("Error retrieving variable %s"), var);
+	}
+	if (ref->expand_variable() != 0) {
+		/* expand_variable prints error messages already, so
+		 * exit quietly here */
+		exit(1);
+	}
+	return ref->expanded;
+}
+
+template <typename T>
+void cond_expr::compare(cond_op op, T lhs, T rhs)
+{
+	switch (op) {
+	case GT_OP:
+		result = lhs > rhs;
+		break;
+	case GE_OP:
+		result = lhs >= rhs;
+		break;
+	case LT_OP:
+		result = lhs < rhs;
+		break;
+	case LE_OP:
+		result = lhs <= rhs;
+		break;
+	default:
+		PDEBUG("Invalid op\n");
+	}
+}
+
+bool nullstr(char *p)
+{
+	return p && !(*p);
+}
+
+long str_set_to_long(std::set<std::string> &src, char **endptr)
+{
+	long converted_src = 0;
+	errno = 0;
+	if (src.size() == 1 && !src.begin()->empty())
+		converted_src = strtol(src.begin()->c_str(), endptr, 0);
+	if (errno == ERANGE)
+		yyerror(_("Value out of valid range\n"));
+	return converted_src;
+}
+
+cond_expr::cond_expr(const char *lhv, cond_op op, const char *rhv)
+{
+	std::set<std::string> lhs = get_set(lhv);
+	std::set<std::string> rhs = get_set(rhv);
+	char *p_lhs = NULL, *p_rhs = NULL;
+	long converted_lhs = 0, converted_rhs = 0;
+
+	if (op == IN_OP) {
+		/* if lhs is a subset of rhs */
+		result = std::includes(rhs.begin(), rhs.end(),
+				       lhs.begin(), lhs.end());
+		return;
+	} else if (op == EQ_OP) {
+		result = lhs == rhs;
+		return;
+	} else if (op == NE_OP) {
+		result = lhs != rhs;
+		return;
+	}
+
+	converted_lhs = str_set_to_long(lhs, &p_lhs);
+	converted_rhs = str_set_to_long(rhs, &p_rhs);
+
+	if (!nullstr(p_lhs) && !nullstr(p_rhs)) {
+		/* sets */
+		compare(op, lhs, rhs);
+	} else if (nullstr(p_lhs) && nullstr(p_rhs)) {
+		/* numbers */
+		compare(op, converted_lhs, converted_rhs);
+	} else {
+		yyerror(_("Can only compare numbers with numbers\n"));
 	}
 }

parser/cond_expr.h

@@ -19,12 +19,31 @@
 #ifndef __AA_COND_EXPR_H
 #define __AA_COND_EXPR_H
 
+#include <set>
+#include <string>
+
+typedef enum {
+	EQ_OP,
+	NE_OP,
+	IN_OP,
+	GT_OP,
+	GE_OP,
+	LT_OP,
+	LE_OP,
+	BOOLEAN_OP,
+	DEFINED_OP,
+} cond_op;
+
 class cond_expr {
 private:
 	bool result;
 public:
 	cond_expr(bool result);
-	cond_expr(const char *var, bool defined);
+	cond_expr(const char *var, cond_op op);
+	cond_expr(const char *var, cond_op op, const char *cond_id);
+	std::set<std::string> get_set(const char *var);
+	template <typename T>
+	void compare(cond_op op, T lhs, T rhs);
 	virtual ~cond_expr()
 	{
 	};

parser/parser_lex.l

@@ -299,7 +299,11 @@ ADD_ASSIGN	\+=
 ARROW		->
 LT_EQUAL	<=
 LT		<
+GT_EQUAL	>=
 GT		>
+EQ_OP		==
+NE_OP		!=
+IN_OP		in
 
 /* IF adding new state please update state_names table and default rule (just
  * above the state_names table) at the eof.
@@ -649,15 +653,39 @@ GT		>
 	}
 }
 
+<INITIAL,EXPR_MODE>{
+	{SET_VARIABLE}	{
+		yylval.set_var = strdup(yytext);
+		RETURN_TOKEN(TOK_SET_VAR);
+	}
+
+	{BOOL_VARIABLE}	{
+		yylval.bool_var = strdup(yytext);
+		RETURN_TOKEN(TOK_BOOL_VAR);
+	}
+}
+
 <EXPR_MODE>{
+	{OPEN_BRACE}	{ POP_AND_RETURN(TOK_OPEN); }
+
 	{OPEN_PAREN}	{ RETURN_TOKEN(TOK_OPENPAREN); }
 	{CLOSE_PAREN}	{ RETURN_TOKEN(TOK_CLOSEPAREN); }
 	and		{ RETURN_TOKEN(TOK_AND); }
 	or		{ RETURN_TOKEN(TOK_OR); }
 	not		{ RETURN_TOKEN(TOK_NOT); }
-	defined		{ RETURN_TOKEN(TOK_DEFINED); }
+	defined	{ RETURN_TOKEN(TOK_DEFINED); }
+	{EQ_OP}	{ RETURN_TOKEN(TOK_EQ_OP); }
+	{NE_OP}	{ RETURN_TOKEN(TOK_NE_OP); }
+	{IN_OP}	{ RETURN_TOKEN(TOK_IN_OP); }
+	{GT}		{ RETURN_TOKEN(TOK_GT); }
+	{GT_EQUAL}	{ RETURN_TOKEN(TOK_GE); }
+	{LT}		{ RETURN_TOKEN(TOK_LT); }
+	{LT_EQUAL}	{ RETURN_TOKEN(TOK_LE); }
 
-	{OPEN_BRACE}	{ POP_AND_RETURN(TOK_OPEN); }
+	({IDS_NOEQ}|{LABEL}|{QUOTED_ID}) {
+		yylval.id = processid(yytext, yyleng);
+		RETURN_TOKEN(TOK_ID);
+	}
 }
 
 #include{WS}+if{WS}+exists/{WS}.*\r?\n	{
@@ -707,18 +735,6 @@ all/({WS}|[^[:alnum:]_]) {
 
 {ADD_ASSIGN}	{ PUSH_AND_RETURN(ASSIGN_MODE, TOK_ADD_ASSIGN); }
 
-<INITIAL,EXPR_MODE>{
-	{SET_VARIABLE}	{
-		yylval.set_var = strdup(yytext);
-		RETURN_TOKEN(TOK_SET_VAR);
-	}
-
-	{BOOL_VARIABLE}	{
-		yylval.bool_var = strdup(yytext);
-		RETURN_TOKEN(TOK_BOOL_VAR);
-	}
-}
-
 {OPEN_BRACE}	{ RETURN_TOKEN(TOK_OPEN); }
 
 {CLOSE_BRACE}	{ RETURN_TOKEN(TOK_CLOSE); }

parser/parser_yacc.y

@@ -154,6 +154,12 @@ static void abi_features(char *filename, bool search);
 %token TOK_PRIORITY
 %token TOK_AND
 %token TOK_OR
+%token TOK_EQ_OP
+%token TOK_NE_OP
+%token TOK_IN_OP
+%token TOK_GT
+%token TOK_GE
+%token TOK_LT
 
  /* rlimits */
 %token TOK_RLIMIT
@@ -984,7 +990,7 @@ notfactor:	TOK_NOT notfactor
 
 factor:	TOK_BOOL_VAR
 	{
-		cond_expr *conds = new cond_expr($1, false);
+		cond_expr *conds = new cond_expr($1, BOOLEAN_OP);
 		PDEBUG("Matched: boolean expr %s value: %d\n", $1, conds->eval());
 		$$ = conds;
 		free($1);
@@ -992,7 +998,7 @@ factor:	TOK_BOOL_VAR
 
 factor:	TOK_DEFINED TOK_SET_VAR
 	{
-		cond_expr *conds = new cond_expr($2, true);
+		cond_expr *conds = new cond_expr($2, DEFINED_OP);
 		PDEBUG("Matched: defined set expr %s value %d\n", $2, conds->eval());
 		$$ = conds;
 		free($2);
@@ -1000,7 +1006,7 @@ factor:	TOK_DEFINED TOK_SET_VAR
 
 factor:	TOK_DEFINED TOK_BOOL_VAR
 	{
-		cond_expr *conds = new cond_expr($2, false);
+		cond_expr *conds = new cond_expr($2, BOOLEAN_OP);
 		PDEBUG("Matched: defined set expr %s value %d\n", $2, conds->eval());
 		$$ = conds;
 		free($2);
@@ -1011,6 +1017,69 @@ factor:   TOK_OPENPAREN expr TOK_CLOSEPAREN
 		$$ = $2;
 	}
 
+factor:	id_or_var TOK_EQ_OP id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, EQ_OP, $3);
+		PDEBUG("Matched: equal set expr %s == %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_NE_OP id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, NE_OP, $3);
+		PDEBUG("Matched: not equal set expr %s != %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_IN_OP id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, IN_OP, $3);
+		PDEBUG("Matched: in set expr %s in %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_GT id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, GT_OP, $3);
+		PDEBUG("Matched: greater set expr %s > %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_GE id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, GE_OP, $3);
+		PDEBUG("Matched: greater or equal set expr %s >= %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_LT id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, LT_OP, $3);
+		PDEBUG("Matched: less set expr %s < %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
+factor:	id_or_var TOK_LE id_or_var
+	{
+		cond_expr *conds = new cond_expr($1, LE_OP, $3);
+		PDEBUG("Matched: less or equal set expr %s <= %s value %d\n", $1, $3, conds->eval());
+		$$ = conds;
+		free($1);
+		free($3);
+	}
+
 id_or_var: TOK_ID { $$ = $1; }
 id_or_var: TOK_SET_VAR { $$ = $1; };
 

parser/tst/Makefile

@@ -17,10 +17,10 @@ endif
 
 all: tests
 
-.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality dirtest valgrind
+.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality dirtest valgrind gen_conditionals
 tests: error_output caching minimize equality dirtest parser_sanity
 
-GEN_TRANS_DIRS=simple_tests/generated_x/ simple_tests/generated_perms_leading/ simple_tests/generated_perms_safe/ simple_tests/generated_dbus
+GEN_TRANS_DIRS=simple_tests/generated_x/ simple_tests/generated_perms_leading/ simple_tests/generated_perms_safe/ simple_tests/generated_dbus simple_tests/generated_conditional/
 
 gen_xtrans: $(GEN_TRANS_DIRS)
 	./gen-xtrans.py
@@ -31,10 +31,13 @@ $(GEN_TRANS_DIRS):
 gen_dbus: $(GEN_TRANS_DIRS)
 	./gen-dbus.py
 
+gen_conditionals: $(GEN_TRANS_DIRS)
+	./gen-conditionals.py
+
 error_output: $(PARSER)
 	LANG=C ./errors.py -p "$(PARSER)" $(PYTEST_ARG)
 
-parser_sanity: $(PARSER) gen_xtrans gen_dbus
+parser_sanity: $(PARSER) gen_xtrans gen_dbus gen_conditionals
 	$(Q)LANG=C APPARMOR_PARSER="$(PARSER)" ${PROVE} ${PROVE_ARG} ${TESTS}
 
 # use this target for faster manual testing if you don't want/need to test all the profiles generated by gen-*.py

parser/tst/gen-conditionals.py

@@ -0,0 +1,75 @@
+#!/usr/bin/python3
+#
+#   Copyright (c) 2025 Canonical, Ltd. (All rights reserved)
+#
+#   This program is free software; you can redistribute it and/or
+#   modify it under the terms of version 2 of the GNU General Public
+#   License published by the Free Software Foundation.
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program; if not, contact Canonical Ltd.
+#
+
+from testlib import write_file
+
+def gen_file(test, xres, op, lhs, rhs, add_else):
+    global count
+
+    content = ''
+    content += '#\n'
+    content += '#=DESCRIPTION {}\n'.format(test)
+    content += '#=EXRESULT {}\n'.format(xres)
+    content += '#\n'
+    if lhs['def'] and lhs != rhs:
+        content += '{} = {}\n'.format(lhs['varname'], lhs['def'])
+    if rhs['def']:
+        content += '{} = {}\n'.format(rhs['varname'], rhs['def'])
+    content += '/usr/bin/foo {\n'
+    content += '  if {} {} {} {{\n'.format(lhs['varname'], op, rhs['varname'])
+    content += '    /bin/true rix,\n'
+    content += '  }'
+    if add_else:
+        content += ' else {\n'
+        content += '    mount,\n'
+        content += '  }\n'
+    else:
+        content += '\n'
+
+    content += '}\n'
+
+    write_file('simple_tests/generated_conditional', '{}{}-{}.sd'.format(test, '-else' if add_else else '', count), content)
+
+    count += 1
+
+ops = {'==': 'equals', '!=': 'notequals', 'in': 'in', '>': 'greater', '>=': 'greaterequals', '<': 'lesser', '<=': 'lesserequals'}
+
+test_vars = [
+    {'varname': '@{VAR_EMPTY}', 'def': '""', 'desc': 'empty', 'number': False},  # empty var
+    {'varname': '@{VAR_ONE_STRING}', 'def': '/path/foo/', 'desc': 'var_one_string', 'number': False},  # one string in var
+    {'varname': '@{VAR_ONE_NUMBER}', 'def': '10', 'desc': 'var_one_number', 'number': True},  # one number in var
+    {'varname': '@{VAR_MULT_STRING}', 'def': '/path/foo/ /path/bar/', 'desc': 'var_mult_string', 'number': False},  # multiple strings in var
+    {'varname': '@{VAR_MULT_NUMBER}', 'def': '10 2 3.1', 'desc': 'var_mult_number', 'number': False},  # multiple numbers in var
+    {'varname': '@{VAR_MIXED}', 'def': '3 /foo 1 /bar/ 10 /path/foo/', 'desc': 'var_mixed', 'number': False},  # mixed var contents
+    {'varname': '10', 'def': '', 'desc': 'number1', 'number': True},  # number directly
+    {'varname': '9', 'def': '', 'desc': 'number2', 'number': True},  # number directly
+    {'varname': '/path/foo/', 'def': '', 'desc': 'string1', 'number': False},  # string directly
+    {'varname': '/path/baz/', 'def': '', 'desc': 'string2', 'number': False},  # string directly
+]
+
+def gen_files():
+    for op in ops:
+        for lhs in test_vars:
+            for rhs in test_vars:
+                for add_else in [True, False]:
+                    test_description = lhs['desc'] + '-' + ops[op]  + '-' + rhs['desc']
+                    xres = 'PASS' if lhs['number'] == rhs['number'] or op in ['in', '==', '!='] else 'FAIL'
+                    gen_file(test_description, xres, op, lhs, rhs, add_else)
+
+count = 0
+gen_files()
+print('Generated {} conditional tests'.format(count))

parser/variable.cc

@@ -66,8 +66,6 @@ char *variable::process_var(const char *var)
 		orig++;
 		len--;
 	} else {
-		PERROR("ASSERT: Found var '%s' without variable prefix\n",
-		       var);
 		return NULL;
 	}