| Message ID | 20250124225547.22684-8-chenste@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ima: kexec: measure events between kexec load and excute | expand |
On 1/24/25 5:55 PM, steven chen wrote: > The amount of memory allocated at kexec load, even with the extra memory > allocated, might not be large enough for the entire measurement list. The > indeterminate interval between kexec 'load' and 'execute' could exacerbate > this problem. > > Define two new IMA events, 'kexec_load' and 'kexec_execute', to be > measured as critical data at kexec 'load' and 'execute' respectively. > Report the allocated kexec segment size, IMA binary log size and the > runtime measurements count as part of those events. > > These events, and the values reported through them, serve as markers in > the IMA log to verify the IMA events are captured during kexec soft > reboot. The presence of a 'kexec_load' event in between the last two > 'boot_aggregate' events in the IMA log implies this is a kexec soft > reboot, and not a cold-boot. And the absence of 'kexec_execute' event > after kexec soft reboot implies missing events in that window which > results in inconsistency with TPM PCR quotes, necessitating a cold boot > for a successful remote attestation. > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > Author: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > Signed-off-by: steven chen <chenste@linux.microsoft.com> > --- > security/integrity/ima/ima_kexec.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c > index c9c916f69ca7..d416ca0382cb 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 256 > + > static struct seq_file ima_kexec_file; > static void *ima_kexec_buffer; > static size_t kexec_segment_size; > @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf) > ima_reset_kexec_file(sf); > } > > +static void ima_measure_kexec_event(const char *event_name) > +{ > + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; > + size_t buf_size = 0; > + long len; > + > + buf_size = ima_get_binary_runtime_size(); > + len = atomic_long_read(&ima_htable.len); > + > + scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN, > + "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;" > + "ima_runtime_measurements_count=%ld;", > + kexec_segment_size, buf_size, len); > + > + ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, > + strlen(ima_kexec_event), false, NULL, 0); > +} > + > static int ima_alloc_kexec_file_buf(size_t segment_size) > { > /* > @@ -60,6 +80,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size) > out: > ima_kexec_file.read_pos = 0; > ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */ > + ima_measure_kexec_event("kexec_load"); > > return 0; > } > @@ -206,6 +227,8 @@ static int ima_update_kexec_buffer(struct notifier_block *self, > > if (ret) > pr_err("Dump measurements failed. Error:%d\n", ret); > + else > + ima_measure_kexec_event("kexec_execute"); > > if (buf_size != 0) > memcpy(ima_kexec_buffer, buf, buf_size); I have been doing kexec's (on ppc64 KVM) applying one patch after another in this series and then testing with this command: evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/binary_runtime_measurements Unfortunately it breaks at this patch. I am not sure what it is due to.
On 1/28/25 10:28 AM, Stefan Berger wrote: > > > On 1/24/25 5:55 PM, steven chen wrote: >> The amount of memory allocated at kexec load, even with the extra memory >> allocated, might not be large enough for the entire measurement list. >> The >> indeterminate interval between kexec 'load' and 'execute' could >> exacerbate >> this problem. >> >> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be >> measured as critical data at kexec 'load' and 'execute' respectively. >> Report the allocated kexec segment size, IMA binary log size and the >> runtime measurements count as part of those events. >> >> These events, and the values reported through them, serve as markers in >> the IMA log to verify the IMA events are captured during kexec soft >> reboot. The presence of a 'kexec_load' event in between the last two >> 'boot_aggregate' events in the IMA log implies this is a kexec soft >> reboot, and not a cold-boot. And the absence of 'kexec_execute' event >> after kexec soft reboot implies missing events in that window which >> results in inconsistency with TPM PCR quotes, necessitating a cold boot >> for a successful remote attestation. >> >> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> >> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com> >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> >> Signed-off-by: steven chen <chenste@linux.microsoft.com> >> --- >> security/integrity/ima/ima_kexec.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ >> ima/ima_kexec.c >> index c9c916f69ca7..d416ca0382cb 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 256 >> + >> static struct seq_file ima_kexec_file; >> static void *ima_kexec_buffer; >> static size_t kexec_segment_size; >> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file >> *sf) >> ima_reset_kexec_file(sf); >> } >> +static void ima_measure_kexec_event(const char *event_name) >> +{ >> + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; >> + size_t buf_size = 0; >> + long len; >> + >> + buf_size = ima_get_binary_runtime_size(); >> + len = atomic_long_read(&ima_htable.len); >> + >> + scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN, >> + "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;" >> + "ima_runtime_measurements_count=%ld;", >> + kexec_segment_size, buf_size, len); >> + >> + ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, >> + strlen(ima_kexec_event), false, NULL, 0); >> +} >> + >> static int ima_alloc_kexec_file_buf(size_t segment_size) >> { >> /* >> @@ -60,6 +80,7 @@ static int ima_alloc_kexec_file_buf(size_t >> segment_size) >> out: >> ima_kexec_file.read_pos = 0; >> ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* >> reserved space */ >> + ima_measure_kexec_event("kexec_load"); >> return 0; >> } >> @@ -206,6 +227,8 @@ static int ima_update_kexec_buffer(struct >> notifier_block *self, >> if (ret) >> pr_err("Dump measurements failed. Error:%d\n", ret); >> + else >> + ima_measure_kexec_event("kexec_execute"); The problem is that this here ^^^^ currently comes after this block: ret = ima_dump_measurement_list(&buf_size, &buf, kexec_segment_size); if (ret) pr_err("Dump measurements failed. Error:%d\n", ret); else ima_measure_kexec_event("kexec_execute"); <--- after the dump It has to be done before so that whatever it adds to the measurement list gets dumped as well. So this works for my testing: + ima_measure_kexec_event("kexec_execute"); + ret = ima_dump_measurement_list(&buf_size, &buf, kexec_segment_size); if (ret) pr_err("Dump measurements failed. Error:%d\n", ret); >> if (buf_size != 0) >> memcpy(ima_kexec_buffer, buf, buf_size); > > > I have been doing kexec's (on ppc64 KVM) applying one patch after > another in this series and then testing with this command: > > evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/ > binary_runtime_measurements > > Unfortunately it breaks at this patch. I am not sure what it is due to. > >
On 1/28/2025 9:29 AM, Stefan Berger wrote: > > > On 1/28/25 10:28 AM, Stefan Berger wrote: >> >> >> On 1/24/25 5:55 PM, steven chen wrote: >>> The amount of memory allocated at kexec load, even with the extra >>> memory >>> allocated, might not be large enough for the entire measurement >>> list. The >>> indeterminate interval between kexec 'load' and 'execute' could >>> exacerbate >>> this problem. >>> >>> Define two new IMA events, 'kexec_load' and 'kexec_execute', to be >>> measured as critical data at kexec 'load' and 'execute' respectively. >>> Report the allocated kexec segment size, IMA binary log size and the >>> runtime measurements count as part of those events. >>> >>> These events, and the values reported through them, serve as markers in >>> the IMA log to verify the IMA events are captured during kexec soft >>> reboot. The presence of a 'kexec_load' event in between the last two >>> 'boot_aggregate' events in the IMA log implies this is a kexec soft >>> reboot, and not a cold-boot. And the absence of 'kexec_execute' event >>> after kexec soft reboot implies missing events in that window which >>> results in inconsistency with TPM PCR quotes, necessitating a cold boot >>> for a successful remote attestation. >>> >>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> >>> Author: Tushar Sugandhi <tusharsu@linux.microsoft.com> >>> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> >>> Signed-off-by: steven chen <chenste@linux.microsoft.com> >>> --- >>> security/integrity/ima/ima_kexec.c | 23 +++++++++++++++++++++++ >>> 1 file changed, 23 insertions(+) >>> >>> diff --git a/security/integrity/ima/ima_kexec.c >>> b/security/integrity/ ima/ima_kexec.c >>> index c9c916f69ca7..d416ca0382cb 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 256 >>> + >>> static struct seq_file ima_kexec_file; >>> static void *ima_kexec_buffer; >>> static size_t kexec_segment_size; >>> @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct >>> seq_file *sf) >>> ima_reset_kexec_file(sf); >>> } >>> +static void ima_measure_kexec_event(const char *event_name) >>> +{ >>> + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; >>> + size_t buf_size = 0; >>> + long len; >>> + >>> + buf_size = ima_get_binary_runtime_size(); >>> + len = atomic_long_read(&ima_htable.len); >>> + >>> + scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN, >>> + "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;" >>> + "ima_runtime_measurements_count=%ld;", >>> + kexec_segment_size, buf_size, len); >>> + >>> + ima_measure_critical_data("ima_kexec", event_name, >>> ima_kexec_event, >>> + strlen(ima_kexec_event), false, NULL, 0); >>> +} >>> + >>> static int ima_alloc_kexec_file_buf(size_t segment_size) >>> { >>> /* >>> @@ -60,6 +80,7 @@ static int ima_alloc_kexec_file_buf(size_t >>> segment_size) >>> out: >>> ima_kexec_file.read_pos = 0; >>> ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* >>> reserved space */ >>> + ima_measure_kexec_event("kexec_load"); >>> return 0; >>> } >>> @@ -206,6 +227,8 @@ static int ima_update_kexec_buffer(struct >>> notifier_block *self, >>> if (ret) >>> pr_err("Dump measurements failed. Error:%d\n", ret); >>> + else >>> + ima_measure_kexec_event("kexec_execute"); > > The problem is that this here ^^^^ currently comes after this block: > > ret = ima_dump_measurement_list(&buf_size, &buf, > kexec_segment_size); > > if (ret) > pr_err("Dump measurements failed. Error:%d\n", ret); > else > ima_measure_kexec_event("kexec_execute"); <--- after > the dump > > It has to be done before so that whatever it adds to the measurement > list gets dumped as well. So this works for my testing: > > + ima_measure_kexec_event("kexec_execute"); > + > ret = ima_dump_measurement_list(&buf_size, &buf, > kexec_segment_size); > > if (ret) > pr_err("Dump measurements failed. Error:%d\n", ret); > > >>> if (buf_size != 0) >>> memcpy(ima_kexec_buffer, buf, buf_size); >> >> >> I have been doing kexec's (on ppc64 KVM) applying one patch after >> another in this series and then testing with this command: >> >> evmctl ima_measurement --ignore-violations /sys/kernel/security/ima/ >> binary_runtime_measurements >> >> Unfortunately it breaks at this patch. I am not sure what it is due to. >> >> Thanks Stefan. Will update it in next release
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index c9c916f69ca7..d416ca0382cb 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 256 + static struct seq_file ima_kexec_file; static void *ima_kexec_buffer; static size_t kexec_segment_size; @@ -36,6 +38,24 @@ static void ima_free_kexec_file_buf(struct seq_file *sf) ima_reset_kexec_file(sf); } +static void ima_measure_kexec_event(const char *event_name) +{ + char ima_kexec_event[IMA_KEXEC_EVENT_LEN]; + size_t buf_size = 0; + long len; + + buf_size = ima_get_binary_runtime_size(); + len = atomic_long_read(&ima_htable.len); + + scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN, + "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;" + "ima_runtime_measurements_count=%ld;", + kexec_segment_size, buf_size, len); + + ima_measure_critical_data("ima_kexec", event_name, ima_kexec_event, + strlen(ima_kexec_event), false, NULL, 0); +} + static int ima_alloc_kexec_file_buf(size_t segment_size) { /* @@ -60,6 +80,7 @@ static int ima_alloc_kexec_file_buf(size_t segment_size) out: ima_kexec_file.read_pos = 0; ima_kexec_file.count = sizeof(struct ima_kexec_hdr); /* reserved space */ + ima_measure_kexec_event("kexec_load"); return 0; } @@ -206,6 +227,8 @@ static int ima_update_kexec_buffer(struct notifier_block *self, if (ret) pr_err("Dump measurements failed. Error:%d\n", ret); + else + ima_measure_kexec_event("kexec_execute"); if (buf_size != 0) memcpy(ima_kexec_buffer, buf, buf_size);