From 2416faac5412716a2178ca3d6d7c48b477af45a3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 17 Aug 2019 05:22:10 -0700 Subject: [PATCH] parser: support matching xattr keys but not values Support profiles that choose to match the presence of an extended attribute without validating its value. This lets AppArmor target xattrs with binary data, such as security.ima and security.evm values. For example, it's now possible to write a profile such as: profile signed_binaries /** xattrs=(security.ima) { # ... } Both presence and value matches can be used in the same profile. To match a signed xattr, target both the xattr and the security.ima value: profile python_script /** xattrs=( security.evm security.apparmor="python" ) { # ... } Updated to work using out of band matching instead of separate data array. Signed-off-by: Eric Chiang Signed-off-by: John Johansen --- parser/parser_lex.l | 2 +- parser/parser_yacc.y | 9 ++++ .../xattrs/{bad_02.sd => ok_10.sd} | 2 +- parser/tst/simple_tests/xattrs/ok_11.sd | 7 +++ parser/tst/simple_tests/xattrs/ok_12.sd | 7 +++ parser/tst/simple_tests/xattrs/ok_13.sd | 7 +++ tests/regression/apparmor/mkprofile.pl | 13 +++-- tests/regression/apparmor/xattrs_profile.sh | 51 +++++++++++++++++-- utils/apparmor/regex.py | 2 +- 9 files changed, 88 insertions(+), 12 deletions(-) rename parser/tst/simple_tests/xattrs/{bad_02.sd => ok_10.sd} (84%) create mode 100644 parser/tst/simple_tests/xattrs/ok_11.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_12.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_13.sd diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 4e5946582..9c6023eeb 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -213,7 +213,7 @@ LIST_VALUE_ID_CHARS ([^ \t\n"!,]{-}[()]|\\[ ]|\\\t|\\\"|\\!|\\,|\\\(|\\\)) LIST_VALUE_QUOTED_ID_CHARS [^\0"]|\\\" LIST_VALUE_ID {LIST_VALUE_ID_CHARS}+ QUOTED_LIST_VALUE_ID \"{LIST_VALUE_QUOTED_ID_CHARS}+\" -ID_CHARS_NOEQ [^ \t\n"!,]{-}[=] +ID_CHARS_NOEQ [^ \t\n"!,]{-}[=)] LEADING_ID_CHARS_NOEQ [^ \t\n"!,]{-}[=()+&] ID_NOEQ {ID_CHARS_NOEQ}|(,{ID_CHARS_NOEQ}) IDS_NOEQ {LEADING_ID_CHARS_NOEQ}{ID_NOEQ}* diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index cd394ff6c..97712af13 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -1205,6 +1205,15 @@ network_rule: TOK_NETWORK TOK_ID TOK_ID TOK_END_OF_RULE $$ = entry; } +cond: TOK_CONDID + { + struct cond_entry *ent; + ent = new_cond_entry($1, 0, NULL); + if (!ent) + yyerror(_("Memory allocation error.")); + $$ = ent; + } + cond: TOK_CONDID TOK_EQUALS TOK_VALUE { struct cond_entry *ent; diff --git a/parser/tst/simple_tests/xattrs/bad_02.sd b/parser/tst/simple_tests/xattrs/ok_10.sd similarity index 84% rename from parser/tst/simple_tests/xattrs/bad_02.sd rename to parser/tst/simple_tests/xattrs/ok_10.sd index 206a34865..ecebbd1be 100644 --- a/parser/tst/simple_tests/xattrs/bad_02.sd +++ b/parser/tst/simple_tests/xattrs/ok_10.sd @@ -1,6 +1,6 @@ # #=Description no xattrs value -#=EXRESULT FAIL +#=EXRESULT PASS # /usr/bin/xattrs-test xattrs=(myvalue) { /foo r, diff --git a/parser/tst/simple_tests/xattrs/ok_11.sd b/parser/tst/simple_tests/xattrs/ok_11.sd new file mode 100644 index 000000000..0250c7808 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_11.sd @@ -0,0 +1,7 @@ +# +#=Description multiple xattrs with no value +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue myvalue2) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_12.sd b/parser/tst/simple_tests/xattrs/ok_12.sd new file mode 100644 index 000000000..810f99b0d --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_12.sd @@ -0,0 +1,7 @@ +# +#=Description xattrs key followed by key value +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue user.foo=bar) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_13.sd b/parser/tst/simple_tests/xattrs/ok_13.sd new file mode 100644 index 000000000..e17e17942 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_13.sd @@ -0,0 +1,7 @@ +# +#=Description xattrs key after key and value +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(user.foo=bar myvalue) { + /foo r, +} diff --git a/tests/regression/apparmor/mkprofile.pl b/tests/regression/apparmor/mkprofile.pl index 8803fce41..ae24c7bd5 100755 --- a/tests/regression/apparmor/mkprofile.pl +++ b/tests/regression/apparmor/mkprofile.pl @@ -379,10 +379,12 @@ sub gen_addimage($) { sub gen_xattr($) { my $rule = shift; my @rules = split (/:/, $rule); - if (@rules != 3) { - (!$nowarn) && print STDERR "Warning: invalid xattr description '$rule', ignored\n"; - } else { + if (@rules == 3) { $xattrs{$rules[1]} = $rules[2]; + } elsif (@rules == 2) { + $xattrs{$rules[1]} = ""; + } else { + (!$nowarn) && print STDERR "Warning: invalid xattr description '$rule', ignored\n"; } } @@ -479,7 +481,10 @@ sub gen_from_args() { } else { print STDOUT " "; } - print STDOUT "$xattr=$xattrs{$xattr}"; + print STDOUT "$xattr"; + if (not $xattrs{$xattr} eq "") { + print STDOUT "=$xattrs{$xattr}"; + } } print STDOUT ") "; } diff --git a/tests/regression/apparmor/xattrs_profile.sh b/tests/regression/apparmor/xattrs_profile.sh index 9ce79b5fc..41c015354 100755 --- a/tests/regression/apparmor/xattrs_profile.sh +++ b/tests/regression/apparmor/xattrs_profile.sh @@ -138,11 +138,7 @@ runchecktest "Profiles with xattrs and same path length" pass profile_1 clean_xattr -# xattr matching doesn't work if the xattr value has a null character. This -# impacts matching security.ima and security.evm values. -# -# A kernel patch has been proposed to fix this: -# https://lists.ubuntu.com/archives/apparmor/2018-December/011882.html +# xattr value matching doesn't work if the xattr value has a null character. genprofile "image=profile_1" \ "addimage:$file" \ @@ -157,3 +153,48 @@ set_xattr "user.foo" "0x610062" # "a\0b" runchecktest "xattr values with null characters don't work" pass unconfined clean_xattr + +# All test cases below this use an xattr_key match +requires_kernel_features domain/attach_conditions/xattr_key + +# xattr value matching doesn't work if the xattr value has a null character. + +genprofile "image=profile_1" \ + "addimage:$file" \ + "path:$file" \ + "/proc/*/attr/current:r" \ + "xattr:user.foo" \ + +runchecktest "Path with no xattrs" pass unconfined +set_xattr "user.foo" "ab" +runchecktest "matches value" pass profile_1 +set_xattr "user.foo" "0x610062" # "a\0b" +runchecktest "xattr values with null characters don't work" pass profile_1 + +clean_xattr + +# Test that xattr keys contribute to the priority of a profile + +genprofile "image=profile_1" \ + "addimage:$file" \ + "path:$file" \ + "/proc/*/attr/current:r" \ + "xattr:user.foo:hello" \ + "xattr:user.bar:bye" \ + -- \ + "image=profile_2" \ + "addimage:$file" \ + "path:$file" \ + "/proc/*/attr/current:r" \ + "xattr:user.foo:hello" \ + "xattr:user.bar" \ + "xattr:user.spam" + +runchecktest "Path with no xattrs" pass unconfined +set_xattr "user.foo" "hello" +set_xattr "user.bar" "bye" +runchecktest "matches value" pass profile_1 +set_xattr "user.spam" "spam" +runchecktest "matches more xattrs in profile_2" pass profile_2 + +clean_xattr diff --git a/utils/apparmor/regex.py b/utils/apparmor/regex.py index 0b2bcb788..6197331c4 100644 --- a/utils/apparmor/regex.py +++ b/utils/apparmor/regex.py @@ -30,7 +30,7 @@ RE_PATH = '/\S*|"/[^"]*"' # filename (starting with '/') withou 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 + '|@{\S+}\S*|"@{\S+}[^"]*"))' # quoted or unquoted filename or variable. %s is the match group name RE_SAFE_OR_UNSAFE = '(?P(safe|unsafe))' -RE_XATTRS = '(\s+xattrs\s*=\s*\((?P([^)=]+=[^)=]+\s?)+)\)\s*)?' +RE_XATTRS = '(\s+xattrs\s*=\s*\((?P([^)=]+(=[^)=]+)?\s?)+)\)\s*)?' RE_FLAGS = '(\s+(flags\s*=\s*)?\((?P[^)]+)\))?' RE_PROFILE_END = re.compile('^\s*\}' + RE_EOL)