Re: [RFC PATCH v2 02/13] ima: always create runtime_measurements sysfs file for ima_hash

[Nicolai Stange]

Mimi Zohar <zohar@xxxxxxxxxxxxx> writes:

> On Sun, 2025-03-23 at 15:09 +0100, Nicolai Stange wrote:
>> runtime_measurements_<hash-algo> sysfs files are getting created for
>> each PCR bank + for SHA-1.
>> 
>> Now that runtime_measurements_<hash-algo> sysfs file creation is being
>> skipped for unsupported hash algorithms, it will become possible that no
>> such file would be provided at all once SHA-1 is made optional in a
>> later patch.
>> 
>> Always create the file for the 'ima_hash' algorithm, even if it's not
>> associated with any of the PCR banks. As IMA initialization will
>> continue to fail if the ima_hash algorithm is not available to the
>> kernel, this guarantees that at least one such file will always be
>> there.
>> 
>> Signed-off-by: Nicolai Stange <nstange@xxxxxxx>
>> ---
>>  security/integrity/ima/ima_fs.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index a8df2fe5f4cb..f030ff7f56da 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -436,10 +436,8 @@ static int __init create_securityfs_measurement_lists(void)
>>  	u16 algo;
>>  	int i;
>>  
>> -	securityfs_measurement_list_count = NR_BANKS(ima_tpm_chip);
>> -
>> -	if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
>> -		securityfs_measurement_list_count++;
>> +	securityfs_measurement_list_count =
>> +		NR_BANKS(ima_tpm_chip) + ima_extra_slots;
>>  
>>  	ascii_securityfs_measurement_lists =
>>  	    kcalloc(securityfs_measurement_list_count, sizeof(struct dentry *),
>
> "ima_hash" is the default file hash algorithm.  Re-using it as the default
> complete measurement list assumes that the subsequent kexec'ed kernels configure
> and define it as the default file hash algorithm as well, which might not be the
> case.

I don't really see why the ima_hashes would have to match between kexecs
for this to work -- all events' template hashes are getting recreated
from scratch anyway after kexec (ima_restore_measurement_list() ->
ima_calc_field_array_hash()).

That is, if ima_hash=sha256 first, and ima_hash=sha384 after kexec, one
would have *runtime_measurements_sha256 first and
*runtime_measurements_sha384 after kexec. And both had exclusively
template hashes of their respective algo in them each.

What am I missing?


> Drop this patch.

Fine by me, but just to confirm, in case there's no TPM attached and
SHA1 was disabled, there would be no /sys/*/*runtime_measurement* at all
then. Is that Ok?

ima_hash was chosen here only, because after this series, it will be the
only single algorithm guaranteed to be available.

Thanks!

Nicolai


> Defer allocating the "extra" non-sha1 bank.  A subsequent patch will select
> SHA256.  Based on the chosen algorithm, define the "extra" non-sha1 bank.
>
-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)