apparmor: convert attaching profiles via xattrs to use dfa matching

This converts profile attachment based on xattrs to a fixed extended
conditional using dfa matching.

This has a couple of advantages
- pattern matching can be used for the xattr match

- xattrs can be optional for an attachment or marked as required

- the xattr attachment conditional will be able to be combined with
  other extended conditionals when the flexible extended conditional
  work lands.

The xattr fixed extended conditional is appended to the xmatch
conditional. If an xattr attachment is specified the profile xmatch
will be generated regardless of whether there is a pattern match on
the executable name.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 1e63ff2e5b8515dba13ff1675b1961d4edcb571b..35e6b240fb14d03eff261256573947df3e4e3ac1 100644 (file)
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2147,6 +2147,10 @@ static struct aa_sfs_entry aa_sfs_entry_signal[] = {
        { }
 };
 
+static struct aa_sfs_entry aa_sfs_entry_attach[] = {
+       AA_SFS_FILE_BOOLEAN("xattr", 1),
+       { }
+};
 static struct aa_sfs_entry aa_sfs_entry_domain[] = {
        AA_SFS_FILE_BOOLEAN("change_hat",       1),
        AA_SFS_FILE_BOOLEAN("change_hatv",      1),
@@ -2155,6 +2159,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = {
        AA_SFS_FILE_BOOLEAN("stack",            1),
        AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap",      1),
        AA_SFS_FILE_BOOLEAN("post_nnp_subset",  1),
+       AA_SFS_DIR("attach_conditions",         aa_sfs_entry_attach),
        AA_SFS_FILE_STRING("version", "1.2"),
        { }
 };

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 6bcafe8d226d021dfade2f78118dc768d787a5c6..6a1279f11fcc60cf54f3dbd4df9c42afb78e4a15 100644 (file)
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -306,11 +306,12 @@ static int change_profile_perms(struct aa_profile *profile,
  * aa_xattrs_match - check whether a file matches the xattrs defined in profile
  * @bprm: binprm struct for the process to validate
  * @profile: profile to match against (NOT NULL)
+ * @state: state to start match in
  *
  * Returns: number of extended attributes that matched, or < 0 on error
  */
 static int aa_xattrs_match(const struct linux_binprm *bprm,
-                          struct aa_profile *profile)
+                          struct aa_profile *profile, unsigned int state)
 {
        int i;
        size_t size;
@@ -321,27 +322,40 @@ static int aa_xattrs_match(const struct linux_binprm *bprm,
        if (!bprm || !profile->xattr_count)
                return 0;
 
+       /* transition from exec match to xattr set */
+       state = aa_dfa_null_transition(profile->xmatch, state);
+
        d = bprm->file->f_path.dentry;
 
        for (i = 0; i < profile->xattr_count; i++) {
                size = vfs_getxattr_alloc(d, profile->xattrs[i], &value,
                                          value_size, GFP_KERNEL);
-               if (size < 0) {
-                       ret = -EINVAL;
-                       goto out;
-               }
+               if (size >= 0) {
+                       u32 perm;
 
-               /* Check the xattr value, not just presence */
-               if (profile->xattr_lens[i]) {
-                       if (profile->xattr_lens[i] != size) {
+                       /* Check the xattr value, not just presence */
+                       state = aa_dfa_match_len(profile->xmatch, state, value,
+                                                size);
+                       perm = dfa_user_allow(profile->xmatch, state);
+                       if (!(perm & MAY_EXEC)) {
                                ret = -EINVAL;
                                goto out;
                        }
-
-                       if (memcmp(value, profile->xattr_values[i], size)) {
+               }
+               /* transition to next element */
+               state = aa_dfa_null_transition(profile->xmatch, state);
+               if (size < 0) {
+                       /*
+                        * No xattr match, so verify if transition to
+                        * next element was valid. IFF so the xattr
+                        * was optional.
+                        */
+                       if (!state) {
                                ret = -EINVAL;
                                goto out;
                        }
+                       /* don't count missing optional xattr as matched */
+                       ret--;
                }
        }
 
@@ -403,13 +417,16 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
                        perm = dfa_user_allow(profile->xmatch, state);
                        /* any accepting state means a valid match. */
                        if (perm & MAY_EXEC) {
-                               int ret = aa_xattrs_match(bprm, profile);
+                               int ret = aa_xattrs_match(bprm, profile, state);
 
                                /* Fail matching if the xattrs don't match */
                                if (ret < 0)
                                        continue;
 
-                               /* The new match isn't more specific
+                               /*
+                                * TODO: allow for more flexible best match
+                                *
+                                * The new match isn't more specific
                                 * than the current best match
                                 */
                                if (profile->xmatch_len == len &&
@@ -428,9 +445,11 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm,
                                xattrs = ret;
                                conflict = false;
                        }
-               } else if (!strcmp(profile->base.name, name) &&
-                          aa_xattrs_match(bprm, profile) >= 0)
-                       /* exact non-re match, no more searching required */
+               } else if (!strcmp(profile->base.name, name))
+                       /*
+                        * old exact non-re match, without conditionals such
+                        * as xattrs. no more searching required
+                        */
                        return profile;
        }
 
@@ -652,7 +671,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
                         * met, and fail execution otherwise
                         */
                        label_for_each(i, new, component) {
-                               if (aa_xattrs_match(bprm, component) < 0) {
+                               if (aa_xattrs_match(bprm, component, state) <
+                                   0) {
                                        error = -EACCES;
                                        info = "required xattrs not present";
                                        perms.allow &= ~MAY_EXEC;

diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h
index 02bde92ebb5c8d2b15acc665dc99dc975c9da232..c93b9ed5549079186643e937484624f7dbee2445 100644 (file)
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -151,8 +151,6 @@ struct aa_profile {
 
        int xattr_count;
        char **xattrs;
-       size_t *xattr_lens;
-       char **xattr_values;
 
        struct aa_rlimit rlimits;
 

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 7fee546ba10ded8ba07f1f4f83e0b5a5c6544b3b..c07493ce237680c7183d93282a310cf4eacbfbde 100644 (file)
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -228,13 +228,9 @@ void aa_free_profile(struct aa_profile *profile)
        aa_free_cap_rules(&profile->caps);
        aa_free_rlimit_rules(&profile->rlimits);
 
-       for (i = 0; i < profile->xattr_count; i++) {
+       for (i = 0; i < profile->xattr_count; i++)
                kzfree(profile->xattrs[i]);
-               kzfree(profile->xattr_values[i]);
-       }
        kzfree(profile->xattrs);
-       kzfree(profile->xattr_lens);
-       kzfree(profile->xattr_values);
        kzfree(profile->dirname);
        aa_put_dfa(profile->xmatch);
        aa_put_dfa(profile->policy.dfa);

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 98d019185e576429bad17b8dc876e73c133a76fa..8a31ddd474d7c4bedeacf641bedbaff5f63d862a 100644 (file)
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -540,8 +540,7 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile)
 
                size = unpack_array(e, NULL);
                profile->xattr_count = size;
-               profile->xattrs = kcalloc(size, sizeof(char *),
-                                               GFP_KERNEL);
+               profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL);
                if (!profile->xattrs)
                        goto fail;
                for (i = 0; i < size; i++) {
@@ -554,38 +553,6 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile)
                        goto fail;
        }
 
-       if (unpack_nameX(e, AA_STRUCT, "xattr_values")) {
-               int i, size;
-
-               size = unpack_array(e, NULL);
-
-               /* Must be the same number of xattr values as xattrs */
-               if (size != profile->xattr_count)
-                       goto fail;
-
-               profile->xattr_lens = kcalloc(size, sizeof(size_t),
-                                                   GFP_KERNEL);
-               if (!profile->xattr_lens)
-                       goto fail;
-
-               profile->xattr_values = kcalloc(size, sizeof(char *),
-                                                     GFP_KERNEL);
-               if (!profile->xattr_values)
-                       goto fail;
-
-               for (i = 0; i < size; i++) {
-                       profile->xattr_lens[i] = unpack_blob(e,
-                                             &profile->xattr_values[i], NULL);
-                       profile->xattr_values[i] =
-                               kvmemdup(profile->xattr_values[i],
-                                        profile->xattr_lens[i]);
-               }
-
-               if (!unpack_nameX(e, AA_ARRAYEND, NULL))
-                       goto fail;
-               if (!unpack_nameX(e, AA_STRUCTEND, NULL))
-                       goto fail;
-       }
        return 1;
 
 fail: