| 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: <505768C3.4030205@schaufler-ca.com>
Date: Mon, 17 Sep 2012 11:15:31 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Rafal Krypa <r.krypa@...sung.com>
CC: linux-security-module@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH] Smack: add support for modification of existing rules
On 9/17/2012 8:15 AM, Rafal Krypa wrote:
> Rule modifications are enabled via /smack/change-rule. Format is as follows:
> "Subject Object rwaxt rwaxt"
>
> First two strings are subject and object labels up to 255 characters.
> Third string contains permissions to enable.
> Fourth string contains permissions to disable.
>
> All unmentioned permissions will be left unchanged.
> If no rule previously existed, it will be created.
>
> Targeted for git://git.gitorious.org/smack-next/kernel.git
>
> Signed-off-by: Rafal Krypa <r.krypa@...sung.com>
Nacked-by: Casey Schaufler <casey@...aufler-ca.com>
The desired behavior of this interface is that it add any
permissions in string 3 and remove any in string 4. What it
does instead is set the permissions in string three and then
remove those in string four. If you have:
Snap Crackle rwxa
# echo 'Snap Crackle - rw' > /smack/change-rule
should result in
Snap Crackle xa
but instead gets
Snap Crackle -
> ---
> Documentation/security/Smack.txt | 11 +++
> security/smack/smackfs.c | 152 ++++++++++++++++++++++++++++----------
> 2 files changed, 122 insertions(+), 41 deletions(-)
>
> diff --git a/Documentation/security/Smack.txt b/Documentation/security/Smack.txt
> index e68536d..ea20fa2 100644
> --- a/Documentation/security/Smack.txt
> +++ b/Documentation/security/Smack.txt
> @@ -118,6 +118,17 @@ access2
> ambient
> This contains the Smack label applied to unlabeled network
> packets.
> +change-rule
> + This interface allows modification of existing access control rules.
> + The format accepted on write is:
> + "%s %s %s %s"
> + where the first string is the subject label, the second the
> + object label, the third the access to allow and the fourth the
> + access to deny. The access strings may contain only the characters
> + "rwxat-". If a rule for a given subject and object exists it will be
> + modified by enabling the permissions in the third string and disabling
> + those in the fourth string. If there is no such rule it will be
> + created using the access specified in the third and the fourth strings.
> cipso
> This interface allows a specific CIPSO header to be assigned
> to a Smack label. The format accepted on write is:
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 99929a5..dfd9d61 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -50,6 +50,7 @@ enum smk_inos {
> SMK_ACCESS2 = 16, /* make an access check with long labels */
> SMK_CIPSO2 = 17, /* load long label -> CIPSO mapping */
> SMK_REVOKE_SUBJ = 18, /* set rules with subject label to '-' */
> + SMK_CHANGE_RULE = 19, /* change or add rules (long labels) */
> };
>
> /*
> @@ -210,10 +211,51 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
> }
>
> /**
> + * smk_perm_from_str - parse smack accesses from a text string
> + * @string: a text string that contains a Smack accesses code
> + *
> + * Returns an integer with respective bits set for specified accesses.
> + */
> +static int smk_perm_from_str(const char *string)
> +{
> + int perm = 0;
> + const char *cp;
> +
> + for (cp = string; ; cp++)
> + switch (*cp) {
> + case '-':
> + break;
> + case 'r':
> + case 'R':
> + perm |= MAY_READ;
> + break;
> + case 'w':
> + case 'W':
> + perm |= MAY_WRITE;
> + break;
> + case 'x':
> + case 'X':
> + perm |= MAY_EXEC;
> + break;
> + case 'a':
> + case 'A':
> + perm |= MAY_APPEND;
> + break;
> + case 't':
> + case 'T':
> + perm |= MAY_TRANSMUTE;
> + break;
> + default:
> + return perm;
> + }
> +}
> +
> +/**
> * smk_fill_rule - Fill Smack rule from strings
> * @subject: subject label string
> * @object: object label string
> * @access: access string
> + * @access_remove: string with permissions to be removed
> * @rule: Smack rule
> * @import: if non-zero, import labels
> * @len: label length limit
> @@ -221,9 +263,10 @@ static int smk_set_access(struct smack_rule *srp, struct list_head *rule_list,
> * Returns 0 on success, -1 on failure
> */
> static int smk_fill_rule(const char *subject, const char *object,
> - const char *access, struct smack_rule *rule,
> - int import, int len)
> + const char *access, const char *access_remove,
> + struct smack_rule *rule, int import, int len)
> {
> + int rc = -1;
> const char *cp;
> struct smack_known *skp;
>
> @@ -255,36 +298,11 @@ static int smk_fill_rule(const char *subject, const char *object,
> rule->smk_object = skp->smk_known;
> }
>
> - rule->smk_access = 0;
> -
> - for (cp = access; *cp != '\0'; cp++) {
> - switch (*cp) {
> - case '-':
> - break;
> - case 'r':
> - case 'R':
> - rule->smk_access |= MAY_READ;
> - break;
> - case 'w':
> - case 'W':
> - rule->smk_access |= MAY_WRITE;
> - break;
> - case 'x':
> - case 'X':
> - rule->smk_access |= MAY_EXEC;
> - break;
> - case 'a':
> - case 'A':
> - rule->smk_access |= MAY_APPEND;
> - break;
> - case 't':
> - case 'T':
> - rule->smk_access |= MAY_TRANSMUTE;
> - break;
> - default:
> - return 0;
> - }
> - }
> + if (access_remove) {
> + rule->smk_access |= smk_perm_from_str(access);
> + rule->smk_access &= ~smk_perm_from_str(access_remove);
> + } else
> + rule->smk_access = smk_perm_from_str(access);
>
> return 0;
> }
> @@ -302,8 +320,8 @@ static int smk_parse_rule(const char *data, struct smack_rule *rule, int import)
> int rc;
>
> rc = smk_fill_rule(data, data + SMK_LABELLEN,
> - data + SMK_LABELLEN + SMK_LABELLEN, rule, import,
> - SMK_LABELLEN);
> + data + SMK_LABELLEN + SMK_LABELLEN, NULL, rule,
> + import, SMK_LABELLEN);
> return rc;
> }
>
> @@ -312,15 +330,17 @@ static int smk_parse_rule(const char *data, struct smack_rule *rule, int import)
> * @data: string to be parsed, null terminated
> * @rule: Smack rule
> * @import: if non-zero, import labels
> + * @change: if non-zero, data is from /smack/change-rule
> *
> * Returns 0 on success, -1 on failure
> */
> static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
> - int import)
> + int import, int change)
> {
> char *subject;
> char *object;
> char *access;
> + char *access_remove;
> int datalen;
> int rc = -1;
>
> @@ -337,10 +357,23 @@ static int smk_parse_long_rule(const char *data, struct smack_rule *rule,
> access = kzalloc(datalen, GFP_KERNEL);
> if (access == NULL)
> goto free_out_o;
> + access_remove = kzalloc(datalen, GFP_KERNEL);
> + if (access_remove == NULL)
> + goto free_out_a;
> +
> + if (change) {
> + if (sscanf(data, "%s %s %s %s",
> + subject, object, access, access_remove) == 4)
> + rc = smk_fill_rule(subject, object, access,
> + access_remove, rule, import, 0);
> + } else {
> + if (sscanf(data, "%s %s %s", subject, object, access) == 3)
> + rc = smk_fill_rule(subject, object, access,
> + NULL, rule, import, 0);
> + }
>
> - if (sscanf(data, "%s %s %s", subject, object, access) == 3)
> - rc = smk_fill_rule(subject, object, access, rule, import, 0);
> -
> + kfree(access_remove);
> +free_out_a:
> kfree(access);
> free_out_o:
> kfree(object);
> @@ -351,6 +384,7 @@ free_out_s:
>
> #define SMK_FIXED24_FMT 0 /* Fixed 24byte label format */
> #define SMK_LONG_FMT 1 /* Variable long label format */
> +#define SMK_CHANGE_FMT 2 /* Rule modification format */
> /**
> * smk_write_rules_list - write() for any /smack rule file
> * @file: file pointer, not actually used
> @@ -359,13 +393,16 @@ free_out_s:
> * @ppos: where to start - must be 0
> * @rule_list: the list of rules to write to
> * @rule_lock: lock for the rule list
> - * @format: /smack/load or /smack/load2 format.
> + * @format: /smack/load or /smack/load2 or /smack/change-rule format.
> *
> * Get one smack access rule from above.
> * The format for SMK_LONG_FMT is:
> * "subject<whitespace>object<whitespace>access[<whitespace>...]"
> * The format for SMK_FIXED24_FMT is exactly:
> * "subject object rwxat"
> + * The format for SMK_CHANGE_FMT is:
> + * "subject<whitespace>object<whitespace>
> + * acc_enable<whitespace>acc_disable[<whitespace>...]"
> */
> static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
> size_t count, loff_t *ppos,
> @@ -417,7 +454,11 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
> * Be sure the data string is terminated.
> */
> data[count] = '\0';
> - if (smk_parse_long_rule(data, rule, 1))
> + if (smk_parse_long_rule(data, rule, 1, 0))
> + goto out_free_rule;
> + } else if (format == SMK_CHANGE_FMT) {
> + data[count] = '\0';
> + if (smk_parse_long_rule(data, rule, 1, 1))
> goto out_free_rule;
> } else {
> /*
> @@ -1796,7 +1837,7 @@ static ssize_t smk_user_access(struct file *file, const char __user *buf,
> return -ENOMEM;
> memcpy(cod, data, count);
> cod[count] = '\0';
> - res = smk_parse_long_rule(cod, &rule, 0);
> + res = smk_parse_long_rule(cod, &rule, 0, 0);
> kfree(cod);
> }
>
> @@ -2064,6 +2105,33 @@ static const struct file_operations smk_revoke_subj_ops = {
> };
>
> /**
> + * smk_write_change_rule - write() for /smack/change-rule
> + * @file: file pointer
> + * @buf: data from user space
> + * @count: bytes sent
> + * @ppos: where to start - must be 0
> + */
> +static ssize_t smk_write_change_rule(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + /*
> + * Must have privilege.
> + */
> + if (!capable(CAP_MAC_ADMIN))
> + return -EPERM;
> +
> + return smk_write_rules_list(file, buf, count, ppos, NULL, NULL,
> + SMK_CHANGE_FMT);
> +}
> +
> +static const struct file_operations smk_change_rule_ops = {
> + .write = smk_write_change_rule,
> + .read = simple_transaction_read,
> + .release = simple_transaction_release,
> + .llseek = generic_file_llseek,
> +};
> +
> +/**
> * smk_fill_super - fill the /smackfs superblock
> * @sb: the empty superblock
> * @data: unused
> @@ -2112,6 +2180,8 @@ static int smk_fill_super(struct super_block *sb, void *data, int silent)
> [SMK_REVOKE_SUBJ] = {
> "revoke-subject", &smk_revoke_subj_ops,
> S_IRUGO|S_IWUSR},
> + [SMK_CHANGE_RULE] = {
> + "change-rule", &smk_change_rule_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