[apparmor] [PATCH v3] apparmor: implement profile-based query interface in apparmorfs
Tyler Hicks
tyhicks at canonical.com
Thu Mar 28 01:06:09 UTC 2013
On 2013-03-14 22:00:46, Tyler Hicks wrote:
> Allow userspace applications to query for allowed, denied, audit, and
> quiet permissions using a profile name and a DFA match string. Userspace
> applications that wish to enforce access controls defined in the
> system's AppArmor policy can use this interface to perform access
> control lookups.
>
> This patch adds a new file, called .access, to the apparmorfs root
> directory. The semantics of the .access file should be hidden behind a
> libapparmor interface, but the process for doing a query looks like
> this:
>
> open("/sys/kernel/security/apparmor/.access", O_RDWR) = 3
> write(3, "profile\0/usr/bin/app\0 system\0org"..., 98) = 98
> read(3, "allow 0x000002\ndeny 0x000000\naud"..., 1024) = 59
> close(3) = 0
>
> The write() buffer contains the prefix specific to the current type of
> query ("profile\0" in this case), the profile name followed by a '\0',
> and the binary DFA match string. The read() buffer contains the query
> results. Here's an example of the query results:
>
> allow 0x000002
> deny 0x000000
> audit 0x000000
> quiet 0x000000
When hunting bugs in the libapparmor query interface I noticed that
these masks only have 6 digits when they should have 8.
>
> The returned masks can be compared to the permission mask of interest.
> In the above example, the permission represented by 0x000002 would be
> allowed and the action would not be audited. The permission represented
> by 0x000001 would not be allowed and an AVC audit message would need to
> be generated.
>
> Acked-by: John Johansen <john.johansen at canonical.com>
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>
> * Changes from v2 to address Seth's feedback:
> - Fixed typo in commit message
> - Adjust the query_profile() function comments to make it clear that the
> query string is NUL separated, rather than misusing the term "NULL"
> * Added John's ack
>
> security/apparmor/apparmorfs.c | 125 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 125 insertions(+)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 3d8619d..78403bb 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -188,6 +188,130 @@ static const struct file_operations aa_fs_profile_remove = {
> .llseek = default_llseek,
> };
>
> +/**
> + * query_profile - queries a profile and writes permissions to buf
> + * @buf: the resulting permissions string is stored here (NOT NULL)
> + * @buf_len: size of buf
> + * @query: binary query string to match against the dfa
> + * @query_len: size of query
> + *
> + * 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 "PROFILE_NAME\0DFA_STRING" where PROFILE_NAME is
> + * the name of the profile, in the current namespace, that is to be queried and
> + * DFA_STRING is a binary string to match against the profile's DFA.
> + *
> + * PROFILE_NAME must be NUL terminated. DFA_STRING may contain NUL characters
> + * but must *not* be NUL terminated.
> + *
> + * Returns: number of characters written to buf or -errno on failure
> + */
> +static ssize_t query_profile(char *buf, size_t buf_len,
> + char *query, size_t query_len)
> +{
> + struct aa_profile *profile;
> + char *profile_name;
> + size_t profile_name_len;
> + u32 allow, audit, quiet;
> +
> + if (!query_len)
> + return -EINVAL;
> +
> + profile_name = query;
> + profile_name_len = strnlen(query, query_len);
> + if (!profile_name_len || profile_name_len == query_len)
> + return -EINVAL;
> +
> + profile = aa_lookup_profile(aa_current_profile()->ns, profile_name);
> + if (!profile)
> + return -ENOENT;
> +
> + if (unconfined(profile)) {
> + allow = 0xFFFFFFFF;
> + audit = 0x00000000;
> + quiet = 0x00000000;
> + } else if (profile->policy.dfa) {
> + /**
> + * The extra byte is to account for the null byte between the
> + * profile name and dfa string. profile_name_len is greater
> + * than zero and less than query_len, so a byte can be safely
> + * added or subtracted.
> + */
> + char *dfa = profile_name + profile_name_len + 1;
> + size_t dfa_len = query_len - profile_name_len - 1;
> + unsigned int state;
> +
> + state = aa_dfa_match_len(profile->policy.dfa,
> + profile->policy.start[0],
> + dfa, dfa_len);
> + allow = dfa_user_allow(profile->policy.dfa, state);
> + audit = dfa_user_audit(profile->policy.dfa, state);
> + quiet = dfa_user_quiet(profile->policy.dfa, state);
> + } else {
> + aa_put_profile(profile);
> + return -EINVAL;
> + }
> + aa_put_profile(profile);
> +
> + return scnprintf(buf, buf_len,
> + "allow %#08x\ndeny %#08x\naudit %#08x\nquiet %#08x\n",
The problem is here. I guess the fancy alternate form thingy (#) counts
the "0x" as 2 of the 8 field width characters.
I'll respond with a patch that does away with the %#08x format and uses
the more common 0x%08x style instead.
Tyler
> + allow, 0, audit, quiet);
> +}
> +
> +#define QUERY_CMD_PROFILE "profile\0"
> +#define QUERY_CMD_PROFILE_LEN 8
> +
> +/**
> + * aa_write_access - generic permissions 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 profile-based query. For this query
> + * ubuf must begin with "profile\0", followed by the profile query specific
> + * format described in the query_profile() function documentation.
> + *
> + * Returns: number of bytes written or -errno on failure
> + */
> +static ssize_t aa_write_access(struct file *file, const char __user *ubuf,
> + size_t count, loff_t *ppos)
> +{
> + char *buf;
> + ssize_t len;
> +
> + if (*ppos)
> + return -ESPIPE;
> +
> + buf = simple_transaction_get(file, ubuf, count);
> + if (IS_ERR(buf))
> + return PTR_ERR(buf);
> +
> + if (count > QUERY_CMD_PROFILE_LEN &&
> + !memcmp(buf, QUERY_CMD_PROFILE, QUERY_CMD_PROFILE_LEN)) {
> + len = query_profile(buf, SIMPLE_TRANSACTION_LIMIT,
> + buf + QUERY_CMD_PROFILE_LEN,
> + count - QUERY_CMD_PROFILE_LEN);
> + } else
> + len = -EINVAL;
> +
> + if (len < 0)
> + return len;
> +
> + simple_transaction_set(file, len);
> +
> + return count;
> +}
> +
> +static const struct file_operations aa_fs_access = {
> + .write = aa_write_access,
> + .read = simple_transaction_read,
> + .release = simple_transaction_release,
> + .llseek = generic_file_llseek,
> +};
> +
> static int aa_fs_seq_show(struct seq_file *seq, void *v)
> {
> struct aa_fs_entry *fs_file = seq->private;
> @@ -787,6 +911,7 @@ static struct aa_fs_entry aa_fs_entry_apparmor[] = {
> AA_FS_FILE_FOPS(".load", 0640, &aa_fs_profile_load),
> AA_FS_FILE_FOPS(".replace", 0640, &aa_fs_profile_replace),
> AA_FS_FILE_FOPS(".remove", 0640, &aa_fs_profile_remove),
> + AA_FS_FILE_FOPS(".access", 0666, &aa_fs_access),
> #ifdef CONFIG_SECURITY_APPARMOR_COMPAT_24
> AA_FS_FILE_FOPS("profiles", 0640, &aa_fs_profiles_fops),
> #endif
> --
> 1.8.1.2
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130327/0f4d1edb/attachment.pgp>
More information about the AppArmor
mailing list