apparmor: compute file permissions on profile load
authorMike Salvatore <mike.salvatore@canonical.com>
Mon, 30 Mar 2020 20:43:29 +0000 (16:43 -0400)
committerJohn Johansen <john.johansen@canonical.com>
Mon, 3 Oct 2022 21:49:02 +0000 (14:49 -0700)
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 <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
security/apparmor/apparmorfs.c
security/apparmor/domain.c
security/apparmor/file.c
security/apparmor/include/file.h
security/apparmor/policy_unpack.c

index 044affb..825b309 100644 (file)
@@ -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))
index 91689d3..2c99edd 100644 (file)
@@ -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;
index e1b7e93..710b7d7 100644 (file)
@@ -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 */
index 029cb20..ab201d6 100644 (file)
@@ -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);
 }
 
 /**
index 10e462d..54175bc 100644 (file)
@@ -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;