apparmor: make str table more generic and be able to have multiple entries

The strtable is currently limited to a single entry string on unpack
even though domain has the concept of multiple entries within it. Make
this a reality as it will be used for tags and more advanced domain
transitions.

Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 267da82..ee2df25 100644 (file)
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -529,7 +529,7 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
        /* TODO: move lookup parsing to unpack time so this is a straight
         *       index into the resultant label
         */
-       for (next = rules->file->trans.table[index]; next;
+       for (next = rules->file->trans.table[index].strs; next;
             next = next_name(xtype, next)) {
                const char *lookup = (*next == '&') ? next + 1 : next;
                *name = next;

diff --git a/security/apparmor/include/lib.h b/security/apparmor/include/lib.h
index 4441970..194be85 100644 (file)
--- a/security/apparmor/include/lib.h
+++ b/security/apparmor/include/lib.h
@@ -30,6 +30,7 @@ extern struct aa_dfa *stacksplitdfa;
 #define DEBUG_DOMAIN 4
 #define DEBUG_POLICY 8
 #define DEBUG_INTERFACE 0x10
+#define DEBUG_UNPACK 0x40
 
 #define DEBUG_ALL 0x1f         /* update if new DEBUG_X added */
 #define DEBUG_PARSE_ERROR (-1)
@@ -119,13 +120,19 @@ static inline bool path_mediated_fs(struct dentry *dentry)
        return !(dentry->d_sb->s_flags & SB_NOUSER);
 }
 
+struct aa_str_table_ent {
+       int count;
+       int size;
+       char *strs;
+};
+
 struct aa_str_table {
        int size;
-       char **table;
+       struct aa_str_table_ent *table;
 };
 
-void aa_free_str_table(struct aa_str_table *table);
 bool aa_resize_str_table(struct aa_str_table *t, int newsize, gfp_t gfp);
+void aa_destroy_str_table(struct aa_str_table *table);
 
 struct counted_str {
        struct kref count;

diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index acf7f51..7ef1b9b 100644 (file)
--- a/security/apparmor/lib.c
+++ b/security/apparmor/lib.c
@@ -44,6 +44,7 @@ static struct val_table_ent debug_values_table[] = {
        { "domain", DEBUG_DOMAIN },
        { "policy", DEBUG_POLICY },
        { "interface", DEBUG_INTERFACE },
+       { "unpack", DEBUG_UNPACK },
        { NULL, 0 }
 };
 
@@ -118,7 +119,7 @@ int aa_print_debug_params(char *buffer)
 
 bool aa_resize_str_table(struct aa_str_table *t, int newsize, gfp_t gfp)
 {
-       char **n;
+       struct aa_str_table_ent *n;
        int i;
 
        if (t->size == newsize)
@@ -129,7 +130,7 @@ bool aa_resize_str_table(struct aa_str_table *t, int newsize, gfp_t gfp)
        for (i = 0; i < min(t->size, newsize); i++)
                n[i] = t->table[i];
        for (; i < t->size; i++)
-               kfree_sensitive(t->table[i]);
+               kfree_sensitive(t->table[i].strs);
        if (newsize > t->size)
                memset(&n[t->size], 0, (newsize-t->size)*sizeof(*n));
        kfree_sensitive(t->table);
@@ -140,10 +141,10 @@ bool aa_resize_str_table(struct aa_str_table *t, int newsize, gfp_t gfp)
 }
 
 /**
- * aa_free_str_table - free entries str table
+ * aa_destroy_str_table - free entries str table
  * @t: the string table to free  (MAYBE NULL)
  */
-void aa_free_str_table(struct aa_str_table *t)
+void aa_destroy_str_table(struct aa_str_table *t)
 {
        int i;
 
@@ -152,7 +153,7 @@ void aa_free_str_table(struct aa_str_table *t)
                        return;
 
                for (i = 0; i < t->size; i++)
-                       kfree_sensitive(t->table[i]);
+                       kfree_sensitive(t->table[i].strs);
                kfree_sensitive(t->table);
                t->table = NULL;
                t->size = 0;

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index b093238..2a8a0e4 100644 (file)
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -104,7 +104,7 @@ static void aa_free_pdb(struct aa_policydb *pdb)
        if (pdb) {
                aa_put_dfa(pdb->dfa);
                kvfree(pdb->perms);
-               aa_free_str_table(&pdb->trans);
+               aa_destroy_str_table(&pdb->trans);
                kfree(pdb);
        }
 }

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 7523971..4f7cb42 100644 (file)
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -450,20 +450,72 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e, int flags)
        return dfa;
 }
 
