apparmor: fix profile verification and enable it

The transition table size was not being set by compat mappings
resulting in the profile verification code not being run. Unfortunately
the checks were also buggy not being correctly updated from the old
accept perms, to the new layout.

Also indicate to userspace that the kernel has the permstable verification
fixes.

BugLink: http://bugs.launchpad.net/bugs/2017903
Fixes: 670f31774ab6 ("apparmor: verify permission table indexes")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Jon Tourville <jontourville@me.com>

diff --git a/security/apparmor/policy_compat.c b/security/apparmor/policy_compat.c
index b2ec7fb04e831bf76fba63648ef1927f7abb80fc..7ff9184ace2942a3edd97a0b3e7f3b6ba47ef18d 100644 (file)
--- a/security/apparmor/policy_compat.c
+++ b/security/apparmor/policy_compat.c
@@ -146,7 +146,8 @@ static struct aa_perms compute_fperms_other(struct aa_dfa *dfa,
  *
  * Returns: remapped perm table
  */
-static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
+static struct aa_perms *compute_fperms(struct aa_dfa *dfa,
+                                      u32 *size)
 {
        aa_state_t state;
        unsigned int state_count;
@@ -159,6 +160,7 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
        table = kvcalloc(state_count * 2, sizeof(struct aa_perms), GFP_KERNEL);
        if (!table)
                return NULL;
+       *size = state_count * 2;
 
        /* zero init so skip the trap state (state == 0) */
        for (state = 1; state < state_count; state++) {
@@ -169,7 +171,8 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa)
        return table;
 }
 
-static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch)
+static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch,
+                                     u32 *size)
 {
        struct aa_perms *perms;
        int state;
@@ -182,6 +185,7 @@ static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch)
        perms = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL);
        if (!perms)
                return NULL;
+       *size = state_count;
 
        /* zero init so skip the trap state (state == 0) */
        for (state = 1; state < state_count; state++)
@@ -242,7 +246,8 @@ static struct aa_perms compute_perms_entry(struct aa_dfa *dfa,
        return perms;
 }
 
-static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version)
+static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version,
+                                     u32 *size)
 {
        unsigned int state;
        unsigned int state_count;
@@ -255,6 +260,7 @@ static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version)
        table = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL);
        if (!table)
                return NULL;
+       *size = state_count;
 
        /* zero init so skip the trap state (state == 0) */
        for (state = 1; state < state_count; state++)
@@ -289,7 +295,7 @@ static void remap_dfa_accept(struct aa_dfa *dfa, unsigned int factor)
 /* TODO: merge different dfa mappings into single map_policy fn */
 int aa_compat_map_xmatch(struct aa_policydb *policy)
 {
-       policy->perms = compute_xmatch_perms(policy->dfa);
+       policy->perms = compute_xmatch_perms(policy->dfa, &policy->size);
        if (!policy->perms)
                return -ENOMEM;
 
@@ -300,7 +306,7 @@ int aa_compat_map_xmatch(struct aa_policydb *policy)
 
 int aa_compat_map_policy(struct aa_policydb *policy, u32 version)
 {
-       policy->perms = compute_perms(policy->dfa, version);
+       policy->perms = compute_perms(policy->dfa, version, &policy->size);
        if (!policy->perms)
                return -ENOMEM;
 
@@ -311,7 +317,7 @@ int aa_compat_map_policy(struct aa_policydb *policy, u32 version)
 
 int aa_compat_map_file(struct aa_policydb *policy)
 {
-       policy->perms = compute_fperms(policy->dfa);
+       policy->perms = compute_fperms(policy->dfa, &policy->size);
        if (!policy->perms)
                return -ENOMEM;
 

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index a357c7b05276cb282b0f08e4515e59747a76bf1a..2a50d3237ee6c6ace1efc0fb419333e3c3970a93 100644 (file)
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1135,22 +1135,16 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
        return 0;
 }
 
-static bool verify_xindex(int xindex, int table_size)
-{
-       int index, xtype;
-       xtype = xindex & AA_X_TYPE_MASK;
-       index = xindex & AA_X_INDEX_MASK;
-       if (xtype == AA_X_TABLE && index >= table_size)
-               return false;
-       return true;
-}
-
-/* verify dfa xindexes are in range of transition tables */
-static bool verify_dfa_xindex(struct aa_dfa *dfa, int table_size)
+/**
+ * verify_dfa_accept_xindex - verify accept indexes are in range of perms table
+ * @dfa: the dfa to check accept indexes are in range
+ * table_size: the permission table size the indexes should be within
+ */
+static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
 {
        int i;
        for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
-               if (!verify_xindex(ACCEPT_TABLE(dfa)[i], table_size))
+               if (ACCEPT_TABLE(dfa)[i] >= table_size)
                        return false;
        }
        return true;
@@ -1187,11 +1181,13 @@ static bool verify_perms(struct aa_policydb *pdb)
                if (!verify_perm(&pdb->perms[i]))
                        return false;
                /* verify indexes into str table */
-               if (pdb->perms[i].xindex >= pdb->trans.size)
+               if ((pdb->perms[i].xindex & AA_X_TYPE_MASK) == AA_X_TABLE &&
+                   (pdb->perms[i].xindex & AA_X_INDEX_MASK) >= pdb->trans.size)
                        return false;
-               if (pdb->perms[i].tag >= pdb->trans.size)
+               if (pdb->perms[i].tag && pdb->perms[i].tag >= pdb->trans.size)
                        return false;
-               if (pdb->perms[i].label >= pdb->trans.size)
+               if (pdb->perms[i].label &&
+                   pdb->perms[i].label >= pdb->trans.size)
                        return false;
        }
 
@@ -1213,10 +1209,10 @@ static int verify_profile(struct aa_profile *profile)
        if (!rules)
                return 0;
 
-       if ((rules->file.dfa && !verify_dfa_xindex(rules->file.dfa,
-                                                 rules->file.trans.size)) ||
+       if ((rules->file.dfa && !verify_dfa_accept_index(rules->file.dfa,
+                                                        rules->file.size)) ||
            (rules->policy.dfa &&
-            !verify_dfa_xindex(rules->policy.dfa, rules->policy.trans.size))) {
+            !verify_dfa_accept_index(rules->policy.dfa, rules->policy.size))) {
                audit_iface(profile, NULL, NULL,
                            "Unpack: Invalid named transition", NULL, -EPROTO);
                return -EPROTO;