| Message ID | 20230703215709.1195644-7-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: measure events between kexec load and execute | expand |
Hi Tushar, On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: > +/* > + * Called during kexec execute so that IMA can update the measurement list. > + */ > +static int ima_update_kexec_buffer(struct notifier_block *self, > + unsigned long action, void *data) > +{ > + void *new_buffer = NULL; > + size_t new_buffer_size, cur_buffer_size; > + bool resume = false; > + > + if (!kexec_in_progress) { > + pr_info("%s: No kexec in progress.\n", __func__); > + return NOTIFY_OK; > + } > + > + if (!ima_kexec_buffer) { > + pr_err("%s: Kexec buffer not set.\n", __func__); > + return NOTIFY_OK; > + } > + > + ima_measurements_suspend(); > + > + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); > + new_buffer_size = ima_get_binary_runtime_size(); > + if (new_buffer_size > cur_buffer_size) { > + pr_err("%s: Measurement list grew too large.\n", __func__); > + resume = true; > + goto out; > + } This changes the current behavior of carrying as many measurements across kexec as possible. True the measurement list won't verify against the TPM PCRs, but not copying the measurements leaves the impression there weren't any previous measurements. This also explains the reason for allocating an IMA buffer (patch 1/10) and not writing the measurements directly into the kexec buffer. > + ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer); > + > + if (!new_buffer) { > + pr_err("%s: Dump measurements failed.\n", __func__); > + resume = true; > + goto out; > + } > + memcpy(ima_kexec_buffer, new_buffer, new_buffer_size); > +out: > + kimage_unmap_segment(ima_kexec_buffer); > + ima_kexec_buffer = NULL; > + > + if (resume) > + ima_measurements_resume(); > + > + return NOTIFY_OK; > +} > + > #endif /* IMA_KEXEC */ > > /*
On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote: > Hi Tushar, > > On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: > > > +/* > > + * Called during kexec execute so that IMA can update the measurement list. > > + */ > > +static int ima_update_kexec_buffer(struct notifier_block *self, > > + unsigned long action, void *data) > > +{ > > + void *new_buffer = NULL; > > + size_t new_buffer_size, cur_buffer_size; > > + bool resume = false; > > + > > + if (!kexec_in_progress) { > > + pr_info("%s: No kexec in progress.\n", __func__); > > + return NOTIFY_OK; > > + } > > + > > + if (!ima_kexec_buffer) { > > + pr_err("%s: Kexec buffer not set.\n", __func__); > > + return NOTIFY_OK; > > + } > > + > > + ima_measurements_suspend(); > > + > > + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); > > + new_buffer_size = ima_get_binary_runtime_size(); > > + if (new_buffer_size > cur_buffer_size) { > > + pr_err("%s: Measurement list grew too large.\n", __func__); > > + resume = true; > > + goto out; > > + } > > This changes the current behavior of carrying as many measurements > across kexec as possible. True the measurement list won't verify > against the TPM PCRs, but not copying the measurements leaves the > impression there weren't any previous measurements. > > This also explains the reason for allocating an IMA buffer (patch 1/10) > and not writing the measurements directly into the kexec buffer. If not carrying even a partial measurement list across kexec is desired, then in addition to the "boot_aggregate" record, define a new record containing the TPM pcrcounter. With this information, attestation servers will at least be able to detect if the measurement list was truncated. thanks, Mimi > > > + ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer); > > + > > + if (!new_buffer) { > > + pr_err("%s: Dump measurements failed.\n", __func__); > > + resume = true; > > + goto out; > > + } > > + memcpy(ima_kexec_buffer, new_buffer, new_buffer_size); > > +out: > > + kimage_unmap_segment(ima_kexec_buffer); > > + ima_kexec_buffer = NULL; > > + > > + if (resume) > > + ima_measurements_resume(); > > + > > + return NOTIFY_OK; > > +} > > + > > #endif /* IMA_KEXEC */ > > > > /* >
Adding Eric to cc. On 7/7/23 08:01, Mimi Zohar wrote: > Hi Tushar, > > On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: > >> +/* >> + * Called during kexec execute so that IMA can update the measurement list. >> + */ >> +static int ima_update_kexec_buffer(struct notifier_block *self, >> + unsigned long action, void *data) >> +{ >> + void *new_buffer = NULL; >> + size_t new_buffer_size, cur_buffer_size; >> + bool resume = false; >> + >> + if (!kexec_in_progress) { >> + pr_info("%s: No kexec in progress.\n", __func__); >> + return NOTIFY_OK; >> + } >> + >> + if (!ima_kexec_buffer) { >> + pr_err("%s: Kexec buffer not set.\n", __func__); >> + return NOTIFY_OK; >> + } >> + >> + ima_measurements_suspend(); >> + >> + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); >> + new_buffer_size = ima_get_binary_runtime_size(); >> + if (new_buffer_size > cur_buffer_size) { >> + pr_err("%s: Measurement list grew too large.\n", __func__); >> + resume = true; >> + goto out; >> + } > This changes the current behavior of carrying as many measurements > across kexec as possible. True the measurement list won't verify > against the TPM PCRs, but not copying the measurements leaves the > impression there weren't any previous measurements. > > This also explains the reason for allocating an IMA buffer (patch 1/10) > and not writing the measurements directly into the kexec buffer. Thanks. I will update this logic depending if we decide to use ima_dump_measurement_list() at kexec ‘execute’, or combination of ima_allocate_buf_at_kexec_load() and ima_populate_buf_at_kexec_execute() at kexec ‘load’ and kexec ‘execute’ respectively. ~Tushar >> + ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer); >> + >> + if (!new_buffer) { >> + pr_err("%s: Dump measurements failed.\n", __func__); >> + resume = true; >> + goto out; >> + } >> + memcpy(ima_kexec_buffer, new_buffer, new_buffer_size); >> +out: >> + kimage_unmap_segment(ima_kexec_buffer); >> + ima_kexec_buffer = NULL; >> + >> + if (resume) >> + ima_measurements_resume(); >> + >> + return NOTIFY_OK; >> +} >> + >> #endif /* IMA_KEXEC */ >> >> /*
Adding Eric to cc. On 7/7/23 12:49, Mimi Zohar wrote: > On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote: >> Hi Tushar, >> >> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: >> >>> +/* >>> + * Called during kexec execute so that IMA can update the measurement list. >>> + */ >>> +static int ima_update_kexec_buffer(struct notifier_block *self, >>> + unsigned long action, void *data) >>> +{ >>> + void *new_buffer = NULL; >>> + size_t new_buffer_size, cur_buffer_size; >>> + bool resume = false; >>> + >>> + if (!kexec_in_progress) { >>> + pr_info("%s: No kexec in progress.\n", __func__); >>> + return NOTIFY_OK; >>> + } >>> + >>> + if (!ima_kexec_buffer) { >>> + pr_err("%s: Kexec buffer not set.\n", __func__); >>> + return NOTIFY_OK; >>> + } >>> + >>> + ima_measurements_suspend(); >>> + >>> + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); >>> + new_buffer_size = ima_get_binary_runtime_size(); >>> + if (new_buffer_size > cur_buffer_size) { >>> + pr_err("%s: Measurement list grew too large.\n", __func__); >>> + resume = true; >>> + goto out; >>> + } >> This changes the current behavior of carrying as many measurements >> across kexec as possible. True the measurement list won't verify >> against the TPM PCRs, but not copying the measurements leaves the >> impression there weren't any previous measurements. >> >> This also explains the reason for allocating an IMA buffer (patch 1/10) >> and not writing the measurements directly into the kexec buffer. > If not carrying even a partial measurement list across kexec is > desired, then in addition to the "boot_aggregate" record, define a new > record containing the TPM pcrcounter. With this information, > attestation servers will at least be able to detect if the measurement > list was truncated. > > thanks, > > Mimi Sure. Recording TPM pcrcounter at boot aggregate and Kexec 'load' should provide the necessary information to the attestation servers. We can implement this if needed, based on how rest of the series evolves. ~Tushar >>> + ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer); >>> + >>> + if (!new_buffer) { >>> + pr_err("%s: Dump measurements failed.\n", __func__); >>> + resume = true; >>> + goto out; >>> + } >>> + memcpy(ima_kexec_buffer, new_buffer, new_buffer_size); >>> +out: >>> + kimage_unmap_segment(ima_kexec_buffer); >>> + ima_kexec_buffer = NULL; >>> + >>> + if (resume) >>> + ima_measurements_resume(); >>> + >>> + return NOTIFY_OK; >>> +} >>> + >>> #endif /* IMA_KEXEC */ >>> >>> /*
On Tue, 2023-07-11 at 12:08 -0700, Tushar Sugandhi wrote: > Adding Eric to cc. > > On 7/7/23 12:49, Mimi Zohar wrote: > > On Fri, 2023-07-07 at 11:01 -0400, Mimi Zohar wrote: > >> Hi Tushar, > >> > >> On Mon, 2023-07-03 at 14:57 -0700, Tushar Sugandhi wrote: > >> > >>> +/* > >>> + * Called during kexec execute so that IMA can update the measurement list. > >>> + */ > >>> +static int ima_update_kexec_buffer(struct notifier_block *self, > >>> + unsigned long action, void *data) > >>> +{ > >>> + void *new_buffer = NULL; > >>> + size_t new_buffer_size, cur_buffer_size; > >>> + bool resume = false; > >>> + > >>> + if (!kexec_in_progress) { > >>> + pr_info("%s: No kexec in progress.\n", __func__); > >>> + return NOTIFY_OK; > >>> + } > >>> + > >>> + if (!ima_kexec_buffer) { > >>> + pr_err("%s: Kexec buffer not set.\n", __func__); > >>> + return NOTIFY_OK; > >>> + } > >>> + > >>> + ima_measurements_suspend(); > >>> + > >>> + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); > >>> + new_buffer_size = ima_get_binary_runtime_size(); > >>> + if (new_buffer_size > cur_buffer_size) { > >>> + pr_err("%s: Measurement list grew too large.\n", __func__); > >>> + resume = true; > >>> + goto out; > >>> + } > >> This changes the current behavior of carrying as many measurements > >> across kexec as possible. True the measurement list won't verify > >> against the TPM PCRs, but not copying the measurements leaves the > >> impression there weren't any previous measurements. > >> > >> This also explains the reason for allocating an IMA buffer (patch 1/10) > >> and not writing the measurements directly into the kexec buffer. > > If not carrying even a partial measurement list across kexec is > > desired, then in addition to the "boot_aggregate" record, define a new > > record containing the TPM pcrcounter. With this information, > > attestation servers will at least be able to detect if the measurement > > list was truncated. > Sure. Recording TPM pcrcounter at boot aggregate and > Kexec 'load' should provide the necessary information to the > attestation servers. We can implement this if needed, based on how > rest of the series evolves. Recording the TPM pcrcounter should be done independently of this patch set. This patch set would have a dependency on it.
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 7deb8df31485..224d88ccfe85 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -18,6 +18,7 @@ struct seq_file ima_kexec_file; struct ima_kexec_hdr ima_khdr; static size_t kexec_segment_size; +static void *ima_kexec_buffer; void ima_clear_kexec_file(void) { @@ -230,6 +231,55 @@ void ima_add_kexec_buffer(struct kimage *image) pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n", kbuf.mem); } + +/* + * Called during kexec execute so that IMA can update the measurement list. + */ +static int ima_update_kexec_buffer(struct notifier_block *self, + unsigned long action, void *data) +{ + void *new_buffer = NULL; + size_t new_buffer_size, cur_buffer_size; + bool resume = false; + + if (!kexec_in_progress) { + pr_info("%s: No kexec in progress.\n", __func__); + return NOTIFY_OK; + } + + if (!ima_kexec_buffer) { + pr_err("%s: Kexec buffer not set.\n", __func__); + return NOTIFY_OK; + } + + ima_measurements_suspend(); + + cur_buffer_size = kexec_segment_size - sizeof(struct ima_kexec_hdr); + new_buffer_size = ima_get_binary_runtime_size(); + if (new_buffer_size > cur_buffer_size) { + pr_err("%s: Measurement list grew too large.\n", __func__); + resume = true; + goto out; + } + + ima_populate_buf_at_kexec_execute(&new_buffer_size, &new_buffer); + + if (!new_buffer) { + pr_err("%s: Dump measurements failed.\n", __func__); + resume = true; + goto out; + } + memcpy(ima_kexec_buffer, new_buffer, new_buffer_size); +out: + kimage_unmap_segment(ima_kexec_buffer); + ima_kexec_buffer = NULL; + + if (resume) + ima_measurements_resume(); + + return NOTIFY_OK; +} + #endif /* IMA_KEXEC */ /*
In the current implementation, the measurement list is not updated in the buffer during the window between kexec load and execute. This leads to measurement loss in the buffer when transitioning from the old to the new kernel. This patch provides a way to update the measurement list in the buffer during a kexec execution. Suspending the measurements during the buffer update ensures the buffer doesn't get corrupted, or goes out of sync with TPM PCRs. Thus it ensures the integrity of measurements is maintained across kernel transitions during a kexec. Introduce a new variable ima_kexec_buffer that is used to hold the address of the IMA kexec buffer. Implement a function ima_update_kexec_buffer() that is called during kexec execute, allowing the IMA to update the measurement list with the events between kexec load and execute. First check if a kexec is in progress and if the IMA kexec buffer is initialized. If these conditions are met, suspend IMA measurements and check if the new buffer size obtained from ima_get_binary_runtime_size() is larger than the current buffer size. If the new buffer size is too large, output an error message, and resume the measurements. If the new buffer size fits, populate the new buffer with the current measurements using ima_populate_buf_at_kexec_execute() and copy it into ima_kexec_buffer. Unmap ima_kexec_buffer segment from the image. Resuming the measurements is not needed in case of successful measurements since the control is being passed to the new kernel anyways through kexec execute. However, the measurements should be resumed if there are any errors flagged in the function. Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> --- security/integrity/ima/ima_kexec.c | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)