-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Sorry about the delay. Here's a second pass at the original pass, with fewer memory allocations, and a more general API for requesting data in general.
The corresponding apparmor branch is at https://code.launchpad.net/~attente/apparmor/dconf-rules-4. On 2015-06-05 09:12 AM, Simon McVittie wrote: > On 05/06/15 12:13, John Johansen wrote: >> On 05/29/2015 09:29 AM, Simon McVittie wrote: >>> Here's a sketch of how [polkit mediation] could look, for >>> instance: >>> >>> audit polkit action=org.freedesktop.udisks2.filesystem-mount, >>> audit deny polkit \ >>> action=org.freedesktop.udisks2.filesystem-mount-system, >>> >>> or if the syntax in policy files was entirely generic, perhaps >>> something more like: >>> >>> userspace class=polkit \ >>> action=org.freedesktop.udisks2.filesystem-mount, audit deny >>> userspace class=polkit \ >>> action=org.freedesktop.udisks2.filesystem-mount-system, >>> >>> Does this sound like a reasonable generalization? >>> >> generally speaking, yes :) >> >> I can't say when polkit will get patched but I expect it will >> happen sooner than later. > > If this becomes something that is concretely required, please talk > to the polkit mailing list - the polkit developers ought to have > an opportunity to review this. I've subscribed to that list to be > able to give D-Bus advice. > > My colleague Philip Withnall and I are not (currently) polkit > maintainers, but we would potentially be interested in reviewing > and/or helping with implementation for this feature. > -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJVcaxIAAoJEGaNijJ4Mbw+Fs4H/2D1GO2vh9RR9eg2d2FX26j+ eDq7lqimCk79gDLXlcsgoJSGgsW9wFZHzC25TKnFI3k+HRoIbhLpJ7lL37BGNSFT tHlOpg4M78tnYXMUfPYge6Oc1e+5VR3UU9YCyLVNO0981+wY6Ek0H5cgiDx9rlOP Q++qiB+k3HnndyHepRDRbv3Bq3GEsNS8NzcunKumafkzycPyRbzWCg4FMDOpOL3v X0rHwuY/SL7ElMGINrF/G9RkfXwgWDxRjTl084BuPmL6p4lwl3B5/EdoZd1X5vyo BliHYaqIyx8Z2XZsndOaSav1hdesu/k65JSjLFDB0RKnTJusbq57tPJvUtHiqsQ= =rhFQ -----END PGP SIGNATURE-----
>From 1b566ff10a8dc1d0a5328f200dcbc51e0a67b85a Mon Sep 17 00:00:00 2001 From: William Hua <[email protected]> Date: Mon, 1 Jun 2015 18:33:25 -0400 Subject: [PATCH] apparmor: add data query support --- security/apparmor/apparmorfs.c | 115 +++++++++++++++++++++++++++++++++++-- security/apparmor/include/policy.h | 20 ++++++- security/apparmor/policy.c | 20 +++++++ security/apparmor/policy_unpack.c | 54 ++++++++++++++++- 4 files changed, 202 insertions(+), 7 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index f86f56e..ab11bd1 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -189,6 +189,97 @@ static const struct file_operations aa_fs_profile_remove = { .llseek = default_llseek, }; +static bool keycmp(void *p1, void *p2) { + const char **k1 = p1; + const char **k2 = p2; + return !strcmp(*k1, *k2); +} + +/** + * query_data - queries a policy and writes its data to buf + * @buf: the resulting data is stored here (NOT NULL) + * @buf_len: size of buf + * @query: query string used to retrieve data + * @query_len: size of query including second NUL byte + * + * The buffers pointed to by buf and query may overlap. The query buffer is + * parsed before buf is written to. + * + * The query should look like "<LABEL>\0<KEY>\0", where <LABEL> is the name of + * the security confinement context and <KEY> is the name of the data to + * retrieve. <LABEL> and <KEY> must not be NUL-terminated. + * + * Don't expect the contents of buf to be preserved on failure. + * + * Returns: number of characters written to buf or -errno on failure + */ +static ssize_t query_data(char *buf, size_t buf_len, + char *query, size_t query_len) +{ + char *out; + const char *key; + struct label_it i; + struct aa_label *label; + struct aa_profile *profile; + struct aa_data *data; + ssize_t blocks = 0; + ssize_t bytes; + u32 hash; + + if (!query_len) + return -EINVAL; /* need a query */ + + key = query + strnlen(query, query_len) + 1; + if (key + 1 >= query + query_len) + return -EINVAL; /* not enough space for a non-empty key */ + if (key + strnlen(key, query + query_len - key) >= query + query_len) + return -EINVAL; /* must end with NUL */ + + if (buf_len < 2 * sizeof(ssize_t)) + return -EINVAL; /* not enough space */ + + label = aa_label_parse(aa_current_label(), query, GFP_KERNEL, false); + if (IS_ERR(label)) + return PTR_ERR(label); + + /* We are going to leave space for two numbers. The first is the total + * number of bytes we are writing after the first number. This is so + * users can read the full output without reallocation. + * + * The second number is the number of data blocks we're writing. An + * application might be confined by multiple policies having data in + * the same key. */ + memset(buf, 0, 2 * sizeof(ssize_t)); + out = buf + 2 * sizeof(ssize_t); + + label_for_each_confined(i, label, profile) { + if (!profile->data) + continue; + + hash = rhashtable_hashfn(profile->data, &key, sizeof(key)); + data = rhashtable_lookup_compare(profile->data, hash, keycmp, &key); + + if (data) { + if (out + sizeof(ssize_t) + data->size > buf + buf_len) { + aa_put_label(label); + return -EINVAL; /* not enough space */ + } + memcpy(out, &data->size, sizeof(ssize_t)); + out += sizeof(ssize_t); + memcpy(out, data->data, data->size); + out += data->size; + blocks++; + } + } + aa_put_label(label); + + bytes = out - buf - sizeof(ssize_t); + memcpy(buf, &bytes, sizeof(ssize_t)); + memcpy(buf + sizeof(ssize_t), &blocks, sizeof(ssize_t)); + + return out - buf; +} + /** * query_label - queries a label and writes permissions to buf * @buf: the resulting permissions string is stored here (NOT NULL) @@ -274,18 +365,27 @@ static ssize_t query_label(char *buf, size_t buf_len, #define QUERY_CMD_LABEL_LEN 6 #define QUERY_CMD_PROFILE "profile\0" #define QUERY_CMD_PROFILE_LEN 8 +#define QUERY_CMD_DATA "data\0" +#define QUERY_CMD_DATA_LEN 5 /** - * aa_write_access - generic permissions query + * aa_write_access - generic permissions and data query * @file: pointer to open apparmorfs/access file * @ubuf: user buffer containing the complete query string (NOT NULL) * @count: size of ubuf * @ppos: position in the file (MUST BE ZERO) * - * Allows for one permission query per open(), write(), and read() sequence. - * The only query currently supported is a label-based query. For this query - * ubuf must begin with "label\0", followed by the profile query specific - * format described in the query_label() function documentation. + * Allows for one permissions or data query per open(), write(), and read() + * sequence. The only queries currently supported are label-based queries for + * permissions or data. + * + * For permissions queries, ubuf must begin with "label\0", followed by the + * profile query specific format described in the query_label() function + * documentation. + * + * For data queries, ubuf must have the form "data\0<LABEL>\0<KEY>\0", where + * <LABEL> is the name of the security confinement context and <KEY> is the + * name of the data to retrieve. * * Returns: number of bytes written or -errno on failure */ @@ -312,6 +412,11 @@ static ssize_t aa_write_access(struct file *file, const char __user *ubuf, len = query_label(buf, SIMPLE_TRANSACTION_LIMIT, buf + QUERY_CMD_LABEL_LEN, count - QUERY_CMD_LABEL_LEN); + } else if (count > QUERY_CMD_DATA_LEN && + !memcmp(buf, QUERY_CMD_DATA, QUERY_CMD_DATA_LEN)) { + len = query_data(buf, SIMPLE_TRANSACTION_LIMIT, + buf + QUERY_CMD_DATA_LEN, + count - QUERY_CMD_DATA_LEN); } else len = -EINVAL; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index da71e27f..48e90a7 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -18,6 +18,7 @@ #include <linux/capability.h> #include <linux/cred.h> #include <linux/kref.h> +#include <linux/rhashtable.h> #include <linux/sched.h> #include <linux/slab.h> #include <linux/socket.h> @@ -142,6 +143,19 @@ struct aa_policydb { }; +/* struct aa_data - generic data structure + * key: name for retrieving this data + * size: size of data in bytes + * data: binary data + * head: reserved for rhashtable + */ +struct aa_data { + const char *key; + size_t size; + char *data; + struct rhash_head head; +}; + /* struct aa_profile - basic confinement data * @base - base components of the profile (name, refcount, lists, lock ...) * @label - label this profile is an extension of @@ -161,9 +175,10 @@ struct aa_policydb { * @caps: capabilities for the profile * @net: network controls for the profile * @rlimits: rlimits for the profile - * * @dents: dentries for the profiles file entries in apparmorfs * @dirname: name of the profile dir in apparmorfs + * @blob: a single binary string holding the allocated data + * @data: hashtable for free-form policy aa_data * * The AppArmor profile contains the basic confinement data. Each profile * has a name, and exists in a namespace. The @name and @exec_match are @@ -205,6 +220,9 @@ struct aa_profile { unsigned char *hash; char *dirname; struct dentry *dents[AAFS_PROF_SIZEOF]; + + void *blob; + struct rhashtable *data; }; extern struct aa_namespace *root_ns; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 7a9d4c8..0d5c477 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -630,6 +630,11 @@ void __init aa_free_root_ns(void) */ void aa_free_profile(struct aa_profile *profile) { + struct rhashtable *rht; + struct aa_data *data; + struct aa_data *next; + size_t i; + AA_DEBUG("%s(%p)\n", __func__, profile); if (!profile) @@ -651,6 +656,21 @@ void aa_free_profile(struct aa_profile *profile) aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); + if (profile->data) { + rht = profile->data; + profile->data = NULL; + for (i = 0; i < rht->tbl->size; i++) { + rht_for_each_entry_safe(data, next, rht->tbl->buckets[i], rht, head) { + kzfree(data); + } + } + rhashtable_destroy(rht); + kzfree(rht); + } + + if (profile->blob) + kzfree(profile->blob); + kzfree(profile->hash); kzfree(profile); } diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 7f63b67..185bbe0 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -20,6 +20,7 @@ #include <asm/unaligned.h> #include <linux/ctype.h> #include <linux/errno.h> +#include <linux/jhash.h> #include "include/apparmor.h" #include "include/audit.h" @@ -484,6 +485,12 @@ fail: return 0; } +static u32 shash(const void *data, u32 len, u32 seed) +{ + const char * const *key = data; + return jhash(*key, strlen(*key), seed); +} + /** * unpack_profile - unpack a serialized profile * @e: serialized data extent information (NOT NULL) @@ -494,6 +501,11 @@ static struct aa_profile *unpack_profile(struct aa_ext *e) { struct aa_profile *profile = NULL; const char *name = NULL; + const char *key = NULL; + char *blob = NULL; + struct rhashtable_params params = { 0 }; + struct aa_ext f; + struct aa_data *data; size_t size = 0; int i, error = -EPROTO; kernel_cap_t tmpcap; @@ -668,14 +680,54 @@ static struct aa_profile *unpack_profile(struct aa_ext *e) if (!unpack_trans_table(e, profile)) goto fail; + size = unpack_blob(e, &blob, "data"); + if (size) { + profile->blob = kmemdup(blob, size, GFP_KERNEL); + if (!profile->blob) + goto fail; + + profile->data = kzalloc(sizeof(*profile->data), GFP_KERNEL); + if (!profile->data) + goto fail; + + params.nelem_hint = 1; + params.key_len = sizeof(void *); + params.key_offset = offsetof(struct aa_data, key); + params.head_offset = offsetof(struct aa_data, head); + params.hashfn = shash; + + if (rhashtable_init(profile->data, ¶ms)) + goto fail; + + f.start = profile->blob; + f.end = f.start + size; + f.pos = f.start; + f.version = e->version; + + while (unpack_str(&f, &key, NULL)) { + data = kzalloc(sizeof(*data), GFP_KERNEL); + data->key = key; + data->size = unpack_blob(&f, &data->data, NULL); + if (!data->size) { + kzfree(data); + goto fail; + } + data->head.next = NULL; + rhashtable_insert(profile->data, &data->head); + } + } + if (!unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; return profile; fail: - if (profile) + /* don't worry about profile->data, aa_free_profile should free it */ + if (profile) { + kzfree(profile->blob); name = NULL; + } else if (!name) name = "unknown"; audit_iface(profile, name, "failed to unpack profile", e, error); -- 2.1.4
0001-apparmor-add-data-query-support.patch.sig
Description: PGP signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
