| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4E6FE1A4.7080907@schaufler-ca.com>
Date: Tue, 13 Sep 2011 16:05:08 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...el.com>
CC: linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH] Smack: check permissions from user space
On 8/28/2011 10:35 PM, Jarkko Sakkinen wrote:
> Adds a new file into SmackFS called 'access'. Wanted
> Smack permission is written into /smack/access.
> After that result can be read from the opened file.
> If access applies result contains 1 and otherwise
> 0. File access is protected from race conditions
> by using simple_transaction_get()/set() API.
Applied to git://gitorious.org/smack-next/kernel.git
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...el.com>
> ---
> security/smack/smack.h | 1 +
> security/smack/smackfs.c | 181 ++++++++++++++++++++++++++++++----------------
> 2 files changed, 119 insertions(+), 63 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 2b6c6a5..82c5a30 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -189,6 +189,7 @@ struct smk_audit_info {
> struct common_audit_data a;
> #endif
> };
> +
> /*
> * These functions are in smack_lsm.c
> */
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index f934601..5a87075 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -27,6 +27,7 @@
> #include <linux/seq_file.h>
> #include <linux/ctype.h>
> #include <linux/audit.h>
> +#include <linux/smack.h>
> #include "smack.h"
>
> /*
> @@ -44,6 +45,7 @@ enum smk_inos {
> SMK_ONLYCAP = 9, /* the only "capable" label */
> SMK_LOGGING = 10, /* logging */
> SMK_LOAD_SELF = 11, /* task specific rules */
> + SMK_ACCESSES = 12, /* access policy */
> };
>
> /*
> @@ -176,71 +178,19 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
> }
>
> /**
> - * smk_write_load_list - write() for any /smack/load
> - * @file: file pointer, not actually used
> - * @buf: where to get the data from
> - * @count: bytes sent
> - * @ppos: where to start - must be 0
> - * @rule_list: the list of rules to write to
> - * @rule_lock: lock for the rule list
> - *
> - * Get one smack access rule from above.
> - * The format is exactly:
> - * char subject[SMK_LABELLEN]
> - * char object[SMK_LABELLEN]
> - * char access[SMK_ACCESSLEN]
> - *
> - * writes must be SMK_LABELLEN+SMK_LABELLEN+SMK_ACCESSLEN bytes.
> + * smk_parse_rule - parse subject, object and access type
> + * @data: string to be parsed whose size is SMK_LOADLEN
> + * @rule: parsed entities are stored in here
> */
> -static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> - size_t count, loff_t *ppos,
> - struct list_head *rule_list,
> - struct mutex *rule_lock)
> +static int smk_parse_rule(const char *data, struct smack_rule *rule)
> {
> - struct smack_rule *rule;
> - char *data;
> - int rc = -EINVAL;
> -
> - /*
> - * No partial writes.
> - * Enough data must be present.
> - */
> - if (*ppos != 0)
> - return -EINVAL;
> - /*
> - * Minor hack for backward compatibility
> - */
> - if (count < (SMK_OLOADLEN) || count > SMK_LOADLEN)
> - return -EINVAL;
> -
> - data = kzalloc(SMK_LOADLEN, GFP_KERNEL);
> - if (data == NULL)
> - return -ENOMEM;
> -
> - if (copy_from_user(data, buf, count) != 0) {
> - rc = -EFAULT;
> - goto out;
> - }
> -
> - /*
> - * More on the minor hack for backward compatibility
> - */
> - if (count == (SMK_OLOADLEN))
> - data[SMK_OLOADLEN] = '-';
> -
> - rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> - if (rule == NULL) {
> - rc = -ENOMEM;
> - goto out;
> - }
> -
> rule->smk_subject = smk_import(data, 0);
> if (rule->smk_subject == NULL)
> - goto out_free_rule;
> + return -1;
>
> rule->smk_object = smk_import(data + SMK_LABELLEN, 0);
> if (rule->smk_object == NULL)
> - goto out_free_rule;
> + return -1;
>
> rule->smk_access = 0;
>
> @@ -252,7 +202,7 @@ static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> rule->smk_access |= MAY_READ;
> break;
> default:
> - goto out_free_rule;
> + return -1;
> }
>
> switch (data[SMK_LABELLEN + SMK_LABELLEN + 1]) {
> @@ -263,7 +213,7 @@ static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> rule->smk_access |= MAY_WRITE;
> break;
> default:
> - goto out_free_rule;
> + return -1;
> }
>
> switch (data[SMK_LABELLEN + SMK_LABELLEN + 2]) {
> @@ -274,7 +224,7 @@ static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> rule->smk_access |= MAY_EXEC;
> break;
> default:
> - goto out_free_rule;
> + return -1;
> }
>
> switch (data[SMK_LABELLEN + SMK_LABELLEN + 3]) {
> @@ -285,7 +235,7 @@ static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> rule->smk_access |= MAY_APPEND;
> break;
> default:
> - goto out_free_rule;
> + return -1;
> }
>
> switch (data[SMK_LABELLEN + SMK_LABELLEN + 4]) {
> @@ -296,9 +246,74 @@ static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> rule->smk_access |= MAY_TRANSMUTE;
> break;
> default:
> - goto out_free_rule;
> + return -1;
> }
>
> + return 0;
> +}
> +
> +/**
> + * smk_write_load_list - write() for any /smack/load
> + * @file: file pointer, not actually used
> + * @buf: where to get the data from
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + * @rule_list: the list of rules to write to
> + * @rule_lock: lock for the rule list
> + *
> + * Get one smack access rule from above.
> + * The format is exactly:
> + * char subject[SMK_LABELLEN]
> + * char object[SMK_LABELLEN]
> + * char access[SMK_ACCESSLEN]
> + *
> + * writes must be SMK_LABELLEN+SMK_LABELLEN+SMK_ACCESSLEN bytes.
> + */
> +static ssize_t smk_write_load_list(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos,
> + struct list_head *rule_list,
> + struct mutex *rule_lock)
> +{
> + struct smack_rule *rule;
> + char *data;
> + int rc = -EINVAL;
> +
> + /*
> + * No partial writes.
> + * Enough data must be present.
> + */
> + if (*ppos != 0)
> + return -EINVAL;
> + /*
> + * Minor hack for backward compatibility
> + */
> + if (count < (SMK_OLOADLEN) || count > SMK_LOADLEN)
> + return -EINVAL;
> +
> + data = kzalloc(SMK_LOADLEN, GFP_KERNEL);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + if (copy_from_user(data, buf, count) != 0) {
> + rc = -EFAULT;
> + goto out;
> + }
> +
> + /*
> + * More on the minor hack for backward compatibility
> + */
> + if (count == (SMK_OLOADLEN))
> + data[SMK_OLOADLEN] = '-';
> +
> + rule = kzalloc(sizeof(*rule), GFP_KERNEL);
> + if (rule == NULL) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + if (smk_parse_rule(data, rule))
> + goto out_free_rule;
> +
> rc = count;
> /*
> * smk_set_access returns true if there was already a rule
> @@ -1425,6 +1440,44 @@ static const struct file_operations smk_load_self_ops = {
> .write = smk_write_load_self,
> .release = seq_release,
> };
> +
> +/**
> + * smk_write_access - handle access check transaction
> + * @file: file pointer
> + * @buf: data from user space
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + */
> +static ssize_t smk_write_access(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct smack_rule rule;
> + char *data;
> +
> + if (!capable(CAP_MAC_ADMIN))
> + return -EPERM;
> +
> + data = simple_transaction_get(file, buf, count);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + if (count < SMK_LOADLEN || smk_parse_rule(data, &rule))
> + return -EINVAL;
> +
> + data[0] = smk_access(rule.smk_subject, rule.smk_object,
> + rule.smk_access, NULL) == 0;
> +
> + simple_transaction_set(file, 1);
> + return SMK_LOADLEN;
> +}
> +
> +static const struct file_operations smk_access_ops = {
> + .write = smk_write_access,
> + .read = simple_transaction_read,
> + .release = simple_transaction_release,
> + .llseek = generic_file_llseek,
> +};
> +
> /**
> * smk_fill_super - fill the /smackfs superblock
> * @sb: the empty superblock
> @@ -1459,6 +1512,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
> "logging", &smk_logging_ops, S_IRUGO|S_IWUSR},
> [SMK_LOAD_SELF] = {
> "load-self", &smk_load_self_ops, S_IRUGO|S_IWUGO},
> + [SMK_ACCESSES] = {
> + "access", &smk_access_ops, S_IRUGO|S_IWUSR},
> /* last one */
> {""}
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists