From bf82f41d9015c48e527bef2e43c7783fba58eb25 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 21 Apr 2020 16:25:29 -0700 Subject: [PATCH 1/5] parser: add support for boolean expressions in IF conditionals Currently the parser doesn't support 'and' and 'or' operations. However these are going to become important once being able to test for what features are supported lands. Signed-off-by: John Johansen --- parser/apparmor.d.pod | 61 +++++++++++++++++++- parser/parser_lex.l | 39 +++++++++---- parser/parser_misc.c | 2 + parser/parser_yacc.y | 47 +++++++++++++-- parser/tst/simple_tests/conditional/bad_4.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_10.sd | 11 ++++ parser/tst/simple_tests/conditional/ok_11.sd | 11 ++++ parser/tst/simple_tests/conditional/ok_12.sd | 11 ++++ parser/tst/simple_tests/conditional/ok_13.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_14.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_15.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_16.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_17.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_18.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_19.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_20.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_21.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_22.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_23.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_24.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_25.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_26.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_27.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_28.sd | 12 ++++ parser/tst/simple_tests/conditional/ok_29.sd | 12 ++++ utils/test/test-parser-simple-tests.py | 20 +++++++ 26 files changed, 402 insertions(+), 16 deletions(-) create mode 100644 parser/tst/simple_tests/conditional/bad_4.sd create mode 100644 parser/tst/simple_tests/conditional/ok_10.sd create mode 100644 parser/tst/simple_tests/conditional/ok_11.sd create mode 100644 parser/tst/simple_tests/conditional/ok_12.sd create mode 100644 parser/tst/simple_tests/conditional/ok_13.sd create mode 100644 parser/tst/simple_tests/conditional/ok_14.sd create mode 100644 parser/tst/simple_tests/conditional/ok_15.sd create mode 100644 parser/tst/simple_tests/conditional/ok_16.sd create mode 100644 parser/tst/simple_tests/conditional/ok_17.sd create mode 100644 parser/tst/simple_tests/conditional/ok_18.sd create mode 100644 parser/tst/simple_tests/conditional/ok_19.sd create mode 100644 parser/tst/simple_tests/conditional/ok_20.sd create mode 100644 parser/tst/simple_tests/conditional/ok_21.sd create mode 100644 parser/tst/simple_tests/conditional/ok_22.sd create mode 100644 parser/tst/simple_tests/conditional/ok_23.sd create mode 100644 parser/tst/simple_tests/conditional/ok_24.sd create mode 100644 parser/tst/simple_tests/conditional/ok_25.sd create mode 100644 parser/tst/simple_tests/conditional/ok_26.sd create mode 100644 parser/tst/simple_tests/conditional/ok_27.sd create mode 100644 parser/tst/simple_tests/conditional/ok_28.sd create mode 100644 parser/tst/simple_tests/conditional/ok_29.sd diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod index 373ddade4..6840ce544 100644 --- a/parser/apparmor.d.pod +++ b/parser/apparmor.d.pod @@ -72,10 +72,12 @@ B = ( [ I ] [ I ] )* B = ( I | I | I | I | I )* Variable assignment and alias rules must come before the profile. -B = I ('=' | '+=') (space separated values) +B = ( I | I ) ('=' | '+=') (space separated values) B = '@{' I [ ( I | '_' ) ... ] '}' +B = '${' I [ ( I | '_' ) ... ] '}' + B = 'alias' I '-E' I ',' B = ( '#include' | 'include' ) [ 'if exists' ] ( I | I ) @@ -130,7 +132,7 @@ B = ( I | I ) [ '\r' ] '\n' B = ( I | I | I | I | I | I | I | I | I | I | I | I | I | I) -B = ( I | I | I ) +B = ( I | I | I | I ) B = 'profile' I [ I ] [ I ] '{' ( I )* '}' @@ -142,6 +144,12 @@ B = I I B = (+ | -)? [[:digit:]]+ +B = 'if' I I [ 'else' I ] + +B = ( I ( 'and' | 'or' ) I | 'not' I | '(' I ')' | I ) + +B = ( 'defined' I | [ 'defined' ] I ) + B = ( 'allow' | 'deny' ) B = [ 'priority' '=' ] [ 'audit' ] [ I ] @@ -1880,6 +1888,13 @@ example) is collapsed. =back +=head3 Boolean Variables + +In addition to the set variables AppArmor supports boolean +variables. These begin with a B<$> and can only be used in conditional +expressions. Boolean variables provide a convenient way to +enable/disable policy rules that have been wrapped in the proper if +condition. =head2 Alias rules @@ -2203,6 +2218,48 @@ An example AppArmor profile: } } +=head2 Conditional rules + +AppArmor provides a mechanism to conditionally enable and disable +rules in a profile. Rules that are to be conditionally used can be +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. + +=over 4 + +=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. + +=item B - boolean B operator, negates the value of the +following expression. + +=item B - boolean B operator, both expressions being +combined with B must be true for the result to be true. + +=item B - boolean B operator, either one or both expressions +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} { + /@{HOME}/.foo_file rw, + } else { + /home/*/.foo_file rw, + } + =head1 FILES =over 4 diff --git a/parser/parser_lex.l b/parser/parser_lex.l index f7b3ff4db..807498f2d 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -336,6 +336,8 @@ GT > %x MQUEUE_MODE %x IOURING_MODE %x INTEGER_MODE +%x EXPR_MODE + %% %{ @@ -347,7 +349,7 @@ GT > } %} -{ +{ {WS}+ { DUMP_PREPROCESS; /* Ignoring whitespace */ } } @@ -647,6 +649,17 @@ GT > } } +{ + {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); } + + {OPEN_BRACE} { POP_AND_RETURN(TOK_OPEN); } +} + #include{WS}+if{WS}+exists/{WS}.*\r?\n { /* Don't use PUSH() macro here as we don't want #include echoed out. * It needs to be handled specially @@ -694,14 +707,16 @@ 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); -} +{ + {SET_VARIABLE} { + yylval.set_var = strdup(yytext); + RETURN_TOKEN(TOK_SET_VAR); + } -{BOOL_VARIABLE} { - yylval.bool_var = strdup(yytext); - RETURN_TOKEN(TOK_BOOL_VAR); + {BOOL_VARIABLE} { + yylval.bool_var = strdup(yytext); + RETURN_TOKEN(TOK_BOOL_VAR); + } } {OPEN_BRACE} { RETURN_TOKEN(TOK_OPEN); } @@ -772,6 +787,9 @@ all/({WS}|[^[:alnum:]_]) { case TOK_MQUEUE: state = MQUEUE_MODE; break; + case TOK_IF: + state = EXPR_MODE; + break; default: /* nothing */ break; } @@ -786,14 +804,14 @@ all/({WS}|[^[:alnum:]_]) { } } -{ +{ \r?\n { DUMP_PREPROCESS; current_lineno++; } } -{ +{ (.|\n) { DUMP_PREPROCESS; /* Something we didn't expect */ @@ -832,4 +850,5 @@ unordered_map state_names = { STATE_TABLE_ENT(MQUEUE_MODE), STATE_TABLE_ENT(IOURING_MODE), STATE_TABLE_ENT(INTEGER_MODE), + STATE_TABLE_ENT(EXPR_MODE), }; diff --git a/parser/parser_misc.c b/parser/parser_misc.c index 4634b3f43..12a71306f 100644 --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -134,6 +134,8 @@ static const unordered_map keyword_table = { {"sqpoll", TOK_SQPOLL}, {"all", TOK_ALL}, {"priority", TOK_PRIORITY}, + {"and", TOK_AND}, + {"or", TOK_OR}, }; /* glibc maps bsd ofile to nofile but musl does not. */ diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 0b5d4111f..86f497002 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -152,6 +152,8 @@ static void abi_features(char *filename, bool search); %token TOK_SQPOLL %token TOK_ALL %token TOK_PRIORITY +%token TOK_AND +%token TOK_OR /* rlimits */ %token TOK_RLIMIT @@ -270,6 +272,9 @@ static void abi_features(char *filename, bool search); %type TOK_VALUE %type valuelist %type expr +%type term +%type notfactor +%type factor %type id_or_var %type opt_id_or_var %type opt_subset_flag @@ -941,14 +946,43 @@ cond_rule: TOK_IF expr block TOK_ELSE cond_rule $$ = ret; } -expr: TOK_NOT expr + +expr: expr TOK_OR term + { + cond_expr *conds = new cond_expr($1->eval() || $3->eval()); + delete $1; + delete $3; + $$ = conds; + } + | term + { + $$ = $1; + } + +term: term TOK_AND notfactor + { + cond_expr *conds = new cond_expr($1->eval() && $3->eval()); + delete $1; + delete $3; + $$ = conds; + } + | notfactor + { + $$ = $1; + } + +notfactor: TOK_NOT notfactor { cond_expr *conds = new cond_expr(!$2->eval()); delete $2; $$ = conds; } + | factor + { + $$ = $1; + } -expr: TOK_BOOL_VAR +factor: TOK_BOOL_VAR { cond_expr *conds = new cond_expr($1, false); PDEBUG("Matched: boolean expr %s value: %d\n", $1, conds->eval()); @@ -956,7 +990,7 @@ expr: TOK_BOOL_VAR free($1); } -expr: TOK_DEFINED TOK_SET_VAR +factor: TOK_DEFINED TOK_SET_VAR { cond_expr *conds = new cond_expr($2, true); PDEBUG("Matched: defined set expr %s value %d\n", $2, conds->eval()); @@ -964,7 +998,7 @@ expr: TOK_DEFINED TOK_SET_VAR free($2); } -expr: TOK_DEFINED TOK_BOOL_VAR +factor: TOK_DEFINED TOK_BOOL_VAR { cond_expr *conds = new cond_expr($2, false); PDEBUG("Matched: defined set expr %s value %d\n", $2, conds->eval()); @@ -972,6 +1006,11 @@ expr: TOK_DEFINED TOK_BOOL_VAR free($2); } +factor: TOK_OPENPAREN expr TOK_CLOSEPAREN + { + $$ = $2; + } + id_or_var: TOK_ID { $$ = $1; } id_or_var: TOK_SET_VAR { $$ = $1; }; diff --git a/parser/tst/simple_tests/conditional/bad_4.sd b/parser/tst/simple_tests/conditional/bad_4.sd new file mode 100644 index 000000000..f3e76e3eb --- /dev/null +++ b/parser/tst/simple_tests/conditional/bad_4.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION @FOO is not a boolean variable, but $FOO is defined +#=EXRESULT FAIL + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not @FOO { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_10.sd b/parser/tst/simple_tests/conditional/ok_10.sd new file mode 100644 index 000000000..82cdf88bf --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_10.sd @@ -0,0 +1,11 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true + +/bin/true { + if $FOO and $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_11.sd b/parser/tst/simple_tests/conditional/ok_11.sd new file mode 100644 index 000000000..aa544023b --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_11.sd @@ -0,0 +1,11 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true + +/bin/true { + if $FOO or $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_12.sd b/parser/tst/simple_tests/conditional/ok_12.sd new file mode 100644 index 000000000..d51c32916 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_12.sd @@ -0,0 +1,11 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true + +/bin/true { + if ( $FOO and $BAR ) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_13.sd b/parser/tst/simple_tests/conditional/ok_13.sd new file mode 100644 index 000000000..989aa864d --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_13.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if $FOO and $BAR and $THREE { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_14.sd b/parser/tst/simple_tests/conditional/ok_14.sd new file mode 100644 index 000000000..1765ca1f0 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_14.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if ($FOO and $BAR) and $THREE { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_15.sd b/parser/tst/simple_tests/conditional/ok_15.sd new file mode 100644 index 000000000..65fba63ac --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_15.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if $FOO and ($BAR and $THREE) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_16.sd b/parser/tst/simple_tests/conditional/ok_16.sd new file mode 100644 index 000000000..3d7e34c4d --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_16.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if $FOO and (($BAR and $THREE)) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_17.sd b/parser/tst/simple_tests/conditional/ok_17.sd new file mode 100644 index 000000000..9a54b8aef --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_17.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if ($FOO and (($BAR and $THREE))) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_18.sd b/parser/tst/simple_tests/conditional/ok_18.sd new file mode 100644 index 000000000..7387606e2 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_18.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if ($FOO) and ($BAR or $THREE) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_19.sd b/parser/tst/simple_tests/conditional/ok_19.sd new file mode 100644 index 000000000..7425d145f --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_19.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if ($FOO and $BAR) or $THREE { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_20.sd b/parser/tst/simple_tests/conditional/ok_20.sd new file mode 100644 index 000000000..d3380dc67 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_20.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if (($FOO and $BAR) or $THREE) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_21.sd b/parser/tst/simple_tests/conditional/ok_21.sd new file mode 100644 index 000000000..1bebcf590 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_21.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if (($FOO and $BAR) or ($THREE)) { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_22.sd b/parser/tst/simple_tests/conditional/ok_22.sd new file mode 100644 index 000000000..5f8cdbe81 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_22.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not $FOO and $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_23.sd b/parser/tst/simple_tests/conditional/ok_23.sd new file mode 100644 index 000000000..6c758652d --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_23.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not $FOO and not $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_24.sd b/parser/tst/simple_tests/conditional/ok_24.sd new file mode 100644 index 000000000..47e441f3f --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_24.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not not $FOO and not $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_25.sd b/parser/tst/simple_tests/conditional/ok_25.sd new file mode 100644 index 000000000..cd150e592 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_25.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if defined $FOO and $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_26.sd b/parser/tst/simple_tests/conditional/ok_26.sd new file mode 100644 index 000000000..8d36de1aa --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_26.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if defined $FOO and defined $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_27.sd b/parser/tst/simple_tests/conditional/ok_27.sd new file mode 100644 index 000000000..1989b4738 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_27.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not defined $FOO and defined $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_28.sd b/parser/tst/simple_tests/conditional/ok_28.sd new file mode 100644 index 000000000..f32500a47 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_28.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not defined $FOO and not defined $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/ok_29.sd b/parser/tst/simple_tests/conditional/ok_29.sd new file mode 100644 index 000000000..bdf19dd50 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_29.sd @@ -0,0 +1,12 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +$BAR=true +$THREE=true + +/bin/true { + if not defined @FOO or not defined $BAR { + /bin/true rix, + } +} diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 833dd8e43..9bca20507 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -360,6 +360,26 @@ syntax_failure = ( 'conditional/ok_8.sd', 'conditional/ok_9.sd', 'conditional/stress_1.sd', + 'conditional/ok_10.sd', + 'conditional/ok_11.sd', + 'conditional/ok_12.sd', + 'conditional/ok_13.sd', + 'conditional/ok_14.sd', + 'conditional/ok_15.sd', + 'conditional/ok_16.sd', + 'conditional/ok_17.sd', + 'conditional/ok_18.sd', + 'conditional/ok_19.sd', + 'conditional/ok_20.sd', + 'conditional/ok_21.sd', + 'conditional/ok_22.sd', + 'conditional/ok_23.sd', + 'conditional/ok_24.sd', + 'conditional/ok_25.sd', + 'conditional/ok_26.sd', + 'conditional/ok_27.sd', + 'conditional/ok_28.sd', + 'conditional/ok_29.sd', # unexpected uppercase vs. lowercase in *x rules 'file/ok_5.sd', # Invalid mode UX From 8d0c248fe40bd60b89538bbd6dda94328d8d10ee Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Fri, 20 Jun 2025 18:43:31 -0300 Subject: [PATCH 2/5] 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 --- parser/apparmor.d.pod | 81 +++++++++++++++++++++--- parser/cond_expr.cc | 110 +++++++++++++++++++++++++++++++-- parser/cond_expr.h | 21 ++++++- parser/parser_lex.l | 44 ++++++++----- parser/parser_yacc.y | 75 +++++++++++++++++++++- parser/tst/Makefile | 9 ++- parser/tst/gen-conditionals.py | 75 ++++++++++++++++++++++ parser/variable.cc | 2 - 8 files changed, 381 insertions(+), 36 deletions(-) create mode 100755 parser/tst/gen-conditionals.py 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; } From 85290ca14c8ac7963df6632708c38daba620d9c9 Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Fri, 18 Jul 2025 12:04:05 -0300 Subject: [PATCH 3/5] utils: add conditionals support --- parser/tst/simple_tests/conditional/bad_5.sd | 10 + parser/tst/simple_tests/conditional/bad_6.sd | 9 + .../conditional/bad_dup_hats_3.sd | 19 + parser/tst/simple_tests/conditional/ok_30.sd | 11 + utils/apparmor/aa.py | 150 +++-- utils/apparmor/profile_storage.py | 4 + utils/apparmor/regex.py | 24 +- utils/apparmor/rule/boolean.py | 9 +- utils/apparmor/rule/conditional.py | 534 ++++++++++++++++++ utils/apparmor/rule/variable.py | 8 +- utils/test/cleanprof_test.complain | 17 + utils/test/cleanprof_test.in | 17 + utils/test/cleanprof_test.out | 21 + utils/test/test-boolean.py | 12 +- utils/test/test-conditional.py | 402 +++++++++++++ utils/test/test-parser-simple-tests.py | 41 -- 16 files changed, 1192 insertions(+), 96 deletions(-) create mode 100644 parser/tst/simple_tests/conditional/bad_5.sd create mode 100644 parser/tst/simple_tests/conditional/bad_6.sd create mode 100644 parser/tst/simple_tests/conditional/bad_dup_hats_3.sd create mode 100644 parser/tst/simple_tests/conditional/ok_30.sd create mode 100644 utils/apparmor/rule/conditional.py create mode 100644 utils/test/test-conditional.py diff --git a/parser/tst/simple_tests/conditional/bad_5.sd b/parser/tst/simple_tests/conditional/bad_5.sd new file mode 100644 index 000000000..91d5dcd9e --- /dev/null +++ b/parser/tst/simple_tests/conditional/bad_5.sd @@ -0,0 +1,10 @@ +#=DESCRIPTION trying to use undefined boolean +#=EXRESULT FAIL + +$FOO=true + +/bin/true { + if $BAR { + /bin/true rix, + } +} diff --git a/parser/tst/simple_tests/conditional/bad_6.sd b/parser/tst/simple_tests/conditional/bad_6.sd new file mode 100644 index 000000000..21ee1acdf --- /dev/null +++ b/parser/tst/simple_tests/conditional/bad_6.sd @@ -0,0 +1,9 @@ +#=DESCRIPTION unfinished else +#=EXRESULT FAIL + +$BAR=true + +/bin/true { + if $BAR { + /bin/true rix, + } else { diff --git a/parser/tst/simple_tests/conditional/bad_dup_hats_3.sd b/parser/tst/simple_tests/conditional/bad_dup_hats_3.sd new file mode 100644 index 000000000..d4c4e5dc7 --- /dev/null +++ b/parser/tst/simple_tests/conditional/bad_dup_hats_3.sd @@ -0,0 +1,19 @@ +#=DESCRIPTION duplicated hats inside a conditional +#=EXRESULT FAIL + +${FOO} = true + +/bin/true { + + ^dupehat { + /bin/false rix, + } + + if not ${FOO} { + /bin/true rm, + } else { + ^dupehat { + capability dac_override, + } + } +} diff --git a/parser/tst/simple_tests/conditional/ok_30.sd b/parser/tst/simple_tests/conditional/ok_30.sd new file mode 100644 index 000000000..b1f5576f7 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_30.sd @@ -0,0 +1,11 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +$FOO=true +@{BAR}=10 + +/bin/true { + if $FOO and @{BAR} >= 10 { + /bin/true rix, + } +} diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index 49bdc54f4..2e0f69586 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -35,13 +35,14 @@ from apparmor.common import ( from apparmor.profile_list import ProfileList, preamble_ruletypes from apparmor.profile_storage import ProfileStorage, add_or_remove_flag, ruletypes from apparmor.regex import ( - RE_HAS_COMMENT_SPLIT, RE_PROFILE_CHANGE_HAT, RE_PROFILE_CONDITIONAL, - RE_PROFILE_CONDITIONAL_BOOLEAN, RE_PROFILE_CONDITIONAL_VARIABLE, RE_PROFILE_END, + RE_HAS_COMMENT_SPLIT, RE_PROFILE_CHANGE_HAT, RE_PROFILE_CONDITIONAL_START, + RE_PROFILE_CONDITIONAL_ELSE, RE_PROFILE_END, RE_PROFILE_HAT_DEF, RE_PROFILE_START, RE_METADATA_LOGPROF_SUGGEST, RE_RULE_HAS_COMMA, parse_profile_start_line, re_match_include) from apparmor.rule.abi import AbiRule from apparmor.rule.file import FileRule from apparmor.rule.include import IncludeRule +from apparmor.rule.conditional import ConditionalBlock from apparmor.logparser import ReadLog from apparmor.translations import init_translation @@ -1718,28 +1719,57 @@ def attach_profile_data(profiles, profile_data): profiles[p] = deepcopy(profile_data[p]) -def parse_profile_data(data, file, do_include, in_preamble): +def parse_conditional(conditional_block, profile_data, data, lineno, file, do_include, in_preamble, profname, profile, debug_lineno): + cond_storage = ProfileStorage(profile_data[profname]['info']['profile'], + profile_data[profname]['info']['hat'], + profile_data[profname]['info']['calledby'] + ' in cond') + ret_lineno, ret_profile_data, end_of_block = parse_block(data[lineno + 1:], file, do_include, in_preamble, profname, profile, cond_storage, debug_lineno, in_if=True) + conditional_block.store_profile_data(ret_profile_data) + lineno = lineno + ret_lineno + + if conditional_block.result: + for cond_profname in ret_profile_data: + if ret_profile_data[cond_profname]['in_cond']: + if profile_data.get(cond_profname, False): + cond_prof_info = ret_profile_data[cond_profname].data['info'] + cond_prof_name = combine_name(cond_prof_info['profile'], cond_prof_info['hat']) + raise AppArmorException( + 'Profile %(profile)s defined twice in %(file)s, last found in line %(line)s' + % {'file': file, 'line': debug_lineno + 1, 'profile': cond_prof_name}) + profile_data[cond_profname] = ret_profile_data[cond_profname] + + if not end_of_block: + i = 1 + if lineno + i < len(data): + next_line = data[lineno + i].strip() + matches = RE_PROFILE_CONDITIONAL_ELSE.search(next_line) + if matches and matches.group('close'): + data[lineno + i] = data[lineno + i].replace('}', '', 1) + else: + profile_data[profname]['cond_block'].add(conditional_block) + conditional_block = None + + return lineno, conditional_block + + +def parse_block(data, file, do_include, in_preamble, profname, profile, prof_storage, src_lineno, in_if=False): profile_data = {} - profile = None hat = None - profname = None in_contained_hat = None parsed_profiles = [] initial_comment = '' lastline = None + conditional_block = None - active_profiles.init_file(file) - - if do_include: - profile = file - hat = None - profname = combine_profname((profile, hat)) - profile_data[profname] = ProfileStorage(profile, hat, 'parse_profile_data() do_include') + if do_include or profname and profname not in profile_data: + profile_data[profname] = prof_storage profile_data[profname]['filename'] = file - - for lineno, line in enumerate(data): - line = line.strip() + lineno = 0 + while lineno < len(data): + line = data[lineno].strip() + debug_lineno = src_lineno + lineno if not line: + lineno += 1 continue # we're dealing with a multiline statement if lastline: @@ -1747,11 +1777,13 @@ def parse_profile_data(data, file, do_include, in_preamble): lastline = None # is line handled by a *Rule class? - (rule_name, rule_obj) = match_line_against_rule_classes(line, profile, file, lineno, in_preamble) + (rule_name, rule_obj) = match_line_against_rule_classes(line, profile, file, debug_lineno, in_preamble) if rule_name: if in_preamble: active_profiles.add_rule(file, rule_name, rule_obj) else: + if profname not in profile_data: + profile_data[profname] = prof_storage profile_data[profname][rule_name].add(rule_obj) if rule_name == 'inc_ie': @@ -1773,16 +1805,16 @@ def parse_profile_data(data, file, do_include, in_preamble): in_preamble = False - (profile, hat, prof_storage) = ProfileStorage.parse(line, file, lineno, profile, hat) - + (profile, hat, prof_storage) = ProfileStorage.parse(line, file, debug_lineno, profile, hat) if profile == hat: hat = None profname = combine_profname((profile, hat)) if profile_data.get(profname, False): + print(file) raise AppArmorException( 'Profile %(profile)s defined twice in %(file)s, last found in line %(line)s' - % {'file': file, 'line': lineno + 1, 'profile': combine_name(profile, hat)}) + % {'file': file, 'line': debug_lineno + 1, 'profile': combine_name(profile, hat)}) profile_data[profname] = prof_storage @@ -1792,12 +1824,11 @@ def parse_profile_data(data, file, do_include, in_preamble): initial_comment = '' - elif RE_PROFILE_END.search(line): - # If profile ends and we're not in one - if not profile: - raise AppArmorException( - _('Syntax Error: Unexpected End of Profile reached in file: %(file)s line: %(line)s') - % {'file': file, 'line': lineno + 1}) + ret_lineno, ret_profile_data, end_of_block = parse_block(data[lineno + 1:], file, do_include, in_preamble, profname, profile, prof_storage, debug_lineno + 1, in_if) + profile_data[profname]['in_cond'] = in_if + lineno = lineno + ret_lineno + + profile_data = {**profile_data, **ret_profile_data} if in_contained_hat: hat = None @@ -1811,32 +1842,49 @@ def parse_profile_data(data, file, do_include, in_preamble): initial_comment = '' - elif RE_PROFILE_CONDITIONAL.search(line): - # Conditional Boolean - pass + elif RE_PROFILE_END.search(line): + # If profile ends and we're not in one + if not profile: + raise AppArmorException( + _('Syntax Error: Unexpected End of Profile reached in file: %(file)s line: %(line)s') + % {'file': file, 'line': debug_lineno + 1}) - elif RE_PROFILE_CONDITIONAL_VARIABLE.search(line): - # Conditional Variable defines - pass + return lineno + 1, profile_data, True - elif RE_PROFILE_CONDITIONAL_BOOLEAN.search(line): - # Conditional Boolean defined - pass + elif RE_PROFILE_CONDITIONAL_START.search(line): + conditional_block = ConditionalBlock(line, active_profiles.files[file]) + lineno, conditional_block = parse_conditional(conditional_block, profile_data, data, lineno, file, do_include, in_preamble, profname, profile, debug_lineno + 1) + + elif RE_PROFILE_CONDITIONAL_ELSE.search(line): + if not in_if and conditional_block is None: + raise AppArmorException(_('Syntax Error: Unexpected else without previous if in file: %(file)s line: %(line)s') + % {'file': file, 'line': debug_lineno + 1}) + + matches = RE_PROFILE_CONDITIONAL_ELSE.search(line) + if matches.group('close'): + return lineno, profile_data, False # not returning next line on purpose + + if conditional_block is None: + raise AppArmorException(_('Syntax Error: Unexpected else found without previous if in file: %(file)s line: %(line)s') + % {'file': file, 'line': debug_lineno + 1}) + conditional_block.add_conditional(line, active_profiles.files[file]) + lineno, conditional_block = parse_conditional(conditional_block, profile_data, data, lineno, file, do_include, in_preamble, profname, profile, debug_lineno + 1) elif RE_PROFILE_CHANGE_HAT.search(line): matches = RE_PROFILE_CHANGE_HAT.search(line).groups() if not profile: raise AppArmorException(_('Syntax Error: Unexpected change hat declaration found in file: %(file)s line: %(line)s') - % {'file': file, 'line': lineno + 1}) + % {'file': file, 'line': debug_lineno + 1}) aaui.UI_Important(_('Ignoring no longer supported change hat declaration "^%(hat)s," found in file: %(file)s line: %(line)s') - % {'hat': matches[0], 'file': file, 'line': lineno + 1}) + % {'hat': matches[0], 'file': file, 'line': debug_lineno + 1}) elif line.startswith('#'): # Handle initial comments if not profile: if line.startswith('# Last Modified:'): + lineno += 1 continue else: initial_comment = initial_comment + line + '\n' @@ -1860,14 +1908,16 @@ def parse_profile_data(data, file, do_include, in_preamble): else: raise AppArmorException( _('Syntax Error: Unknown line found in file %(file)s line %(lineno)s:\n %(line)s') - % {'file': file, 'lineno': lineno + 1, 'line': line}) + % {'file': file, 'lineno': debug_lineno, 'line': line}) + + lineno += 1 if lastline: # lastline gets merged into line (and reset to None) when reading the next line. # If it isn't empty, this means there's something unparsable at the end of the profile raise AppArmorException( _('Syntax Error: Unknown line found in file %(file)s line %(lineno)s:\n %(line)s') - % {'file': file, 'lineno': lineno + 1, 'line': lastline}) + % {'file': file, 'lineno': debug_lineno + 1, 'line': lastline}) # Below is not required I'd say if not do_include: @@ -1887,6 +1937,30 @@ def parse_profile_data(data, file, do_include, in_preamble): _("Syntax Error: Missing '}' or ','. Reached end of file %(file)s while inside profile %(profile)s") % {'file': file, 'profile': profile}) + return lineno, profile_data, False + + +def parse_profile_data(data, file, do_include, in_preamble): + profile_data = {} + profile = None + hat = None + profname = None + prof_storage = None + + active_profiles.init_file(file) + + if do_include: + profile = file + hat = None + profname = combine_profname((profile, hat)) + prof_storage = ProfileStorage(profile, hat, 'parse_profile_data() do_include') + + _lineno, profile_data, end_of_block = parse_block(data, file, do_include, in_preamble, profname, profile, prof_storage, 0) + + for prof in list(profile_data): # get the keys + if profile_data[prof]['in_cond']: + profile_data.pop(prof) + return profile_data diff --git a/utils/apparmor/profile_storage.py b/utils/apparmor/profile_storage.py index 26fd91ee8..346fdeba3 100644 --- a/utils/apparmor/profile_storage.py +++ b/utils/apparmor/profile_storage.py @@ -35,6 +35,7 @@ from apparmor.rule.io_uring import IOUringRule, IOUringRuleset from apparmor.rule.mount import MountRule, MountRuleset from apparmor.rule.pivot_root import PivotRootRule, PivotRootRuleset from apparmor.rule.unix import UnixRule, UnixRuleset +from apparmor.rule.conditional import ConditionalBlock, ConditionalBlockset from apparmor.translations import init_translation @@ -58,6 +59,7 @@ ruletypes = { 'mount': {'rule': MountRule, 'ruleset': MountRuleset}, 'pivot_root': {'rule': PivotRootRule, 'ruleset': PivotRootRuleset}, 'unix': {'rule': UnixRule, 'ruleset': UnixRuleset}, + 'cond_block': {'rule': ConditionalBlock, 'ruleset': ConditionalBlockset}, } @@ -88,6 +90,7 @@ class ProfileStorage: data['profile_keyword'] = False data['is_hat'] = False # profile or hat? data['hat_keyword'] = False # True for 'hat foo', False for '^foo' + data['in_cond'] = False # used for profiles created inside conditionals self.data = data @@ -178,6 +181,7 @@ class ProfileStorage: 'inc_ie', 'rlimit', 'capability', + 'cond_block', 'network', 'dbus', 'mount', diff --git a/utils/apparmor/regex.py b/utils/apparmor/regex.py index a7a93ea22..d1e53b8c4 100644 --- a/utils/apparmor/regex.py +++ b/utils/apparmor/regex.py @@ -28,7 +28,7 @@ RE_COMMA_EOL = r'\s*,' + RE_EOL # optional whitespace, comma + RE_EOL RE_PROFILE_NAME = r'(?P<%s>(\S+|"[^"]+"))' # string without spaces, or quoted string. %s is the match group name RE_PATH = r'/\S*|"/[^"]*"' # filename (starting with '/') without spaces, or quoted filename. -RE_VAR = r'@{[^}\s]+}' +RE_VAR = r'@{?[^}\s]+}?' RE_DICT_ENTRY = r'\s*(?P[^,\s=]+)(?:=(?P[^,\s=]+))?\s*' RE_PROFILE_PATH = '(?P<%s>(' + RE_PATH + '))' # quoted or unquoted filename. %s is the match group name RE_PROFILE_PATH_OR_VAR = '(?P<%s>(' + RE_PATH + '|' + RE_VAR + r'\S*|"' + RE_VAR + '[^"]*"))' # quoted or unquoted filename or variable. %s is the match group name @@ -37,6 +37,10 @@ RE_XATTRS = r'(\s+xattrs\s*=\s*\((?P([^)=]+(=[^)=]+)?\s?)*)\)\s*)?' RE_FLAGS = r'(\s+(flags\s*=\s*)?\((?P[^)]+)\))?' RE_VARIABLE = re.compile(RE_VAR) +RE_ID = r'(?P[^,!#\s=@$()"]+|"(\w|\s)*")' +RE_VARIABLES = r'(?P(?P@|\$)\{?(?P\w+)\}?)' +RE_ID_OR_VAR = r'(' + RE_VARIABLES + r'|' + RE_ID + r')' +RE_ALL_VARIABLES = re.compile(RE_VARIABLES % {'label': ''}) RE_PROFILE_END = re.compile(r'^\s*\}' + RE_EOL) RE_PROFILE_ALL = re.compile(RE_PRIORITY_AUDIT_DENY + r'all' + RE_COMMA_EOL) @@ -45,9 +49,21 @@ RE_PROFILE_ALIAS = re.compile(r'^\s*alias\s+(?P"??.+?"??)\s+->\s*(?P< RE_PROFILE_RLIMIT = re.compile(r'^\s*set\s+rlimit\s+(?P[a-z]+)\s*<=\s*(?P[^ ]+(\s+[a-zA-Z]+)?)' + RE_COMMA_EOL) RE_PROFILE_BOOLEAN = re.compile(r'^\s*(?P\$\{?\w*\}?)\s*=\s*(?Ptrue|false)\s*,?' + RE_EOL, flags=re.IGNORECASE) RE_PROFILE_VARIABLE = re.compile(r'^\s*(?P@\{?\w+\}?)\s*(?P\+?=)\s*(?P@*.+?)' + RE_EOL) -RE_PROFILE_CONDITIONAL = re.compile(r'^\s*if\s+(not\s+)?(\$\{?\w*\}?)\s*\{' + RE_EOL) -RE_PROFILE_CONDITIONAL_VARIABLE = re.compile(r'^\s*if\s+(not\s+)?defined\s+(@\{?\w+\}?)\s*\{\s*(#.*)?$') -RE_PROFILE_CONDITIONAL_BOOLEAN = re.compile(r'^\s*if\s+(not\s+)?defined\s+(\$\{?\w+\}?)\s*\{\s*(#.*)?$') + +RE_BOOLEAN_OP = r'(?P(?P(not\s+)*)(?Pdefined\s+)?' + RE_VARIABLES % {'label': '%(term)s'} + r')' +RE_COMPARE_OP_QUOTED = r'(?P(?P(not\s+)*)(?P"?' + RE_ID_OR_VAR % {'label': '_left%(term)s'} + r'"?)\s+(?P==|!=|in|>|>=|<|<=)\s+(?P"?' + RE_ID_OR_VAR % {'label': '_right%(term)s'} + r'"?))' # used only by transform_cond +RE_COMPARE_OP = RE_COMPARE_OP_QUOTED.replace('"?', '') + +RE_FACTOR = r'(?P\()?(' + RE_COMPARE_OP + r'|' + RE_BOOLEAN_OP + r')(?P\))?' + +RE_TERM = r'(?P\()*\s*((?P' + RE_FACTOR % {'term': '_1%(expr)s'} + r')\s+(?Pand|or)\s+(?P' + RE_FACTOR % {'term': '_2%(expr)s'} + ')|' + RE_FACTOR % {'term': '_0%(expr)s'} + r')\s*(?P\))*' + +RE_CONDITION = r'(?P(?P\()?(?P' + RE_TERM % {'expr': '_first'} + r')(\s+(?Pand|or)\s+(?P' + RE_TERM % {'expr': '_second'} + r'))*(?P\))?)' + +RE_PROFILE_CONDITIONAL = r'\s*if\s+' + RE_CONDITION + r'\s*\{' + +RE_PROFILE_CONDITIONAL_START = re.compile(r'^' + RE_PROFILE_CONDITIONAL + RE_EOL) +RE_PROFILE_CONDITIONAL_ELSE = re.compile(r'^\s*(?P\})?\s*else((?P\s+' + RE_PROFILE_CONDITIONAL + r')|(\s*\{))' + RE_EOL) RE_PROFILE_NETWORK = re.compile(RE_PRIORITY_AUDIT_DENY + r'network(?P
\s+.*)?' + RE_COMMA_EOL) RE_PROFILE_CHANGE_HAT = re.compile(r'^\s*\^("??.+?"??)' + RE_COMMA_EOL) RE_PROFILE_HAT_DEF = re.compile(r'^(?P\s*)(?P\^|hat\s+)(?P"??[^)]+?"??)' + RE_FLAGS + r'\s*\{' + RE_EOL) diff --git a/utils/apparmor/rule/boolean.py b/utils/apparmor/rule/boolean.py index ec1b901e8..b3364a083 100644 --- a/utils/apparmor/rule/boolean.py +++ b/utils/apparmor/rule/boolean.py @@ -51,7 +51,7 @@ class BooleanRule(BaseRule): raise AppArmorException('Passed invalid value to %s: %s' % (self.__class__.__name__, value)) self.varname = varname - self.value = value + self.value = value == 'true' @classmethod def _create_instance(cls, raw_rule, matches): @@ -70,7 +70,7 @@ class BooleanRule(BaseRule): space = ' ' * depth - return '%s%s = %s' % (space, self.varname, self.value) + return '%s%s = %s' % (space, self.varname, str(self.value).lower()) def _is_covered_localvars(self, other_rule): """check if other_rule is covered by this rule object""" @@ -78,7 +78,10 @@ class BooleanRule(BaseRule): if self.varname != other_rule.varname: return False - if not self._is_covered_list(self.value, None, set(other_rule.value), None, 'value'): + if type(self.value) is not bool or type(other_rule.value) is not bool: + raise AppArmorBug(_('Invalid value for boolean variable')) + + if self.value != other_rule.value: return False # still here? -> then it is covered diff --git a/utils/apparmor/rule/conditional.py b/utils/apparmor/rule/conditional.py new file mode 100644 index 000000000..dd3f97361 --- /dev/null +++ b/utils/apparmor/rule/conditional.py @@ -0,0 +1,534 @@ +# +# ---------------------------------------------------------------------- +# Copyright (C) 2025 Canonical, Ltd. +# +# 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 as 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. +# +# ---------------------------------------------------------------------- + +import ast +import re + +from apparmor.common import AppArmorException, AppArmorBug +from apparmor.regex import strip_quotes, RE_PROFILE_CONDITIONAL_START, RE_PROFILE_CONDITIONAL_ELSE, RE_ALL_VARIABLES, RE_BOOLEAN_OP, RE_ID_OR_VAR, RE_CONDITION, RE_COMPARE_OP_QUOTED +from apparmor.rule import BaseRule, BaseRuleset, parse_comment, quote_if_needed +from apparmor.translations import init_translation + +_ = init_translation() + + +class ConditionalBlock(BaseRule): + """Class to handle and store a conditional block containing if, + and optionally, else ifs and else""" + + _match_re = None + result = False + + def __init__(self, raw_rule, prof_storage, audit=False, deny=False, allow_keyword=False, + comment='', log_event=None, priority=None): + + super().__init__(audit=audit, deny=deny, allow_keyword=allow_keyword, + comment=comment, log_event=log_event, priority=priority) + + # conditional blocks don't support priority, allow keyword, audit or deny - yet + self.ensure_modifiers_not_supported() + + self.cond_list = [] + conditional = ConditionalStart.create_instance(raw_rule) + self.result = conditional.evaluate(prof_storage) + self.cond_list.append(conditional) + + def store_profile_data(self, profile_data): + self.cond_list[-1].profile_data = profile_data + + def add_conditional(self, raw_rule, prof_storage): + conditional = ConditionalElse.create_instance(raw_rule) + result = conditional.evaluate(prof_storage) + self.cond_list.append(conditional) + if self.result: + self.result = False + else: + self.result = result + + def get_clean(self, depth=0): + clean = '' + for cond in self.cond_list: + if clean: + clean += ' ' + clean += cond.get_clean(depth) + return clean + + @classmethod + def _create_instance(cls, raw_rule, matches): + raise NotImplementedError("'%s' is not supposed to be called directly" % (str(cls))) + + def _is_covered_localvars(self, other_rule): + """check if other_rule is covered by this rule object""" + if len(self.cond_list) != len(other_rule.cond_list): + return False + else: + for idx in range(len(self.cond_list)): + if not self.cond_list[idx].is_covered(other_rule.cond_list[idx]): + return False + return True + + def _is_equal_localvars(self, other_rule, strict): + """compare if rule-specific conditionals are equal""" + if len(self.cond_list) != len(other_rule.cond_list): + return False + else: + for idx in range(len(self.cond_list)): + if not self.cond_list[idx].is_equal(other_rule.cond_list[idx]): + return False + return True + + def _logprof_header_localvars(self): + conditions = [] + for cond in self.cond_list: + conditions.append(cond.logprof_header()) + return _('ConditionalBlock'), (conditions) + + +class ConditionalRule(BaseRule): + """Class to handle and store a single conditional rule""" + + IF = 1 + ELSEIF = 2 + ELSE = 3 + rule_name = 'conditional' + _match_re = re.compile(RE_CONDITION) + + def __init__(self, condition, ast_tree, + audit=False, deny=False, allow_keyword=False, + comment='', log_event=None, priority=None): + + super().__init__(audit=audit, deny=deny, allow_keyword=allow_keyword, + comment=comment, log_event=log_event, priority=priority) + + # conditionals don't support priority, allow keyword, audit or deny + self.ensure_modifiers_not_supported() + + if condition not in [self.IF, self.ELSEIF, self.ELSE]: + raise AppArmorBug('Passed invalid condition to %s: %s' % (self.__class__.__name__, condition)) + if ast_tree is not None and not isinstance(ast_tree, AppArmorAst): + raise AppArmorBug('Passed invalid AST tree type to %s: %s' % (self.__class__.__name__, type(ast_tree))) + + self.condition = condition + self.ast_tree = ast_tree + + @classmethod + def _create_instance(cls, raw_rule, matches): + """parse raw_rule and return instance of this class""" + + if cls == ConditionalStart: + conditional = ConditionalRule.IF + else: + if matches.group('if'): + conditional = ConditionalRule.ELSEIF + else: + conditional = ConditionalRule.ELSE + + comment = parse_comment(matches) + + expr = cls._match_re.search(raw_rule) + ast_tree = None + if conditional != ConditionalRule.ELSE: + ast_tree = AppArmorAst(expr.group('expr')) + + return cls(conditional, ast_tree, comment) + + def evaluate(self, prof_storage): + # else should always evaluate to true, since that's the + # default if all previous "ifs" evaluated to false + if self.condition == ConditionalRule.ELSE: + return True + + return self.ast_tree.evaluate(prof_storage) + + def get_clean(self, depth=0): + """return rule (in clean/default formatting)""" + + space = ' ' * depth + leading_space = '' + if self.condition == ConditionalRule.IF: + conditional = 'if ' + leading_space = space + elif self.condition == ConditionalRule.ELSEIF: + conditional = 'else if ' + elif self.condition == ConditionalRule.ELSE: + conditional = 'else ' + else: + raise AppArmorBug('Invalid condition type in %s' % (self.__class__.__name__)) + + expr = '' + if self.ast_tree: + expr = self.ast_tree.get_clean() + + data = [] + data.append('%s%s%s{' % (leading_space, conditional, expr)) + + for profname in self.profile_data: + if self.profile_data[profname]['in_cond']: + from apparmor.aa import write_piece + data.extend(write_piece(self.profile_data, depth + 1, profname, profname)) + else: + data += self.profile_data[profname].get_rules_clean(depth + 1) + + data.append('%s}' % space) + return '\n'.join(data) + + def _is_covered_localvars(self, other_rule): + """check if other_rule is covered by this rule object""" + # conditional is only covered if equal + if self.is_equal(other_rule): + return True + return False + + def _is_equal_localvars(self, rule_obj, strict): + """compare if rule-specific conditionals are equal""" + if self.condition != rule_obj.condition: + return False + if self.profile_data != rule_obj.profile_data: + return False + return self.ast_tree.is_equal(rule_obj.ast_tree) + + def _logprof_header_localvars(self): + return _('Conditional'), self.get_clean() + + +class ConditionalStart(ConditionalRule): + """Class to handle and store a single conditional rule""" + + _match_re = RE_PROFILE_CONDITIONAL_START + + +class ConditionalElse(ConditionalRule): + """Class to handle and store a single conditional rule""" + + _match_re = RE_PROFILE_CONDITIONAL_ELSE + + +class ConditionalBlockset(BaseRuleset): + """Class to handle and store a collection of conditional rule blocks""" + + +class AppArmorAst(): + astcomp_to_string = { + ast.Eq: '==', + ast.NotEq: '!=', + ast.Lt: '<', + ast.LtE: '<=', + ast.Gt: '>', + ast.GtE: '>=', + ast.In: 'in', + } + + def __init__(self, expr): + self.tree = ast.parse(self.transform_cond(expr)) + + def get_clean(self): + noop, expr = self.get_clean_tree(self.tree.body[0]) + expr += ' ' + return expr + + def get_clean_tree(self, node): + if isinstance(node, ast.Expr): + node = node.value + if isinstance(node, ast.BoolOp): + op = '' + clean = '' + if isinstance(node.op, ast.And): + op = 'and' + else: + op = 'or' + for value in node.values: + if clean: + clean += ' ' + op + ' ' + ret_op, ret_clean = self.get_clean_tree(value) + if ret_op == 'or' and op == 'and': + ret_clean = '(' + ret_clean + ')' + clean += ret_clean + return op, clean + elif isinstance(node, ast.UnaryOp): + if not isinstance(node.op, ast.Not): + raise AppArmorBug('Invalid unary operation in %s' % (self.__class__.__name__)) + op, child = self.get_clean_tree(node.operand) + if op == 'not': # remove canceling nots + return 'val', '%s' % (child[len('not '):]) + return 'not', 'not %s' % (child) + elif isinstance(node, ast.Constant): + val = quote_if_needed(strip_quotes(node.value)) # strip first because it might it can be quoted but not need it + term = Term.create_instance(val) + return 'val', str(term) + elif isinstance(node, ast.Name): + return 'func', node.id + elif isinstance(node, ast.Call): + noop, name = self.get_clean_tree(node.func) + if name != 'defined': + raise AppArmorBug('Invalid function name in %s' % (self.__class__.__name__)) + noop, var = self.get_clean_tree(node.args[0]) + return 'defined', '%s %s' % (name, var) + elif isinstance(node, ast.Compare): + noop, left = self.get_clean_tree(node.left) + noop, right = self.get_clean_tree(node.comparators[0]) + op = self.astcomp_to_string[type(node.ops[0])] + return 'cmp', '%s %s %s' % (left, op, right) + else: + raise AppArmorBug('Unsupported node type in %s' % (self.__class__.__name__)) + + def evaluate(self, prof_storage): + noop, result = self.evaluate_tree(self.tree.body[0], prof_storage, True) + return result + + def evaluate_tree(self, node, prof_storage, resolve=False): + result = None + if isinstance(node, ast.Expr): + node = node.value + if isinstance(node, ast.BoolOp): + op = node.op + for value in node.values: + ret_op, ret_result = self.evaluate_tree(value, prof_storage, True) + if result is None: + result = ret_result + else: + if isinstance(op, ast.And): + result = result and ret_result + else: + result = result or ret_result + return op, result + elif isinstance(node, ast.UnaryOp): + if not isinstance(node.op, ast.Not): + raise AppArmorBug('Invalid unary operation in %s' % (self.__class__.__name__)) + result = not self.evaluate_tree(node.operand, prof_storage, True) + return node.op, result + elif isinstance(node, ast.Constant): + val = quote_if_needed(strip_quotes(node.value)) # strip first because it might it can be quoted but not need it + term = Term.create_instance(val) + if resolve: + cond = BooleanCondition('', term) + result = cond.evaluate(prof_storage) + return term, result + elif isinstance(node, ast.Name): + return node.id, result + elif isinstance(node, ast.Call): + func, noop = self.evaluate_tree(node.func, prof_storage) + if func != 'defined': + raise AppArmorBug('Invalid function name in %s' % (self.__class__.__name__)) + # there should be only one arg + variable, noop = self.evaluate_tree(node.args[0], prof_storage) + cond = BooleanCondition(func, variable) + return 'defined', cond.evaluate(prof_storage) + elif isinstance(node, ast.Compare): + left, noop = self.evaluate_tree(node.left, prof_storage) + right, noop = self.evaluate_tree(node.comparators[0], prof_storage) + op = self.astcomp_to_string[type(node.ops[0])] + cond = CompareCondition(left, op, right) + return 'cmp', cond.evaluate(prof_storage) + else: + raise AppArmorBug('Unsupported node type in %s' % (self.__class__.__name__)) + + def compare_ast(self, node1, node2): + if type(node1) is not type(node2): + return False + if isinstance(node1, ast.AST): + for k, v in vars(node1).items(): + if k in ('lineno', 'col_offset', 'ctx', 'end_lineno', 'end_col_offset'): + continue + if not self.compare_ast(v, getattr(node2, k)): + return False + return True + elif isinstance(node1, list): + if len(node1) != len(node2): + return False + for i in range(len(node1)): + if not self.compare_ast(node1[i], node2[i]): + return False + return True + else: + return node1 == node2 + + def is_equal(self, other_tree): + return self.compare_ast(self.tree, other_tree.tree) + + def transform_cond(self, text): + """Used to transform policy conditional into Python format, so ast can be used""" + + def boolean_op(match): + not_op = match.group('boolean_not') + defined = match.group('defined') + var = match.group('var') + var = '"%s"' % (var) + if defined: + var = 'defined(%s)' % (var) + return '%s%s' % (not_op, var) + + def compare_op(match): + not_op = match.group('compare_not') + left = match.group('left') + op = match.group('op') + right = match.group('right') + + if match.group('id_left') and not (left.startswith('"') or left.endswith('"')): + left = '"%s"' % (left) + if match.group('id_right') and not (right.startswith('"') or right.endswith('"')): + right = '"%s"' % (right) + + return '%s%s %s %s' % (not_op, left, op, right) + + replaced = re.sub(RE_BOOLEAN_OP % {'term': ''}, boolean_op, text) + replaced = re.sub(RE_COMPARE_OP_QUOTED % {'term': ''}, compare_op, replaced) + return replaced + + +class Term(): + match_re = re.compile(RE_ID_OR_VAR % {'label': ''}) + + @classmethod + def create_instance(cls, raw_term): + """parse raw_term and return instance of this class""" + matches = cls.match_re.search(raw_term) + if not matches: + raise AppArmorBug('Unable to parse term in %s' % (cls.__class__.__name__)) + if matches.group('id'): + return Id(matches.group('id')) + else: + var_type = matches.group('var_type') + varname = matches.group('varname') + var = matches.group('var') + return Variable(var_type, varname, var) + + +class Variable(Term): + def __init__(self, var_type, varname, var): + self.varname = varname + self.var = var + if var_type == '$': + self.var_type = 'boolean' + else: + self.var_type = 'variable' + + def get_variable_rule(self, prof_storage): + for rule in prof_storage[self.var_type].rules: + filtered = RE_ALL_VARIABLES.search(rule.varname) + if self.varname == filtered.group('varname'): + return rule + return None + + def get_set(self, prof_storage): + variable_rule = self.get_variable_rule(prof_storage) + if variable_rule is None: + raise AppArmorException(_('Error retrieving variable %(var)s') % {'var': self.var}) + return variable_rule.values + + def __repr__(self): + return self.var + + +class Id(Term): + def __init__(self, value): + self.value = value + + def get_set(self, prof_storage): + return {self.value} + + def __repr__(self): + return self.value + + +class BooleanCondition(): + def __init__(self, defined: str, variable: Term): + if not isinstance(defined, str): + raise AppArmorBug('Passed invalid defined value to %s: %s' % (self.__class__.__name__, defined)) + if not isinstance(variable, Term): + raise AppArmorBug('Passed invalid variable type to %s: %s' % (self.__class__.__name__, type(variable))) + + self.defined = defined + self.variable = variable + + def evaluate(self, prof_storage): + matched = self.variable.get_variable_rule(prof_storage) + + if not self.defined: # boolean op + if self.variable.var_type == 'boolean': + if matched: + return matched.value + else: + raise AppArmorException(_('Cannot find previous declaration of %(var)s') % {'var': self.variable}) + else: + raise AppArmorException(_('Unexpected variable in boolean operation: %(var)s') % {'var': self.variable}) + else: + return bool(matched) + + +class CompareCondition(): + valid_ops = ['==', '!=', 'in', '>', '>=', '<', '<='] + + def __init__(self, left_term: Term, op: str, right_term: Term): + if op not in self.valid_ops: + raise AppArmorBug('Passed invalid op value to %s: %s' % (self.__class__.__name__, op)) + if not isinstance(left_term, Term): + raise AppArmorBug('Passed invalid left term type to %s: %s' % (self.__class__.__name__, type(left_term))) + if not isinstance(right_term, Term): + raise AppArmorBug('Passed invalid right term type to %s: %s' % (self.__class__.__name__, type(right_term))) + + self.left_term = left_term + self.op = op + self.right_term = right_term + + def compare(self, op, lhs, rhs): + if type(lhs) is not type(rhs): + raise AppArmorBug('Trying to compare elements of different types in %s' % (self.__class__.__name__)) + + if (op == '>'): + return lhs > rhs + elif (op == '>='): + return lhs >= rhs + elif (op == '<'): + return lhs < rhs + elif (op == '<='): + return lhs <= rhs + else: + raise AppArmorBug('Invalid op in %s: %s' % (self.__class__.__name__, op)) + + def evaluate(self, prof_storage): + lhs = self.left_term.get_set(prof_storage) + rhs = self.right_term.get_set(prof_storage) + + if not isinstance(lhs, set) or not isinstance(rhs, set): + raise AppArmorBug('Passed invalid type for condition term in %s' % (self.__class__.__name__)) + + converted_lhs = None + converted_rhs = None + + if self.op == 'in': + return lhs.issubset(rhs) + elif self.op == '==': + return lhs == rhs + elif self.op == '!=': + return lhs != rhs + + try: + if len(lhs) == 1: + converted_lhs = int(next(iter(lhs))) + except ValueError: + pass + + try: + if len(rhs) == 1: + converted_rhs = int(next(iter(rhs))) + except ValueError: + pass + + if converted_lhs is None and converted_rhs is None: # sets + return self.compare(self.op, lhs, rhs) + elif converted_lhs is not None and converted_rhs is not None: # numbers + return self.compare(self.op, converted_lhs, converted_rhs) + else: + raise AppArmorException(_('Can only compare numbers with numbers')) diff --git a/utils/apparmor/rule/variable.py b/utils/apparmor/rule/variable.py index a88fc42de..ee4865ade 100644 --- a/utils/apparmor/rule/variable.py +++ b/utils/apparmor/rule/variable.py @@ -40,10 +40,10 @@ class VariableRule(BaseRule): if not isinstance(varname, str): raise AppArmorBug('Passed unknown type for varname to %s: %s' % (self.__class__.__name__, varname)) - if not varname.startswith('@{'): - raise AppArmorException("Passed invalid varname to %s (doesn't start with '@{'): %s" % (self.__class__.__name__, varname)) - if not varname.endswith('}'): - raise AppArmorException("Passed invalid varname to %s (doesn't end with '}'): %s" % (self.__class__.__name__, varname)) + if not varname.startswith('@'): + raise AppArmorException("Passed invalid varname to %s (doesn't start with '@'): %s" % (self.__class__.__name__, varname)) + if (varname.startswith('@{') and not varname.endswith('}')) or (not varname.startswith('@{') and varname.endswith('}')): + raise AppArmorException("Passed invalid varname to %s (mismatched braces): %s" % (self.__class__.__name__, varname)) if not isinstance(mode, str): raise AppArmorBug('Passed unknown type for variable assignment mode to %s: %s' % (self.__class__.__name__, mode)) diff --git a/utils/test/cleanprof_test.complain b/utils/test/cleanprof_test.complain index 1ad59d756..cdcf525af 100644 --- a/utils/test/cleanprof_test.complain +++ b/utils/test/cleanprof_test.complain @@ -107,3 +107,20 @@ profile foo//bar { /usr/bin/a/simple/cleanprof/test/profile//child { /home/test-child r, } + +profile uniquename /attach { + if not not + not not + not not defined $foo { + /bin/true rix, + } else if x in @xy { + if @{asdf} > " a test" { + /greaterthan r, + } else { + file, + } + /bin/false rix, + } else { + /dev/null r, + } +} diff --git a/utils/test/cleanprof_test.in b/utils/test/cleanprof_test.in index e9f13299c..8c8624c45 100644 --- a/utils/test/cleanprof_test.in +++ b/utils/test/cleanprof_test.in @@ -107,3 +107,20 @@ profile foo//bar { /usr/bin/a/simple/cleanprof/test/profile//child { /home/test-child r, } + +profile uniquename /attach { + if not not + not not + not not defined $foo { + /bin/true rix, + } else if x in @xy { + if @{asdf} > " a test" { + /greaterthan r, + } else { + file, + } + /bin/false rix, + } else { + /dev/null r, + } +} diff --git a/utils/test/cleanprof_test.out b/utils/test/cleanprof_test.out index aa501b1b1..441e606c1 100644 --- a/utils/test/cleanprof_test.out +++ b/utils/test/cleanprof_test.out @@ -86,3 +86,24 @@ profile foo//bar { /home/namedchild r, } +profile uniquename /attach { + if defined $foo { + /bin/true rix, + + } else if x in @xy { + if @{asdf} > " a test" { + /greaterthan r, + + } else { + file, + + } + + /bin/false rix, + + } else { + /dev/null r, + + } + +} diff --git a/utils/test/test-boolean.py b/utils/test/test-boolean.py index cc588e3f9..1cbab26a7 100644 --- a/utils/test/test-boolean.py +++ b/utils/test/test-boolean.py @@ -44,12 +44,12 @@ class BooleanTest(AATest): class BooleanTestParse(BooleanTest): tests = ( # rawrule comment varname value - ('$foo=true', exp('', '$foo', 'true')), - ('$foo = false', exp('', '$foo', 'false')), - ('$foo=TrUe', exp('', '$foo', 'true')), - ('$foo = FaLsE', exp('', '$foo', 'false')), - (' $foo = true ', exp('', '$foo', 'true')), - (' $foo = true # comment', exp(' # comment', '$foo', 'true')), + ('$foo=true', exp('', '$foo', True)), + ('$foo = false', exp('', '$foo', False)), + ('$foo=TrUe', exp('', '$foo', True)), + ('$foo = FaLsE', exp('', '$foo', False)), + (' $foo = true ', exp('', '$foo', True)), + (' $foo = true # comment', exp(' # comment', '$foo', True)), ) def _run_test(self, rawrule, expected): diff --git a/utils/test/test-conditional.py b/utils/test/test-conditional.py new file mode 100644 index 000000000..e7eea5715 --- /dev/null +++ b/utils/test/test-conditional.py @@ -0,0 +1,402 @@ +#!/usr/bin/python3 +# ---------------------------------------------------------------------- +# Copyright (C) 2025 Canonical, Ltd. +# +# 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 as 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. +# +# ---------------------------------------------------------------------- + +import unittest + +from apparmor.common import AppArmorException, AppArmorBug, combine_profname +from apparmor.profile_storage import ProfileStorage +from apparmor.profile_list import ProfileList +from apparmor.rule.file import FileRule +from apparmor.rule.variable import VariableRule +from apparmor.rule.boolean import BooleanRule +from apparmor.rule.conditional import ConditionalBlock, AppArmorAst, Term, CompareCondition, ConditionalRule, BooleanCondition +from common_test import AATest, setup_all_loops + + +class TestConditional(AATest): + """ Base class to test conditionals, profile_data does not contain any rules """ + filename = 'somefile' + condition_contents = '\n' + + tests = ( + # ConditionalBlock clean rule + (['if $FOO {', '} else if $BAR {', '} else {'], 'if $FOO {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not $FOO {', '} else if not $BAR {', '} else {'], 'if not $FOO {' + condition_contents + '} else if not $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if defined $FOO {', '} else if not defined $BAR {', '} else {'], 'if defined $FOO {' + condition_contents + '} else if not defined $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else if defined @{VAR1} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else if defined @{VAR1} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {'], 'if not defined @{VAR2} {' + condition_contents + '}'), + ) + + @classmethod + def setUpClass(cls): + super(TestConditional, cls).setUpClass() + cls.active_profiles = ProfileList() + cls.profile_data = {} + cls.condition_contents = '\n' + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.active_profiles.add_rule(cls.filename, 'variable', VariableRule.create_instance('@{VAR1} = "test"')) + cls.active_profiles.add_rule(cls.filename, 'variable', VariableRule.create_instance('@{VAR2} = "test1 test2"')) + cls.active_profiles.add_rule(cls.filename, 'variable', VariableRule.create_instance('@{VAR3} = 10')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$FOO=true')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$BAR=false')) + + def _run_test(self, params, expected): + conditional_block = None + for condition in params: + if condition == params[0]: + conditional_block = ConditionalBlock(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + else: + conditional_block.add_conditional(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + + self.assertEqual(conditional_block.get_clean(), expected) + + +class TestConditionalComplex(TestConditional): + """ Class to test complex conditionals (and, ors, comparisons and boolean operations), profile_data contains one file rule """ + # directly related to how setUpClass sets up profile_data + condition_contents = '\n /bin/false rix,\n\n' + + tests = ( + # ConditionalBlock clean rule + (['if $FOO and ($BAR) {', '} else if $BAR {', '} else {'], 'if $FOO and $BAR {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if $FOO and ($BAR or defined @{VAR1}) {', '} else if $BAR {', '} else {'], 'if $FOO and ($BAR or defined @{VAR1}) {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if $FOO or ($BAR or defined @{VAR1}) {', '} else if $BAR {', '} else {'], 'if $FOO or $BAR or defined @{VAR1} {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if $FOO or ($BAR and defined @{VAR1}) {', '} else if $BAR {', '} else {'], 'if $FOO or $BAR and defined @{VAR1} {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not $FOO {', '} else if not not $BAR {', '} else {'], 'if not $FOO {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if ((test in @{VAR1})) and defined $FOO {'], 'if test in @{VAR1} and defined $FOO {' + condition_contents + '}'), + (['if test3 in @{VAR2} {'], 'if test3 in @{VAR2} {' + condition_contents + '}'), + (['if 9 < @{VAR3} {'], 'if 9 < @{VAR3} {' + condition_contents + '}'), + (['if test in "test2 test" {'], 'if test in "test2 test" {' + condition_contents + '}'), + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalComplex, cls).setUpClass() + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.profile_data[profile]['file'].add(FileRule.create_instance('/bin/false rix,')) + + +class TestConditionalSimple(TestConditional): + """ Class to test simple conditionals (boolean and defined operations), profile_data contains one file rule """ + # directly related to how setUpClass sets up profile_data + condition_contents = '\n /bin/false rix,\n\n' + + tests = ( + # ConditionalBlock clean rule + (['if $FOO {', '} else if $BAR {', '} else {'], 'if $FOO {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not $FOO {', '} else if not $BAR {', '} else {'], 'if not $FOO {' + condition_contents + '} else if not $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if defined $FOO {', '} else if not defined ${BAR} {', '} else {'], 'if defined $FOO {' + condition_contents + '} else if not defined ${BAR} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else if defined @{VAR1} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else if defined @{VAR1} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {'], 'if not defined @{VAR2} {' + condition_contents + '}'), + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalSimple, cls).setUpClass() + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.profile_data[profile]['file'].add(FileRule.create_instance('/bin/false rix,')) + + +class TestConditionalSubProfile(TestConditional): + """ Class to test simple conditionals, profile_data contains hat definition """ + # directly related to how setUpClass sets up profile_data + condition_contents = '\n /** w,\n /bin/false rix,\n\n ^bar {\n /bin/true rix,\n\n }\n' + + tests = ( + # ConditionalBlock clean rule + (['if $FOO {', '} else if $BAR {', '} else {'], 'if $FOO {' + condition_contents + '} else if $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not $FOO {', '} else if not $BAR {', '} else {'], 'if not $FOO {' + condition_contents + '} else if not $BAR {' + condition_contents + '} else {' + condition_contents + '}'), + (['if defined $FOO {', '} else if not defined ${BAR} {', '} else {'], 'if defined $FOO {' + condition_contents + '} else if not defined ${BAR} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else if defined @{VAR1} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else if defined @{VAR1} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {', '} else {'], 'if not defined @{VAR2} {' + condition_contents + '} else {' + condition_contents + '}'), + (['if not defined @{VAR2} {'], 'if not defined @{VAR2} {' + condition_contents + '}'), + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalSubProfile, cls).setUpClass() + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.profile_data[profile]['file'].add(FileRule.create_instance('/bin/false rix,')) + cls.profile_data[profile]['file'].add(FileRule.create_instance('/** w,')) + (profile, hat, prof_storage) = ProfileStorage.parse('^bar {', cls.filename, 0, profile, None) + profname = combine_profname((profile, hat)) + cls.profile_data[profname] = prof_storage + cls.profile_data[profname]['in_cond'] = True + cls.profile_data[profname]['file'].add(FileRule.create_instance('/bin/true rix,')) + + +class TestConditionalLogprof(TestConditional): + """ Class to test output of Logprof """ + # directly related to how setUpClass sets up profile_data + condition_contents = '\n /** w,\n /bin/false rix,\n\n ^bar {\n /bin/true rix,\n\n }\n' + + tests = ( + (['if $FOO {', '} else if $BAR {', '} else {'], + ['ConditionalBlock', [['Conditional', 'if $FOO {' + condition_contents + '}'], + ['Conditional', 'else if $BAR {' + condition_contents + '}'], + ['Conditional', 'else {' + condition_contents + '}']]]), + (['if not $FOO {', '} else if not $BAR {', '} else {'], + ['ConditionalBlock', [['Conditional', 'if not $FOO {' + condition_contents + '}'], + ['Conditional', 'else if not $BAR {' + condition_contents + '}'], + ['Conditional', 'else {' + condition_contents + '}']]]), + (['if defined $FOO {', '} else if not defined $BAR {', '} else {'], + ['ConditionalBlock', [['Conditional', 'if defined $FOO {' + condition_contents + '}'], + ['Conditional', 'else if not defined $BAR {' + condition_contents + '}'], + ['Conditional', 'else {' + condition_contents + '}']]]), + (['if not defined @{VAR2} {', '} else if defined @{VAR1} {', '} else {'], + ['ConditionalBlock', [['Conditional', 'if not defined @{VAR2} {' + condition_contents + '}'], + ['Conditional', 'else if defined @{VAR1} {' + condition_contents + '}'], + ['Conditional', 'else {' + condition_contents + '}']]]), + (['if not defined @{VAR2} {', '} else {'], + ['ConditionalBlock', [['Conditional', 'if not defined @{VAR2} {' + condition_contents + '}'], + ['Conditional', 'else {' + condition_contents + '}']]]), + (['if not defined @{VAR2} {'], + ['ConditionalBlock', [['Conditional', 'if not defined @{VAR2} {' + condition_contents + '}']]]), + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalLogprof, cls).setUpClass() + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.profile_data[profile]['file'].add(FileRule.create_instance('/bin/false rix,')) + cls.profile_data[profile]['file'].add(FileRule.create_instance('/** w,')) + (profile, hat, prof_storage) = ProfileStorage.parse('^bar {', cls.filename, 0, profile, None) + profname = combine_profname((profile, hat)) + cls.profile_data[profname] = prof_storage + cls.profile_data[profname]['in_cond'] = True + cls.profile_data[profname]['file'].add(FileRule.create_instance('/bin/true rix,')) + + def _run_test(self, params, expected): + conditional_block = None + for condition in params: + if condition == params[0]: + conditional_block = ConditionalBlock(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + else: + conditional_block.add_conditional(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + + self.assertEqual(conditional_block.logprof_header(), expected) + + +class TestConditionalEquality(AATest): + """ Class to test equality tests """ + filename = 'somefile' + profile_data = {} + + tests = ( + # should always be different + # first condition second condition + ((['if not $FOO {'], profile_data), (['if $FOO {'], profile_data)), + ((['if defined $FOO {'], profile_data), (['if $FOO {'], profile_data)), + ((['if $BAR {'], profile_data), (['if $FOO {'], profile_data)), + ((['if $FOO {'], None), (['if $FOO {'], profile_data)), + ((['if $FOO {', '} else if $BAR {'], profile_data), (['if $FOO {', '} else {'], profile_data)), + ((['if $FOO and $BAR {'], profile_data), (['if $FOO and $BAR and defined $BAZ{'], profile_data)), + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalEquality, cls).setUpClass() + cls.active_profiles = ProfileList() + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.profile_data[profile]['file'].add(FileRule.create_instance('/bin/false rix,')) + cls.profile_data[profile]['file'].add(FileRule.create_instance('/** w,')) + (profile, hat, prof_storage) = ProfileStorage.parse('^bar {', cls.filename, 0, profile, None) + profname = combine_profname((profile, hat)) + cls.profile_data[profname] = prof_storage + cls.profile_data[profname]['in_cond'] = True + cls.profile_data[profname]['file'].add(FileRule.create_instance('/bin/true rix,')) + cls.active_profiles.add_rule(cls.filename, 'variable', VariableRule.create_instance('@{VAR1} = "test"')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$FOO=true')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$BAR=false')) + + def init_cond(self, conds, profile_data): + block = None + for cond in conds: + if cond == conds[0]: + block = ConditionalBlock(cond, self.active_profiles.files[self.filename]) + block.store_profile_data(profile_data) + else: + block.add_conditional(cond, self.active_profiles.files[self.filename]) + block.store_profile_data(profile_data) + return block + + def _run_test(self, cond1, cond2): + conditional_block1 = self.init_cond(cond1[0], cond1[1]) + conditional_block2 = self.init_cond(cond2[0], cond2[1]) + + self.assertFalse(conditional_block1.is_equal(conditional_block2)) + self.assertFalse(conditional_block1.is_covered(conditional_block2)) + + def test_is_equal_condition(self): + conditional_block1 = self.init_cond(['if $FOO {', '} else if $BAR {', '} else {'], self.profile_data) + conditional_block2 = self.init_cond(['if $FOO {', '} else {'], self.profile_data) + + self.assertFalse(conditional_block1.is_equal(conditional_block2)) + self.assertFalse(conditional_block1.is_covered(conditional_block2)) + + def test_is_equal(self): + conditional_block1 = ConditionalBlock('if not defined $FOO {', self.active_profiles.files[self.filename]) + conditional_block1.store_profile_data(self.profile_data) + + self.assertTrue(conditional_block1.is_equal(conditional_block1)) + self.assertTrue(conditional_block1.is_covered(conditional_block1)) + + +class TestConditionalException(AATest): + filename = 'somefile' + profile_data = {} + + tests = ( + (['if @{VAR1} {', '} else if $BAR {', '} else {'], AppArmorException), # set variable used in boolean comparison + (['if $FOO {', '} else if @{VAR} {', '} else {'], AppArmorException), # undefined variable in else if + (['if ${BAZ} {', '} else {'], AppArmorException), # undefined boolean variable with else + (['if ${VAR} {'], AppArmorException), # undefined boolean variable + (['if foo in @{UNDERFINED} {'], AppArmorException), # undefined set variable + ) + + @classmethod + def setUpClass(cls): + super(TestConditionalException, cls).setUpClass() + cls.active_profiles = ProfileList() + cls.condition_contents = '\n' + (profile, hat, prof_storage) = ProfileStorage.parse('profile foo {', cls.filename, 0, False, False) + cls.profile_data[profile] = prof_storage + cls.active_profiles.add_rule(cls.filename, 'variable', VariableRule.create_instance('@{VAR1} = "test"')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$FOO=true')) + cls.active_profiles.add_rule(cls.filename, 'boolean', BooleanRule.create_instance('$BAR=false')) + + def _run_test(self, params, expected): + conditional_block = None + with self.assertRaises(expected): + for condition in params: + if condition == params[0]: + conditional_block = ConditionalBlock(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + else: + conditional_block.add_conditional(condition, self.active_profiles.files[self.filename]) + conditional_block.store_profile_data(self.profile_data) + + def test_create_instance_block1(self): + with self.assertRaises(NotImplementedError): + ConditionalBlock.create_instance('test') + + def test_create_instance_block2(self): + with self.assertRaises(NotImplementedError): + ConditionalBlock._create_instance('test', None) + + +class TestAppArmorAstException(AATest): + tests = ( + ('foo = "1"', AppArmorBug), # AppArmorAst only supports the actual condition + ('~ $FOO', AppArmorBug), # Invalid unary operator + ('func(${BAZ})', AppArmorBug), # Invalid function name + ) + + def _run_test(self, params, expected): + ast = AppArmorAst(params) + with self.assertRaises(expected): + ast.get_clean() + with self.assertRaises(expected): + ast.evaluate(None) + + +class TestMiscClassException(AATest): + def test_invalid_op_1(self): + term = Term.create_instance('10') + op = 'is' + with self.assertRaises(AppArmorBug): + CompareCondition(term, op, term) + + def test_invalid_op_2(self): + term = Term.create_instance('10') + cond = CompareCondition(term, 'in', term) + cond.op = 'is' + with self.assertRaises(AppArmorBug): + cond.evaluate(None) + + def test_invalid_term(self): + with self.assertRaises(AppArmorBug): + Term.create_instance('#') + + def test_invalid_conditional_condition(self): + with self.assertRaises(AppArmorBug): + ConditionalRule('invalid', AppArmorAst('defined $BAR')) + + def test_invalid_conditional_ast_tree(self): + with self.assertRaises(AppArmorBug): + ConditionalRule(ConditionalRule.IF, 'defined $BAR') + + def test_invalid_conditional_clean(self): + cond = ConditionalRule(ConditionalRule.IF, AppArmorAst('defined $BAR')) + cond.condition = 'invalid' + with self.assertRaises(AppArmorBug): + cond.get_clean() + + def test_invalid_boolean_defined(self): + with self.assertRaises(AppArmorBug): + BooleanCondition(None, Term.create_instance('10')) + + def test_invalid_boolean_varible(self): + with self.assertRaises(AppArmorBug): + BooleanCondition('defined', '10') + + def test_invalid_compare_left(self): + with self.assertRaises(AppArmorBug): + CompareCondition(None, '==', Term.create_instance('10')) + + def test_invalid_compare_right(self): + with self.assertRaises(AppArmorBug): + CompareCondition(Term.create_instance('10'), '==', None) + + def test_invalid_compare_op(self): + with self.assertRaises(AppArmorBug): + CompareCondition(Term.create_instance('10'), '~', Term.create_instance('10')) + + def test_invalid_compare(self): + term = Term.create_instance('10') + cond = CompareCondition(term, '==', term) + with self.assertRaises(AppArmorBug): + cond.compare(cond.op, term.get_set(None), 10) + + def test_invalid_compare_evaluate(self): + term = InvalidTerm(10) + cond = CompareCondition(term, '==', term) + with self.assertRaises(AppArmorBug): + cond.evaluate(None) + + +class InvalidTerm(Term): + def __init__(self, value): + self.value = value + + def get_set(self, prof_storage): + return self.value # not a set + + +setup_all_loops(__name__) +if __name__ == '__main__': + unittest.main(verbosity=1) diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index 9bca20507..fbba878f4 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -340,47 +340,6 @@ syntax_failure = ( # missing profile keywords 'profile/re_named_ok2.sd', - # Syntax Errors caused by boolean conditions (parse_profile_data() gets confused by the closing '}') - 'conditional/defined_1.sd', - 'conditional/defined_2.sd', - 'conditional/else_1.sd', - 'conditional/else_2.sd', - 'conditional/else_3.sd', - 'conditional/else_if_1.sd', - 'conditional/else_if_2.sd', - 'conditional/else_if_3.sd', - 'conditional/else_if_5.sd', - 'conditional/ok_1.sd', - 'conditional/ok_2.sd', - 'conditional/ok_3.sd', - 'conditional/ok_4.sd', - 'conditional/ok_5.sd', - 'conditional/ok_6.sd', - 'conditional/ok_7.sd', - 'conditional/ok_8.sd', - 'conditional/ok_9.sd', - 'conditional/stress_1.sd', - 'conditional/ok_10.sd', - 'conditional/ok_11.sd', - 'conditional/ok_12.sd', - 'conditional/ok_13.sd', - 'conditional/ok_14.sd', - 'conditional/ok_15.sd', - 'conditional/ok_16.sd', - 'conditional/ok_17.sd', - 'conditional/ok_18.sd', - 'conditional/ok_19.sd', - 'conditional/ok_20.sd', - 'conditional/ok_21.sd', - 'conditional/ok_22.sd', - 'conditional/ok_23.sd', - 'conditional/ok_24.sd', - 'conditional/ok_25.sd', - 'conditional/ok_26.sd', - 'conditional/ok_27.sd', - 'conditional/ok_28.sd', - 'conditional/ok_29.sd', - # unexpected uppercase vs. lowercase in *x rules 'file/ok_5.sd', # Invalid mode UX 'file/ok_2.sd', # Invalid mode RWM From f9690cdb6c2559d953babf35a4d47787f9b3bacd Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Wed, 30 Jul 2025 18:24:06 -0300 Subject: [PATCH 4/5] parser/tst: create generic target to generate parser test cases Instead of having to add a new target to build generated test cases in .gitlab-ci.yml, use a generic "gen_tests" that calls the others. Signed-off-by: Georgia Garcia --- .gitlab-ci.yml | 3 +-- parser/tst/Makefile | 8 +++++--- spread.yaml | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 13aebbe74..514d146e7 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -103,8 +103,7 @@ test-utils: - printf '\e[0K%s:%s:%s\r\e[0K\n' section_end "$(date +%s)" install_extra_deps # See apparmor/apparmor#221 - - make -C parser/tst gen_dbus - - make -C parser/tst gen_xtrans + - make -C parser/tst gen_tests - make -C utils check - make -C utils/test coverage-regression artifacts: diff --git a/parser/tst/Makefile b/parser/tst/Makefile index 0ecb6ec67..d36d9ff45 100644 --- a/parser/tst/Makefile +++ b/parser/tst/Makefile @@ -17,7 +17,7 @@ endif all: tests -.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality dirtest valgrind gen_conditionals +.PHONY: tests error_output gen_dbus gen_xtrans parser_sanity caching minimize equality dirtest valgrind gen_conditionals gen_tests 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 simple_tests/generated_conditional/ @@ -34,10 +34,12 @@ gen_dbus: $(GEN_TRANS_DIRS) gen_conditionals: $(GEN_TRANS_DIRS) ./gen-conditionals.py +gen_tests: gen_xtrans gen_dbus gen_conditionals + error_output: $(PARSER) LANG=C ./errors.py -p "$(PARSER)" $(PYTEST_ARG) -parser_sanity: $(PARSER) gen_xtrans gen_dbus gen_conditionals +parser_sanity: $(PARSER) gen_tests $(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 @@ -57,7 +59,7 @@ equality: $(PARSER) dirtest: $(PARSER) LANG=C APPARMOR_PARSER="$(PARSER) $(PARSER_ARGS)" ./dirtest.sh -valgrind: $(PARSER) gen_xtrans gen_dbus +valgrind: $(PARSER) gen_tests LANG=C ./valgrind_simple.py -p "$(PARSER) $(PARSER_ARGS)" -v simple_tests $(PARSER): diff --git a/spread.yaml b/spread.yaml index eeee192bc..2e27bd038 100644 --- a/spread.yaml +++ b/spread.yaml @@ -284,7 +284,7 @@ suites: summary: Unit tests for the Python utilities. prepare: | # Generate apparmor profiles that the tests rely on. - make -C "$SPREAD_PATH"/parser/tst gen_xtrans gen_dbus + make -C "$SPREAD_PATH"/parser/tst gen_tests # Spread does not support programmatically generated test variants. # Ensure that the list baked into utils/test/task.yaml contains all # the files matching utils/test/test-*.py From 19f0ac1773973b922ee922255b47f2f987b46d57 Mon Sep 17 00:00:00 2001 From: Georgia Garcia Date: Wed, 13 Aug 2025 12:33:18 -0300 Subject: [PATCH 5/5] parser: fix validation of "defined" boolean variable conditional "Defined" operations on boolean variables were returning the value of the boolean variable instead of checking if it existed or not. That caused the parser to fail to compile the profile if the boolean variable was not defined, which is the whole purpose of the "defined" operation. Signed-off-by: Georgia Garcia --- parser/parser_yacc.y | 2 +- parser/tst/simple_tests/conditional/ok_31.sd | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 parser/tst/simple_tests/conditional/ok_31.sd diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 5612359be..b721aaa67 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -1006,7 +1006,7 @@ factor: TOK_DEFINED TOK_SET_VAR factor: TOK_DEFINED TOK_BOOL_VAR { - cond_expr *conds = new cond_expr($2, BOOLEAN_OP); + cond_expr *conds = new cond_expr($2, DEFINED_OP); PDEBUG("Matched: defined set expr %s value %d\n", $2, conds->eval()); $$ = conds; free($2); diff --git a/parser/tst/simple_tests/conditional/ok_31.sd b/parser/tst/simple_tests/conditional/ok_31.sd new file mode 100644 index 000000000..678989e73 --- /dev/null +++ b/parser/tst/simple_tests/conditional/ok_31.sd @@ -0,0 +1,8 @@ +#=DESCRIPTION conditional within profile +#=EXRESULT PASS + +/bin/true { + if defined $UNKNOWN { + /bin/true rix, + } +}