linux-kernel - Re: [PATCH v8 3/7] ima: kexec: skip IMA segment validation after kexec soft reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8814f0f-3613-4efd-afb2-655a5c0c9f38@linux.microsoft.com>
Date: Fri, 21 Feb 2025 13:06:37 -0800
From: steven chen <chenste@...ux.microsoft.com>
To: Mimi Zohar <zohar@...ux.ibm.com>, stefanb@...ux.ibm.com,
 roberto.sassu@...weicloud.com, roberto.sassu@...wei.com,
 eric.snowberg@...cle.com, ebiederm@...ssion.com, paul@...l-moore.com,
 code@...icks.com, bauermann@...abnow.com, linux-integrity@...r.kernel.org,
 kexec@...ts.infradead.org, linux-security-module@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: madvenka@...ux.microsoft.com, nramas@...ux.microsoft.com,
 James.Bottomley@...senPartnership.com, bhe@...hat.com, vgoyal@...hat.com,
 dyoung@...hat.com
Subject: Re: [PATCH v8 3/7] ima: kexec: skip IMA segment validation after
 kexec soft reboot

On 2/21/2025 7:41 AM, Mimi Zohar wrote:
> On Tue, 2025-02-18 at 14:54 -0800, steven chen wrote:
>> kexec_calculate_store_digests() calculates and stores the digest of the
>> segment at kexec_file_load syscall where the IMA segment is also
>> allocated.  With this series, the IMA segment will be updated with the
>> measurement log at kexec execute stage when soft reboot is initiated.
>> Therefore, it may fail digest verification in verify_sha256_digest()
>> after kexec soft reboot into the new kernel. Therefore, the digest
>> calculation/verification of the IMA segment needs to be skipped.
>>
>> Skip the calculating and storing digest of the IMA segment in
>> kexec_calculate_store_digests() so that it is not added to the
>> 'purgatory_sha_regions'.
>>
>> Since verify_sha256_digest() only verifies 'purgatory_sha_regions',
>> no change is needed in verify_sha256_digest() in this context.
>>
>> With this change, the IMA segment is not included in the digest
>> calculation, storage, and verification.
> Basically you're saying because the hash verification will fail, don't include
> the IMA buffer.  What's missing is the reason for not caring whether the IMA
> hash is included or not.
>
> I understand this is the best we can do without making some major kexec changes.
>
>> Signed-off-by: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
>> Signed-off-by: steven chen <chenste@...ux.microsoft.com>
>> Reviewed-by: Stefan Berger <stefanb@...ux.ibm.com>
> After updating the patch description,
>
> Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>
>
>> ---
>>   include/linux/kexec.h              |  3 +++
>>   kernel/kexec_file.c                | 22 ++++++++++++++++++++++
>>   security/integrity/ima/ima_kexec.c |  3 +++
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index 4dbf806bccef..bd554ced9fb2 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -362,6 +362,9 @@ struct kimage {
>>   
>>   	phys_addr_t ima_buffer_addr;
>>   	size_t ima_buffer_size;
>> +
>> +	unsigned long ima_segment_index;
>> +	bool is_ima_segment_index_set;
>>   #endif
>>   
>>   	/* Core ELF header buffer */
>> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
>> index 3eedb8c226ad..606132253c79 100644
>> --- a/kernel/kexec_file.c
>> +++ b/kernel/kexec_file.c
>> @@ -38,6 +38,21 @@ void set_kexec_sig_enforced(void)
>>   }
>>   #endif
>>   
>> +#ifdef CONFIG_IMA_KEXEC
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> +	if (image->is_ima_segment_index_set && i == image->ima_segment_index)
>> +		return true;
>> +	else
>> +		return false;
>> +}
>> +#else
>> +static bool check_ima_segment_index(struct kimage *image, int i)
>> +{
>> +	return false;
>> +}
>> +#endif
>> +
>>   static int kexec_calculate_store_digests(struct kimage *image);
>>   
>>   /* Maximum size in bytes for kernel/initrd files. */
>> @@ -764,6 +779,13 @@ static int kexec_calculate_store_digests(struct kimage
>> *image)
>>   		if (ksegment->kbuf == pi->purgatory_buf)
>>   			continue;
>>   
>> +		/*
>> +		 * Skip the segment if ima_segment_index is set and matches
>> +		 * the current index
>> +		 */
>> +		if (check_ima_segment_index(image, i))
>> +			continue;
>> +
>>   		ret = crypto_shash_update(desc, ksegment->kbuf,
>>   					  ksegment->bufsz);
>>   		if (ret)
>> diff --git a/security/integrity/ima/ima_kexec.c
>> b/security/integrity/ima/ima_kexec.c
>> index 89088f1fa989..704676fa6615 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -160,6 +160,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   	kbuf.buffer = kexec_buffer;
>>   	kbuf.bufsz = kexec_buffer_size;
>>   	kbuf.memsz = kexec_segment_size;
>> +	image->is_ima_segment_index_set = false;
>>   	ret = kexec_add_buffer(&kbuf);
>>   	if (ret) {
>>   		pr_err("Error passing over kexec measurement buffer.\n");
>> @@ -170,6 +171,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   	image->ima_buffer_addr = kbuf.mem;
>>   	image->ima_buffer_size = kexec_segment_size;
>>   	image->ima_buffer = kexec_buffer;
>> +	image->ima_segment_index = image->nr_segments - 1;
>> +	image->is_ima_segment_index_set = true;
>>   
>>   	/*
>>   	 * kexec owns kexec_buffer after kexec_add_buffer() is called

Thanks, Mimi. I will update in next version.