[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHC9VhRa+NwKzLfQBmHfMgUp6_d5soQG7JBq-Vn=MUeUAt4tuQ@mail.gmail.com>
Date: Thu, 2 Mar 2023 14:04:42 -0500
From: Paul Moore <paul@...l-moore.com>
To: Fan Wu <wufan@...ux.microsoft.com>
Cc: corbet@....net, zohar@...ux.ibm.com, jmorris@...ei.org,
serge@...lyn.com, tytso@....edu, ebiggers@...nel.org,
axboe@...nel.dk, agk@...hat.com, snitzer@...nel.org,
eparis@...hat.com, linux-doc@...r.kernel.org,
linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-fscrypt@...r.kernel.org, linux-block@...r.kernel.org,
dm-devel@...hat.com, linux-audit@...hat.com,
roberto.sassu@...wei.com, linux-kernel@...r.kernel.org,
Deven Bowers <deven.desai@...ux.microsoft.com>
Subject: Re: [RFC PATCH v9 05/16] ipe: add userspace interface
On Mon, Jan 30, 2023 at 5:58 PM Fan Wu <wufan@...ux.microsoft.com> wrote:
>
> From: Deven Bowers <deven.desai@...ux.microsoft.com>
>
> As is typical with LSMs, IPE uses securityfs as its interface with
> userspace. for a complete list of the interfaces and the respective
> inputs/outputs, please see the documentation under
> admin-guide/LSM/ipe.rst
>
> Signed-off-by: Deven Bowers <deven.desai@...ux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@...ux.microsoft.com>
...
> ---
> security/ipe/Makefile | 2 +
> security/ipe/fs.c | 101 +++++++++
> security/ipe/fs.h | 17 ++
> security/ipe/ipe.c | 3 +
> security/ipe/ipe.h | 2 +
> security/ipe/policy.c | 135 ++++++++++++
> security/ipe/policy.h | 7 +
> security/ipe/policy_fs.c | 459 +++++++++++++++++++++++++++++++++++++++
> 8 files changed, 726 insertions(+)
> create mode 100644 security/ipe/fs.c
> create mode 100644 security/ipe/fs.h
> create mode 100644 security/ipe/policy_fs.c
...
> diff --git a/security/ipe/policy.c b/security/ipe/policy.c
> index 772d876b1087..a5e9c6e5691b 100644
> --- a/security/ipe/policy.c
> +++ b/security/ipe/policy.c
> @@ -4,12 +4,39 @@
> */
>
> #include "ipe.h"
> +#include "eval.h"
> +#include "fs.h"
> #include "policy.h"
> #include "policy_parser.h"
> #include "digest.h"
>
> #include <linux/verification.h>
>
> +/* lock for synchronizing writers across ipe policy */
> +DEFINE_SPINLOCK(ipe_policy_lock);
> +
> +/**
> + * ver_to_u64 - Convert an internal ipe_policy_version to a u64.
> + * @p: Policy to extract the version from.
> + *
> + * Bits (LSB is index 0):
> + * [48,32] -> Major
> + * [32,16] -> Minor
> + * [16, 0] -> Revision
> + *
> + * Return: u64 version of the embedded version structure.
> + */
> +static inline u64 ver_to_u64(const struct ipe_policy *const p)
> +{
> + u64 r = 0;
No need to set @r to 0 since you set it to the version immediately below.
> + r = (((u64)p->parsed->version.major) << 32)
> + | (((u64)p->parsed->version.minor) << 16)
> + | ((u64)(p->parsed->version.rev));
> +
> + return r;
> +}
> +
> /**
> * ipe_free_policy - Deallocate a given IPE policy.
> * @p: Supplies the policy to free.
> @@ -21,6 +48,7 @@ void ipe_free_policy(struct ipe_policy *p)
> if (IS_ERR_OR_NULL(p))
> return;
>
> + ipe_del_policyfs_node(p);
> free_parsed_policy(p->parsed);
> if (!p->pkcs7)
> kfree(p->text);
> @@ -39,6 +67,70 @@ static int set_pkcs7_data(void *ctx, const void *data, size_t len,
> return 0;
> }
>
> +/**
> + * ipe_update_policy - parse a new policy and replace @old with it.
> + * @addr: Supplies a pointer to the i_private for saving policy.
> + * @text: Supplies a pointer to the plain text policy.
> + * @textlen: Supplies the length of @text.
> + * @pkcs7: Supplies a pointer to a buffer containing a pkcs7 message.
> + * @pkcs7len: Supplies the length of @pkcs7len.
> + *
> + * @text/@...tlen is mutually exclusive with @pkcs7/@...s7len - see
> + * ipe_new_policy.
> + *
> + * Return:
> + * * !IS_ERR - OK
> + * * -ENOENT - Policy doesn't exist
> + * * -EINVAL - New policy is invalid
> + */
> +struct ipe_policy *ipe_update_policy(struct ipe_policy __rcu **addr,
> + const char *text, size_t textlen,
> + const char *pkcs7, size_t pkcs7len)
> +{
> + int rc = 0;
> + struct ipe_policy *old, *new;
> +
> + old = ipe_get_policy_rcu(*addr);
> + if (!old) {
> + rc = -ENOENT;
> + goto err;
> + }
> +
> + new = ipe_new_policy(text, textlen, pkcs7, pkcs7len);
> + if (IS_ERR(new)) {
> + rc = PTR_ERR(new);
> + goto err;
> + }
> +
> + if (strcmp(new->parsed->name, old->parsed->name)) {
> + rc = -EINVAL;
> + goto err;
> + }
> +
> + if (ver_to_u64(old) > ver_to_u64(new)) {
> + rc = -EINVAL;
> + goto err;
> + }
> +
> + if (ipe_is_policy_active(old)) {
I don't understand the is-active check, you want to make @new the new
active policy regardless, right? Could this is-active check ever be
false?
> + spin_lock(&ipe_policy_lock);
> + rcu_assign_pointer(ipe_active_policy, new);
> + spin_unlock(&ipe_policy_lock);
> + synchronize_rcu();
> + }
> +
> + rcu_assign_pointer(*addr, new);
> +
> + swap(new->policyfs, old->policyfs);
> + ipe_free_policy(old);
> +
> + goto out;
> +err:
> + ipe_free_policy(new);
> +out:
> + return (rc < 0) ? ERR_PTR(rc) : new;
> +}
> +
> /**
> * ipe_new_policy - Allocate and parse an ipe_policy structure.
> *
> @@ -117,3 +209,46 @@ struct ipe_policy *ipe_get_policy_rcu(struct ipe_policy __rcu *p)
>
> return rv;
> }
> +
> +/**
> + * ipe_set_active_pol - Make @p the active policy.
> + * @p: Supplies a pointer to the policy to make active.
> + */
> +int ipe_set_active_pol(const struct ipe_policy *p)
> +{
> + int rc = 0;
> + struct ipe_policy *ap = NULL;
> +
> + ap = ipe_get_policy_rcu(ipe_active_policy);
> + if (ap && ver_to_u64(ap) > ver_to_u64(p)) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + spin_lock(&ipe_policy_lock);
> + rcu_assign_pointer(ipe_active_policy, p);
> + spin_unlock(&ipe_policy_lock);
> + synchronize_rcu();
> +
> +out:
> + return rc;
> +}
> +
> +/**
> + * ipe_is_policy_active - Determine wehther @p is the active policy.
> + * @p: Supplies a pointer to the policy to check.
> + *
> + * Return:
> + * * true - @p is the active policy
> + * * false - @p is not the active policy
> + */
> +bool ipe_is_policy_active(const struct ipe_policy *p)
> +{
> + bool rv;
> +
> + rcu_read_lock();
> + rv = rcu_access_pointer(ipe_active_policy) == p;
> + rcu_read_unlock();
> +
> + return rv;
> +}
--
paul-moore.com
Powered by blists - more mailing lists