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 <[email protected]>
Nacked-by: Casey Schaufler <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
