Received-SPF: pass (google.com: domain of linux-nfs+bounces-3-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <884eb5167283c7ce0604b3dae9807d99b661eff8.camel@huaweicloud.com>
Subject: Re: [PATCH v6 25/25] security: Enforce ordering of 'ima' and 'evm'
 LSMs
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: Casey Schaufler <casey@schaufler-ca.com>, viro@zeniv.linux.org.uk, 
 brauner@kernel.org, chuck.lever@oracle.com, jlayton@kernel.org,
 neilb@suse.de,  kolga@netapp.com, Dai.Ngo@oracle.com, tom@talpey.com,
 paul@paul-moore.com,  jmorris@namei.org, serge@hallyn.com,
 zohar@linux.ibm.com,  dmitry.kasatkin@gmail.com, dhowells@redhat.com,
 jarkko@kernel.org,  stephen.smalley.work@gmail.com, eparis@parisplace.org,
 mic@digikod.net
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-nfs@vger.kernel.org, linux-security-module@vger.kernel.org, 
	linux-integrity@vger.kernel.org, keyrings@vger.kernel.org, 
	selinux@vger.kernel.org, Roberto Sassu <roberto.sassu@huawei.com>
Date: Tue, 21 Nov 2023 08:57:56 +0100
In-Reply-To: <24a9f95d-6a28-47d3-a0cf-48e1698e2445@schaufler-ca.com>
References: <20231120173318.1132868-1-roberto.sassu@huaweicloud.com>
	 <20231120173318.1132868-26-roberto.sassu@huaweicloud.com>
	 <24a9f95d-6a28-47d3-a0cf-48e1698e2445@schaufler-ca.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

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@huawei.com>
> >=20
> > The ordering of LSM_ORDER_LAST LSMs depends on how they are placed in t=
he
> > .lsm_info.init section of the kernel image.
> >=20
> > Without making any assumption on the LSM ordering based on how they are
> > compiled, enforce that ordering at LSM infrastructure level.
> >=20
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  security/security.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >=20
> > 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)
> >  	}
> >  }
> > =20
> > +/* Find an LSM with a given name. */
> > +static struct lsm_info __init *find_lsm(const char *name)
> > +{
> > +	struct lsm_info *lsm;
> > +
> > +	for (lsm =3D __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)
> > =20
> >  	/* LSM_ORDER_LAST is always last. */
> >  	for (lsm =3D __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;
> > +
>=20
> 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 othe=
r
> 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=20
> must_precede and must_follow lists. Maybe a must_not_combine list, too,
> although I'm hoping to make that unnecessary.=20

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.
>=20
> >  		if (lsm->order =3D=3D LSM_ORDER_LAST)
> >  			append_ordered_lsm(lsm, "   last");
> >  	}
> > =20
> > +	/* Ensure that the 'ima' and 'evm' LSMs are last and in this order. *=
/
> > +	lsm =3D find_lsm("ima");
> > +	if (lsm)
> > +		append_ordered_lsm(lsm, "   last");
> > +
> > +	lsm =3D find_lsm("evm");
> > +	if (lsm)
> > +		append_ordered_lsm(lsm, "   last");
> > +
> >  	/* Disable all LSMs not in the ordered list. */
> >  	for (lsm =3D __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> >  		if (exists_ordered_lsm(lsm))