diff mbox series

Carry forward IMA measurement log on kexec on x86_64

Message ID YmKyvlF3my1yWTvK@noodles-fedora-PC23Y6EG (mailing list archive)
State New
Headers show
Series Carry forward IMA measurement log on kexec on x86_64 | expand

Commit Message

Jonathan McDowell April 22, 2022, 1:50 p.m. UTC
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(-)

Comments

Mimi Zohar April 25, 2022, 4:29 p.m. UTC | #1
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;
Jonathan McDowell April 26, 2022, 12:08 p.m. UTC | #2
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 */
Mimi Zohar April 26, 2022, 1:49 p.m. UTC | #3
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 */
Jonathan McDowell April 26, 2022, 4:48 p.m. UTC | #4
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.
Mimi Zohar April 26, 2022, 6:10 p.m. UTC | #5
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
Jonathan McDowell April 28, 2022, 10:40 a.m. UTC | #6
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.
Mimi Zohar April 28, 2022, 12:25 p.m. UTC | #7
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 mbox series

Patch

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;