Currently, xattrs values must match the xmatch DFA to match a profile. This lets users construct profiles to match a file with a specific key and value with a basic regex.

profile test xattrs(security.apparmor=/usr/bin/*) {}

The xmatch DFA doesn't handle null characters in the xattrs value, since this is the special character used to indicate that the DFA is transitioning from matching the profile path to matching the xattr value.

However, both IMA and EVM xattr values hold signatures which potentially have a null character. It's currently impossible to write a profile that requires a the presence of an EVM signature without checking the value.

profile test xattrs(security.evm security.apparmor=/usr/bin/*) {}

Add an additional "xattr_keys" array to the profile that only checks the presence of extended attributes, and not their values.

A modified apparmor_parser that was used to test these changes can be found at:

https://gitlab.com/ericchiang/apparmor/commits/parser-xattrs-keys

To test, build the parser and run the following command:

$ echo '/usr/bin/* xattrs=(user.foo user.bar=bar) {}' | \ ./apparmor_parser -r $ setfattr -n "user.foo" -v "foo" /usr/bin/whoami $ setfattr -n "user.bar" -v "bar" /usr/bin/whoami $ whoami # command fails

Signed-off-by: Eric Chiang ericchiang@google.com CC: stable@vger.kernel.org --- security/apparmor/domain.c | 22 +++++++++++++++++++--- security/apparmor/include/policy.h | 6 ++++++ security/apparmor/policy.c | 3 +++ security/apparmor/policy_unpack.c | 18 ++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 08c88de0ffda..53c46e3e01f0 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -319,14 +319,30 @@ static int aa_xattrs_match(const struct linux_binprm *bprm, char *value = NULL; int value_size = 0, ret = profile->xattr_count;

- if (!bprm || !profile->xattr_count) + if (!bprm) return 0;

+ d = bprm->file->f_path.dentry; + + if (profile->xattr_keys_count) { + /* validate that these attributes are present, ignore values */ + for (i = 0; i < profile->xattr_keys_count; i++) { + size = vfs_getxattr_alloc(d, profile->xattr_keys[i], + &value, value_size, + GFP_KERNEL); + if (size < 0) { + ret = -EINVAL; + goto out; + } + } + } + + if (!profile->xattr_count) + goto out; + /* 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); diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 8e6707c837be..8ed1d30de7ce 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -112,6 +112,10 @@ struct aa_data { * @policy: general match rules governing policy * @file: The set of rules governing basic file access and domain transitions * @caps: capabilities for the profile + * @xattr_count: number of xattrs values + * @xattrs: extended attributes whose values must match the xmatch + * @xattr_keys_count: number of xattr keys values + * @xattr_keys: extended attributes that must be present to match the profile * @rlimits: rlimits for the profile * * @dents: dentries for the profiles file entries in apparmorfs @@ -152,6 +156,8 @@ struct aa_profile {

int xattr_count; char **xattrs; + int xattr_keys_count; + char **xattr_keys;

struct aa_rlimit rlimits;

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index df9c5890a878..e0f9cf8b8318 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -231,6 +231,9 @@ void aa_free_profile(struct aa_profile *profile) for (i = 0; i < profile->xattr_count; i++) kzfree(profile->xattrs[i]); kzfree(profile->xattrs); + for (i = 0; i < profile->xattr_keys_count; i++) + kzfree(profile->xattr_keys[i]); + kzfree(profile->xattr_keys); for (i = 0; i < profile->secmark_count; i++) kzfree(profile->secmark[i].label); kzfree(profile->secmark); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 379682e2a8d5..d1fd75093260 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -535,6 +535,24 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) goto fail; }

+ if (unpack_nameX(e, AA_STRUCT, "xattr_keys")) { + int i, size; + + size = unpack_array(e, NULL); + profile->xattr_keys_count = size; + profile->xattr_keys = kcalloc(size, sizeof(char *), GFP_KERNEL); + if (!profile->xattr_keys) + goto fail; + for (i = 0; i < size; i++) { + if (!unpack_strdup(e, &profile->xattr_keys[i], NULL)) + goto fail; + } + if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + goto fail; + if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + goto fail; + } + return 1;

fail: