From 7d9958890f4a1bf4975140776c25e639bd2dafb2 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 3 Jul 2023 23:52:57 -0700 Subject: [PATCH] parser: finish basic infrastructure for rule merging Currently only file rules get merged. Finish adding basic support for rule merging and make the default the behavior to dedup merge rules that are exact matches. Signed-off-by: John Johansen --- parser/parser.h | 4 ++ parser/parser_merge.c | 15 ++++- parser/parser_misc.c | 65 +++++++++++++++++++ parser/parser_regex.c | 2 +- parser/parser_variable.c | 2 +- parser/profile.cc | 41 ++++++------ parser/profile.h | 2 +- parser/rule.h | 133 +++++++++++++++++++++++++++++++++------ 8 files changed, 220 insertions(+), 44 deletions(-) diff --git a/parser/parser.h b/parser/parser.h index 0399db8be..e5b785480 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -99,6 +99,8 @@ struct value_list { struct value_list *next; }; +int cmp_value_list(value_list *lhs, value_list *rhs); + struct cond_entry { char *name; int eq; /* where equals was used in specifying list */ @@ -454,6 +456,8 @@ extern struct cod_entry *new_entry(char *id, perms_t perms, char *link_id); /* returns -1 if value != true or false, otherwise 0 == false, 1 == true */ extern int str_to_boolean(const char* str); +extern int null_strcmp(const char *s1, const char *s2); +extern bool strcomp (const char *lhs, const char *rhs); extern struct cod_entry *copy_cod_entry(struct cod_entry *cod); extern void free_cod_entries(struct cod_entry *list); void debug_cod_entries(struct cod_entry *list); diff --git a/parser/parser_merge.c b/parser/parser_merge.c index 77c295e19..b8041ebb5 100644 --- a/parser/parser_merge.c +++ b/parser/parser_merge.c @@ -72,7 +72,7 @@ static int process_file_entries(Profile *prof) table = (struct cod_entry **) malloc(sizeof(struct cod_entry *) * (count + 1)); if (!table) { PERROR(_("Couldn't merge entries. Out of Memory\n")); - return ENOMEM; + return -ENOMEM; } for (cur = prof->entries, n = 0; cur; cur = cur->next, n++) @@ -84,6 +84,7 @@ static int process_file_entries(Profile *prof) prof->entries = table[0]; free(table); + count = 0; /* walk the sorted table merging similar entries */ for (cur = prof->entries, next = cur->next; next; next = cur->next) { if (file_comp(&cur, &next) != 0) { @@ -102,12 +103,20 @@ static int process_file_entries(Profile *prof) next->next = NULL; free_cod_entries(next); + count++; } - return 0; + return count; } int profile_merge_rules(Profile *prof) { - return process_file_entries(prof); + int res, tmp = process_file_entries(prof); + if (tmp < 0) + return -tmp; + res = prof->merge_rules(); + if (res < 0) + return -res; + // TODO: output message eliminated rules res + tmp; + return 0; } diff --git a/parser/parser_misc.c b/parser/parser_misc.c index 651b4ce98..ed80d068f 100644 --- a/parser/parser_misc.c +++ b/parser/parser_misc.c @@ -34,6 +34,8 @@ #include #include +#include + #include "capability.h" #include "lib.h" #include "parser.h" @@ -271,6 +273,25 @@ static const char *strn_token(const char *str, size_t &len) return start; } +int null_strcmp(const char *s1, const char *s2) +{ + if (s1) { + if (s2) + return strcmp(s1, s2); + return 1; + } else if (s2) { + return -1; + } + + // both null + return 0; +} + +bool strcomp (const char *lhs, const char *rhs) +{ + return null_strcmp(lhs, rhs) < 0; +} + /* * Returns: -1: error * 0: no change - capability already in table @@ -1065,6 +1086,50 @@ void debug_cod_entries(struct cod_entry *list) } } +// these need to move to stl +int ordered_cmp_value_list(value_list *lhs, value_list *rhs) +{ + std::vector lhstable; + std::vector rhstable; + + struct value_list *entry; + list_for_each(lhs, entry) { + lhstable.push_back(entry->value); + } + list_for_each(rhs, entry) { + rhstable.push_back(entry->value); + } + + int res = lhstable.size() - rhstable.size(); + if (res) + return res; + + std::sort(lhstable.begin(), lhstable.end(), strcomp); + std::sort(rhstable.begin(), rhstable.end(), strcomp); + + for (unsigned long i = 0; i < lhstable.size(); i++) { + res = null_strcmp(lhstable[i], rhstable[i]); + if (res) + return res; + } + + return 0; +} + +int cmp_value_list(value_list *lhs, value_list *rhs) +{ + if (lhs) { + if (rhs) { + return ordered_cmp_value_list(lhs, rhs); + } + return 1; + } else if (rhs) { + return -1; + } + + return 0; +} + struct value_list *new_value_list(char *value) { struct value_list *val = (struct value_list *) calloc(1, sizeof(struct value_list)); diff --git a/parser/parser_regex.c b/parser/parser_regex.c index dfab8328a..95d018342 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -848,7 +848,7 @@ int clear_and_convert_entry(std::string& buffer, char *entry) int post_process_policydb_ents(Profile *prof) { for (RuleList::iterator i = prof->rule_ents.begin(); i != prof->rule_ents.end(); i++) { - if ((*i)->skip_processing()) + if ((*i)->skip()) continue; if ((*i)->gen_policy_re(*prof) == RULE_ERROR) return FALSE; diff --git a/parser/parser_variable.c b/parser/parser_variable.c index 4f3e8f2f0..61cdcc3a8 100644 --- a/parser/parser_variable.c +++ b/parser/parser_variable.c @@ -267,7 +267,7 @@ static int process_variables_in_entries(struct cod_entry *entry_list) static int process_variables_in_rules(Profile &prof) { for (RuleList::iterator i = prof.rule_ents.begin(); i != prof.rule_ents.end(); i++) { - if ((*i)->skip_processing()) + if ((*i)->skip()) continue; int error = (*i)->expand_variables(); if (error) diff --git a/parser/profile.cc b/parser/profile.cc index e16546450..9eeb39018 100644 --- a/parser/profile.cc +++ b/parser/profile.cc @@ -121,38 +121,39 @@ Profile::~Profile() free(net.quiet); } -static bool comp (rule_t *lhs, rule_t *rhs) { return (*lhs < *rhs); } +static bool comp (rule_t *lhs, rule_t *rhs) +{ + return (*lhs < *rhs); +} -bool Profile::merge_rules(void) +// TODO: move to block rule +// returns number of rules merged +// returns negative number on error +int Profile::merge_rules(void) { int count = 0; + std::vector table; - for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end(); ) { - if ((*i)->is_mergeable()) - count++; + for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end(); i++) { + if ((*i)->is_mergeable() && !(*i)->skip()) + table.push_back(*i); } - if (count < 2) + if (table.size() < 2) return 0; - - std::vector table(count); - int n = 0; - for (RuleList::iterator i = rule_ents.begin(); i != rule_ents.end(); ) { - if ((*i)->is_mergeable()) - table[n++] = *i; - } - std::sort(table.begin(), table.end(), comp); - - for (int i = 0, j = 1; j < count; j++) { + unsigned long n = table.size(); + for (unsigned long i = 0, j = 1; j < n; j++) { + if (table[j]->skip()) + continue; if (table[i]->cmp(*table[j]) == 0) { - if (!table[i]->merge(*table[j])) - return false; + if (table[i]->merge(*table[j])) + count++; continue; } i = j; } - return true; + return count; } @@ -318,7 +319,7 @@ void post_process_file_entries(Profile *prof) void post_process_rule_entries(Profile *prof) { for (RuleList::iterator i = prof->rule_ents.begin(); i != prof->rule_ents.end(); i++) { - if ((*i)->skip_processing()) + if ((*i)->skip()) continue; (*i)->post_parse_profile(*prof); } diff --git a/parser/profile.h b/parser/profile.h index 683e97695..c73b59faf 100644 --- a/parser/profile.h +++ b/parser/profile.h @@ -249,7 +249,7 @@ public: * Requires the merged rules have customized methods * cmp(), is_mergeable() and merge() */ - virtual bool merge_rules(void); + virtual int merge_rules(void); void dump(void) { diff --git a/parser/rule.h b/parser/rule.h index f4931a890..a040333ba 100644 --- a/parser/rule.h +++ b/parser/rule.h @@ -38,6 +38,10 @@ class Profile; // RULE_TYPE_CLASS needs to be last because various class follow it #define RULE_TYPE_CLASS 3 +// rule_cast should only be used after a comparison of rule_type to ensure +// that it is valid. Change to dynamic_cast for debugging +//#define rule_cast dynamic_cast +#define rule_cast static_cast typedef enum { RULE_FLAG_NONE = 0, RULE_FLAG_DELETED = 1, // rule deleted - skip @@ -48,21 +52,38 @@ typedef enum { RULE_FLAG_NONE = 0, // added because it is implied } rule_flags_t; +inline rule_flags_t operator|(rule_flags_t a, rule_flags_t b) +{ + return static_cast(static_cast(a) | static_cast(b)); +} + +inline rule_flags_t operator&(rule_flags_t a, rule_flags_t b) +{ + return static_cast(static_cast(a) & static_cast(b)); +} + +inline rule_flags_t& operator|=(rule_flags_t &a, const rule_flags_t &b) +{ + a = a | b; + return a; +} + class rule_t { public: int rule_type; rule_flags_t flags; - rule_t(int t): rule_type(t), flags(RULE_FLAG_NONE) { } + rule_t *removed_by; + + rule_t(int t): rule_type(t), flags(RULE_FLAG_NONE), removed_by(NULL) { } virtual ~rule_t() { }; bool is_type(int type) { return rule_type == type; } // rule has been marked as should be skipped by regular processing - bool skip_processing() + bool skip() { - return (flags == RULE_FLAG_DELETED || - flags == RULE_FLAG_MERGED); + return (flags & RULE_FLAG_DELETED); } //virtual bool operator<(rule_t const &rhs)const = 0; virtual std::ostream &dump(std::ostream &os) = 0; @@ -81,15 +102,37 @@ public: // to support expansion in include names and profile names virtual int expand_variables(void) = 0; - // called by duplicate rule merge/elimination after final expand_vars - virtual bool is_mergeable(void) { return false; } virtual int cmp(rule_t const &rhs) const { - return rule_type < rhs.rule_type; + return rule_type - rhs.rule_type; } virtual bool operator<(rule_t const &rhs) const { return cmp(rhs) < 0; } - virtual bool merge(rule_t &rhs __attribute__ ((unused))) { return false; }; + // called by duplicate rule merge/elimination after final expand_vars + // to get default rule dedup + // child object need to provide + // - cmp, operator< + // - is_mergeable() returning true + // if a child object wants to provide merging of permissions, + // it needs to provide a custom cmp fn that doesn't include + // permissions and a merge routine that does more than flagging + // as dup as below + virtual bool is_mergeable(void) { return false; } + + // returns true if merged + virtual bool merge(rule_t &rhs) + { + if (rule_type != rhs.rule_type) + return false; + if (skip() || rhs.skip()) + return false; + // default merge is just dedup + flags |= RULE_FLAG_MERGED; + rhs.flags |= (RULE_FLAG_MERGED | RULE_FLAG_DELETED); + rhs.removed_by = this; + + return true; + }; // called late frontend to generate data for regex backend virtual int gen_policy_re(Profile &prof) = 0; @@ -162,6 +205,32 @@ public: return os; } + + int cmp(prefixes const &rhs) const { + if ((uint) audit < (uint) rhs.audit) + return -1; + if ((uint) audit > (uint) rhs.audit) + return 1; + if ((uint) rule_mode < (uint) rhs.rule_mode) + return -1; + if ((uint) rule_mode > (uint) rhs.rule_mode) + return 1; + if (owner < rhs.owner) + return -1; + if (owner > rhs.owner) + return 1; + return 0; + } + + bool operator<(prefixes const &rhs) const { + if ((uint) audit < (uint) rhs.audit) + return true; + if ((uint) rule_mode < (uint) rhs.rule_mode) + return true; + if (owner < rhs.owner) + return true; + return false; + } }; class prefix_rule_t: public rule_t, public prefixes { @@ -221,21 +290,32 @@ public: return true; } - virtual bool operator<(prefixes const &rhs) const { - if ((uint) audit < (uint) rhs.audit) - return true; - if ((uint) rule_mode < (uint) rhs.rule_mode) - return true; - if (owner < rhs.owner) - return true; - return false; + int cmp(prefixes const &rhs) const { + return prefixes::cmp(rhs); } - virtual bool operator<(prefix_rule_t const &rhs) const { + + virtual bool operator<(prefixes const &rhs) const { + const prefixes *ptr = this; + return *ptr < rhs; + } + + virtual int cmp(rule_t const &rhs) const { + int res = rule_t::cmp(rhs); + if (res) + return res; + prefix_rule_t const &pr = rule_cast(rhs); + const prefixes *lhsptr = this, *rhsptr = ≺ + return lhsptr->cmp(*rhsptr); + } + + virtual bool operator<(rule_t const &rhs) const { if (rule_type < rhs.rule_type) return true; if (rhs.rule_type < rule_type) return false; - return *this < (prefixes const &)rhs; + prefix_rule_t const &pr = rule_cast(rhs); + const prefixes *rhsptr = ≺ + return *this < *rhsptr; } virtual ostream &dump(ostream &os) { @@ -253,6 +333,16 @@ public: int aa_class(void) { return rule_type - RULE_TYPE_CLASS; } + /* inherit cmp */ + + /* we do not inherit operator< from so class_rules children + * can in herit the generic one that redirects to cmp() + * that does get overriden + */ + virtual bool operator<(rule_t const &rhs) const { + return cmp(rhs) < 0; + } + virtual ostream &dump(ostream &os) { prefix_rule_t::dump(os); @@ -267,6 +357,13 @@ class perms_rule_t: public class_rule_t { public: perms_rule_t(int c): class_rule_t(c), perms(0) { }; + virtual int cmp(rule_t const &rhs) const { + int res = class_rule_t::cmp(rhs); + if (res) + return res; + return perms - (rule_cast(rhs)).perms; + } + /* defaut perms, override/mask off if none default used */ virtual ostream &dump(ostream &os) {