On Mon, 2023-11-20 at 16:50 -0800, Casey Schaufler wrote:
> On 11/20/2023 9:33 AM, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> >
> > The ordering of LSM_ORDER_LAST LSMs depends on how they are placed in the
> > .lsm_info.init section of the kernel image.
> >
> > Without making any assumption on the LSM ordering based on how they are
> > compiled, enforce that ordering at LSM infrastructure level.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > ---
> > security/security.c | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/security/security.c b/security/security.c
> > index 351a124b771c..b98db79ca500 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -263,6 +263,18 @@ static void __init initialize_lsm(struct lsm_info *lsm)
> > }
> > }
> >
> > +/* Find an LSM with a given name. */
> > +static struct lsm_info __init *find_lsm(const char *name)
> > +{
> > + struct lsm_info *lsm;
> > +
> > + for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++)
> > + if (!strcmp(lsm->name, name))
> > + return lsm;
> > +
> > + return NULL;
> > +}
> > +
> > /*
> > * Current index to use while initializing the lsm id list.
> > */
> > @@ -333,10 +345,23 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> >
> > /* LSM_ORDER_LAST is always last. */
> > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> > + /* Do it later, to enforce the expected ordering. */
> > + if (!strcmp(lsm->name, "ima") || !strcmp(lsm->name, "evm"))
> > + continue;
> > +
>
> Hard coding the ordering of LSMs is incredibly ugly and unlikely to scale.
> Not to mention perplexing the next time someone creates an LSM that "has to be last".
Uhm, yes, not the best solution.
> Why isn't LSM_ORDER_LAST sufficient? If it really isn't, how about adding
> and using LSM_ORDER_LAST_I_REALLY_MEAN_IT* ?
I don't know if the order at run-time reflects the order in the
Makefile (EVM is compiled after IMA). If it does, there is no need for
this patch.
> Alternatively, a declaration of ordering requirements with regard to other
> LSMs in lsm_info. You probably don't care where ima is relative to Yama,
> but you need to be after SELinux and before evm. lsm_info could have
> must_precede and must_follow lists. Maybe a must_not_combine list, too,
> although I'm hoping to make that unnecessary.
Uhm, I agree. Will think about how to make it more straightforward.
> And you should be using LSM_ID values instead of LSM names.
Ok.
Thanks
Roberto
> ---
> * Naming subject to Paul's sensibilities, of course.
>
> > if (lsm->order == LSM_ORDER_LAST)
> > append_ordered_lsm(lsm, " last");
> > }
> >
> > + /* Ensure that the 'ima' and 'evm' LSMs are last and in this order. */
> > + lsm = find_lsm("ima");
> > + if (lsm)
> > + append_ordered_lsm(lsm, " last");
> > +
> > + lsm = find_lsm("evm");
> > + if (lsm)
> > + append_ordered_lsm(lsm, " last");
> > +
> > /* Disable all LSMs not in the ordered list. */
> > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> > if (exists_ordered_lsm(lsm))
