Re: [PATCH v2] ima: remove the IMA_TEMPLATE Kconfig option

[Mimi Zohar]

On Wed, 2022-04-06 at 14:16 +0800, GUO Zihua wrote:
> It is discovered thatO allowing template "ima" as the compiled default
> would cause the following issue: the boot command line option
> "ima_hash=" must be behind "ima_template=", otherwise "ima_hash=" might
> be rejected.

The format of a proper patch description describes the current status,
provides a succinct problem description, followed by the solution.

The original 'ima' measurement list template contains a hash, defined
as 20 bytes, and a null terminated pathname, limited to 255
characters.  Other measurement list templates permit both larger hashes
and longer pathnames.  When the "ima" template is configured as the
default, a new measurement list template (ima_template=) must be
specified before specifying a larger hash algorithm (ima_hash=) on the
boot command line.

To avoid this boot command line ordering issue, remove the legacy "ima"
template configuration option, allowing it to 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. Users who needs to use "ima" template
> could still do it by specifying "ima_template=ima" in boot command line.
> 
> 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 | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index f3a9cc201c8c..f392cac7a7d1 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.
> +

Other than perhaps changing "contains" to "contained", there's no
reason for changing the text.  Adding an additional line is fine - The
configured default template can be replaced by specifying
"ima_template="  on the boot command line.

>  	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
>  
> @@ -102,19 +98,19 @@ choice
>  
>  	config IMA_DEFAULT_HASH_SHA256
>  		bool "SHA256"
> -		depends on CRYPTO_SHA256=y && !IMA_TEMPLATE
> +		depends on CRYPTO_SHA256=y
>  
>  	config IMA_DEFAULT_HASH_SHA512
>  		bool "SHA512"
> -		depends on CRYPTO_SHA512=y && !IMA_TEMPLATE
> +		depends on CRYPTO_SHA512=y
>  
>  	config IMA_DEFAULT_HASH_WP512
>  		bool "WP512"
> -		depends on CRYPTO_WP512=y && !IMA_TEMPLATE
> +		depends on CRYPTO_WP512=y
>  
>  	config IMA_DEFAULT_HASH_SM3
>  		bool "SM3"
> -		depends on CRYPTO_SM3=y && !IMA_TEMPLATE
> +		depends on CRYPTO_SM3=y
>  endchoice
>  
>  config IMA_DEFAULT_HASH