From a42fd8c6f4c76d2685c2e5ecc88f84af5997c5b4 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 6 Dec 2018 10:54:46 -0800 Subject: [PATCH] parser: add support for matching based on extended file attributes Add userland support for matching based on extended file attributes. This leverages DFA based matching already in the kernel: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8e51f908 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=73f488cd Matching is exposed via flags on the profile: /usr/bin/* xattrs=(user.foo=bar user.bar=**) { # ... } Profiles list the set of extended attributes that a file MUST have, and a regex to match the value of that extended attributes. Additional extended attributes on the file don't effect the match. Signed-off-by: Eric Chiang --- parser/libapparmor_re/aare_rules.cc | 38 +++++++++++++++++++++ parser/libapparmor_re/aare_rules.h | 1 + parser/libapparmor_re/expr-tree.cc | 7 ++-- parser/libapparmor_re/expr-tree.h | 21 ++++++++++++ parser/parser_interface.c | 24 +++++++++++++ parser/parser_lex.l | 2 +- parser/parser_regex.c | 41 ++++++++++++++++++++++- parser/parser_yacc.y | 18 ++++++++-- parser/profile.h | 4 +++ parser/tst/simple_tests/xattrs/bad_01.sd | 7 ++++ parser/tst/simple_tests/xattrs/bad_02.sd | 7 ++++ parser/tst/simple_tests/xattrs/bad_03.sd | 7 ++++ parser/tst/simple_tests/xattrs/hats_01.sd | 10 ++++++ parser/tst/simple_tests/xattrs/ok_01.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_02.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_03.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_04.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_05.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_06.sd | 7 ++++ parser/tst/simple_tests/xattrs/ok_07.sd | 8 +++++ parser/tst/simple_tests/xattrs/ok_08.sd | 8 +++++ parser/tst/simple_tests/xattrs/ok_09.sd | 7 ++++ utils/apparmor/regex.py | 2 +- 23 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 parser/tst/simple_tests/xattrs/bad_01.sd create mode 100644 parser/tst/simple_tests/xattrs/bad_02.sd create mode 100644 parser/tst/simple_tests/xattrs/bad_03.sd create mode 100644 parser/tst/simple_tests/xattrs/hats_01.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_01.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_02.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_03.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_04.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_05.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_06.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_07.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_08.sd create mode 100644 parser/tst/simple_tests/xattrs/ok_09.sd diff --git a/parser/libapparmor_re/aare_rules.cc b/parser/libapparmor_re/aare_rules.cc index fd719db48..f5be7418f 100644 --- a/parser/libapparmor_re/aare_rules.cc +++ b/parser/libapparmor_re/aare_rules.cc @@ -124,6 +124,44 @@ bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit, return true; } +/* + * append_rule is like add_rule, but appends the rule to any existing rules + * with a null transition. The appended rule matches with the same permissions + * as the rule it's appended to. + * + * This is used by xattrs matching where, after matching the path, the DFA is + * advanced by a null character for each xattr. + */ +bool aare_rules::append_rule(const char *rule, dfaflags_t flags) +{ + Node *tree = NULL; + if (regex_parse(&tree, rule)) + return false; + + if (flags & DFA_DUMP_RULE_EXPR) { + cerr << "rule: "; + cerr << rule; + cerr << " -> "; + tree->dump(cerr); + cerr << "\n\n"; + } + + /* + * For each matching state, we want to create an optional path + * separated by a null character. + * + * When matching xattrs, the DFA must end up in an accepting state for + * the path, then each value of the xattrs. Using an optional node + * lets each rule end up in an accepting state. + */ + tree = new OptionalNode(new CatNode(new CharNode(0), tree)); + PermExprMap::iterator it; + for (it = expr_map.begin(); it != expr_map.end(); it++) { + expr_map[it->first] = new CatNode(it->second, tree); + } + return true; +} + /* create a dfa from the ruleset * returns: buffer contain dfa tables, @size set to the size of the tables * else NULL on failure, @min_match_len set to the shortest string diff --git a/parser/libapparmor_re/aare_rules.h b/parser/libapparmor_re/aare_rules.h index 3cdfa0963..ac0fba5d3 100644 --- a/parser/libapparmor_re/aare_rules.h +++ b/parser/libapparmor_re/aare_rules.h @@ -104,6 +104,7 @@ class aare_rules { uint32_t audit, dfaflags_t flags); bool add_rule_vec(int deny, uint32_t perms, uint32_t audit, int count, const char **rulev, dfaflags_t flags); + bool append_rule(const char *rule, dfaflags_t flags); void *create_dfa(size_t *size, int *min_match_len, dfaflags_t flags); }; diff --git a/parser/libapparmor_re/expr-tree.cc b/parser/libapparmor_re/expr-tree.cc index 69c24a072..94761703f 100644 --- a/parser/libapparmor_re/expr-tree.cc +++ b/parser/libapparmor_re/expr-tree.cc @@ -534,6 +534,9 @@ static void count_tree_nodes(Node *t, struct node_counts *counts) } else if (dynamic_cast(t)) { counts->star++; count_tree_nodes(t->child[0], counts); + } else if (dynamic_cast(t)) { + counts->optional++; + count_tree_nodes(t->child[0], counts); } else if (dynamic_cast(t)) { counts->charnode++; } else if (dynamic_cast(t)) { @@ -559,7 +562,7 @@ Node *simplify_tree(Node *t, dfaflags_t flags) int i; if (flags & DFA_DUMP_TREE_STATS) { - struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0 }; + struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }; count_tree_nodes(t, &counts); fprintf(stderr, "expr tree: c %d, [] %d, [^] %d, | %d, + %d, * %d, . %d, cat %d\n", @@ -595,7 +598,7 @@ Node *simplify_tree(Node *t, dfaflags_t flags) } } if (flags & DFA_DUMP_TREE_STATS) { - struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0 }; + struct node_counts counts = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }; count_tree_nodes(t, &counts); fprintf(stderr, "simplified expr tree: c %d, [] %d, [^] %d, | %d, + %d, * %d, . %d, cat %d\n", diff --git a/parser/libapparmor_re/expr-tree.h b/parser/libapparmor_re/expr-tree.h index 6990cdef6..21a3b07a2 100644 --- a/parser/libapparmor_re/expr-tree.h +++ b/parser/libapparmor_re/expr-tree.h @@ -482,6 +482,26 @@ public: bool contains_null() { return child[0]->contains_null(); } }; +/* Match a node zero or one times. */ +class OptionalNode: public OneChildNode { +public: + OptionalNode(Node *left): OneChildNode(left) { nullable = true; } + void compute_firstpos() { firstpos = child[0]->firstpos; } + void compute_lastpos() { lastpos = child[0]->lastpos; } + int eq(Node *other) + { + if (dynamic_cast(other)) + return child[0]->eq(other->child[0]); + return 0; + } + ostream &dump(ostream &os) + { + os << '('; + child[0]->dump(os); + return os << ")?"; + } +}; + /* Match a node one or more times. (This is a unary operator.) */ class PlusNode: public OneChildNode { public: @@ -713,6 +733,7 @@ struct node_counts { int alt; int plus; int star; + int optional; int any; int cat; }; diff --git a/parser/parser_interface.c b/parser/parser_interface.c index 07ea0f6db..32f5279e2 100644 --- a/parser/parser_interface.c +++ b/parser/parser_interface.c @@ -371,6 +371,28 @@ void sd_serialize_xtable(std::ostringstream &buf, char **table) sd_write_structend(buf); } +void sd_serialize_xattrs(std::ostringstream &buf, struct cond_entry_list xattrs) +{ + int count; + struct cond_entry *entry; + + if (!(xattrs.list)) + return; + + count = 0; + for (entry = xattrs.list; entry; entry = entry->next) { + count++; + } + + sd_write_struct(buf, "xattrs"); + sd_write_array(buf, NULL, count); + for (entry = xattrs.list; entry; entry = entry->next) { + sd_write_string(buf, entry->name, NULL); + } + sd_write_arrayend(buf); + sd_write_structend(buf); +} + void sd_serialize_profile(std::ostringstream &buf, Profile *profile, int flattened) { @@ -432,6 +454,8 @@ void sd_serialize_profile(std::ostringstream &buf, Profile *profile, sd_write_uint32(buf, 0); sd_write_structend(buf); + sd_serialize_xattrs(buf, profile->xattrs); + sd_serialize_rlimits(buf, &profile->rlimits); if (profile->net.allow && kernel_supports_network) { diff --git a/parser/parser_lex.l b/parser/parser_lex.l index 20c845693..52a6e6656 100644 --- a/parser/parser_lex.l +++ b/parser/parser_lex.l @@ -308,7 +308,7 @@ GT > } { - peer/{WS}*={WS}*\( { + (peer|xattrs)/{WS}*={WS}*\( { /* we match to the = in the lexer so that we can switch scanner * state. By the time the parser see the = it may be too late * as bison may have requested the next token from the scanner diff --git a/parser/parser_regex.c b/parser/parser_regex.c index 72df37aa8..1b3c2b659 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -432,12 +432,29 @@ static const char *local_name(const char *name) return name; } +/* + * get_xattr_value returns the value of an xattr expression, performing NULL + * checks along the way. The method returns NULL if the xattr match doesn't + * have an xattrs (though this case currently isn't permitted by the parser). + */ +char *get_xattr_value(struct cond_entry *entry) +{ + if (!entry->eq) + return NULL; + if (!entry->vals) + return NULL; + return entry->vals->value; +} + static int process_profile_name_xmatch(Profile *prof) { std::string tbuf; pattern_t ptype; const char *name; + struct cond_entry *entry; + const char *xattr_value; + /* don't filter_slashes for profile names */ if (prof->attachment) name = prof->attachment; @@ -451,7 +468,7 @@ static int process_profile_name_xmatch(Profile *prof) if (ptype == ePatternInvalid) { PERROR(_("%s: Invalid profile name '%s' - bad regular expression\n"), progname, name); return FALSE; - } else if (ptype == ePatternBasic && !(prof->altnames || prof->attachment)) { + } else if (ptype == ePatternBasic && !(prof->altnames || prof->attachment || prof->xattrs.list)) { /* no regex so do not set xmatch */ prof->xmatch = NULL; prof->xmatch_len = 0; @@ -479,6 +496,28 @@ static int process_profile_name_xmatch(Profile *prof) } } } + if (prof->xattrs.list) { + for (entry = prof->xattrs.list; entry; entry = entry->next) { + xattr_value = get_xattr_value(entry); + if (!xattr_value) + xattr_value = "**"; // Default to allowing any value. + /* len is measured because it's required to + * convert the regex to pcre, but doesn't impact + * xmatch_len. The kernel uses the number of + * xattrs matched to prioritized in addition to + * xmatch_len. + */ + int len; + tbuf.clear(); + convert_aaregex_to_pcre(xattr_value, 0, + glob_default, tbuf, + &len); + if (!rules->append_rule(tbuf.c_str(), dfaflags)) { + delete rules; + return FALSE; + } + } + } prof->xmatch = rules->create_dfa(&prof->xmatch_size, &prof->xmatch_len, dfaflags); delete rules; if (!prof->xmatch) diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index ab3c9fc59..3c4e69ee6 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -306,9 +306,9 @@ opt_id: { /* nothing */ $$ = NULL; } opt_id_or_var: { /* nothing */ $$ = NULL; } | id_or_var { $$ = $1; } -profile_base: TOK_ID opt_id_or_var flags TOK_OPEN rules TOK_CLOSE +profile_base: TOK_ID opt_id_or_var opt_cond_list flags TOK_OPEN rules TOK_CLOSE { - Profile *prof = $5; + Profile *prof = $6; bool self_stack = false; if (!prof) { @@ -342,7 +342,13 @@ profile_base: TOK_ID opt_id_or_var flags TOK_OPEN rules TOK_CLOSE prof->attachment = $2; if ($2 && !($2[0] == '/' || strncmp($2, "@{", 2) == 0)) yyerror(_("Profile attachment must begin with a '/' or variable.")); - prof->flags = $3; + if ($3.name) { + if (strcmp($3.name, "xattrs") != 0) + yyerror(_("profile id: invalid conditional group %s=()"), $3.name); + free ($3.name); + prof->xattrs = $3; + } + prof->flags = $4; if (force_complain && kernel_abi_version == 5) /* newer abis encode force complain as part of the * header @@ -393,6 +399,12 @@ hat: hat_start profile_base Profile *prof = $2; if ($2) PDEBUG("Matched: hat %s { ... }\n", prof->name); + /* + * It isn't clear what a xattrs match on a hat profile + * should do, disallow it for now. + */ + if ($2->xattrs.list) + yyerror("hat profiles can't use xattrs matches"); prof->flags.hat = 1; $$ = prof; diff --git a/parser/profile.h b/parser/profile.h index 7121c0a75..048a7cf9c 100644 --- a/parser/profile.h +++ b/parser/profile.h @@ -120,6 +120,8 @@ public: size_t xmatch_size; int xmatch_len; + struct cond_entry_list xattrs; + /* char *sub_name; */ /* subdomain name or NULL */ /* int default_deny; */ /* TRUE or FALSE */ int local; @@ -151,6 +153,8 @@ public: xmatch_size = 0; xmatch_len = 0; + xattrs.list = NULL; + local = local_mode = local_audit = 0; parent = NULL; diff --git a/parser/tst/simple_tests/xattrs/bad_01.sd b/parser/tst/simple_tests/xattrs/bad_01.sd new file mode 100644 index 000000000..241dd1308 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/bad_01.sd @@ -0,0 +1,7 @@ +# +#=Description wrong conditional group +#=EXRESULT FAIL +# +/usr/bin/xattrs-test peer=(myvalue=foo) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/bad_02.sd b/parser/tst/simple_tests/xattrs/bad_02.sd new file mode 100644 index 000000000..206a34865 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/bad_02.sd @@ -0,0 +1,7 @@ +# +#=Description no xattrs value +#=EXRESULT FAIL +# +/usr/bin/xattrs-test xattrs=(myvalue) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/bad_03.sd b/parser/tst/simple_tests/xattrs/bad_03.sd new file mode 100644 index 000000000..3e5f09a3a --- /dev/null +++ b/parser/tst/simple_tests/xattrs/bad_03.sd @@ -0,0 +1,7 @@ +# +#=Description flags before xattrs +#=EXRESULT FAIL +# +/usr/bin/xattrs-test flags=(complain) xattrs=(myvalue=foo) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/hats_01.sd b/parser/tst/simple_tests/xattrs/hats_01.sd new file mode 100644 index 000000000..42b8a0cbe --- /dev/null +++ b/parser/tst/simple_tests/xattrs/hats_01.sd @@ -0,0 +1,10 @@ +# +#=Description hat profile with xattrs +#=EXRESULT FAIL +# +/usr/bin/xattrs-test { + ^hat xattrs=(myvalue=foo) { + /foo r, + } + /foo w, +} diff --git a/parser/tst/simple_tests/xattrs/ok_01.sd b/parser/tst/simple_tests/xattrs/ok_01.sd new file mode 100644 index 000000000..53bdfb312 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_01.sd @@ -0,0 +1,7 @@ +# +#=Description basic xattr value +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue=foo) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_02.sd b/parser/tst/simple_tests/xattrs/ok_02.sd new file mode 100644 index 000000000..eb10c17f6 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_02.sd @@ -0,0 +1,7 @@ +# +#=Description xattrs with quoted value +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue="foo.bar") { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_03.sd b/parser/tst/simple_tests/xattrs/ok_03.sd new file mode 100644 index 000000000..2cc5a44ab --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_03.sd @@ -0,0 +1,7 @@ +# +#=Description match any value of an xattr +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue="*") { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_04.sd b/parser/tst/simple_tests/xattrs/ok_04.sd new file mode 100644 index 000000000..401510510 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_04.sd @@ -0,0 +1,7 @@ +# +#=Description key with '.' character +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(hello.world=foo) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_05.sd b/parser/tst/simple_tests/xattrs/ok_05.sd new file mode 100644 index 000000000..798e81f08 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_05.sd @@ -0,0 +1,7 @@ +# +#=Description multiple xattrs +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(hello.world=foo goodbye.word=bar) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_06.sd b/parser/tst/simple_tests/xattrs/ok_06.sd new file mode 100644 index 000000000..c61b1063f --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_06.sd @@ -0,0 +1,7 @@ +# +#=Description xattrs then flags +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue=foo) flags=(audit, mediate_deleted) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_07.sd b/parser/tst/simple_tests/xattrs/ok_07.sd new file mode 100644 index 000000000..7ef7aab40 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_07.sd @@ -0,0 +1,8 @@ +# +#=Description named profile +#=EXRESULT PASS +# + +profile xattrs-test /usr/bin/hi xattrs=(user.foo=* user.bar=*) { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_08.sd b/parser/tst/simple_tests/xattrs/ok_08.sd new file mode 100644 index 000000000..94fa6c52d --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_08.sd @@ -0,0 +1,8 @@ +# +#=Description named profile without path +#=EXRESULT PASS +# + +profile xattrs-test xattrs=(user.foo="bar") { + /foo r, +} diff --git a/parser/tst/simple_tests/xattrs/ok_09.sd b/parser/tst/simple_tests/xattrs/ok_09.sd new file mode 100644 index 000000000..c60feb491 --- /dev/null +++ b/parser/tst/simple_tests/xattrs/ok_09.sd @@ -0,0 +1,7 @@ +# +#=Description profile with xattrs then flags +#=EXRESULT PASS +# +/usr/bin/xattrs-test xattrs=(myvalue=foo) flags=(complain) { + /foo r, +} diff --git a/utils/apparmor/regex.py b/utils/apparmor/regex.py index 4d16fb8f9..6ec966483 100644 --- a/utils/apparmor/regex.py +++ b/utils/apparmor/regex.py @@ -45,7 +45,7 @@ RE_PROFILE_CONDITIONAL_VARIABLE = re.compile('^\s*if\s+(not\s+)?defined\s+(@\{?\ RE_PROFILE_CONDITIONAL_BOOLEAN = re.compile('^\s*if\s+(not\s+)?defined\s+(\$\{?\w+\}?)\s*\{\s*(#.*)?$') RE_PROFILE_NETWORK = re.compile(RE_AUDIT_DENY + 'network(?P
\s+.*)?' + RE_COMMA_EOL) RE_PROFILE_CHANGE_HAT = re.compile('^\s*\^(\"??.+?\"??)' + RE_COMMA_EOL) -RE_PROFILE_HAT_DEF = re.compile('^(?P\s*)(?P\^|hat\s+)(?P\"??[^)]+?\"??)' + RE_XATTRS + RE_FLAGS + '\s*\{' + RE_EOL) +RE_PROFILE_HAT_DEF = re.compile('^(?P\s*)(?P\^|hat\s+)(?P\"??[^)]+?\"??)' + RE_FLAGS + '\s*\{' + RE_EOL) RE_PROFILE_DBUS = re.compile(RE_AUDIT_DENY + '(dbus\s*,|dbus(?P
\s+[^#]*)\s*,)' + RE_EOL) RE_PROFILE_MOUNT = re.compile(RE_AUDIT_DENY + '((mount|remount|umount|unmount)(\s+[^#]*)?\s*,)' + RE_EOL) RE_PROFILE_SIGNAL = re.compile(RE_AUDIT_DENY + '(signal\s*,|signal(?P
\s+[^#]*)\s*,)' + RE_EOL)