Re: [PATCH] ima: remove template "ima" as the compiled default

[Guozihua (Scott)]

On 2022/4/5 4:22, Mimi Zohar wrote:
> Hi Guo,
>
> The Subject line above sounds like the default template is currently
> "ima", which it isn't.   Perhaps "ima: remove the IMA_TEMPLATE Kconfig
> option" is more accurate.
>
> On Mon, 2022-03-21 at 15:47 +0800, GUO Zihua wrote:
> > Template "ima" is a legacy template which limits the hash algorithm to
> > either sha1 or md5. None of them should be considered "strong" these
> > days. Besides, allowing template "ima" as the compiled default would
> > also cause the following issue: the cmdline option "ima_hash=" must be
> > behind "ima_template=", otherwise "ima_hash=" might be rejected.
>
> True "ima" is a legacy template, but the purpose of removing the
> IMA_TEMPLATE from the Kconfig is to address the remaining boot command
> line ordering issue not previously addressed.  This is reasonable
> because the "ima" template is limited to SHA1 and MD5.  If someone
> still needs to use the "ima" template, "ima_template=ima" could still
> be specified on the boot command line.
>
> > The root cause of this issue is that during the processing of ima_hash,
> > we would try to check whether the hash algorithm is compatible with the
> > template. If the template is not set at the moment we do the check, we
> > check the algorithm against the compiled default template. If the
> > complied default template is "ima", then we reject any hash algorithm
> > other than sha1 and md5.
> >
> > For example, if the compiled default template is "ima", and the default
> > algorithm is sha1 (which is the current default). In the cmdline, we put
> > in "ima_hash=sha256 ima_template=ima-ng". The expected behavior would be
> > that ima starts with ima-ng as the template and sha256 as the hash
> > algorithm. However, during the processing of "ima_hash=",
> > "ima_template=" has not been processed yet, and hash_setup would check
> > the configured hash algorithm against the compiled default: ima, and
> > reject sha256. So at the end, the hash algorithm that is actually used
> > will be sha1.
> >
> > With template "ima" removed from the compiled default, we ensure that the
> > default tempalte would at least be "ima-ng" which allows for basically
> > any hash algorithm.
> >
> > This change would not break the algorithm compatibility checking for
> > IMA.
> >
> > Fixes: 4286587dccd43 ("ima: add Kconfig default measurement list template")
> > Signed-off-by: GUO Zihua <guozihua@xxxxxxxxxx>
> > ---
> >   security/integrity/ima/Kconfig | 14 +++++---------
> >   1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index f3a9cc201c8c..9513df2ac19e 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -65,14 +65,11 @@ choice
> >   	help
> >   	  Select the default IMA measurement template.
> >   
> > -	  The original 'ima' measurement list template contains a
> > -	  hash, defined as 20 bytes, and a null terminated pathname,
> > -	  limited to 255 characters.  The 'ima-ng' measurement list
> > -	  template permits both larger hash digests and longer
> > -	  pathnames.
> > -
> > -	config IMA_TEMPLATE
> > -		bool "ima"
> > +	  The 'ima-ng' measurement list template permits various hash
> > +	  digests and long pathnames. The compiled default template
> > +	  can be overwritten using the kernel command line
> > +	  'ima_template=' option.
> > +
> >   	config IMA_NG_TEMPLATE
> >   		bool "ima-ng (default)"
> >   	config IMA_SIG_TEMPLATE
> > @@ -82,7 +79,6 @@ endchoice
> >   config IMA_DEFAULT_TEMPLATE
> >   	string
> >   	depends on IMA
> > -	default "ima" if IMA_TEMPLATE
> >   	default "ima-ng" if IMA_NG_TEMPLATE
> >   	default "ima-sig" if IMA_SIG_TEMPLATE
> >   
>
> The IMA_TEMPLATE definition is removed, but leaves a few references to
> it.

Thank you very much for the detailed review Mimi! I'll fix these right now.

--
Best
GUO Zihua