On 2020-04-15 09:25:41, [email protected] wrote:
> From: Deven Bowers <[email protected]>
> 
> 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 <[email protected]>
> ---

...

> 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

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to