diff mbox series

[v2,7/7] ima: record log size at kexec load and execute

Message ID 20231005182602.634615-8-tusharsu@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series ima: kexec: measure events between kexec load and execute | expand

Commit Message

Tushar Sugandhi Oct. 5, 2023, 6:26 p.m. UTC
The window between kexec 'load' and 'execute' could be arbitrarily long.
Even with the large chunk of memory allocated at kexec 'load', it may
run out which would result in missing events in IMA log after the system
soft reboots to the new Kernel.  This would result in IMA measurements
getting out of sync with the TPM PCR quotes which would result in remote
attestation failing for that system.  There is currently no way for the
new Kernel to know if the IMA log TPM PCR quote out of sync problem is
because of the missing measurements during kexec.

Define two new IMA events, 'kexec_load' and 'kexec_execute', to be 
measured at kexec 'load' and 'execute' respectively.

IMA measures 'boot_aggregate' as the first event when the system boots - 
either cold boot or kexec soft boot.  In case when the system goes
through multiple soft reboots, the number of 'boot_aggregate' events in
IMA log corresponds to total number of boots (cold boot plus multiple
kexec soft reboots).  With this change, there would be additional 
'kexec_load' and 'kexec_execute' events in between the two 
'boot_aggregate' events.  In rare cases, when the system runs out of
memory during kexec soft reboot, 'kexec_execute' won't be copied since
its one of the very last event measured just before kexec soft reboot.
The absence of the event 'kexec_execute' in between the two 
boot_aggregate' events would signal the attestation service that the IMA
log on the system is out of sync with TPM PCR quotes and the system needs
to be cold booted for the remote attestation to succeed again.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 security/integrity/ima/ima_kexec.c | 35 +++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

Comments

Stefan Berger Oct. 13, 2023, 12:27 a.m. UTC | #1
On 10/5/23 14:26, Tushar Sugandhi wrote:
> The window between kexec 'load' and 'execute' could be arbitrarily long.
> Even with the large chunk of memory allocated at kexec 'load', it may
> run out which would result in missing events in IMA log after the system
> soft reboots to the new Kernel.  This would result in IMA measurements
> getting out of sync with the TPM PCR quotes which would result in remote
> attestation failing for that system.  There is currently no way for the
> new Kernel to know if the IMA log TPM PCR quote out of sync problem is
> because of the missing measurements during kexec.
>
> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
> measured at kexec 'load' and 'execute' respectively.
>
> IMA measures 'boot_aggregate' as the first event when the system boots -
> either cold boot or kexec soft boot.  In case when the system goes
> through multiple soft reboots, the number of 'boot_aggregate' events in
> IMA log corresponds to total number of boots (cold boot plus multiple
> kexec soft reboots).  With this change, there would be additional
> 'kexec_load' and 'kexec_execute' events in between the two
> 'boot_aggregate' events.  In rare cases, when the system runs out of
> memory during kexec soft reboot, 'kexec_execute' won't be copied since
> its one of the very last event measured just before kexec soft reboot.
> The absence of the event 'kexec_execute' in between the two
> boot_aggregate' events would signal the attestation service that the IMA
> log on the system is out of sync with TPM PCR quotes and the system needs
> to be cold booted for the remote attestation to succeed again.
>
>
> @@ -198,6 +208,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>   static int ima_update_kexec_buffer(struct notifier_block *self,
>   				   unsigned long action, void *data)
>   {
> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>   	void *buf = NULL;
>   	size_t buf_size;
>   	bool resume = false;
> @@ -213,9 +224,31 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
>   		return NOTIFY_OK;
>   	}
>   
> +	buf_size = ima_get_binary_runtime_size();
> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
> +		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;",
> +		  kexec_segment_size, buf_size);
> +
> +	/*
> +	 * This is one of the very last events measured by IMA before kexec
> +	 * soft rebooting into the new Kernal.
> +	 * This event can be used as a marker after the system soft reboots
> +	 * to the new Kernel to check if there was sufficient memory allocated
> +	 * at kexec 'load' to capture the events measured between the window
> +	 * of kexec 'load' and 'execute'.
> +	 * This event needs to be present in the IMA log, in between the two
> +	 * 'boot_aggregate' events that are logged for the previous boot and
> +	 * the current soft reboot. If it is not present after the system soft
> +	 * reboots into the new Kernel, it would mean the IMA log is not
> +	 * consistent with the TPM PCR quotes, and the system needs to be
> +	 * cold-booted for the attestation to succeed again.
> +	 */
> +	ima_measure_critical_data("ima_kexec", "kexec_execute",
> +				  ima_kexec_event, strlen(ima_kexec_event),
> +				  false, NULL, 0);
> +
>   	ima_measurements_suspend();
>   
> -	buf_size = ima_get_binary_runtime_size();

