Re: [RFC PATCH v3 03/12] security: add ipe lsm policy parser and policy loading
From: Tyler Hicks <hidden>
Date: 2020-07-15 19:17:12
Also in:
dm-devel, linux-block, linux-integrity, lkml
On 2020-04-15 09:25:41, deven.desai@linux.microsoft.com wrote:
From: Deven Bowers <redacted> Adds the policy parser and the policy loading to IPE, along with the related sysfs, securityfs entries, and audit events. Signed-off-by: Deven Bowers <redacted> ---
...
quoted hunk ↗ jump to hunk
diff --git a/security/ipe/ipe-sysfs.c b/security/ipe/ipe-sysfs.c index 1c65185c531d..a250da29c3b5 100644 --- a/security/ipe/ipe-sysfs.c +++ b/security/ipe/ipe-sysfs.c@@ -5,6 +5,7 @@ #include "ipe.h" #include "ipe-audit.h" +#include "ipe-secfs.h" #include <linux/sysctl.h> #include <linux/fs.h>@@ -45,6 +46,79 @@ static int ipe_switch_mode(struct ctl_table *table, int write, #endif /* CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH */ +#ifdef CONFIG_SECURITYFS + +/** + * ipe_switch_active_policy: Handler to switch the policy IPE is enforcing. + * @table: Sysctl table entry from the variable, sysctl_table. + * @write: Integer indicating whether this is a write or a read. + * @buffer: Data passed to sysctl. This is the policy id to activate, + * for this function. + * @lenp: Pointer to the size of @buffer. + * @ppos: Offset into @buffer. + * + * This wraps proc_dointvec_minmax, and if there's a change, emits an + * audit event. + * + * Return: + * 0 - OK + * -ENOMEM - Out of memory + * -ENOENT - Policy identified by @id does not exist + * Other - See proc_dostring and retrieve_backed_dentry + */ +static int ipe_switch_active_policy(struct ctl_table *table, int write, + void __user *buffer, size_t *lenp, + loff_t *ppos) +{ + int rc = 0; + char *id = NULL; + size_t size = 0; + + if (write) {
I see that the policy files in securityfs, such as new_policy, are checking for CAP_MAC_ADMIN but there's no check here for CAP_MAC_ADMIN when switching the active policy. I think we should enforce that cap here, too. Thinking about it some more, I find it a little odd that we're spreading the files necessary to update a policy across both procfs (sysctl) and securityfs. It looks like procfs will have different semantics than securityfs after looking at proc_sys_permission(). I suggest moving strict_parse and active_policy under securityfs for a unified experience and common location when updating policy. Tyler
quoted hunk ↗ jump to hunk
+ id = kzalloc((*lenp) + 1, GFP_KERNEL); + if (!id) + return -ENOMEM; + + table->data = id; + table->maxlen = (*lenp) + 1; + + rc = proc_dostring(table, write, buffer, lenp, ppos); + if (rc != 0) + goto out; + + rc = ipe_set_active_policy(id, strlen(id)); + } else { + if (!rcu_access_pointer(ipe_active_policy)) { + table->data = ""; + table->maxlen = 1; + return proc_dostring(table, write, buffer, lenp, ppos); + } + + rcu_read_lock(); + size = strlen(rcu_dereference(ipe_active_policy)->policy_name); + rcu_read_unlock(); + + id = kzalloc(size + 1, GFP_KERNEL); + if (!id) + return -ENOMEM; + + rcu_read_lock(); + strncpy(id, rcu_dereference(ipe_active_policy)->policy_name, + size); + rcu_read_unlock(); + + table->data = id; + table->maxlen = size; + + rc = proc_dostring(table, write, buffer, lenp, ppos); + } +out: + kfree(id); + return rc; +} + +#endif /* CONFIG_SECURITYFS */ + static struct ctl_table_header *sysctl_header; static const struct ctl_path sysctl_path[] = {@@ -75,6 +149,24 @@ static struct ctl_table sysctl_table[] = { .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_ONE, }, +#ifdef CONFIG_SECURITYFS + { + .procname = "strict_parse", + .data = &ipe_strict_parse, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, + }, + { + .procname = "active_policy", + .data = NULL, + .maxlen = 0, + .mode = 0644, + .proc_handler = ipe_switch_active_policy, + }, +#endif /* CONFIG_SECURITYFS */ {} };diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c index b6553e370f98..07f855ffb79a 100644 --- a/security/ipe/ipe.c +++ b/security/ipe/ipe.c@@ -6,6 +6,7 @@ #include "ipe.h" #include "ipe-policy.h" #include "ipe-hooks.h" +#include "ipe-secfs.h" #include "ipe-sysfs.h" #include <linux/module.h>@@ -60,3 +61,4 @@ DEFINE_LSM(ipe) = { int ipe_enforce = 1; int ipe_success_audit; +int ipe_strict_parse;diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h index 6a47f55b05d9..bf6cf7744b0e 100644 --- a/security/ipe/ipe.h +++ b/security/ipe/ipe.h@@ -16,5 +16,6 @@ extern int ipe_enforce; extern int ipe_success_audit; +extern int ipe_strict_parse; #endif /* IPE_H */-- 2.26.0