diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod index 6840ce544..237c1583a 100644 --- a/parser/apparmor.d.pod +++ b/parser/apparmor.d.pod @@ -144,11 +144,15 @@ B = I I B = (+ | -)? [[:digit:]]+ -B = 'if' I I [ 'else' I ] +B = 'if' I I [ ( 'else' 'if' I I )* ] [ 'else' I ] B = ( I ( 'and' | 'or' ) I | 'not' I | '(' I ')' | I ) -B = ( 'defined' I | [ 'defined' ] I ) +B = ( 'defined' I | [ 'defined' ] I | ( I | I ) I ( I | I ) ) + +B = ( 'in' | '==' | '!=' | '>' | '>=' | '<' | '<=' ) + +B = '{' ( I )* '}' B = ( 'allow' | 'deny' ) @@ -2226,17 +2230,53 @@ wrapped in an I condition block with the condition expression being controlled by setting variable values. The condition expression can be composed of variables, boolean -variables, a B check, open '(' and close ')' parentheses, and -the boolean operators B, B, and B. Boolean operators are -evaluated left to write with priority in order of the following list. +variables, a B check, the comparison operators B, B<==>, +B, B<>>, B<>=>, B<<>, and B<<=>, open '(' and close ')' parentheses, +and the boolean operators B, B, and B. Boolean operators +are evaluated left to write with priority in order of the +following list. =over 4 +=item B - 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 - tests whether the following variable or boolean variable has been defined/created. =item B - open '(' and close ')' paretheses are used to -to group operations by priority. +group operations by priority. =item B - boolean B operator, negates the value of the following expression. @@ -2249,9 +2289,6 @@ being combined with B 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 diff --git a/parser/cond_expr.cc b/parser/cond_expr.cc index f601618a1..ffa784db8 100644 --- a/parser/cond_expr.cc +++ b/parser/cond_expr.cc @@ -16,6 +16,8 @@ * Ltd. */ +#include + #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 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 +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 &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 lhs = get_set(lhv); + std::set 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")); } } diff --git a/parser/cond_expr.h b/parser/cond_expr.h index 13bd8d524..9ab405e47 100644 --- a/parser/cond_expr.h +++ b/parser/cond_expr.h @@ -19,12 +19,31 @@ #ifndef __AA_COND_EXPR_H #define __AA_COND_EXPR_H +#include +#include + +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 get_set(const char *var); + template + void compare(cond_op op, T lhs, T rhs); virtual ~cond_expr() { }; diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 807498f2d..6664f63bc 100644 --- a/parser/parser_lex.l +++ b/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 > } } +{ + {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} { 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); } -{ - {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); } diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 86f497002..5612359be 100644 --- a/parser/parser_yacc.y +++ b/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; }; diff --git a/parser/tst/Makefile b/parser/tst/Makefile index 460cf6493..0ecb6ec67 100644 --- a/parser/tst/Makefile +++ b/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 diff --git a/parser/tst/gen-conditionals.py b/parser/tst/gen-conditionals.py new file mode 100755 index 000000000..c8610ae9d --- /dev/null +++ b/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)) diff --git a/parser/variable.cc b/parser/variable.cc index b0deb4097..1e5ad4900 100644 --- a/parser/variable.cc +++ b/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; }