| Message ID | YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Carry forward IMA measurement log on kexec on x86_64 | expand |
Hi Jonathan, On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote: > On kexec file load Integrity Measurement Architecture (IMA) subsystem > may verify the IMA signature of the kernel and initramfs, and measure > it. The command line parameters passed to the kernel in the kexec call > may also be measured by IMA. A remote attestation service can verify > a TPM quote based on the TPM event log, the IMA measurement list, and > the TPM PCR data. This can be achieved only if the IMA measurement log > is carried over from the current kernel to the next kernel across > the kexec call. > > powerpc and ARM64 both achieve this using device tree with a > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > device tree, so the IMA infrastructure is extended to allow non device > tree platforms to provide a log buffer. x86 then passes the IMA buffer > to the new kernel via the setup_data mechanism. > > Signed-off-by: Jonathan McDowell <noodles@fb.com> FYI, after applying, building, and booting a kernel with this patch, "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline -- initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but "kexec -s -e" fails to reboot, at least on a test laptop even with only the "boot_aggregate" measurement record. Without enabling CONFIG_IMA_KEXEC, kexec boots properly. thanks, Mimi > --- > arch/x86/Kconfig | 1 + > arch/x86/include/uapi/asm/bootparam.h | 9 ++++ > arch/x86/kernel/e820.c | 6 +-- > arch/x86/kernel/kexec-bzimage64.c | 37 ++++++++++++++++- > arch/x86/kernel/setup.c | 26 ++++++++++++ > include/linux/ima.h | 1 + > security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++- > 7 files changed, 134 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index b0142e01002e..bde4959d9bdc 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -2017,6 +2017,7 @@ config KEXEC_FILE > bool "kexec file based system call" > select KEXEC_CORE > select BUILD_BIN2C > + select HAVE_IMA_KEXEC if IMA > depends on X86_64 > depends on CRYPTO=y > depends on CRYPTO_SHA256=y > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h > index b25d3f82c2f3..2f7b138a9388 100644 > --- a/arch/x86/include/uapi/asm/bootparam.h > +++ b/arch/x86/include/uapi/asm/bootparam.h > @@ -10,6 +10,7 @@ > #define SETUP_EFI 4 > #define SETUP_APPLE_PROPERTIES 5 > #define SETUP_JAILHOUSE 6 > +#define SETUP_IMA 7 > > #define SETUP_INDIRECT (1<<31) > > @@ -171,6 +172,14 @@ struct jailhouse_setup_data { > } __attribute__((packed)) v2; > } __attribute__((packed)); > > +/* > + * IMA buffer setup data information from the previous kernel during kexec > + */ > +struct ima_setup_data { > + __u64 addr; > + __u64 size; > +} __attribute__((packed)); > + > /* The so-called "zeropage" */ > struct boot_params { > struct screen_info screen_info; /* 0x000 */ > diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c > index f267205f2d5a..9dac24680ff8 100644 > --- a/arch/x86/kernel/e820.c > +++ b/arch/x86/kernel/e820.c > @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void) > e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN); > > /* > - * SETUP_EFI is supplied by kexec and does not need to be > - * reserved. > + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need > + * to be reserved. > */ > - if (data->type != SETUP_EFI) > + if (data->type != SETUP_EFI && data->type != SETUP_IMA) > e820__range_update_kexec(pa_data, > sizeof(*data) + data->len, > E820_TYPE_RAM, E820_TYPE_RESERVED_KERN); > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > index 170d0fd68b1f..07625da33075 100644 > --- a/arch/x86/kernel/kexec-bzimage64.c > +++ b/arch/x86/kernel/kexec-bzimage64.c > @@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, > } > #endif /* CONFIG_EFI */ > > +#ifdef CONFIG_IMA_KEXEC > +static void > +setup_ima_state(const struct kimage *image, struct boot_params *params, > + unsigned long params_load_addr, > + unsigned int ima_setup_data_offset) > +{ > + struct setup_data *sd = (void *)params + ima_setup_data_offset; > + struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data); > + unsigned long setup_data_phys; > + > + if (!image->ima_buffer_size) > + return; > + > + sd->type = SETUP_IMA; > + sd->len = sizeof(*ima); > + > + ima->addr = image->ima_buffer_addr; > + ima->size = image->ima_buffer_size; > + > + /* Add setup data */ > + setup_data_phys = params_load_addr + ima_setup_data_offset; > + sd->next = params->hdr.setup_data; > + params->hdr.setup_data = setup_data_phys; > +} > +#endif /* CONFIG_IMA_KEXEC */ > + > static int > setup_boot_parameters(struct kimage *image, struct boot_params *params, > unsigned long params_load_addr, > @@ -247,6 +273,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, > efi_setup_data_offset); > #endif > + > +#ifdef CONFIG_IMA_KEXEC > + /* Setup IMA log buffer state */ > + setup_ima_state(image, params, params_load_addr, > + efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data)); > +#endif > + > /* Setup EDD info */ > memcpy(params->eddbuf, boot_params.eddbuf, > EDDMAXNR * sizeof(struct edd_info)); > @@ -401,7 +434,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel, > params_cmdline_sz = ALIGN(params_cmdline_sz, 16); > kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) + > sizeof(struct setup_data) + > - sizeof(struct efi_setup_data); > + sizeof(struct efi_setup_data) + > + sizeof(struct setup_data) + > + sizeof(struct ima_setup_data); > > params = kzalloc(kbuf.bufsz, GFP_KERNEL); > if (!params) > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index c95b9ac5a457..8b0e7725f918 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -11,6 +11,7 @@ > #include <linux/dma-map-ops.h> > #include <linux/dmi.h> > #include <linux/efi.h> > +#include <linux/ima.h> > #include <linux/init_ohci1394_dma.h> > #include <linux/initrd.h> > #include <linux/iscsi_ibft.h> > @@ -335,6 +336,28 @@ static void __init reserve_initrd(void) > } > #endif /* CONFIG_BLK_DEV_INITRD */ > > +#ifdef CONFIG_IMA_KEXEC > +static void __init add_early_ima_buffer(u64 phys_addr) > +{ > + struct ima_setup_data *data; > + > + data = early_memremap(phys_addr + sizeof(struct setup_data), > + sizeof(*data)); > + if (!data) { > + pr_warn("setup: failed to memremap ima_setup_data entry\n"); > + return; > + } > + memblock_reserve(data->addr, data->size); > + ima_set_kexec_buffer(data->addr, data->size); > + early_memunmap(data, sizeof(*data)); > +} > +#else > +static void __init add_early_ima_buffer(u64 phys_addr) > +{ > + pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n"); > +} > +#endif > + > static void __init parse_setup_data(void) > { > struct setup_data *data; > @@ -360,6 +383,9 @@ static void __init parse_setup_data(void) > case SETUP_EFI: > parse_efi_setup(pa_data, data_len); > break; > + case SETUP_IMA: > + add_early_ima_buffer(pa_data); > + break; > default: > break; > } > diff --git a/include/linux/ima.h b/include/linux/ima.h > index 426b1744215e..f58aed7acad4 100644 > --- a/include/linux/ima.h > +++ b/include/linux/ima.h > @@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {} > > #ifdef CONFIG_IMA_KEXEC > extern void ima_add_kexec_buffer(struct kimage *image); > +extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size); > #endif > > #else > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c > index 13753136f03f..419c50cfe6b9 100644 > --- a/security/integrity/ima/ima_kexec.c > +++ b/security/integrity/ima/ima_kexec.c > @@ -10,6 +10,7 @@ > #include <linux/seq_file.h> > #include <linux/vmalloc.h> > #include <linux/kexec.h> > +#include <linux/memblock.h> > #include <linux/of.h> > #include <linux/ima.h> > #include "ima.h" > @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image) > } > #endif /* IMA_KEXEC */ > > +#ifndef CONFIG_OF > +static phys_addr_t ima_early_kexec_buffer_phys; > +static size_t ima_early_kexec_buffer_size; > + > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size) > +{ > + if (size == 0) > + return; > + > + ima_early_kexec_buffer_phys = phys_addr; > + ima_early_kexec_buffer_size = size; > +} > + > +int __init ima_free_kexec_buffer(void) > +{ > + int rc; > + > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) > + return -ENOTSUPP; > + > + if (ima_early_kexec_buffer_size == 0) > + return -ENOENT; > + > + rc = memblock_phys_free(ima_early_kexec_buffer_phys, > + ima_early_kexec_buffer_size); > + if (rc) > + return rc; > + > + ima_early_kexec_buffer_phys = 0; > + ima_early_kexec_buffer_size = 0; > + > + return 0; > +} > + > +int __init ima_get_kexec_buffer(void **addr, size_t *size) > +{ > + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) > + return -ENOTSUPP; > + > + if (ima_early_kexec_buffer_size == 0) > + return -ENOENT; > + > + *addr = __va(ima_early_kexec_buffer_phys); > + *size = ima_early_kexec_buffer_size; > + > + return 0; > +} > + > +#else > + > +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size) > +{ > + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n"); > +} > +#endif /* CONFIG_OF */ > + > /* > * Restore the measurement list from the previous kernel. > */ > -void ima_load_kexec_buffer(void) > +void __init ima_load_kexec_buffer(void) > { > void *kexec_buffer = NULL; > size_t kexec_buffer_size = 0;
On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote: > Hi Jonathan, > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote: > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > may verify the IMA signature of the kernel and initramfs, and measure > > it. The command line parameters passed to the kernel in the kexec call > > may also be measured by IMA. A remote attestation service can verify > > a TPM quote based on the TPM event log, the IMA measurement list, and > > the TPM PCR data. This can be achieved only if the IMA measurement log > > is carried over from the current kernel to the next kernel across > > the kexec call. > > > > powerpc and ARM64 both achieve this using device tree with a > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > device tree, so the IMA infrastructure is extended to allow non device > > tree platforms to provide a log buffer. x86 then passes the IMA buffer > > to the new kernel via the setup_data mechanism. > > > > Signed-off-by: Jonathan McDowell <noodles@fb.com> > > FYI, after applying, building, and booting a kernel with this patch, > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline -- > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but > "kexec -s -e" fails to reboot, at least on a test laptop even with only > the "boot_aggregate" measurement record. > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly. Thanks for giving it a try. At a guess your laptop is booting with EFI, whereas for my testing I was using qemu with legacy BIOS. I've managed to reproduce the issue with qemu+OVMF and isolated the mistake in the setup data calculation I made when EFI is involved. If you have time can you try with the below on top of the original patch? diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 07625da33075..cdc73e081585 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -277,7 +277,9 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, #ifdef CONFIG_IMA_KEXEC /* Setup IMA log buffer state */ setup_ima_state(image, params, params_load_addr, - efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data)); + efi_setup_data_offset + + sizeof(struct setup_data) + + sizeof(struct efi_setup_data)); #endif /* Setup EDD info */
On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote: > On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote: > > Hi Jonathan, > > > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote: > > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > > may verify the IMA signature of the kernel and initramfs, and measure > > > it. The command line parameters passed to the kernel in the kexec call > > > may also be measured by IMA. A remote attestation service can verify > > > a TPM quote based on the TPM event log, the IMA measurement list, and > > > the TPM PCR data. This can be achieved only if the IMA measurement log > > > is carried over from the current kernel to the next kernel across > > > the kexec call. > > > > > > powerpc and ARM64 both achieve this using device tree with a > > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > > device tree, so the IMA infrastructure is extended to allow non device > > > tree platforms to provide a log buffer. x86 then passes the IMA buffer > > > to the new kernel via the setup_data mechanism. > > > > > > Signed-off-by: Jonathan McDowell <noodles@fb.com> > > > > FYI, after applying, building, and booting a kernel with this patch, > > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline -- > > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but > > "kexec -s -e" fails to reboot, at least on a test laptop even with only > > the "boot_aggregate" measurement record. > > > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly. > > Thanks for giving it a try. At a guess your laptop is booting with > EFI, whereas for my testing I was using qemu with legacy BIOS. I've > managed to reproduce the issue with qemu+OVMF and isolated the mistake > in the setup data calculation I made when EFI is involved. If you have > time can you try with the below on top of the original patch? Thank you! With the change, as expected there are two "boot_aggregate" records in the measurement list. With a custom policy, the measurement list verifies. # grep boot_aggregate /sys/kernel/security/ima/ascii_runtime_measurements 10 fe0b821290b1bd229e0d34c5571f48eeff403119 ima-sig sha1:a87d47e560d148cd1f4c8da677a84ddbe27f12f8 boot_aggregate 10 fe0b821290b1bd229e0d34c5571f48eeff403119 ima-sig sha1:a87d47e560d148cd1f4c8da677a84ddbe27f12f8 boot_aggregate # cat /sys/kernel/security/ima/runtime_measurements_count 5597 # evmctl ima_measurement /sys/kernel/security/ima/binary_runtime_measurements Matched per TPM bank calculated digest(s). FYI, the builtin "ima_policy=tcb" results in measurement violations. Normally, the measurement list can still be verified using the evmctl "--ignore-violations" option. For some reason with the "tcb" policy, the measurement list doesn't verify even with the "--ignore-violations" option after kexec. I assume this is a result of additional measurements being added after the kexec load, which aren't being carried across kexec. thanks, Mimi > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > index 07625da33075..cdc73e081585 100644 > --- a/arch/x86/kernel/kexec-bzimage64.c > +++ b/arch/x86/kernel/kexec-bzimage64.c > @@ -277,7 +277,9 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > #ifdef CONFIG_IMA_KEXEC > /* Setup IMA log buffer state */ > setup_ima_state(image, params, params_load_addr, > - efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data)); > + efi_setup_data_offset + > + sizeof(struct setup_data) + > + sizeof(struct efi_setup_data)); > #endif > > /* Setup EDD info */
On Tue, Apr 26, 2022 at 09:49:53AM -0400, Mimi Zohar wrote: > On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote: > > On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote: > > > Hi Jonathan, > > > > > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote: > > > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > > > may verify the IMA signature of the kernel and initramfs, and measure > > > > it. The command line parameters passed to the kernel in the kexec call > > > > may also be measured by IMA. A remote attestation service can verify > > > > a TPM quote based on the TPM event log, the IMA measurement list, and > > > > the TPM PCR data. This can be achieved only if the IMA measurement log > > > > is carried over from the current kernel to the next kernel across > > > > the kexec call. > > > > > > > > powerpc and ARM64 both achieve this using device tree with a > > > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > > > device tree, so the IMA infrastructure is extended to allow non device > > > > tree platforms to provide a log buffer. x86 then passes the IMA buffer > > > > to the new kernel via the setup_data mechanism. > > > > > > > > Signed-off-by: Jonathan McDowell <noodles@fb.com> > > > > > > FYI, after applying, building, and booting a kernel with this patch, > > > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline -- > > > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but > > > "kexec -s -e" fails to reboot, at least on a test laptop even with only > > > the "boot_aggregate" measurement record. > > > > > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly. > > > > Thanks for giving it a try. At a guess your laptop is booting with > > EFI, whereas for my testing I was using qemu with legacy BIOS. I've > > managed to reproduce the issue with qemu+OVMF and isolated the mistake > > in the setup data calculation I made when EFI is involved. If you have > > time can you try with the below on top of the original patch? > > Thank you! With the change, as expected there are two "boot_aggregate" > records in the measurement list. With a custom policy, the measurement > list verifies. Excellent, thanks for verifying. I'll get the fixed v2 out. ... > FYI, the builtin "ima_policy=tcb" results in measurement violations. > Normally, the measurement list can still be verified using the evmctl > "--ignore-violations" option. For some reason with the "tcb" policy, > the measurement list doesn't verify even with the "--ignore-violations" > option after kexec. I assume this is a result of additional > measurements being added after the kexec load, which aren't being > carried across kexec. I believe with "tcb" things like the subsequent exec of kexec to actually do the reboot will end up measured, and as the kexec buffer is static it won't include that. Also there's an issue about the fact that we measure the kexec pieces even if we don't actually do the kexec; there's no marker that confirms the kexec took place. It's separate to this patch (in that it affects the device tree kexec infrastructure too) but it's conceivable that an attacker could measure in the new kernel details and not actually do the kexec, and that's not distinguishable from the kexec happening. One approach might be to add a marker in the kexec ima buffer such that if it's not present we know the kexec hasn't happened, but I need to think through that a bit more. J.
On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote: > On Tue, Apr 26, 2022 at 09:49:53AM -0400, Mimi Zohar wrote: > > On Tue, 2022-04-26 at 12:08 +0000, Jonathan McDowell wrote: > > > On Mon, Apr 25, 2022 at 12:29:17PM -0400, Mimi Zohar wrote: > > > > Hi Jonathan, > > > > > > > > On Fri, 2022-04-22 at 13:50 +0000, Jonathan McDowell wrote: > > > > > On kexec file load Integrity Measurement Architecture (IMA) subsystem > > > > > may verify the IMA signature of the kernel and initramfs, and measure > > > > > it. The command line parameters passed to the kernel in the kexec call > > > > > may also be measured by IMA. A remote attestation service can verify > > > > > a TPM quote based on the TPM event log, the IMA measurement list, and > > > > > the TPM PCR data. This can be achieved only if the IMA measurement log > > > > > is carried over from the current kernel to the next kernel across > > > > > the kexec call. > > > > > > > > > > powerpc and ARM64 both achieve this using device tree with a > > > > > "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of > > > > > device tree, so the IMA infrastructure is extended to allow non device > > > > > tree platforms to provide a log buffer. x86 then passes the IMA buffer > > > > > to the new kernel via the setup_data mechanism. > > > > > > > > > > Signed-off-by: Jonathan McDowell <noodles@fb.com> > > > > > > > > FYI, after applying, building, and booting a kernel with this patch, > > > > "kexec -s -l /boot/vmlinuz-5.18.0-rc4+ --reuse-cmdline -- > > > > initrd=/boot/initramfs-5.18.0-rc4+.img" properly loads the kernel, but > > > > "kexec -s -e" fails to reboot, at least on a test laptop even with only > > > > the "boot_aggregate" measurement record. > > > > > > > > Without enabling CONFIG_IMA_KEXEC, kexec boots properly. > > > > > > Thanks for giving it a try. At a guess your laptop is booting with > > > EFI, whereas for my testing I was using qemu with legacy BIOS. I've > > > managed to reproduce the issue with qemu+OVMF and isolated the mistake > > > in the setup data calculation I made when EFI is involved. If you have > > > time can you try with the below on top of the original patch? > > > > Thank you! With the change, as expected there are two "boot_aggregate" > > records in the measurement list. With a custom policy, the measurement > > list verifies. > > Excellent, thanks for verifying. I'll get the fixed v2 out. > > ... > > FYI, the builtin "ima_policy=tcb" results in measurement violations. > > Normally, the measurement list can still be verified using the evmctl > > "--ignore-violations" option. For some reason with the "tcb" policy, > > the measurement list doesn't verify even with the "--ignore-violations" > > option after kexec. I assume this is a result of additional > > measurements being added after the kexec load, which aren't being > > carried across kexec. > > I believe with "tcb" things like the subsequent exec of kexec to > actually do the reboot will end up measured, and as the kexec buffer is > static it won't include that. > > Also there's an issue about the fact that we measure the kexec pieces > even if we don't actually do the kexec; there's no marker that confirms > the kexec took place. It's separate to this patch (in that it affects > the device tree kexec infrastructure too) but it's conceivable that an > attacker could measure in the new kernel details and not actually do the > kexec, and that's not distinguishable from the kexec happening. > > One approach might be to add a marker in the kexec ima buffer such that > if it's not present we know the kexec hasn't happened, but I need to > think through that a bit more. I'm not quite sure what you mean by "we measure the kexec pieces". The kexec file load syscall calls kernel_read_file_from_fd() to read the kernel image into a buffer. The measurement record included in the IMA measurement list a hash of the buffer data, which is exactly the same as the hash of the kernel image. The kernel kexec self tests only do the kexec load, not the execute. For each kexec execute you'll see an additional "boot_aggregate" record in the IMA measurement list. At least for the moment I don't see a need for additional marker. thanks, Mimi
On Tue, Apr 26, 2022 at 02:10:58PM -0400, Mimi Zohar wrote: > On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote: > > Also there's an issue about the fact that we measure the kexec pieces > > even if we don't actually do the kexec; there's no marker that confirms > > the kexec took place. It's separate to this patch (in that it affects > > the device tree kexec infrastructure too) but it's conceivable that an > > attacker could measure in the new kernel details and not actually do the > > kexec, and that's not distinguishable from the kexec happening. > > > > One approach might be to add a marker in the kexec ima buffer such that > > if it's not present we know the kexec hasn't happened, but I need to > > think through that a bit more. > > I'm not quite sure what you mean by "we measure the kexec pieces". The > kexec file load syscall calls kernel_read_file_from_fd() to read the > kernel image into a buffer. The measurement record included in the IMA > measurement list a hash of the buffer data, which is exactly the same > as the hash of the kernel image. > > The kernel kexec self tests only do the kexec load, not the execute. > For each kexec execute you'll see an additional "boot_aggregate" record > in the IMA measurement list. At least for the moment I don't see a > need for additional marker. You're right, of course. I'd missed the fact we measure the boot_aggregate into IMA_MEASURE_PCR_IDX on boot, so although we'll update PCRs related to the kexec on load the IMA PCR won't get updated until we've actually done the reboot. So no need for anything extra. J.
On Thu, 2022-04-28 at 10:40 +0000, Jonathan McDowell wrote: > On Tue, Apr 26, 2022 at 02:10:58PM -0400, Mimi Zohar wrote: > > On Tue, 2022-04-26 at 16:48 +0000, Jonathan McDowell wrote: > > > Also there's an issue about the fact that we measure the kexec pieces > > > even if we don't actually do the kexec; there's no marker that confirms > > > the kexec took place. It's separate to this patch (in that it affects > > > the device tree kexec infrastructure too) but it's conceivable that an > > > attacker could measure in the new kernel details and not actually do the > > > kexec, and that's not distinguishable from the kexec happening. > > > > > > One approach might be to add a marker in the kexec ima buffer such that > > > if it's not present we know the kexec hasn't happened, but I need to > > > think through that a bit more. > > > > I'm not quite sure what you mean by "we measure the kexec pieces". The > > kexec file load syscall calls kernel_read_file_from_fd() to read the > > kernel image into a buffer. The measurement record included in the IMA > > measurement list a hash of the buffer data, which is exactly the same > > as the hash of the kernel image. > > > > The kernel kexec self tests only do the kexec load, not the execute. > > For each kexec execute you'll see an additional "boot_aggregate" record > > in the IMA measurement list. At least for the moment I don't see a > > need for additional marker. > > You're right, of course. I'd missed the fact we measure the > boot_aggregate into IMA_MEASURE_PCR_IDX on boot, so although we'll > update PCRs related to the kexec on load the IMA PCR won't get updated > until we've actually done the reboot. So no need for anything extra. To clarify, after the kexec load, the IMA measurement list contains the kexec'ed kernel image measurement. The TPM was also extended with that measurement. Nothing prevents verifying the IMA measurement list at this point, before the kexec execute, though depending on policy it might result in additional measurements. The IMA "pcr=" policy rule option allows specifying a different PCR than the default IMA_MEASURE_PCR_IDX. To summarize, with CONFIG_IMA_ARCH_POLICY enabled, both measurements - kexec'ed kernel image, boot_aggregate - are being added to the IMA measurement list and extended into the default TPM PCR. Measuring the kexec'ed kernel image and extending the TPM with the measurement happens at some point before the system is rebooted. The "boot_aggregate" is the first measurement after boot/soft boot. thanks, Mimi
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b0142e01002e..bde4959d9bdc 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2017,6 +2017,7 @@ config KEXEC_FILE bool "kexec file based system call" select KEXEC_CORE select BUILD_BIN2C + select HAVE_IMA_KEXEC if IMA depends on X86_64 depends on CRYPTO=y depends on CRYPTO_SHA256=y diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index b25d3f82c2f3..2f7b138a9388 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -10,6 +10,7 @@ #define SETUP_EFI 4 #define SETUP_APPLE_PROPERTIES 5 #define SETUP_JAILHOUSE 6 +#define SETUP_IMA 7 #define SETUP_INDIRECT (1<<31) @@ -171,6 +172,14 @@ struct jailhouse_setup_data { } __attribute__((packed)) v2; } __attribute__((packed)); +/* + * IMA buffer setup data information from the previous kernel during kexec + */ +struct ima_setup_data { + __u64 addr; + __u64 size; +} __attribute__((packed)); + /* The so-called "zeropage" */ struct boot_params { struct screen_info screen_info; /* 0x000 */ diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c index f267205f2d5a..9dac24680ff8 100644 --- a/arch/x86/kernel/e820.c +++ b/arch/x86/kernel/e820.c @@ -1017,10 +1017,10 @@ void __init e820__reserve_setup_data(void) e820__range_update(pa_data, sizeof(*data)+data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN); /* - * SETUP_EFI is supplied by kexec and does not need to be - * reserved. + * SETUP_EFI and SETUP_IMA are supplied by kexec and do not need + * to be reserved. */ - if (data->type != SETUP_EFI) + if (data->type != SETUP_EFI && data->type != SETUP_IMA) e820__range_update_kexec(pa_data, sizeof(*data) + data->len, E820_TYPE_RAM, E820_TYPE_RESERVED_KERN); diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 170d0fd68b1f..07625da33075 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -186,6 +186,32 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, } #endif /* CONFIG_EFI */ +#ifdef CONFIG_IMA_KEXEC +static void +setup_ima_state(const struct kimage *image, struct boot_params *params, + unsigned long params_load_addr, + unsigned int ima_setup_data_offset) +{ + struct setup_data *sd = (void *)params + ima_setup_data_offset; + struct ima_setup_data *ima = (void *)sd + sizeof(struct setup_data); + unsigned long setup_data_phys; + + if (!image->ima_buffer_size) + return; + + sd->type = SETUP_IMA; + sd->len = sizeof(*ima); + + ima->addr = image->ima_buffer_addr; + ima->size = image->ima_buffer_size; + + /* Add setup data */ + setup_data_phys = params_load_addr + ima_setup_data_offset; + sd->next = params->hdr.setup_data; + params->hdr.setup_data = setup_data_phys; +} +#endif /* CONFIG_IMA_KEXEC */ + static int setup_boot_parameters(struct kimage *image, struct boot_params *params, unsigned long params_load_addr, @@ -247,6 +273,13 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, efi_setup_data_offset); #endif + +#ifdef CONFIG_IMA_KEXEC + /* Setup IMA log buffer state */ + setup_ima_state(image, params, params_load_addr, + efi_setup_data_offset + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data)); +#endif + /* Setup EDD info */ memcpy(params->eddbuf, boot_params.eddbuf, EDDMAXNR * sizeof(struct edd_info)); @@ -401,7 +434,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel, params_cmdline_sz = ALIGN(params_cmdline_sz, 16); kbuf.bufsz = params_cmdline_sz + ALIGN(efi_map_sz, 16) + sizeof(struct setup_data) + - sizeof(struct efi_setup_data); + sizeof(struct efi_setup_data) + + sizeof(struct setup_data) + + sizeof(struct ima_setup_data); params = kzalloc(kbuf.bufsz, GFP_KERNEL); if (!params) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index c95b9ac5a457..8b0e7725f918 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -11,6 +11,7 @@ #include <linux/dma-map-ops.h> #include <linux/dmi.h> #include <linux/efi.h> +#include <linux/ima.h> #include <linux/init_ohci1394_dma.h> #include <linux/initrd.h> #include <linux/iscsi_ibft.h> @@ -335,6 +336,28 @@ static void __init reserve_initrd(void) } #endif /* CONFIG_BLK_DEV_INITRD */ +#ifdef CONFIG_IMA_KEXEC +static void __init add_early_ima_buffer(u64 phys_addr) +{ + struct ima_setup_data *data; + + data = early_memremap(phys_addr + sizeof(struct setup_data), + sizeof(*data)); + if (!data) { + pr_warn("setup: failed to memremap ima_setup_data entry\n"); + return; + } + memblock_reserve(data->addr, data->size); + ima_set_kexec_buffer(data->addr, data->size); + early_memunmap(data, sizeof(*data)); +} +#else +static void __init add_early_ima_buffer(u64 phys_addr) +{ + pr_warn("Passed IMA kexec data, but CONFIG_IMA_KEXEC not set. Ignoring.\n"); +} +#endif + static void __init parse_setup_data(void) { struct setup_data *data; @@ -360,6 +383,9 @@ static void __init parse_setup_data(void) case SETUP_EFI: parse_efi_setup(pa_data, data_len); break; + case SETUP_IMA: + add_early_ima_buffer(pa_data); + break; default: break; } diff --git a/include/linux/ima.h b/include/linux/ima.h index 426b1744215e..f58aed7acad4 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -48,6 +48,7 @@ static inline void ima_appraise_parse_cmdline(void) {} #ifdef CONFIG_IMA_KEXEC extern void ima_add_kexec_buffer(struct kimage *image); +extern void ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size); #endif #else diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c index 13753136f03f..419c50cfe6b9 100644 --- a/security/integrity/ima/ima_kexec.c +++ b/security/integrity/ima/ima_kexec.c @@ -10,6 +10,7 @@ #include <linux/seq_file.h> #include <linux/vmalloc.h> #include <linux/kexec.h> +#include <linux/memblock.h> #include <linux/of.h> #include <linux/ima.h> #include "ima.h" @@ -134,10 +135,66 @@ void ima_add_kexec_buffer(struct kimage *image) } #endif /* IMA_KEXEC */ +#ifndef CONFIG_OF +static phys_addr_t ima_early_kexec_buffer_phys; +static size_t ima_early_kexec_buffer_size; + +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size) +{ + if (size == 0) + return; + + ima_early_kexec_buffer_phys = phys_addr; + ima_early_kexec_buffer_size = size; +} + +int __init ima_free_kexec_buffer(void) +{ + int rc; + + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) + return -ENOTSUPP; + + if (ima_early_kexec_buffer_size == 0) + return -ENOENT; + + rc = memblock_phys_free(ima_early_kexec_buffer_phys, + ima_early_kexec_buffer_size); + if (rc) + return rc; + + ima_early_kexec_buffer_phys = 0; + ima_early_kexec_buffer_size = 0; + + return 0; +} + +int __init ima_get_kexec_buffer(void **addr, size_t *size) +{ + if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC)) + return -ENOTSUPP; + + if (ima_early_kexec_buffer_size == 0) + return -ENOENT; + + *addr = __va(ima_early_kexec_buffer_phys); + *size = ima_early_kexec_buffer_size; + + return 0; +} + +#else + +void __init ima_set_kexec_buffer(phys_addr_t phys_addr, size_t size) +{ + pr_warn("CONFIG_OF enabled, ignoring call to set buffer details.\n"); +} +#endif /* CONFIG_OF */ + /* * Restore the measurement list from the previous kernel. */ -void ima_load_kexec_buffer(void) +void __init ima_load_kexec_buffer(void) { void *kexec_buffer = NULL; size_t kexec_buffer_size = 0;
On kexec file load Integrity Measurement Architecture (IMA) subsystem may verify the IMA signature of the kernel and initramfs, and measure it. The command line parameters passed to the kernel in the kexec call may also be measured by IMA. A remote attestation service can verify a TPM quote based on the TPM event log, the IMA measurement list, and the TPM PCR data. This can be achieved only if the IMA measurement log is carried over from the current kernel to the next kernel across the kexec call. powerpc and ARM64 both achieve this using device tree with a "linux,ima-kexec-buffer" node. x86 platforms generally don't make use of device tree, so the IMA infrastructure is extended to allow non device tree platforms to provide a log buffer. x86 then passes the IMA buffer to the new kernel via the setup_data mechanism. Signed-off-by: Jonathan McDowell <noodles@fb.com> --- arch/x86/Kconfig | 1 + arch/x86/include/uapi/asm/bootparam.h | 9 ++++ arch/x86/kernel/e820.c | 6 +-- arch/x86/kernel/kexec-bzimage64.c | 37 ++++++++++++++++- arch/x86/kernel/setup.c | 26 ++++++++++++ include/linux/ima.h | 1 + security/integrity/ima/ima_kexec.c | 59 ++++++++++++++++++++++++++- 7 files changed, 134 insertions(+), 5 deletions(-)