From 10a75c431fd5aa16f493d38ea79ccc58e9254677 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 8 Sep 2021 00:25:28 -0700 Subject: [PATCH] parser: rename post_process() method and move code around The post_process() method is misnamed, it fires when the profile is finished parsing but fires before variable expansion. Rename it to better reflect what it does and move the trigger code into profile as a start of cleaning this stage up. Also document the order the hooks fire in Signed-off-by: John Johansen --- parser/af_rule.h | 1 - parser/af_unix.h | 1 - parser/dbus.h | 1 - parser/mount.cc | 2 +- parser/mount.h | 2 +- parser/mqueue.h | 1 - parser/parser.h | 2 - parser/parser_policy.c | 189 +------------------------------------- parser/parser_yacc.y | 3 +- parser/profile.cc | 201 +++++++++++++++++++++++++++++++++++++++++ parser/profile.h | 3 + parser/ptrace.h | 1 - parser/rule.h | 16 +++- parser/signal.h | 1 - parser/userns.h | 1 - 15 files changed, 223 insertions(+), 202 deletions(-) diff --git a/parser/af_rule.h b/parser/af_rule.h index 422bc6b69..5e845b2c1 100644 --- a/parser/af_rule.h +++ b/parser/af_rule.h @@ -68,7 +68,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof) = 0; - virtual void post_process(Profile &prof unused) { }; }; #endif /* __AA_AF_RULE_H */ diff --git a/parser/af_unix.h b/parser/af_unix.h index 861b25ecc..2ad6e9e84 100644 --- a/parser/af_unix.h +++ b/parser/af_unix.h @@ -61,7 +61,6 @@ public: virtual ostream &dump_peer(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; protected: virtual void warn_once(const char *name) override; diff --git a/parser/dbus.h b/parser/dbus.h index f8da1d07d..a564ef8ae 100644 --- a/parser/dbus.h +++ b/parser/dbus.h @@ -61,7 +61,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; protected: virtual void warn_once(const char *name) override; diff --git a/parser/mount.cc b/parser/mount.cc index 66b2bdc0d..7e0b76823 100644 --- a/parser/mount.cc +++ b/parser/mount.cc @@ -1067,7 +1067,7 @@ fail: return RULE_ERROR; } -void mnt_rule::post_process(Profile &prof) +void mnt_rule::post_parse_profile(Profile &prof) { if (trans) { perms_t perms = 0; diff --git a/parser/mount.h b/parser/mount.h index 4eecee8eb..86cd1d99d 100644 --- a/parser/mount.h +++ b/parser/mount.h @@ -166,7 +166,7 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused); + virtual void post_parse_profile(Profile &prof unused); protected: virtual void warn_once(const char *name) override; diff --git a/parser/mqueue.h b/parser/mqueue.h index e9f0a4721..b26a988a7 100644 --- a/parser/mqueue.h +++ b/parser/mqueue.h @@ -106,7 +106,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; protected: virtual void warn_once(const char *name) override; diff --git a/parser/parser.h b/parser/parser.h index a638d9a93..55ef2e500 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -499,8 +499,6 @@ extern void add_to_list(Profile *profile); extern void add_hat_to_policy(Profile *policy, Profile *hat); extern int add_entry_to_x_table(Profile *prof, char *name); extern void add_entry_to_policy(Profile *policy, struct cod_entry *entry); -extern void post_process_file_entries(Profile *prof); -extern void post_process_rule_entries(Profile *prof); extern int post_process_policy(int debug_only); extern int process_profile_regex(Profile *prof); extern int process_profile_variables(Profile *prof); diff --git a/parser/parser_policy.c b/parser/parser_policy.c index cbf17677c..c57dda8af 100644 --- a/parser/parser_policy.c +++ b/parser/parser_policy.c @@ -70,188 +70,6 @@ void add_hat_to_policy(Profile *prof, Profile *hat) } } -int add_entry_to_x_table(Profile *prof, char *name) -{ - int i; - for (i = (AA_EXEC_LOCAL >> 10) + 1; i < AA_EXEC_COUNT; i++) { - if (!prof->exec_table[i]) { - prof->exec_table[i] = name; - return i; - } else if (strcmp(prof->exec_table[i], name) == 0) { - /* name already in table */ - free(name); - return i; - } - } - free(name); - return 0; -} - -static int add_named_transition(Profile *prof, struct cod_entry *entry) -{ - char *name = NULL; - - /* check to see if it is a local transition */ - if (!label_contains_ns(entry->nt_name)) { - char *sub = strstr(entry->nt_name, "//"); - /* does the subprofile name match the rule */ - - if (sub && strncmp(prof->name, sub, sub - entry->nt_name) && - strcmp(sub + 2, entry->name) == 0) { - free(entry->nt_name); - entry->nt_name = NULL; - return AA_EXEC_LOCAL >> 10; - } else if (((entry->perms & AA_USER_EXEC_MODIFIERS) == - SHIFT_PERMS(AA_EXEC_LOCAL, AA_USER_SHIFT)) || - ((entry->perms & AA_OTHER_EXEC_MODIFIERS) == - SHIFT_PERMS(AA_EXEC_LOCAL, AA_OTHER_SHIFT))) { - if (strcmp(entry->nt_name, entry->name) == 0) { - free(entry->nt_name); - entry->nt_name = NULL; - return AA_EXEC_LOCAL >> 10; - } - /* specified as cix so profile name is implicit */ - name = (char *) malloc(strlen(prof->name) + strlen(entry->nt_name) - + 3); - if (!name) { - PERROR("Memory allocation error\n"); - exit(1); - } - sprintf(name, "%s//%s", prof->name, entry->nt_name); - free(entry->nt_name); - entry->nt_name = NULL; - } else { - /** - * pass control of the memory pointed to by nt_name - * from entry to add_entry_to_x_table() - */ - name = entry->nt_name; - entry->nt_name = NULL; - } - } else { - /** - * pass control of the memory pointed to by nt_name - * from entry to add_entry_to_x_table() - */ - name = entry->nt_name; - entry->nt_name = NULL; - } - - return add_entry_to_x_table(prof, name); -} - -void add_entry_to_policy(Profile *prof, struct cod_entry *entry) -{ - entry->next = prof->entries; - prof->entries = entry; -} - -static bool add_proc_access(Profile *prof, const char *rule) -{ - /* FIXME: should use @{PROC}/@{PID}/attr/{apparmor/,}{current,exec} */ - struct cod_entry *new_ent; - /* allow probe for new interfaces */ - char *buffer = strdup("/proc/*/attr/apparmor/"); - if (!buffer) { - PERROR("Memory allocation error\n"); - return FALSE; - } - new_ent = new_entry(buffer, AA_MAY_READ, NULL); - if (!new_ent) { - free(buffer); - PERROR("Memory allocation error\n"); - return FALSE; - } - add_entry_to_policy(prof, new_ent); - - /* allow probe if apparmor is enabled for the old interface */ - buffer = strdup("/sys/module/apparmor/parameters/enabled"); - if (!buffer) { - PERROR("Memory allocation error\n"); - return FALSE; - } - new_ent = new_entry(buffer, AA_MAY_READ, NULL); - if (!new_ent) { - free(buffer); - PERROR("Memory allocation error\n"); - return FALSE; - } - add_entry_to_policy(prof, new_ent); - - /* allow setting on new and old interfaces */ - buffer = strdup(rule); - if (!buffer) { - PERROR("Memory allocation error\n"); - return FALSE; - } - new_ent = new_entry(buffer, AA_MAY_WRITE, NULL); - if (!new_ent) { - free(buffer); - PERROR("Memory allocation error\n"); - return FALSE; - } - add_entry_to_policy(prof, new_ent); - - return TRUE; -} - -#define CHANGEPROFILE_PATH "/proc/*/attr/{apparmor/,}{current,exec}" -void post_process_file_entries(Profile *prof) -{ - struct cod_entry *entry; - perms_t cp_perms = 0; - - list_for_each(prof->entries, entry) { - if (entry->nt_name) { - perms_t perms = 0; - int n = add_named_transition(prof, entry); - if (!n) { - PERROR("Profile %s has too many specified profile transitions.\n", prof->name); - exit(1); - } - if (entry->perms & AA_USER_EXEC) - perms |= SHIFT_PERMS(n << 10, AA_USER_SHIFT); - if (entry->perms & AA_OTHER_EXEC) - perms |= SHIFT_PERMS(n << 10, AA_OTHER_SHIFT); - entry->perms = ((entry->perms & ~AA_ALL_EXEC_MODIFIERS) | - (perms & AA_ALL_EXEC_MODIFIERS)); - } - /* FIXME: currently change_profile also implies onexec */ - cp_perms |= entry->perms & (AA_CHANGE_PROFILE); - } - - /* if there are change_profile rules, this implies that we need - * access to some /proc/ interfaces - */ - if (cp_perms & AA_CHANGE_PROFILE) { - if (!add_proc_access(prof, CHANGEPROFILE_PATH)) - exit(1); - } -} - -void post_process_rule_entries(Profile *prof) -{ - for (RuleList::iterator i = prof->rule_ents.begin(); i != prof->rule_ents.end(); i++) - (*i)->post_process(*prof); -} - - -#define CHANGEHAT_PATH "/proc/[0-9]*/attr/{apparmor/,}current" - -/* add file rules to access /proc files to call change_hat() - */ -static int profile_add_hat_rules(Profile *prof) -{ - /* don't add hat rules if not hat or profile doesn't have hats */ - if (!(prof->flags.flags & FLAG_HAT) && prof->hat_table.empty()) - return 0; - - if (!add_proc_access(prof, CHANGEHAT_PATH)) - return ENOMEM; - - return 0; -} - int load_policy_list(ProfileList &list, int option, aa_kernel_interface *kernel_interface, int cache_fd) { @@ -392,12 +210,7 @@ int post_process_profile(Profile *profile, int debug_only) { int error = 0; - error = profile_add_hat_rules(profile); - if (error) { - PERROR(_("ERROR adding hat access rule for profile %s\n"), - profile->name); - return error; - } + profile->add_implied_rules(); error = process_profile_variables(profile); if (error) { diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index a19868e97..b9a55bf6e 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -397,8 +397,7 @@ profile_base: TOK_ID opt_id_or_var opt_cond_list flags TOK_OPEN */ prof->flags.mode = MODE_COMPLAIN; - post_process_file_entries(prof); - post_process_rule_entries(prof); + prof->post_parse_profile(); prof->flags.debug(cerr); /* restore previous blocks include cache */ diff --git a/parser/profile.cc b/parser/profile.cc index a3b982aff..79095d1d8 100644 --- a/parser/profile.cc +++ b/parser/profile.cc @@ -14,6 +14,7 @@ #include "profile.h" #include "rule.h" +#include "parser.h" #include #include @@ -118,3 +119,203 @@ Profile::~Profile() free(net.quiet); } +int add_entry_to_x_table(Profile *prof, char *name) +{ + int i; + for (i = (AA_EXEC_LOCAL >> 10) + 1; i < AA_EXEC_COUNT; i++) { + if (!prof->exec_table[i]) { + prof->exec_table[i] = name; + return i; + } else if (strcmp(prof->exec_table[i], name) == 0) { + /* name already in table */ + free(name); + return i; + } + } + free(name); + return 0; +} + +void add_entry_to_policy(Profile *prof, struct cod_entry *entry) +{ + entry->next = prof->entries; + prof->entries = entry; +} + +static int add_named_transition(Profile *prof, struct cod_entry *entry) +{ + char *name = NULL; + + /* check to see if it is a local transition */ + if (!label_contains_ns(entry->nt_name)) { + char *sub = strstr(entry->nt_name, "//"); + /* does the subprofile name match the rule */ + + if (sub && strncmp(prof->name, sub, sub - entry->nt_name) && + strcmp(sub + 2, entry->name) == 0) { + free(entry->nt_name); + entry->nt_name = NULL; + return AA_EXEC_LOCAL >> 10; + } else if (((entry->perms & AA_USER_EXEC_MODIFIERS) == + SHIFT_PERMS(AA_EXEC_LOCAL, AA_USER_SHIFT)) || + ((entry->perms & AA_OTHER_EXEC_MODIFIERS) == + SHIFT_PERMS(AA_EXEC_LOCAL, AA_OTHER_SHIFT))) { + if (strcmp(entry->nt_name, entry->name) == 0) { + free(entry->nt_name); + entry->nt_name = NULL; + return AA_EXEC_LOCAL >> 10; + } + /* specified as cix so profile name is implicit */ + name = (char *) malloc(strlen(prof->name) + strlen(entry->nt_name) + + 3); + if (!name) { + PERROR("Memory allocation error\n"); + exit(1); + } + sprintf(name, "%s//%s", prof->name, entry->nt_name); + free(entry->nt_name); + entry->nt_name = NULL; + } else { + /** + * pass control of the memory pointed to by nt_name + * from entry to add_entry_to_x_table() + */ + name = entry->nt_name; + entry->nt_name = NULL; + } + } else { + /** + * pass control of the memory pointed to by nt_name + * from entry to add_entry_to_x_table() + */ + name = entry->nt_name; + entry->nt_name = NULL; + } + + return add_entry_to_x_table(prof, name); +} + +static bool add_proc_access(Profile *prof, const char *rule) +{ + /* FIXME: should use @{PROC}/@{PID}/attr/{apparmor/,}{current,exec} */ + struct cod_entry *new_ent; + /* allow probe for new interfaces */ + char *buffer = strdup("/proc/*/attr/apparmor/"); + if (!buffer) { + PERROR("Memory allocation error\n"); + return FALSE; + } + new_ent = new_entry(buffer, AA_MAY_READ, NULL); + if (!new_ent) { + free(buffer); + PERROR("Memory allocation error\n"); + return FALSE; + } + add_entry_to_policy(prof, new_ent); + + /* allow probe if apparmor is enabled for the old interface */ + buffer = strdup("/sys/module/apparmor/parameters/enabled"); + if (!buffer) { + PERROR("Memory allocation error\n"); + return FALSE; + } + new_ent = new_entry(buffer, AA_MAY_READ, NULL); + if (!new_ent) { + free(buffer); + PERROR("Memory allocation error\n"); + return FALSE; + } + add_entry_to_policy(prof, new_ent); + + /* allow setting on new and old interfaces */ + buffer = strdup(rule); + if (!buffer) { + PERROR("Memory allocation error\n"); + return FALSE; + } + new_ent = new_entry(buffer, AA_MAY_WRITE, NULL); + if (!new_ent) { + free(buffer); + PERROR("Memory allocation error\n"); + return FALSE; + } + add_entry_to_policy(prof, new_ent); + + return TRUE; +} + +#define CHANGEPROFILE_PATH "/proc/*/attr/{apparmor/,}{current,exec}" +void post_process_file_entries(Profile *prof) +{ + struct cod_entry *entry; + perms_t cp_perms = 0; + + list_for_each(prof->entries, entry) { + if (entry->nt_name) { + perms_t perms = 0; + int n = add_named_transition(prof, entry); + if (!n) { + PERROR("Profile %s has too many specified profile transitions.\n", prof->name); + exit(1); + } + if (entry->perms & AA_USER_EXEC) + perms |= SHIFT_PERMS(n << 10, AA_USER_SHIFT); + if (entry->perms & AA_OTHER_EXEC) + perms |= SHIFT_PERMS(n << 10, AA_OTHER_SHIFT); + entry->perms = ((entry->perms & ~AA_ALL_EXEC_MODIFIERS) | + (perms & AA_ALL_EXEC_MODIFIERS)); + } + /* FIXME: currently change_profile also implies onexec */ + cp_perms |= entry->perms & (AA_CHANGE_PROFILE); + } + + /* if there are change_profile rules, this implies that we need + * access to some /proc/ interfaces + */ + if (cp_perms & AA_CHANGE_PROFILE) { + if (!add_proc_access(prof, CHANGEPROFILE_PATH)) + exit(1); + } +} + +void post_process_rule_entries(Profile *prof) +{ + for (RuleList::iterator i = prof->rule_ents.begin(); i != prof->rule_ents.end(); i++) + (*i)->post_parse_profile(*prof); +} + + +#define CHANGEHAT_PATH "/proc/[0-9]*/attr/{apparmor/,}current" + +/* add file rules to access /proc files to call change_hat() + */ +static int profile_add_hat_rules(Profile *prof) +{ + /* don't add hat rules if not hat or profile doesn't have hats */ + if (!(prof->flags.flags & FLAG_HAT) && prof->hat_table.empty()) + return 0; + + if (!add_proc_access(prof, CHANGEHAT_PATH)) + return ENOMEM; + + return 0; +} + +void Profile::post_parse_profile(void) +{ + post_process_file_entries(this); + post_process_rule_entries(this); +} + +void Profile::add_implied_rules(void) +{ + int error; + + error = profile_add_hat_rules(this); + if (error) { + PERROR(_("ERROR adding hat access rule for profile %s\n"), + name); + //return error; + } + +} diff --git a/parser/profile.h b/parser/profile.h index bf9ca2308..e324d9f1d 100644 --- a/parser/profile.h +++ b/parser/profile.h @@ -314,6 +314,9 @@ public: { cout << get_name(fqp);; } + + void post_parse_profile(void); + void add_implied_rules(void); }; diff --git a/parser/ptrace.h b/parser/ptrace.h index dbe1eb308..98454e952 100644 --- a/parser/ptrace.h +++ b/parser/ptrace.h @@ -43,7 +43,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; virtual bool valid_prefix(prefixes &p, const char *&error) { if (p.owner) { diff --git a/parser/rule.h b/parser/rule.h index ad307c66f..aa8ff2c72 100644 --- a/parser/rule.h +++ b/parser/rule.h @@ -37,9 +37,23 @@ public: //virtual bool operator<(rule_t const &rhs)const = 0; virtual std::ostream &dump(std::ostream &os) = 0; + + // Follow methods in order of being called by the parse + + // called when profile is finished parsing + virtual void post_parse_profile(Profile &prof __attribute__ ((unused))) { }; + + // called before final expansion of variables. So implied rules + // can reference variables + virtual void add_implied_rules(Profile &prof __attribute__ ((unused))) { }; + + // currently only called post parse + // needs to change to being interatively called during parse + // to support expansion in include names and profile names virtual int expand_variables(void) = 0; + + // called late frontend to generate data for regex backend virtual int gen_policy_re(Profile &prof) = 0; - virtual void post_process(Profile &prof) = 0; protected: const char *warned_name = NULL; diff --git a/parser/signal.h b/parser/signal.h index b6ca7c9ee..af8469cc9 100644 --- a/parser/signal.h +++ b/parser/signal.h @@ -56,7 +56,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; protected: virtual void warn_once(const char *name) override; diff --git a/parser/userns.h b/parser/userns.h index 0333e9cac..624d53cbb 100644 --- a/parser/userns.h +++ b/parser/userns.h @@ -41,7 +41,6 @@ public: virtual ostream &dump(ostream &os); virtual int expand_variables(void); virtual int gen_policy_re(Profile &prof); - virtual void post_process(Profile &prof unused) { }; protected: virtual void warn_once(const char *name) override;