From 408d53e923bd852d5d80243a642004163db53a87 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Mon, 30 Mar 2020 16:43:29 -0400 Subject: apparmor: compute file permissions on profile load Rather than computing file permissions for each file access, file permissions can be computed once on profile load and stored for lookup. Signed-off-by: Mike Salvatore Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 2 +- security/apparmor/domain.c | 10 +-- security/apparmor/file.c | 128 ++++++++++++++++++++++++++------------ security/apparmor/include/file.h | 15 ++++- security/apparmor/policy_unpack.c | 3 + 5 files changed, 110 insertions(+), 48 deletions(-) (limited to 'security') diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 044affb1ce83..825b3093dcdd 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -624,7 +624,7 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms, if (state) { struct path_cond cond = { }; - tmp = aa_compute_fperms(dfa, state, &cond); + tmp = *(aa_lookup_fperms(&(profile->file), state, &cond)); } } else if (profile->policy.dfa) { if (!PROFILE_MEDIATES(profile, *match_str)) diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 91689d34d281..2c99edd8953a 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -162,7 +162,7 @@ next: if (!state) goto fail; } - *perms = aa_compute_fperms(profile->file.dfa, state, &cond); + *perms = *(aa_lookup_fperms(&(profile->file), state, &cond)); aa_apply_modes_to_perms(profile, perms); if ((perms->allow & request) != request) return -EACCES; @@ -215,7 +215,7 @@ static int label_components_match(struct aa_profile *profile, return 0; next: - tmp = aa_compute_fperms(profile->file.dfa, state, &cond); + tmp = *(aa_lookup_fperms(&(profile->file), state, &cond)); aa_apply_modes_to_perms(profile, &tmp); aa_perms_accum(perms, &tmp); label_for_each_cont(i, label, tp) { @@ -224,7 +224,7 @@ next: state = match_component(profile, tp, stack, start); if (!state) goto fail; - tmp = aa_compute_fperms(profile->file.dfa, state, &cond); + tmp = *(aa_lookup_fperms(&(profile->file), state, &cond)); aa_apply_modes_to_perms(profile, &tmp); aa_perms_accum(perms, &tmp); } @@ -661,7 +661,7 @@ static struct aa_label *profile_transition(struct aa_profile *profile, } /* find exec permissions for name */ - state = aa_str_perms(profile->file.dfa, state, name, cond, &perms); + state = aa_str_perms(&(profile->file), state, name, cond, &perms); if (perms.allow & MAY_EXEC) { /* exec permission determine how to transition */ new = x_to_label(profile, bprm, name, perms.xindex, &target, @@ -756,7 +756,7 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec, } /* find exec permissions for name */ - state = aa_str_perms(profile->file.dfa, state, xname, cond, &perms); + state = aa_str_perms(&(profile->file), state, xname, cond, &perms); if (!(perms.allow & AA_MAY_ONEXEC)) { info = "no change_onexec valid for executable"; goto audit; diff --git a/security/apparmor/file.c b/security/apparmor/file.c index e1b7e93602e4..710b7d7517eb 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -201,47 +201,97 @@ static u32 map_old_perms(u32 old) return new; } +static void __aa_compute_fperms_allow(struct aa_perms *perms, + struct aa_dfa *dfa, + unsigned int state) +{ + perms->allow |= AA_MAY_GETATTR; + + /* change_profile wasn't determined by ownership in old mapping */ + if (ACCEPT_TABLE(dfa)[state] & 0x80000000) + perms->allow |= AA_MAY_CHANGE_PROFILE; + if (ACCEPT_TABLE(dfa)[state] & 0x40000000) + perms->allow |= AA_MAY_ONEXEC; +} + +static struct aa_perms __aa_compute_fperms_user(struct aa_dfa *dfa, + unsigned int state) +{ + struct aa_perms perms = { }; + + perms.allow = map_old_perms(dfa_user_allow(dfa, state)); + perms.audit = map_old_perms(dfa_user_audit(dfa, state)); + perms.quiet = map_old_perms(dfa_user_quiet(dfa, state)); + perms.xindex = dfa_user_xindex(dfa, state); + + __aa_compute_fperms_allow(&perms, dfa, state); + + return perms; +} + +static struct aa_perms __aa_compute_fperms_other(struct aa_dfa *dfa, + unsigned int state) +{ + struct aa_perms perms = { }; + + perms.allow = map_old_perms(dfa_other_allow(dfa, state)); + perms.audit = map_old_perms(dfa_other_audit(dfa, state)); + perms.quiet = map_old_perms(dfa_other_quiet(dfa, state)); + perms.xindex = dfa_other_xindex(dfa, state); + + __aa_compute_fperms_allow(&perms, dfa, state); + + return perms; +} + +/** + * aa_compute_fperms - convert dfa compressed perms to internal perms and store + * them so they can be retrieved later. + * @file_rules: a file_rules structure containing a dfa (NOT NULL) for which + * permissions will be computed (NOT NULL) + * + * TODO: convert from dfa + state to permission entry + */ +void aa_compute_fperms(struct aa_file_rules *file_rules) +{ + int state; + int state_count = file_rules->dfa->tables[YYTD_ID_BASE]->td_lolen; + + // DFAs are restricted from having a state_count of less than 2 + file_rules->fperms_table = kvzalloc( + state_count * 2 * sizeof(struct aa_perms), GFP_KERNEL); + + // Since fperms_table is initialized with zeroes via kvzalloc(), we can + // skip the trap state (state == 0) + for (state = 1; state < state_count; state++) { + file_rules->fperms_table[state * 2] = + __aa_compute_fperms_user(file_rules->dfa, state); + file_rules->fperms_table[state * 2 + 1] = + __aa_compute_fperms_other(file_rules->dfa, state); + } +} + /** - * aa_compute_fperms - convert dfa compressed perms to internal perms - * @dfa: dfa to compute perms for (NOT NULL) + * aa_lookup_fperms - convert dfa compressed perms to internal perms + * @dfa: dfa to lookup perms for (NOT NULL) * @state: state in dfa * @cond: conditions to consider (NOT NULL) * - * TODO: convert from dfa + state to permission entry, do computation conversion - * at load time. + * TODO: convert from dfa + state to permission entry * - * Returns: computed permission set + * Returns: a pointer to a file permission set */ -struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state, - struct path_cond *cond) +struct aa_perms default_perms = {}; +struct aa_perms *aa_lookup_fperms(struct aa_file_rules *file_rules, + unsigned int state, struct path_cond *cond) { - /* FIXME: change over to new dfa format - * currently file perms are encoded in the dfa, new format - * splits the permissions from the dfa. This mapping can be - * done at profile load - */ - struct aa_perms perms = { }; + if (!(file_rules->fperms_table)) + return &default_perms; - if (uid_eq(current_fsuid(), cond->uid)) { - perms.allow = map_old_perms(dfa_user_allow(dfa, state)); - perms.audit = map_old_perms(dfa_user_audit(dfa, state)); - perms.quiet = map_old_perms(dfa_user_quiet(dfa, state)); - perms.xindex = dfa_user_xindex(dfa, state); - } else { - perms.allow = map_old_perms(dfa_other_allow(dfa, state)); - perms.audit = map_old_perms(dfa_other_audit(dfa, state)); - perms.quiet = map_old_perms(dfa_other_quiet(dfa, state)); - perms.xindex = dfa_other_xindex(dfa, state); - } - perms.allow |= AA_MAY_GETATTR; + if (uid_eq(current_fsuid(), cond->uid)) + return &(file_rules->fperms_table[state * 2]); - /* change_profile wasn't determined by ownership in old mapping */ - if (ACCEPT_TABLE(dfa)[state] & 0x80000000) - perms.allow |= AA_MAY_CHANGE_PROFILE; - if (ACCEPT_TABLE(dfa)[state] & 0x40000000) - perms.allow |= AA_MAY_ONEXEC; - - return perms; + return &(file_rules->fperms_table[state * 2 + 1]); } /** @@ -254,13 +304,13 @@ struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state, * * Returns: the final state in @dfa when beginning @start and walking @name */ -unsigned int aa_str_perms(struct aa_dfa *dfa, unsigned int start, +unsigned int aa_str_perms(struct aa_file_rules *file_rules, unsigned int start, const char *name, struct path_cond *cond, struct aa_perms *perms) { unsigned int state; - state = aa_dfa_match(dfa, start, name); - *perms = aa_compute_fperms(dfa, state, cond); + state = aa_dfa_match(file_rules->dfa, start, name); + *perms = *(aa_lookup_fperms(file_rules, state, cond)); return state; } @@ -273,7 +323,7 @@ int __aa_path_perm(const char *op, struct aa_profile *profile, const char *name, if (profile_unconfined(profile)) return 0; - aa_str_perms(profile->file.dfa, profile->file.start, name, cond, perms); + aa_str_perms(&(profile->file), profile->file.start, name, cond, perms); if (request & ~perms->allow) e = -EACCES; return aa_audit_file(profile, perms, op, request, name, NULL, NULL, @@ -380,7 +430,7 @@ static int profile_path_link(struct aa_profile *profile, error = -EACCES; /* aa_str_perms - handles the case of the dfa being NULL */ - state = aa_str_perms(profile->file.dfa, profile->file.start, lname, + state = aa_str_perms(&(profile->file), profile->file.start, lname, cond, &lperms); if (!(lperms.allow & AA_MAY_LINK)) @@ -388,7 +438,7 @@ static int profile_path_link(struct aa_profile *profile, /* test to see if target can be paired with link */ state = aa_dfa_null_transition(profile->file.dfa, state); - aa_str_perms(profile->file.dfa, state, tname, cond, &perms); + aa_str_perms(&(profile->file), state, tname, cond, &perms); /* force audit/quiet masks for link are stored in the second entry * in the link pair. @@ -410,7 +460,7 @@ static int profile_path_link(struct aa_profile *profile, /* Do link perm subset test requiring allowed permission on link are * a subset of the allowed permissions on target. */ - aa_str_perms(profile->file.dfa, profile->file.start, tname, cond, + aa_str_perms(&(profile->file), profile->file.start, tname, cond, &perms); /* AA_MAY_LINK is not considered in the subset test */ diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h index 029cb20e322d..ab201d625a34 100644 --- a/security/apparmor/include/file.h +++ b/security/apparmor/include/file.h @@ -181,11 +181,13 @@ struct aa_file_rules { /* struct perms perms; */ struct aa_domain trans; /* TODO: add delegate table */ + struct aa_perms *fperms_table; }; -struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state, - struct path_cond *cond); -unsigned int aa_str_perms(struct aa_dfa *dfa, unsigned int start, +void aa_compute_fperms(struct aa_file_rules *file_rules); +struct aa_perms *aa_lookup_fperms(struct aa_file_rules *file_rules, + unsigned int state, struct path_cond *cond); +unsigned int aa_str_perms(struct aa_file_rules *file_rules, unsigned int start, const char *name, struct path_cond *cond, struct aa_perms *perms); @@ -204,10 +206,17 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, void aa_inherit_files(const struct cred *cred, struct files_struct *files); +static inline void aa_free_fperms_table(struct aa_perms *fperms_table) +{ + if (fperms_table) + kvfree(fperms_table); +} + static inline void aa_free_file_rules(struct aa_file_rules *rules) { aa_put_dfa(rules->dfa); aa_free_domain_entries(&rules->trans); + aa_free_fperms_table(rules->fperms_table); } /** diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 10e462d00321..54175bca4256 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -22,6 +22,7 @@ #include "include/audit.h" #include "include/cred.h" #include "include/crypto.h" +#include "include/file.h" #include "include/match.h" #include "include/path.h" #include "include/policy.h" @@ -878,6 +879,8 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) } else profile->file.dfa = aa_get_dfa(nulldfa); + aa_compute_fperms(&(profile->file)); + if (!unpack_trans_table(e, profile)) { info = "failed to unpack profile transition table"; goto fail; -- cgit v1.2.3