This should be removed earlier, in 2/7.



>   	ret = ima_dump_measurement_list(&buf_size, &buf,
>   					kexec_segment_size);
>
Tushar Sugandhi Oct. 20, 2023, 8:40 p.m. UTC | #2
On 10/12/23 17:27, Stefan Berger wrote:
> 
> On 10/5/23 14:26, Tushar Sugandhi wrote:
>> The window between kexec 'load' and 'execute' could be arbitrarily long.
>> Even with the large chunk of memory allocated at kexec 'load', it may
>> run out which would result in missing events in IMA log after the system
>> soft reboots to the new Kernel.  This would result in IMA measurements
>> getting out of sync with the TPM PCR quotes which would result in remote
>> attestation failing for that system.  There is currently no way for the
>> new Kernel to know if the IMA log TPM PCR quote out of sync problem is
>> because of the missing measurements during kexec.
>>
>> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
>> measured at kexec 'load' and 'execute' respectively.
>>
>> IMA measures 'boot_aggregate' as the first event when the system boots -
>> either cold boot or kexec soft boot.  In case when the system goes
>> through multiple soft reboots, the number of 'boot_aggregate' events in
>> IMA log corresponds to total number of boots (cold boot plus multiple
>> kexec soft reboots).  With this change, there would be additional
>> 'kexec_load' and 'kexec_execute' events in between the two
>> 'boot_aggregate' events.  In rare cases, when the system runs out of
>> memory during kexec soft reboot, 'kexec_execute' won't be copied since
>> its one of the very last event measured just before kexec soft reboot.
>> The absence of the event 'kexec_execute' in between the two
>> boot_aggregate' events would signal the attestation service that the IMA
>> log on the system is out of sync with TPM PCR quotes and the system needs
>> to be cold booted for the remote attestation to succeed again.
>>
>>
>> @@ -198,6 +208,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   static int ima_update_kexec_buffer(struct notifier_block *self,
>>                      unsigned long action, void *data)
>>   {
>> +    char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>>       void *buf = NULL;
>>       size_t buf_size;
>>       bool resume = false;
>> @@ -213,9 +224,31 @@ static int ima_update_kexec_buffer(struct 
>> notifier_block *self,
>>           return NOTIFY_OK;
>>       }
>> +    buf_size = ima_get_binary_runtime_size();
>> +    scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> +          "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;",
>> +          kexec_segment_size, buf_size);
>> +
>> +    /*
>> +     * This is one of the very last events measured by IMA before kexec
>> +     * soft rebooting into the new Kernal.
>> +     * This event can be used as a marker after the system soft reboots
>> +     * to the new Kernel to check if there was sufficient memory 
>> allocated
>> +     * at kexec 'load' to capture the events measured between the window
>> +     * of kexec 'load' and 'execute'.
>> +     * This event needs to be present in the IMA log, in between the two
>> +     * 'boot_aggregate' events that are logged for the previous boot and
>> +     * the current soft reboot. If it is not present after the system 
>> soft
>> +     * reboots into the new Kernel, it would mean the IMA log is not
>> +     * consistent with the TPM PCR quotes, and the system needs to be
>> +     * cold-booted for the attestation to succeed again.
>> +     */
>> +    ima_measure_critical_data("ima_kexec", "kexec_execute",
>> +                  ima_kexec_event, strlen(ima_kexec_event),
>> +                  false, NULL, 0);
>> +
>>       ima_measurements_suspend();
>> -    buf_size = ima_get_binary_runtime_size();
> 
> This should be removed earlier, in 2/7.
> 
> 
> 
Agreed. Will do.

~Tushar

