This cleans things up a bit and fixes a bug where not all rules are getting properly counted so that the addition of policy_mediation rules fails to generate the policy dfa in some cases.
Because the policy dfa is being generated correctly now we need to fix some tests to use the new -M flag to specify the expected features set of the test. Signed-off-by: John Johansen <john.johan...@canonical.com> --- parser/dbus.c | 24 ++++------ parser/libapparmor_re/aare_rules.cc | 80 +++++++++++------------------------- parser/libapparmor_re/aare_rules.h | 34 +++++++-------- parser/mount.c | 47 +++++++++------------ parser/parser_regex.c | 79 +++++++++++++++-------------------- parser/profile.cc | 4 - parser/profile.h | 6 +- 7 files changed, 114 insertions(+), 160 deletions(-) --- 2.9-test.orig/parser/dbus.c +++ 2.9-test/parser/dbus.c @@ -284,28 +284,26 @@ } if (mode & AA_DBUS_BIND) { - if (!aare_add_rule_vec(prof.policy.rules, deny, - mode & AA_DBUS_BIND, - audit & AA_DBUS_BIND, - 2, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, mode & AA_DBUS_BIND, + audit & AA_DBUS_BIND, + 2, vec, dfaflags)) goto fail; } if (mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE)) { - if (!aare_add_rule_vec(prof.policy.rules, deny, - mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE), - audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE), - 6, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, + mode & (AA_DBUS_SEND | AA_DBUS_RECEIVE), + audit & (AA_DBUS_SEND | AA_DBUS_RECEIVE), + 6, vec, dfaflags)) goto fail; } if (mode & AA_DBUS_EAVESDROP) { - if (!aare_add_rule_vec(prof.policy.rules, deny, - mode & AA_DBUS_EAVESDROP, - audit & AA_DBUS_EAVESDROP, - 1, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, + mode & AA_DBUS_EAVESDROP, + audit & AA_DBUS_EAVESDROP, + 1, vec, dfaflags)) goto fail; } - prof.policy.count++; return RULE_OK; fail: --- 2.9-test.orig/parser/libapparmor_re/aare_rules.cc +++ 2.9-test/parser/libapparmor_re/aare_rules.cc @@ -34,38 +34,20 @@ #include "chfa.h" #include "../immunix.h" -struct aare_ruleset { - int reverse; - Node *root; -}; -aare_ruleset_t *aare_new_ruleset(int reverse) -{ - aare_ruleset_t *container = (aare_ruleset_t *) malloc(sizeof(aare_ruleset_t)); - if (!container) - return NULL; - - container->root = NULL; - container->reverse = reverse; - - return container; -} -void aare_delete_ruleset(aare_ruleset_t *rules) +aare_rules::~aare_rules(void) { - if (rules) { - if (rules->root) - rules->root->release(); - free(rules); - } + if (root) + root->release(); aare_reset_matchflags(); } -int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny, - uint32_t perms, uint32_t audit, dfaflags_t flags) +bool aare_rules::add_rule(const char *rule, int deny, uint32_t perms, + uint32_t audit, dfaflags_t flags) { - return aare_add_rule_vec(rules, deny, perms, audit, 1, &rule, flags); + return add_rule_vec(deny, perms, audit, 1, &rule, flags); } #define FLAGS_WIDTH 2 @@ -94,9 +76,8 @@ #undef RESET_FLAGS } -int aare_add_rule_vec(aare_ruleset_t *rules, int deny, - uint32_t perms, uint32_t audit, - int count, const char **rulev, dfaflags_t flags) +bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit, + int count, const char **rulev, dfaflags_t flags) { Node *tree = NULL, *accept; int exact_match; @@ -105,15 +86,15 @@ assert(perms != 0); if (regex_parse(&tree, rulev[0])) - return 0; + return false; for (int i = 1; i < count; i++) { Node *subtree = NULL; Node *node = new CharNode(0); if (!node) - return 0; + return false; tree = new CatNode(tree, node); if (regex_parse(&subtree, rulev[i])) - return 0; + return false; tree = new CatNode(tree, subtree); } @@ -132,26 +113,15 @@ exact_match = 0; } - if (rules->reverse) + if (reverse) flip_tree(tree); /* 0x7f == 4 bits x mods + 1 bit unsafe mask + 1 bit ix, + 1 pux after shift */ #define EXTRACT_X_INDEX(perm, shift) (((perm) >> (shift + 7)) & 0x7f) -//if (perms & ALL_AA_EXEC_TYPE && (!perms & AA_EXEC_BITS)) -// fprintf(stderr, "adding X rule without MAY_EXEC: 0x%x %s\n", perms, rulev[0]); - -//if (perms & ALL_EXEC_TYPE) -// fprintf(stderr, "adding X rule %s 0x%x\n", rulev[0], perms); - -//if (audit) -//fprintf(stderr, "adding rule with audit bits set: 0x%x %s\n", audit, rulev[0]); - -//if (perms & AA_CHANGE_HAT) -// fprintf(stderr, "adding change_hat rule %s\n", rulev[0]); -/* the permissions set is assumed to be non-empty if any audit - * bits are specified */ + /* the permissions set is assumed to be non-empty if any audit + * bits are specified */ accept = NULL; for (unsigned int n = 0; perms && n < (sizeof(perms) * 8); n++) { uint32_t mask = 1 << n; @@ -230,44 +200,44 @@ cerr << "\n\n"; } - if (rules->root) - rules->root = new AltNode(rules->root, new CatNode(tree, accept)); + if (root) + root = new AltNode(root, new CatNode(tree, accept)); else - rules->root = new CatNode(tree, accept); + root = new CatNode(tree, accept); - return 1; + rule_count++; + 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 */ -void *aare_create_dfa(aare_ruleset_t *rules, size_t *size, - dfaflags_t flags) +void *aare_rules::create_dfa(size_t *size, dfaflags_t flags) { char *buffer = NULL; - label_nodes(rules->root); + label_nodes(root); if (flags & DFA_DUMP_TREE) { cerr << "\nDFA: Expression Tree\n"; - rules->root->dump(cerr); + root->dump(cerr); cerr << "\n\n"; } if (flags & DFA_CONTROL_TREE_SIMPLE) { - rules->root = simplify_tree(rules->root, flags); + root = simplify_tree(root, flags); if (flags & DFA_DUMP_SIMPLE_TREE) { cerr << "\nDFA: Simplified Expression Tree\n"; - rules->root->dump(cerr); + root->dump(cerr); cerr << "\n\n"; } } stringstream stream; try { - DFA dfa(rules->root, flags); + DFA dfa(root, flags); if (flags & DFA_DUMP_UNIQ_PERMS) dfa.dump_uniq_perms("dfa"); --- 2.9-test.orig/parser/libapparmor_re/aare_rules.h +++ 2.9-test/parser/libapparmor_re/aare_rules.h @@ -24,26 +24,24 @@ #include <stdint.h> #include "apparmor_re.h" +#include "expr-tree.h" -#ifdef __cplusplus -extern "C" { -#endif +class aare_rules { + Node *root; +public: + int reverse; + int rule_count; + aare_rules(): root(NULL), reverse(0), rule_count(0) { }; + aare_rules(int reverse): root(NULL), reverse(reverse), rule_count(0) { }; + ~aare_rules(); + + bool add_rule(const char *rule, int deny, uint32_t perms, + 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); + void *create_dfa(size_t *size, dfaflags_t flags); +}; -struct aare_ruleset; - -typedef struct aare_ruleset aare_ruleset_t; - -aare_ruleset_t *aare_new_ruleset(int reverse); -void aare_delete_ruleset(aare_ruleset_t *rules); -int aare_add_rule(aare_ruleset_t *rules, const char *rule, int deny, uint32_t perms, - uint32_t audit, dfaflags_t flags); -int aare_add_rule_vec(aare_ruleset_t *rules, int deny, uint32_t perms, - uint32_t audit, int count, const char **rulev, - dfaflags_t flags); -void *aare_create_dfa(aare_ruleset_t *rules, size_t *size, dfaflags_t flags); void aare_reset_matchflags(void); -#ifdef __cplusplus -} -#endif #endif /* __LIBAA_RE_RULES_H */ --- 2.9-test.orig/parser/mount.c +++ 2.9-test/parser/mount.c @@ -614,9 +614,9 @@ tmpallow = allow; /* rule for match without required data || data MATCH_CONT */ - if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow, - audit | AA_AUDIT_MNT_DATA, 4, - vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, tmpallow, + audit | AA_AUDIT_MNT_DATA, 4, + vec, dfaflags)) goto fail; count++; @@ -626,10 +626,9 @@ if (!build_mnt_opts(optsbuf, opts)) goto fail; vec[4] = optsbuf.c_str(); - if (!aare_add_rule_vec(prof.policy.rules, deny, - allow, - audit | AA_AUDIT_MNT_DATA, - 5, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, + audit | AA_AUDIT_MNT_DATA, + 5, vec, dfaflags)) goto fail; count++; } @@ -657,8 +656,8 @@ if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags)) goto fail; vec[3] = flagsbuf; - if (!aare_add_rule_vec(prof.policy.rules, deny, allow, - audit, 4, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec, + dfaflags)) goto fail; count++; } @@ -686,8 +685,8 @@ if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags)) goto fail; vec[3] = flagsbuf; - if (!aare_add_rule_vec(prof.policy.rules, deny, allow, - audit, 4, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec, + dfaflags)) goto fail; count++; } @@ -716,8 +715,8 @@ if (!build_mnt_flags(flagsbuf, PATH_MAX, tmpflags, tmpinv_flags)) goto fail; vec[3] = flagsbuf; - if (!aare_add_rule_vec(prof.policy.rules, deny, allow, - audit, 4, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 4, vec, + dfaflags)) goto fail; count++; } @@ -756,9 +755,9 @@ tmpallow = allow; /* rule for match without required data || data MATCH_CONT */ - if (!aare_add_rule_vec(prof.policy.rules, deny, tmpallow, - audit | AA_AUDIT_MNT_DATA, 4, - vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, tmpallow, + audit | AA_AUDIT_MNT_DATA, 4, + vec, dfaflags)) goto fail; count++; @@ -768,10 +767,9 @@ if (!build_mnt_opts(optsbuf, opts)) goto fail; vec[4] = optsbuf.c_str(); - if (!aare_add_rule_vec(prof.policy.rules, deny, - allow, - audit | AA_AUDIT_MNT_DATA, - 5, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, + audit | AA_AUDIT_MNT_DATA, + 5, vec, dfaflags)) goto fail; count++; } @@ -782,8 +780,8 @@ if (!convert_entry(mntbuf, mnt_point)) goto fail; vec[0] = mntbuf.c_str(); - if (!aare_add_rule_vec(prof.policy.rules, deny, allow, - audit, 1, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 1, vec, + dfaflags)) goto fail; count++; } @@ -796,8 +794,8 @@ if (!clear_and_convert_entry(devbuf, device)) goto fail; vec[1] = devbuf.c_str(); - if (!aare_add_rule_vec(prof.policy.rules, deny, allow, - audit, 2, vec, dfaflags)) + if (!prof.policy.rules->add_rule_vec(deny, allow, audit, 2, vec, + dfaflags)) goto fail; count++; } @@ -806,7 +804,6 @@ /* didn't actually encode anything */ goto fail; - prof.policy.count++; return RULE_OK; fail: --- 2.9-test.orig/parser/parser_regex.c +++ 2.9-test/parser/parser_regex.c @@ -431,11 +431,11 @@ prof->xmatch_size = 0; } else { /* build a dfa */ - aare_ruleset_t *rule = aare_new_ruleset(0); - if (!rule) + aare_rules *rules = new aare_rules(); + if (!rules) return FALSE; - if (!aare_add_rule(rule, tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) { - aare_delete_ruleset(rule); + if (!rules->add_rule(tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) { + delete rules; return FALSE; } if (prof->altnames) { @@ -450,15 +450,14 @@ len = strlen(alt->name); if (len < prof->xmatch_len) prof->xmatch_len = len; - if (!aare_add_rule(rule, tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) { - aare_delete_ruleset(rule); + if (!rules->add_rule(tbuf.c_str(), 0, AA_MAY_EXEC, 0, dfaflags)) { + delete rules; return FALSE; } } } - prof->xmatch = aare_create_dfa(rule, &prof->xmatch_size, - dfaflags); - aare_delete_ruleset(rule); + prof->xmatch = rules->create_dfa(&prof->xmatch_size, dfaflags); + delete rules; if (!prof->xmatch) return FALSE; } @@ -466,7 +465,7 @@ return TRUE; } -static int process_dfa_entry(aare_ruleset_t *dfarules, struct cod_entry *entry) +static int process_dfa_entry(aare_rules *dfarules, struct cod_entry *entry) { std::string tbuf; pattern_t ptype; @@ -498,13 +497,13 @@ * entry of the pair */ if (entry->deny && (entry->mode & AA_LINK_BITS)) { - if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny, - entry->mode & ~AA_LINK_BITS, - entry->audit & ~AA_LINK_BITS, dfaflags)) + if (!dfarules->add_rule(tbuf.c_str(), entry->deny, + entry->mode & ~AA_LINK_BITS, + entry->audit & ~AA_LINK_BITS, dfaflags)) return FALSE; } else if (entry->mode & ~AA_CHANGE_PROFILE) { - if (!aare_add_rule(dfarules, tbuf.c_str(), entry->deny, entry->mode, - entry->audit, dfaflags)) + if (!dfarules->add_rule(tbuf.c_str(), entry->deny, entry->mode, + entry->audit, dfaflags)) return FALSE; } @@ -526,7 +525,7 @@ perms |= LINK_TO_LINK_SUBSET(perms); vec[1] = "/[^/].*"; } - if (!aare_add_rule_vec(dfarules, entry->deny, perms, entry->audit & AA_LINK_BITS, 2, vec, dfaflags)) + if (!dfarules->add_rule_vec(entry->deny, perms, entry->audit & AA_LINK_BITS, 2, vec, dfaflags)) return FALSE; } if (entry->mode & AA_CHANGE_PROFILE) { @@ -545,12 +544,12 @@ vec[index++] = tbuf.c_str(); /* regular change_profile rule */ - if (!aare_add_rule_vec(dfarules, 0, AA_CHANGE_PROFILE | AA_ONEXEC, 0, index - 1, &vec[1], dfaflags)) + if (!dfarules->add_rule_vec(0, AA_CHANGE_PROFILE | AA_ONEXEC, 0, index - 1, &vec[1], dfaflags)) return FALSE; /* onexec rules - both rules are needed for onexec */ - if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, 1, vec, dfaflags)) + if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, 1, vec, dfaflags)) return FALSE; - if (!aare_add_rule_vec(dfarules, 0, AA_ONEXEC, 0, index, vec, dfaflags)) + if (!dfarules->add_rule_vec(0, AA_ONEXEC, 0, index, vec, dfaflags)) return FALSE; } return TRUE; @@ -560,15 +559,12 @@ { int ret = TRUE; struct cod_entry *entry; - int count = 0; list_for_each(prof->entries, entry) { if (!process_dfa_entry(prof->dfa.rules, entry)) ret = FALSE; - count++; } - prof->dfa.count = count; return ret; } @@ -579,17 +575,17 @@ if (!process_profile_name_xmatch(prof)) goto out; - prof->dfa.rules = aare_new_ruleset(0); + prof->dfa.rules = new aare_rules(); if (!prof->dfa.rules) goto out; if (!post_process_entries(prof)) goto out; - if (prof->dfa.count > 0) { - prof->dfa.dfa = aare_create_dfa(prof->dfa.rules, &prof->dfa.size, - dfaflags); - aare_delete_ruleset(prof->dfa.rules); + if (prof->dfa.rules->rule_count > 0) { + prof->dfa.dfa = prof->dfa.rules->create_dfa(&prof->dfa.size, + dfaflags); + delete prof->dfa.rules; prof->dfa.rules = NULL; if (!prof->dfa.dfa) goto out; @@ -683,7 +679,7 @@ { int error = -1; - prof->policy.rules = aare_new_ruleset(0); + prof->policy.rules = new aare_rules(); if (!prof->policy.rules) goto out; @@ -694,26 +690,21 @@ * to be supported */ - if (kernel_supports_mount) { - if (!aare_add_rule(prof->policy.rules, mediates_mount, 0, AA_MAY_READ, 0, dfaflags)) + if (kernel_supports_mount && + !prof->policy.rules->add_rule(mediates_mount, 0, AA_MAY_READ, 0, dfaflags)) goto out; - prof->policy.count++; - } - if (kernel_supports_dbus) { - if (!aare_add_rule(prof->policy.rules, mediates_dbus, 0, AA_MAY_READ, 0, dfaflags)) - goto out; - prof->policy.count++; - } - if (prof->policy.count > 0) { - prof->policy.dfa = aare_create_dfa(prof->policy.rules, - &prof->policy.size, - dfaflags); - aare_delete_ruleset(prof->policy.rules); + if (kernel_supports_dbus && + !prof->policy.rules->add_rule(mediates_dbus, 0, AA_MAY_READ, 0, dfaflags)) + goto out; + if (prof->policy.rules->rule_count > 0) { + prof->policy.dfa = prof->policy.rules->create_dfa(&prof->policy.size, dfaflags); + delete prof->policy.rules; + prof->policy.rules = NULL; if (!prof->policy.dfa) goto out; } else { - aare_delete_ruleset(prof->policy.rules); + delete prof->policy.rules; prof->policy.rules = NULL; } @@ -722,7 +713,7 @@ error = 0; out: - aare_delete_ruleset(prof->policy.rules); + delete prof->policy.rules; prof->policy.rules = NULL; return error; --- 2.9-test.orig/parser/profile.cc +++ 2.9-test/parser/profile.cc @@ -67,11 +67,11 @@ for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end(); i++) delete *i; if (dfa.rules) - aare_delete_ruleset(dfa.rules); + delete dfa.rules; if (dfa.dfa) free(dfa.dfa); if (policy.rules) - aare_delete_ruleset(policy.rules); + delete policy.rules; if (policy.dfa) free(policy.dfa); if (xmatch) --- 2.9-test.orig/parser/profile.h +++ 2.9-test/parser/profile.h @@ -18,6 +18,7 @@ #include "parser.h" #include "rule.h" +#include "libapparmor_re/aare_rules.h" class Profile; @@ -120,12 +121,11 @@ }; struct dfa_stuff { - aare_ruleset_t *rules; - int count; + aare_rules *rules; void *dfa; size_t size; - dfa_stuff(void): rules(NULL), count(0), dfa(NULL), size(0) { } + dfa_stuff(void): rules(NULL), dfa(NULL), size(0) { } }; class Profile { -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor