Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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.
--
thanks,
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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 */
/*
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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 */
/*
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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 */
> >
> > /*
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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 */
>
> /*
--
thanks,
Mimi
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 06/10] ima: update buffer at kexec execute with ima measurements
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
---
security/integrity/ima/ima_kexec.c | 50 ++
1 file changed, 50 insertions(+)
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 */
/*
--
2.25.1
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