>>       ret = ima_dump_measurement_list(&buf_size, &buf,
>>                       kexec_segment_size);
Tushar Sugandhi Nov. 14, 2023, 10:48 p.m. UTC | #3
On 10/27/23 07:56, Mimi Zohar wrote:
> Hi Tushar,
> 
> On Thu, 2023-10-05 at 11:26 -0700, Tushar Sugandhi wrote:
>> The window between kexec 'load' and 'execute' could be arbitrarily long.
>> Even with the large chunk of memory allocated at kexec 'load', it may
>> run out which would result in missing events in IMA log after the system
>> soft reboots to the new Kernel.  This would result in IMA measurements
>> getting out of sync with the TPM PCR quotes which would result in remote
>> attestation failing for that system.  There is currently no way for the
>> new Kernel to know if the IMA log TPM PCR quote out of sync problem is
>> because of the missing measurements during kexec.
>>
>> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be
>> measured at kexec 'load' and 'execute' respectively.
>>
>> IMA measures 'boot_aggregate' as the first event when the system boots -
>> either cold boot or kexec soft boot.  In case when the system goes
>> through multiple soft reboots, the number of 'boot_aggregate' events in
>> IMA log corresponds to total number of boots (cold boot plus multiple
>> kexec soft reboots).  With this change, there would be additional
>> 'kexec_load' and 'kexec_execute' events in between the two
>> 'boot_aggregate' events.  In rare cases, when the system runs out of
>> memory during kexec soft reboot, 'kexec_execute' won't be copied since
>> its one of the very last event measured just before kexec soft reboot.
>> The absence of the event 'kexec_execute' in between the two
>> boot_aggregate' events would signal the attestation service that the IMA
>> log on the system is out of sync with TPM PCR quotes and the system needs
>> to be cold booted for the remote attestation to succeed again.
>>
>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
> 
> Adding a new type of critical data is fine.  The patch description
> should describe the reason for it.  Please update the Subject line and
> the patch description accordingly.
> 
Thanks.  I described the reasons in the code comment below, because I 
thought it is important enough to stay with the code to give context. 
But I can move it from the code comment to a patch description.
>> ---
>>   security/integrity/ima/ima_kexec.c | 35 +++++++++++++++++++++++++++++-
>>   1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>> index 6cd5f46a7208..0f9c424fe808 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -17,6 +17,8 @@
>>   #include "ima.h"
>>   
>>   #ifdef CONFIG_IMA_KEXEC
>> +#define IMA_KEXEC_EVENT_LEN 128
>> +
>>   struct seq_file ima_kexec_file;
>>   struct ima_kexec_hdr ima_khdr;
>>   static void *ima_kexec_buffer;
>> @@ -34,6 +36,8 @@ void ima_clear_kexec_file(void)
>>   
>>   static int ima_alloc_kexec_buf(size_t kexec_segment_size)
>>   {
>> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>> +
>>   	if ((kexec_segment_size == 0) ||
>>   	    (kexec_segment_size == ULONG_MAX) ||
>>   	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
>> @@ -64,6 +68,12 @@ static int ima_alloc_kexec_buf(size_t kexec_segment_size)
>>   	memset(&ima_khdr, 0, sizeof(ima_khdr));
>>   	ima_khdr.version = 1;
>>   
>> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> +		  "kexec_segment_size=%lu;", kexec_segment_size);
>> +
>> +	ima_measure_critical_data("ima_kexec", "kexec_load", ima_kexec_event,
>> +				  strlen(ima_kexec_event), false, NULL, 0);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -198,6 +208,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>   static int ima_update_kexec_buffer(struct notifier_block *self,
>>   				   unsigned long action, void *data)
>>   {
>> +	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
>>   	void *buf = NULL;
>>   	size_t buf_size;
>>   	bool resume = false;
>> @@ -213,9 +224,31 @@ static int ima_update_kexec_buffer(struct notifier_block *self,
>>   		return NOTIFY_OK;
>>   	}
>>   
>> +	buf_size = ima_get_binary_runtime_size();
>> +	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> +		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;",
>> +		  kexec_segment_size, buf_size);
>> +
>> +	/*
>> +	 * This is one of the very last events measured by IMA before kexec
>> +	 * soft rebooting into the new Kernal.
>> +	 * This event can be used as a marker after the system soft reboots
>> +	 * to the new Kernel to check if there was sufficient memory allocated
>> +	 * at kexec 'load' to capture the events measured between the window
>> +	 * of kexec 'load' and 'execute'.
>> +	 * This event needs to be present in the IMA log, in between the two
>> +	 * 'boot_aggregate' events that are logged for the previous boot and
>> +	 * the current soft reboot. If it is not present after the system soft
>> +	 * reboots into the new Kernel, it would mean the IMA log is not
>> +	 * consistent with the TPM PCR quotes, and the system needs to be
>> +	 * cold-booted for the attestation to succeed again.
>> +	 */
> 
> Comments like this don't belong in the code.  Please refer to section
> "8) Commenting" of Documentation/process/coding-style.rst.
Will do.

~Tushar
> 
>> +	ima_measure_critical_data("ima_kexec", "kexec_execute",
>> +				  ima_kexec_event, strlen(ima_kexec_event),
>> +				  false, NULL, 0);
>> +
>>   	ima_measurements_suspend();
>>   
>> -	buf_size = ima_get_binary_runtime_size();
>>   	ret = ima_dump_measurement_list(&buf_size, &buf,
>>   					kexec_segment_size);
>>
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 6cd5f46a7208..0f9c424fe808 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -17,6 +17,8 @@ 
 #include "ima.h"
 
 #ifdef CONFIG_IMA_KEXEC
+#define IMA_KEXEC_EVENT_LEN 128
+
 struct seq_file ima_kexec_file;
 struct ima_kexec_hdr ima_khdr;
 static void *ima_kexec_buffer;
@@ -34,6 +36,8 @@  void ima_clear_kexec_file(void)
 
 static int ima_alloc_kexec_buf(size_t kexec_segment_size)
 {
+	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
+
 	if ((kexec_segment_size == 0) ||
 	    (kexec_segment_size == ULONG_MAX) ||
 	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
@@ -64,6 +68,12 @@  static int ima_alloc_kexec_buf(size_t kexec_segment_size)
 	memset(&ima_khdr, 0, sizeof(ima_khdr));
 	ima_khdr.version = 1;
 
+	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
+		  "kexec_segment_size=%lu;", kexec_segment_size);
+
+	ima_measure_critical_data("ima_kexec", "kexec_load", ima_kexec_event,
+				  strlen(ima_kexec_event), false, NULL, 0);
+
 	return 0;
 }
 
@@ -198,6 +208,7 @@  void ima_add_kexec_buffer(struct kimage *image)
 static int ima_update_kexec_buffer(struct notifier_block *self,
 				   unsigned long action, void *data)
 {
+	char ima_kexec_event[IMA_KEXEC_EVENT_LEN];
 	void *buf = NULL;
 	size_t buf_size;
 	bool resume = false;
@@ -213,9 +224,31 @@  static int ima_update_kexec_buffer(struct notifier_block *self,
 		return NOTIFY_OK;
 	}
 
+	buf_size = ima_get_binary_runtime_size();
+	scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
+		  "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;",
+		  kexec_segment_size, buf_size);
+
+	/*
+	 * This is one of the very last events measured by IMA before kexec
+	 * soft rebooting into the new Kernal.
+	 * This event can be used as a marker after the system soft reboots
+	 * to the new Kernel to check if there was sufficient memory allocated
+	 * at kexec 'load' to capture the events measured between the window
+	 * of kexec 'load' and 'execute'.
+	 * This event needs to be present in the IMA log, in between the two
+	 * 'boot_aggregate' events that are logged for the previous boot and
+	 * the current soft reboot. If it is not present after the system soft
+	 * reboots into the new Kernel, it would mean the IMA log is not
+	 * consistent with the TPM PCR quotes, and the system needs to be
+	 * cold-booted for the attestation to succeed again.
+	 */
+	ima_measure_critical_data("ima_kexec", "kexec_execute",
+				  ima_kexec_event, strlen(ima_kexec_event),
+				  false, NULL, 0);
+
 	ima_measurements_suspend();
 
-	buf_size = ima_get_binary_runtime_size();
 	ret = ima_dump_measurement_list(&buf_size, &buf,
 					kexec_segment_size);