diff mbox series

ima: remove template "ima" as the compiled default

Message ID 20220321074737.138002-1-guozihua@huawei.com (mailing list archive)
State New, archived
Headers show
Series ima: remove template "ima" as the compiled default | expand

Commit Message

Guozihua (Scott) March 21, 2022, 7:47 a.m. UTC
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.

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@huawei.com>
---
 security/integrity/ima/Kconfig | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

Guozihua (Scott) March 24, 2022, 1:20 a.m. UTC | #1
On 2022/3/21 15:47, 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.
> 
> 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@huawei.com>
> ---
>   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
>   

Hi all,

Any thought on this patch?

Thanks!
Mimi Zohar April 4, 2022, 8:22 p.m. UTC | #2
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@huawei.com>
> ---
>  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.
Guozihua (Scott) April 6, 2022, 1:48 a.m. UTC | #3
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@huawei.com>
>> ---
>>   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.
diff mbox series

Patch

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