From c9d01a325d5f4cb6243c94b784d73b8782ece718 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 7 Jul 2020 09:42:41 -0700 Subject: [PATCH] parser: don't apply exec mapping computations to the policydb v8 network permissions extend into the range used by exec mapping so it is important to not blindly do execmapping on both the file dfa and policydb dfa any more. Track what type of dfa and its permissions we are building so we can properly apply exec mapping only when building the file dfa. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521 Signed-off-by: John Johansen --- parser/libapparmor_re/aare_rules.cc | 5 +++-- parser/libapparmor_re/aare_rules.h | 3 ++- parser/libapparmor_re/hfa.cc | 26 +++++++++++++++----------- parser/libapparmor_re/hfa.h | 25 ++++++++++++++----------- parser/parser_regex.c | 6 +++--- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/parser/libapparmor_re/aare_rules.cc b/parser/libapparmor_re/aare_rules.cc index 3f3367a89..124499044 100644 --- a/parser/libapparmor_re/aare_rules.cc +++ b/parser/libapparmor_re/aare_rules.cc @@ -195,7 +195,8 @@ bool aare_rules::append_rule(const char *rule, bool oob, bool with_perm, * else NULL on failure, @min_match_len set to the shortest string * that can match the dfa for determining xmatch priority. */ -void *aare_rules::create_dfa(size_t *size, int *min_match_len, dfaflags_t flags) +void *aare_rules::create_dfa(size_t *size, int *min_match_len, dfaflags_t flags, + bool filedfa) { char *buffer = NULL; @@ -249,7 +250,7 @@ void *aare_rules::create_dfa(size_t *size, int *min_match_len, dfaflags_t flags) stringstream stream; try { - DFA dfa(root, flags); + DFA dfa(root, flags, filedfa); if (flags & DFA_DUMP_UNIQ_PERMS) dfa.dump_uniq_perms("dfa"); diff --git a/parser/libapparmor_re/aare_rules.h b/parser/libapparmor_re/aare_rules.h index 9c8df25ae..ab88f0af0 100644 --- a/parser/libapparmor_re/aare_rules.h +++ b/parser/libapparmor_re/aare_rules.h @@ -105,7 +105,8 @@ class aare_rules { bool add_rule_vec(int deny, uint32_t perms, uint32_t audit, int count, const char **rulev, dfaflags_t flags, bool oob); bool append_rule(const char *rule, bool oob, bool with_perm, dfaflags_t flags); - void *create_dfa(size_t *size, int *min_match_len, dfaflags_t flags); + void *create_dfa(size_t *size, int *min_match_len, dfaflags_t flags, + bool filedfa); }; #endif /* __LIBAA_RE_RULES_H */ diff --git a/parser/libapparmor_re/hfa.cc b/parser/libapparmor_re/hfa.cc index 500275654..6b0109133 100644 --- a/parser/libapparmor_re/hfa.cc +++ b/parser/libapparmor_re/hfa.cc @@ -309,7 +309,7 @@ State *DFA::add_new_state(NodeSet *anodes, NodeSet *nnodes, State *other) ProtoState proto; proto.init(nnodev, anodes); - State *state = new State(node_map.size(), proto, other); + State *state = new State(node_map.size(), proto, other, filedfa); pair x = node_map.insert(proto, state); if (x.second == false) { delete state; @@ -420,7 +420,7 @@ void DFA::process_work_queue(const char *header, dfaflags_t flags) /** * Construct a DFA from a syntax tree. */ -DFA::DFA(Node *root, dfaflags_t flags): root(root) +DFA::DFA(Node *root, dfaflags_t flags, bool buildfiledfa): root(root), filedfa(buildfiledfa) { diffcount = 0; /* set by diff_encode */ max_range = 256; @@ -785,7 +785,7 @@ void DFA::minimize(dfaflags_t flags) if (flags & DFA_DUMP_MIN_PARTS) cerr << **i << ", "; (*i)->label = -1; - rep->perms.add((*i)->perms); + rep->perms.add((*i)->perms, filedfa); } if (rep->perms.is_accept()) final_accept++; @@ -1340,7 +1340,7 @@ static inline int diff_qualifiers(uint32_t perm1, uint32_t perm2) * have any exact matches, then they override the execute and safe * execute flags. */ -int accept_perms(NodeSet *state, perms_t &perms) +int accept_perms(NodeSet *state, perms_t &perms, bool filedfa) { int error = 0; uint32_t exact_match_allow = 0; @@ -1357,7 +1357,7 @@ int accept_perms(NodeSet *state, perms_t &perms) continue; if (dynamic_cast(match)) { /* exact match only ever happens with x */ - if (!is_merged_x_consistent(exact_match_allow, + if (filedfa && !is_merged_x_consistent(exact_match_allow, match->flag)) error = 1;; exact_match_allow |= match->flag; @@ -1366,16 +1366,20 @@ int accept_perms(NodeSet *state, perms_t &perms) perms.deny |= match->flag; perms.quiet |= match->audit; } else { - if (!is_merged_x_consistent(perms.allow, match->flag)) + if (filedfa && !is_merged_x_consistent(perms.allow, match->flag)) error = 1; perms.allow |= match->flag; perms.audit |= match->audit; } } - perms.allow |= exact_match_allow & ~(ALL_AA_EXEC_TYPE); - perms.audit |= exact_audit & ~(ALL_AA_EXEC_TYPE); - + if (filedfa) { + perms.allow |= exact_match_allow & ~(ALL_AA_EXEC_TYPE); + perms.audit |= exact_audit & ~(ALL_AA_EXEC_TYPE); + } else { + perms.allow |= exact_match_allow; + perms.audit |= exact_audit; + } if (exact_match_allow & AA_USER_EXEC) { perms.allow = (exact_match_allow & AA_USER_EXEC_TYPE) | (perms.allow & ~AA_USER_EXEC_TYPE); @@ -1386,10 +1390,10 @@ int accept_perms(NodeSet *state, perms_t &perms) (perms.allow & ~AA_OTHER_EXEC_TYPE); perms.exact |= AA_OTHER_EXEC_TYPE; } - if (AA_USER_EXEC & perms.deny) + if (filedfa && (AA_USER_EXEC & perms.deny)) perms.deny |= AA_USER_EXEC_TYPE; - if (AA_OTHER_EXEC & perms.deny) + if (filedfa && (AA_OTHER_EXEC & perms.deny)) perms.deny |= AA_OTHER_EXEC_TYPE; perms.allow &= ~perms.deny; diff --git a/parser/libapparmor_re/hfa.h b/parser/libapparmor_re/hfa.h index 7de4e88c1..3ad7aaaa4 100644 --- a/parser/libapparmor_re/hfa.h +++ b/parser/libapparmor_re/hfa.h @@ -59,11 +59,11 @@ public: } void clear(void) { allow = deny = audit = quiet = 0; } - void add(perms_t &rhs) + void add(perms_t &rhs, bool filedfa) { deny |= rhs.deny; - if (!is_merged_x_consistent(allow & ALL_USER_EXEC, + if (filedfa && !is_merged_x_consistent(allow & ALL_USER_EXEC, rhs.allow & ALL_USER_EXEC)) { if ((exact & AA_USER_EXEC_TYPE) && !(rhs.exact & AA_USER_EXEC_TYPE)) { @@ -74,10 +74,10 @@ public: (rhs.allow & AA_USER_EXEC_TYPE); } else throw 1; - } else + } else if (filedfa) allow |= rhs.allow & AA_USER_EXEC_TYPE; - if (!is_merged_x_consistent(allow & ALL_OTHER_EXEC, + if (filedfa && !is_merged_x_consistent(allow & ALL_OTHER_EXEC, rhs.allow & ALL_OTHER_EXEC)) { if ((exact & AA_OTHER_EXEC_TYPE) && !(rhs.exact & AA_OTHER_EXEC_TYPE)) { @@ -88,11 +88,13 @@ public: (rhs.allow & AA_OTHER_EXEC_TYPE); } else throw 1; - } else + } else if (filedfa) allow |= rhs.allow & AA_OTHER_EXEC_TYPE; - - allow = (allow | (rhs.allow & ~ALL_AA_EXEC_TYPE)); + if (filedfa) + allow = (allow | (rhs.allow & ~ALL_AA_EXEC_TYPE)); + else + allow |= rhs.allow; audit |= rhs.audit; quiet = (quiet | rhs.quiet); @@ -131,7 +133,7 @@ public: uint32_t allow, deny, audit, quiet, exact; }; -int accept_perms(NodeSet *state, perms_t &perms); +int accept_perms(NodeSet *state, perms_t &perms, bool filedfa); /* * ProtoState - NodeSet and ancillery information used to create a state @@ -195,7 +197,7 @@ struct DiffDag { */ class State { public: - State(int l, ProtoState &n, State *other): + State(int l, ProtoState &n, State *other, bool filedfa): label(l), flags(0), perms(), trans() { int error; @@ -208,7 +210,7 @@ public: proto = n; /* Compute permissions associated with the State. */ - error = accept_perms(n.anodes, perms); + error = accept_perms(n.anodes, perms, filedfa); if (error) { //cerr << "Failing on accept perms " << error << "\n"; throw error; @@ -316,7 +318,7 @@ class DFA { list work_queue; public: - DFA(Node *root, dfaflags_t flags); + DFA(Node *root, dfaflags_t flags, bool filedfa); virtual ~DFA(); State *match_len(State *state, const char *str, size_t len); @@ -347,6 +349,7 @@ public: Node *root; State *nonmatching, *start; Partition states; + bool filedfa; }; void dump_equivalence_classes(ostream &os, map &eq); diff --git a/parser/parser_regex.c b/parser/parser_regex.c index 01c87eb4d..f3052440f 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -557,7 +557,7 @@ static int process_profile_name_xmatch(Profile *prof) } } build: - prof->xmatch = rules->create_dfa(&prof->xmatch_size, &prof->xmatch_len, dfaflags); + prof->xmatch = rules->create_dfa(&prof->xmatch_size, &prof->xmatch_len, dfaflags, true); delete rules; if (!prof->xmatch) return FALSE; @@ -755,7 +755,7 @@ int process_profile_regex(Profile *prof) if (prof->dfa.rules->rule_count > 0) { int xmatch_len = 0; prof->dfa.dfa = prof->dfa.rules->create_dfa(&prof->dfa.size, - &xmatch_len, dfaflags); + &xmatch_len, dfaflags, true); delete prof->dfa.rules; prof->dfa.rules = NULL; if (!prof->dfa.dfa) @@ -973,7 +973,7 @@ int process_profile_policydb(Profile *prof) if (prof->policy.rules->rule_count > 0) { int xmatch_len = 0; prof->policy.dfa = prof->policy.rules->create_dfa(&prof->policy.size, - &xmatch_len, dfaflags); + &xmatch_len, dfaflags, false); delete prof->policy.rules; prof->policy.rules = NULL;