+static int process_strs_entry(char *str, int size, bool multi)
+{
+       int c = 1;
+
+       if (size <= 0)
+               return -1;
+       if (multi) {
+               if (size < 2)
+                       return -2;
+               /* multi ends with double \0 */
+               if (str[size - 2])
+                       return -3;
+       }
+
+       char *save = str;
+       char *pos = str;
+       char *end = multi ? str + size - 2 : str + size - 1;
+       /* count # of internal \0 */
+       while (str < end) {
+               if (str == pos) {
+                       /* starts with ... */
+                       if (!*str) {
+                               AA_DEBUG(DEBUG_UNPACK,
+                                        "starting with null save=%lu size %d c=%d",
+                                        str - save, size, c);
+                               return -4;
+                       }
+                       if (isspace(*str))
+                               return -5;
+                       if (*str == ':') {
+                               /* :ns_str\0str\0
+                                * first character after : must be valid
+                                */
+                               if (!str[1])
+                                       return -6;
+                       }
+               } else if (!*str) {
+                       if (*pos == ':')
+                               *str = ':';
+                       else
+                               c++;
+                       pos = str +  1;
+               }
+               str++;
+       } /* while */
+
+       return c;
+}
+
 /**
- * unpack_trans_table - unpack a profile transition table
+ * unpack_strs_table - unpack a profile transition table
  * @e: serialized data extent information  (NOT NULL)
+ * @name: name of table (MAY BE NULL)
+ * @multi: allow multiple strings on a single entry
  * @strs: str table to unpack to (NOT NULL)
  *
  * Returns: true if table successfully unpacked or not present
  */
-static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
+static bool unpack_strs_table(struct aa_ext *e, const char *name, bool multi,
+                             struct aa_str_table *strs)
 {
        void *saved_pos = e->pos;
-       char **table = NULL;
+       struct aa_str_table_ent *table = NULL;
 
        /* exec table is optional */
-       if (aa_unpack_nameX(e, AA_STRUCT, "xtable")) {
+       if (aa_unpack_nameX(e, AA_STRUCT, name)) {
                u16 size;
                int i;
 
@@ -475,7 +527,8 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
                         * for size check here
                         */
                        goto fail;
-               table = kcalloc(size, sizeof(char *), GFP_KERNEL);
+               table = kcalloc(size, sizeof(struct aa_str_table_ent),
+                               GFP_KERNEL);
                if (!table)
                        goto fail;
 
@@ -483,41 +536,23 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
                strs->size = size;
                for (i = 0; i < size; i++) {
                        char *str;
-                       int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL);
+                       int c, size2 = aa_unpack_strdup(e, &str, NULL);
                        /* aa_unpack_strdup verifies that the last character is
                         * null termination byte.
                         */
-                       if (!size2)
+                       c = process_strs_entry(str, size2, multi);
+                       if (c <= 0) {
+                               AA_DEBUG(DEBUG_UNPACK, "process_strs %d", c);
                                goto fail;
-                       table[i] = str;
-                       /* verify that name doesn't start with space */
-                       if (isspace(*str))
-                               goto fail;
-
-                       /* count internal #  of internal \0 */
-                       for (c = j = 0; j < size2 - 1; j++) {
-                               if (!str[j]) {
-                                       pos = j;
-                                       c++;
-                               }
                        }
-                       if (*str == ':') {
-                               /* first character after : must be valid */
-                               if (!str[1])
-                                       goto fail;
-                               /* beginning with : requires an embedded \0,
-                                * verify that exactly 1 internal \0 exists
-                                * trailing \0 already verified by aa_unpack_strdup
-                                *
-                                * convert \0 back to : for label_parse
-                                */
-                               if (c == 1)
-                                       str[pos] = ':';
-                               else if (c > 1)
-                                       goto fail;
-                       } else if (c)
+                       if (!multi && c > 1) {
+                               AA_DEBUG(DEBUG_UNPACK, "!multi && c > 1");
                                /* fail - all other cases with embedded \0 */
                                goto fail;
+                       }
+                       table[i].strs = str;
+                       table[i].count = c;
+                       table[i].size = size2;
                }
                if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL))
                        goto fail;
@@ -527,7 +562,7 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs)
        return true;
 
 fail:
-       aa_free_str_table(strs);
+       aa_destroy_str_table(strs);
        e->pos = saved_pos;
        return false;
 }
@@ -795,13 +830,14 @@ static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy,
         * transition table may be present even when the dfa is
         * not. For compatibility reasons unpack and discard.
         */
-       if (!unpack_trans_table(e, &pdb->trans) && required_trans) {
+       if (!unpack_strs_table(e, "xtable", false, &pdb->trans) &&
+           required_trans) {
                *info = "failed to unpack profile transition table";
                goto fail;
        }
 
        if (!pdb->dfa && pdb->trans.table)
-               aa_free_str_table(&pdb->trans);
+               aa_destroy_str_table(&pdb->trans);
 
        /* TODO:
         * - move compat mapping here, requires dfa merging first
@@ -1268,7 +1304,7 @@ static bool verify_perms(struct aa_policydb *pdb)
        }
        /* deal with incorrectly constructed string tables */
        if (xmax == -1) {
-               aa_free_str_table(&pdb->trans);
+               aa_destroy_str_table(&pdb->trans);
        } else if (pdb->trans.size > xmax + 1) {
                if (!aa_resize_str_table(&pdb->trans, xmax + 1, GFP_KERNEL))
                        return false;