Re: [RFC PATCH v9 05/16] ipe: add userspace interface

[Fan Wu]

On Tue, Apr 11, 2023 at 05:45:41PM -0400, Paul Moore wrote:
> On Mon, Apr 10, 2023 at 3:10???PM Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Thu, Mar 02, 2023 at 02:04:42PM -0500, Paul Moore wrote:
> > > On Mon, Jan 30, 2023 at 5:58???PM Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > From: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
> > >
> > > ...
> > >
> > > > ---
> > > >  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
> 
> ...
> 
> > > > +/**
> > > > + * 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/@textlen is mutually exclusive with @pkcs7/@pkcs7len - 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?
> >
> > Actually this is needed. Policy updates can be applied to any deployed
> > policy, which may be saved in two places: the securityfs file node
> > and the ipe_active_policy pointer. To update a policy, this function first
> > checks if the policy saved in the securityfs file node is currently active.
> > If so, it updates the ipe_active_policy pointer to point to the new policy,
> > and finally updates the policy pointer in the securityfs to the new policy.
> 
> Ah, okay.  I must have forgotten, or not realized, that multiple
> policies could be loaded and not active.
> 
> I guess this does make me wonder about keeping a non-active policy
> loaded in the kernel, what purpose does that serve?
> 

The non-active policy doesn't serve anything unless it is activated. User can
even delete a policy if that is no longer needed. Non-active is just the default
state when a new policy is loaded.

If IPE supports namespace, there is another use case where different containers
can select different policies as the active policy from among multiple loaded
policies. Deven has presented a demo of this during LSS 2021. But this goes
beyond the scope of this version.

-Fan

> -- 
> paul-moore.com