| 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: <20200715191617.GD3673@sequoia>
Date: Wed, 15 Jul 2020 14:16:17 -0500
From: Tyler Hicks <tyhicks@...ux.microsoft.com>
To: deven.desai@...ux.microsoft.com
Cc: agk@...hat.com, axboe@...nel.dk, snitzer@...hat.com,
jmorris@...ei.org, serge@...lyn.com, zohar@...ux.ibm.com,
linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org, dm-devel@...hat.com,
linux-block@...r.kernel.org, jannh@...gle.com,
pasha.tatashin@...een.com, sashal@...nel.org,
jaskarankhurana@...ux.microsoft.com, nramas@...ux.microsoft.com,
mdsakib@...ux.microsoft.com, linux-kernel@...r.kernel.org,
corbet@....net
Subject: Re: [RFC PATCH v3 03/12] security: add ipe lsm policy parser and
policy loading
On 2020-04-15 09:25:41, deven.desai@...ux.microsoft.com wrote:
> From: Deven Bowers <deven.desai@...ux.microsoft.com>
>
> 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 <deven.desai@...ux.microsoft.com>
> ---
...
> 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
> + 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
Powered by blists - more mailing lists