[apparmor] [PATCH 2/3] apparmor: convert attaching profiles via xattrs to use, dfa matching

John Johansen john.johansen at canonical.com
Thu Feb 8 20:38:57 UTC 2018


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 at canonical.com>
---
 security/apparmor/apparmorfs.c     |  5 ++++
 security/apparmor/domain.c         | 52 ++++++++++++++++++++++++++------------
 security/apparmor/include/policy.h |  2 --
 security/apparmor/policy.c         |  6 +----
 security/apparmor/policy_unpack.c  | 32 -----------------------
 5 files changed, 42 insertions(+), 55 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 1e63ff2e5b85..35e6b240fb14 100644
--- 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 ca8353f3e7a0..9651e18cbae5 100644
--- 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--;
 		}
 	}
 
@@ -402,13 +416,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 &&
@@ -427,9 +444,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;
 	}
 
@@ -651,7 +670,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 9eab920b68eb..15f2404e70ba 100644
--- a/security/apparmor/include/policy.h
+++ b/security/apparmor/include/policy.h
@@ -153,8 +153,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 437cc917a5eb..69af9c6a6089 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -229,13 +229,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 81c52cd26126..7d4f79884846 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -556,38 +556,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 = kmalloc_array(size, sizeof(size_t),
-						    GFP_KERNEL);
-		if (!profile->xattr_lens)
-			goto fail;
-
-		profile->xattr_values = kmalloc_array(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:
-- 
2.14.1




More information about the AppArmor mailing list