Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1675119451-23180-1-git-send-email-wufan@linux.microsoft.com>
 <1675119451-23180-6-git-send-email-wufan@linux.microsoft.com>
 <CAHC9VhRa+NwKzLfQBmHfMgUp6_d5soQG7JBq-Vn=MUeUAt4tuQ@mail.gmail.com> <20230410191035.GB18827@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
In-Reply-To: <20230410191035.GB18827@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
From:   Paul Moore <paul@paul-moore.com>
Date:   Tue, 11 Apr 2023 17:45:41 -0400
Message-ID: <CAHC9VhQDvWDshaZvJrHmjcwyHFxv9oYTN9bn0xiTtFZQRp+GPg@mail.gmail.com>
Subject: Re: [RFC PATCH v9 05/16] ipe: add userspace interface
To:     Fan Wu <wufan@linux.microsoft.com>
Cc:     corbet@lwn.net, zohar@linux.ibm.com, jmorris@namei.org,
        serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
        axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
        eparis@redhat.com, linux-doc@vger.kernel.org,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
        dm-devel@redhat.com, linux-audit@redhat.com,
        roberto.sassu@huawei.com, linux-kernel@vger.kernel.org,
        Deven Bowers <deven.desai@linux.microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 10, 2023 at 3:10=E2=80=AFPM Fan Wu <wufan@linux.microsoft.com> =
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@linux.microsoft.com> wr=
ote:
> > >
> > > From: Deven Bowers <deven.desai@linux.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@linux.microsoft.com>
> > > Signed-off-by: Fan Wu <wufan@linux.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

...

> > > +/**
> > > + * 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 pkcs7l=
en)
> > > +{
> > > +       int rc =3D 0;
> > > +       struct ipe_policy *old, *new;
> > > +
> > > +       old =3D ipe_get_policy_rcu(*addr);
> > > +       if (!old) {
> > > +               rc =3D -ENOENT;
> > > +               goto err;
> > > +       }
> > > +
> > > +       new =3D ipe_new_policy(text, textlen, pkcs7, pkcs7len);
> > > +       if (IS_ERR(new)) {
> > > +               rc =3D PTR_ERR(new);
> > > +               goto err;
> > > +       }
> > > +
> > > +       if (strcmp(new->parsed->name, old->parsed->name)) {
> > > +               rc =3D -EINVAL;
> > > +               goto err;
> > > +       }
> > > +
> > > +       if (ver_to_u64(old) > ver_to_u64(new)) {
> > > +               rc =3D -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 firs=
t
> checks if the policy saved in the securityfs file node is currently activ=
e.
> If so, it updates the ipe_active_policy pointer to point to the new polic=
y,
> and finally updates the policy pointer in the securityfs to the new polic=
y.

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?

--=20
paul-moore.com