[PATCH v2] ipe: allow secondary and platform keyrings to install/update policies

[Luca Boccassi]

On Fri, 20 Sept 2024 at 04:02, Serge E. Hallyn <serge at hallyn.com> wrote:
>
> On Sun, Sep 15, 2024 at 11:11:19AM +0200, luca.boccassi at gmail.com wrote:
> > From: Luca Boccassi <bluca at debian.org>
> >
> > The current policy management makes it impossible to use IPE
> > in a general purpose distribution. In such cases the users are not
> > building the kernel, the distribution is, and access to the private
> > key included in the trusted keyring is, for obvious reason, not
> > available.
> > This means that users have no way to enable IPE, since there will
> > be no built-in generic policy, and no access to the key to sign
> > updates validated by the trusted keyring.
> >
> > Just as we do for dm-verity, kernel modules and more, allow the
> > secondary and platform keyrings to also validate policies. This
> > allows users enrolling their own keys in UEFI db or MOK to also
> > sign policies, and enroll them. This makes it sensible to enable
> > IPE in general purpose distributions, as it becomes usable by
> > any user wishing to do so. Keys in these keyrings can already
> > load kernels and kernel modules, so there is no security
> > downgrade.
> >
> > Add a kconfig each, like dm-verity does, but default to enabled if
> > the dependencies are available.
> >
> > Signed-off-by: Luca Boccassi <bluca at debian.org>
> > ---
> > v2: add Kconfig entries following the dm-verity model
> >     update documentation
> >
> >  Documentation/admin-guide/LSM/ipe.rst |  5 ++++-
> >  security/ipe/Kconfig                  | 19 +++++++++++++++++++
> >  security/ipe/policy.c                 | 14 +++++++++++++-
> >  3 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/ipe/policy.c b/security/ipe/policy.c
> > index d8e7db857a2e..bf5aa97911e1 100644
> > --- a/security/ipe/policy.c
> > +++ b/security/ipe/policy.c
> > @@ -169,9 +169,21 @@ struct ipe_policy *ipe_new_policy(const char *text, size_t textlen,
> >                       goto err;
> >               }
> >
> > -             rc = verify_pkcs7_signature(NULL, 0, new->pkcs7, pkcs7len, NULL,
> > +             rc = verify_pkcs7_signature(NULL, 0, new->pkcs7, pkcs7len,
> > +#ifdef CONFIG_IPE_POLICY_SIG_SECONDARY_KEYRING
> > +                                         VERIFY_USE_SECONDARY_KEYRING,
> > +#else
> > +                                         NULL,
> > +#endif
> >                                           VERIFYING_UNSPECIFIED_SIGNATURE,
> >                                           set_pkcs7_data, new);
> > +#ifdef CONFIG_IPE_POLICY_SIG_PLATFORM_KEYRING
> > +             if (rc == -ENOKEY)
>
> If the secondary key *is* there, but returns -EKEYREJECTED,
> do you want to fall back to trying the platform keyring, or not?

I like the idea in principle, however for ease of use personally I'd
prefer if the behaviour was the same as dm-verity, given the close
relationship - maybe we can start with this version, then I can
propose the same change in a single series for both components later,
so that we either change it in both or neither. Would that work?