[PATCH 0/1] x86/kexec: UKI support

[PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

Hello,

this patch implements UKI support for kexec_file_load. It will require support
in the kexec-tools userspace utility. For testing purposes the following can be used:
https://github.com/Cydox/kexec-test/

There has been discussion on this topic in an issue on GitHub that is linked below
for reference.


Some links:
- Related discussion: https://github.com/systemd/systemd/issues/28538
- Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/

Jan Hendrik Farr (1):
  x86/kexec: UKI support

 arch/x86/include/asm/kexec-uki.h       |   7 ++
 arch/x86/include/asm/parse_pefile.h    |  32 +++++++
 arch/x86/kernel/Makefile               |   2 +
 arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
 arch/x86/kernel/machine_kexec_64.c     |   2 +
 arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
 crypto/asymmetric_keys/mscode_parser.c |   2 +-
 crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
 crypto/asymmetric_keys/verify_pefile.h |  16 ----
 9 files changed, 278 insertions(+), 116 deletions(-)
 create mode 100644 arch/x86/include/asm/kexec-uki.h
 create mode 100644 arch/x86/include/asm/parse_pefile.h
 create mode 100644 arch/x86/kernel/kexec-uki.c
 create mode 100644 arch/x86/kernel/parse_pefile.c

-- 
2.40.1

[PATCH 1/1] x86/kexec: UKI support

[Jan Hendrik Farr]

make the kernel accept UKIs (Unified Kernel Images) for kexec_file_load.

UKIs contain the kernel bzImage, initrd, and cmdline all packaged up as
one EFI application. The main advantage of this is that the whole
combination is signed together as a package for secure boot.

This implementation parses the UKI and passes the bzImage, initrd, and
cmdline to the normal bzImage loader.

Signed-off-by: Jan Hendrik Farr <kernel@jfarr.cc>
---
 arch/x86/include/asm/kexec-uki.h       |   7 ++
 arch/x86/include/asm/parse_pefile.h    |  32 +++++++
 arch/x86/kernel/Makefile               |   2 +
 arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
 arch/x86/kernel/machine_kexec_64.c     |   2 +
 arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
 crypto/asymmetric_keys/mscode_parser.c |   2 +-
 crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
 crypto/asymmetric_keys/verify_pefile.h |  16 ----
 9 files changed, 278 insertions(+), 116 deletions(-)
 create mode 100644 arch/x86/include/asm/kexec-uki.h
 create mode 100644 arch/x86/include/asm/parse_pefile.h
 create mode 100644 arch/x86/kernel/kexec-uki.c
 create mode 100644 arch/x86/kernel/parse_pefile.c

diff --git a/arch/x86/include/asm/kexec-uki.h b/arch/x86/include/asm/kexec-uki.h
new file mode 100644
index 000000000000..87fd2c6fb091
--- /dev/null
+++ b/arch/x86/include/asm/kexec-uki.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_KEXEC_UKI_H
+#define _ASM_KEXEC_UKI_H
+
+extern const struct kexec_file_ops kexec_uki_ops;
+
+#endif  /* _ASM_KEXEC_UKI_H */
diff --git a/arch/x86/include/asm/parse_pefile.h b/arch/x86/include/asm/parse_pefile.h
new file mode 100644
index 000000000000..c29f8c98ee66
--- /dev/null
+++ b/arch/x86/include/asm/parse_pefile.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
+ *
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#ifndef _ASM_PARSE_PEFILE_H
+#define _ASM_PARSE_PEFILE_H
+
+#include <linux/pe.h>
+
+struct pefile_context {
+	unsigned	header_size;
+	unsigned	image_checksum_offset;
+	unsigned	cert_dirent_offset;
+	unsigned	n_data_dirents;
+	unsigned	n_sections;
+	unsigned	certs_size;
+	unsigned	sig_offset;
+	unsigned	sig_len;
+	const struct section_header *secs;
+
+	/* PKCS#7 MS Individual Code Signing content */
+	const void	*digest;		/* Digest */
+	unsigned	digest_len;		/* Digest length */
+	const char	*digest_algo;		/* Digest algorithm */
+};
+int pefile_parse_binary(const void *pebuf, unsigned int pelen,
+			       struct pefile_context *ctx);
+
+#endif // _ASM_PARSE_PEFILE_H
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 3269a0e23d3a..8a37a977bf72 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -101,6 +101,8 @@ obj-$(CONFIG_CRASH_CORE)	+= crash_core_$(BITS).o
 obj-$(CONFIG_KEXEC_CORE)	+= machine_kexec_$(BITS).o
 obj-$(CONFIG_KEXEC_CORE)	+= relocate_kernel_$(BITS).o crash.o
 obj-$(CONFIG_KEXEC_FILE)	+= kexec-bzimage64.o
+obj-$(CONFIG_KEXEC_FILE)	+= kexec-uki.o
+obj-$(CONFIG_KEXEC_FILE)	+= parse_pefile.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
 obj-y				+= kprobes/
 obj-$(CONFIG_MODULES)		+= module.o
diff --git a/arch/x86/kernel/kexec-uki.c b/arch/x86/kernel/kexec-uki.c
new file mode 100644
index 000000000000..9275196a6b71
--- /dev/null
+++ b/arch/x86/kernel/kexec-uki.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kexec UKI loader
+ *
+ * Copyright (C) 2023 Jan Hendrik Farr
+ *
+ * Authors:
+ *      Jan Hendrik Farr <kernel@jfarr.cc>
+ */
+
+#define pr_fmt(fmt)	"kexec-uki: " fmt
+
+#include <linux/kernel.h>
+#include "linux/pe.h"
+#include <linux/kexec.h>
+#include <linux/err.h>
+
+#include <asm/kexec-uki.h>
+#include <asm/kexec-bzimage64.h>
+#include <asm/parse_pefile.h>
+
+static int find_section(struct pefile_context *ctx, const char *name,
+			const struct section_header **sec)
+{
+	for (int i = 0; i < ctx->n_sections; i++) {
+		const struct section_header *cur_sec = &ctx->secs[i];
+
+		if (!strncmp(cur_sec->name, name, ARRAY_SIZE(cur_sec->name))) {
+			*sec = cur_sec;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int uki_probe(const char *buf, unsigned long len)
+{
+	int ret = -ENOEXEC;
+	struct pefile_context pe_ctx;
+
+	int r = pefile_parse_binary(buf, len, &pe_ctx);
+
+	if (r) {
+		pr_info("Not a UKI. Not a valid PE file\n");
+		return ret;
+	}
+
+	const struct section_header *_;
+
+	if (find_section(&pe_ctx, ".linux", &_) ||
+	    find_section(&pe_ctx, ".initrd", &_) ||
+	    find_section(&pe_ctx, ".cmdline", &_)) {
+		pr_info("Not a UKI. Missing .linux, .initrd, or .cmdline\n");
+		return ret;
+	}
+
+
+	pr_info("It's a UKI\n");
+	return 0;
+}
+
+static void *uki_load(struct kimage *image, char *kernel,
+		      unsigned long kernel_len, char *initrd,
+		      unsigned long initrd_len, char *cmdline,
+		      unsigned long cmdline_len)
+{
+	struct pefile_context pe_ctx;
+	int r = pefile_parse_binary(kernel, kernel_len, &pe_ctx);
+
+	if (r)
+		return ERR_PTR(r);
+
+	pr_debug("pefile_parse_binary return %d, number of sections: %d\n",
+		 r, pe_ctx.n_sections);
+
+	const struct section_header *sec_linux, *sec_initrd, *sec_cmdline;
+	int r_linux = find_section(&pe_ctx, ".linux", &sec_linux);
+	int r_initrd = find_section(&pe_ctx, ".initrd", &sec_initrd);
+	int r_cmdline = find_section(&pe_ctx, ".cmdline", &sec_cmdline);
+
+	if (r_linux || r_initrd || r_cmdline)
+		return ERR_PTR(-EINVAL);
+
+	void *ret = kexec_bzImage64_ops.load(
+		image,
+		kernel + sec_linux->data_addr,
+		sec_linux->raw_data_size,
+		kernel + sec_initrd->data_addr,
+		sec_initrd->raw_data_size,
+		kernel + sec_cmdline->data_addr,
+		sec_cmdline->raw_data_size
+	);
+
+	if (IS_ERR(ret))
+		pr_warn("bzImage64_load error\n");
+
+	return ret;
+}
+
+static int uki_cleanup(void *loader_data)
+{
+	return kexec_bzImage64_ops.cleanup(loader_data);
+}
+
+const struct kexec_file_ops kexec_uki_ops = {
+	.probe = uki_probe,
+	.load = uki_load,
+	.cleanup = uki_cleanup,
+#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
+	.verify_sig = kexec_kernel_verify_pe_sig,
+#endif
+};
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 1a3e2c05a8a5..072f5aac52b9 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -25,6 +25,7 @@
 #include <asm/io_apic.h>
 #include <asm/debugreg.h>
 #include <asm/kexec-bzimage64.h>
+#include <asm/kexec-uki.h>
 #include <asm/setup.h>
 #include <asm/set_memory.h>
 #include <asm/cpu.h>
@@ -81,6 +82,7 @@ static int map_acpi_tables(struct x86_mapping_info *info, pgd_t *level4p) { retu
 #ifdef CONFIG_KEXEC_FILE
 const struct kexec_file_ops * const kexec_file_loaders[] = {
 		&kexec_bzImage64_ops,
+		&kexec_uki_ops,
 		NULL
 };
 #endif
diff --git a/arch/x86/kernel/parse_pefile.c b/arch/x86/kernel/parse_pefile.c
new file mode 100644
index 000000000000..7737c94a1848
--- /dev/null
+++ b/arch/x86/kernel/parse_pefile.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Parse a PE binary
+ *
+ * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2023 Jan Hendrik Farr
+ *
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#include <asm/parse_pefile.h>
+#include <linux/err.h>
+
+#define pr_fmt(fmt)	"parse_pefile: " fmt
+#include <linux/module.h>
+/*
+ * Parse a PE binary.
+ */
+int pefile_parse_binary(const void *pebuf, unsigned int pelen,
+			       struct pefile_context *ctx)
+{
+	const struct mz_hdr *mz = pebuf;
+	const struct pe_hdr *pe;
+	const struct pe32_opt_hdr *pe32;
+	const struct pe32plus_opt_hdr *pe64;
+	const struct data_directory *ddir;
+	const struct data_dirent *dde;
+	const struct section_header *secs, *sec;
+	size_t cursor, datalen = pelen;
+
+
+#define chkaddr(base, x, s)						\
+	do {								\
+		if ((x) < base || (s) >= datalen || (x) > datalen - (s)) \
+			return -ELIBBAD;				\
+	} while (0)
+
+	chkaddr(0, 0, sizeof(*mz));
+	if (mz->magic != MZ_MAGIC)
+		return -ELIBBAD;
+	cursor = sizeof(*mz);
+
+	chkaddr(cursor, mz->peaddr, sizeof(*pe));
+	pe = pebuf + mz->peaddr;
+	if (pe->magic != PE_MAGIC)
+		return -ELIBBAD;
+	cursor = mz->peaddr + sizeof(*pe);
+
+	chkaddr(0, cursor, sizeof(pe32->magic));
+	pe32 = pebuf + cursor;
+	pe64 = pebuf + cursor;
+
+	switch (pe32->magic) {
+	case PE_OPT_MAGIC_PE32:
+		chkaddr(0, cursor, sizeof(*pe32));
+		ctx->image_checksum_offset =
+			(unsigned long)&pe32->csum - (unsigned long)pebuf;
+		ctx->header_size = pe32->header_size;
+		cursor += sizeof(*pe32);
+		ctx->n_data_dirents = pe32->data_dirs;
+		break;
+
+	case PE_OPT_MAGIC_PE32PLUS:
+		chkaddr(0, cursor, sizeof(*pe64));
+		ctx->image_checksum_offset =
+			(unsigned long)&pe64->csum - (unsigned long)pebuf;
+		ctx->header_size = pe64->header_size;
+		cursor += sizeof(*pe64);
+		ctx->n_data_dirents = pe64->data_dirs;
+		break;
+
+	default:
+		pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
+		return -ELIBBAD;
+	}
+
+	pr_debug("checksum @ %x\n", ctx->image_checksum_offset);
+	pr_debug("header size = %x\n", ctx->header_size);
+
+	if (cursor >= ctx->header_size || ctx->header_size >= datalen)
+		return -ELIBBAD;
+
+	if (ctx->n_data_dirents > (ctx->header_size - cursor) / sizeof(*dde))
+		return -ELIBBAD;
+
+	ddir = pebuf + cursor;
+	cursor += sizeof(*dde) * ctx->n_data_dirents;
+
+	ctx->cert_dirent_offset =
+		(unsigned long)&ddir->certs - (unsigned long)pebuf;
+	ctx->certs_size = ddir->certs.size;
+
+	if (ddir->certs.virtual_address && ddir->certs.size) {
+
+		chkaddr(ctx->header_size, ddir->certs.virtual_address,
+			ddir->certs.size);
+		ctx->sig_offset = ddir->certs.virtual_address;
+		ctx->sig_len = ddir->certs.size;
+		pr_debug("cert = %x @%x [%*ph]\n",
+			 ctx->sig_len, ctx->sig_offset,
+			 ctx->sig_len, pebuf + ctx->sig_offset);
+
+	}
+
+	ctx->n_sections = pe->sections;
+	if (ctx->n_sections > (ctx->header_size - cursor) / sizeof(*sec))
+		return -ELIBBAD;
+	ctx->secs = secs = pebuf + cursor;
+
+	return 0;
+}
diff --git a/crypto/asymmetric_keys/mscode_parser.c b/crypto/asymmetric_keys/mscode_parser.c
index 839591ad21ac..063b348bb4c3 100644
--- a/crypto/asymmetric_keys/mscode_parser.c
+++ b/crypto/asymmetric_keys/mscode_parser.c
@@ -11,7 +11,7 @@
 #include <linux/err.h>
 #include <linux/oid_registry.h>
 #include <crypto/pkcs7.h>
-#include "verify_pefile.h"
+#include <asm/parse_pefile.h>
 #include "mscode.asn1.h"
 
 /*
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index f440767bd727..fbb319094c0a 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -14,106 +14,9 @@
 #include <linux/asn1.h>
 #include <linux/verification.h>
 #include <crypto/hash.h>
+#include <asm/parse_pefile.h>
 #include "verify_pefile.h"
 
-/*
- * Parse a PE binary.
- */
-static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
-			       struct pefile_context *ctx)
-{
-	const struct mz_hdr *mz = pebuf;
-	const struct pe_hdr *pe;
-	const struct pe32_opt_hdr *pe32;
-	const struct pe32plus_opt_hdr *pe64;
-	const struct data_directory *ddir;
-	const struct data_dirent *dde;
-	const struct section_header *secs, *sec;
-	size_t cursor, datalen = pelen;
-
-	kenter("");
-
-#define chkaddr(base, x, s)						\
-	do {								\
-		if ((x) < base || (s) >= datalen || (x) > datalen - (s)) \
-			return -ELIBBAD;				\
-	} while (0)
-
-	chkaddr(0, 0, sizeof(*mz));
-	if (mz->magic != MZ_MAGIC)
-		return -ELIBBAD;
-	cursor = sizeof(*mz);
-
-	chkaddr(cursor, mz->peaddr, sizeof(*pe));
-	pe = pebuf + mz->peaddr;
-	if (pe->magic != PE_MAGIC)
-		return -ELIBBAD;
-	cursor = mz->peaddr + sizeof(*pe);
-
-	chkaddr(0, cursor, sizeof(pe32->magic));
-	pe32 = pebuf + cursor;
-	pe64 = pebuf + cursor;
-
-	switch (pe32->magic) {
-	case PE_OPT_MAGIC_PE32:
-		chkaddr(0, cursor, sizeof(*pe32));
-		ctx->image_checksum_offset =
-			(unsigned long)&pe32->csum - (unsigned long)pebuf;
-		ctx->header_size = pe32->header_size;
-		cursor += sizeof(*pe32);
-		ctx->n_data_dirents = pe32->data_dirs;
-		break;
-
-	case PE_OPT_MAGIC_PE32PLUS:
-		chkaddr(0, cursor, sizeof(*pe64));
-		ctx->image_checksum_offset =
-			(unsigned long)&pe64->csum - (unsigned long)pebuf;
-		ctx->header_size = pe64->header_size;
-		cursor += sizeof(*pe64);
-		ctx->n_data_dirents = pe64->data_dirs;
-		break;
-
-	default:
-		pr_warn("Unknown PEOPT magic = %04hx\n", pe32->magic);
-		return -ELIBBAD;
-	}
-
-	pr_debug("checksum @ %x\n", ctx->image_checksum_offset);
-	pr_debug("header size = %x\n", ctx->header_size);
-
-	if (cursor >= ctx->header_size || ctx->header_size >= datalen)
-		return -ELIBBAD;
-
-	if (ctx->n_data_dirents > (ctx->header_size - cursor) / sizeof(*dde))
-		return -ELIBBAD;
-
-	ddir = pebuf + cursor;
-	cursor += sizeof(*dde) * ctx->n_data_dirents;
-
-	ctx->cert_dirent_offset =
-		(unsigned long)&ddir->certs - (unsigned long)pebuf;
-	ctx->certs_size = ddir->certs.size;
-
-	if (!ddir->certs.virtual_address || !ddir->certs.size) {
-		pr_warn("Unsigned PE binary\n");
-		return -ENODATA;
-	}
-
-	chkaddr(ctx->header_size, ddir->certs.virtual_address,
-		ddir->certs.size);
-	ctx->sig_offset = ddir->certs.virtual_address;
-	ctx->sig_len = ddir->certs.size;
-	pr_debug("cert = %x @%x [%*ph]\n",
-		 ctx->sig_len, ctx->sig_offset,
-		 ctx->sig_len, pebuf + ctx->sig_offset);
-
-	ctx->n_sections = pe->sections;
-	if (ctx->n_sections > (ctx->header_size - cursor) / sizeof(*sec))
-		return -ELIBBAD;
-	ctx->secs = secs = pebuf + cursor;
-
-	return 0;
-}
 
 /*
  * Check and strip the PE wrapper from around the signature and check that the
@@ -431,6 +334,13 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
 	if (ret < 0)
 		return ret;
 
+	const struct data_dirent *certs = pebuf + ctx.cert_dirent_offset;
+
+	if (!certs->virtual_address || !certs->size) {
+		pr_warn("Unsigned PE binary\n");
+		return -ENODATA;
+	}
+
 	ret = pefile_strip_sig_wrapper(pebuf, &ctx);
 	if (ret < 0)
 		return ret;
@@ -439,8 +349,10 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
 				     pebuf + ctx.sig_offset, ctx.sig_len,
 				     trusted_keys, usage,
 				     mscode_parse, &ctx);
-	if (ret < 0)
+	if (ret < 0) {
+		pr_warn("invalid PE file signature\n");
 		goto error;
+	}
 
 	pr_debug("Digest: %u [%*ph]\n",
 		 ctx.digest_len, ctx.digest_len, ctx.digest);
diff --git a/crypto/asymmetric_keys/verify_pefile.h b/crypto/asymmetric_keys/verify_pefile.h
index e1628e100cde..5ab2f9a5b2ef 100644
--- a/crypto/asymmetric_keys/verify_pefile.h
+++ b/crypto/asymmetric_keys/verify_pefile.h
@@ -8,22 +8,6 @@
 #include <crypto/pkcs7.h>
 #include <crypto/hash_info.h>
 
-struct pefile_context {
-	unsigned	header_size;
-	unsigned	image_checksum_offset;
-	unsigned	cert_dirent_offset;
-	unsigned	n_data_dirents;
-	unsigned	n_sections;
-	unsigned	certs_size;
-	unsigned	sig_offset;
-	unsigned	sig_len;
-	const struct section_header *secs;
-
-	/* PKCS#7 MS Individual Code Signing content */
-	const void	*digest;		/* Digest */
-	unsigned	digest_len;		/* Digest length */
-	const char	*digest_algo;		/* Digest algorithm */
-};
 
 #define kenter(FMT, ...)					\
 	pr_devel("==> %s("FMT")\n", __func__, ##__VA_ARGS__)
-- 
2.40.1

Re: [PATCH 0/1] x86/kexec: UKI support

[Luca Boccassi]

On Sat, 9 Sept 2023 at 17:19, Jan Hendrik Farr <kernel@jfarr.cc> wrote:
>
> Hello,
>
> this patch implements UKI support for kexec_file_load. It will require support
> in the kexec-tools userspace utility. For testing purposes the following can be used:
> https://github.com/Cydox/kexec-test/
>
> There has been discussion on this topic in an issue on GitHub that is linked below
> for reference.
>
>
> Some links:
> - Related discussion: https://github.com/systemd/systemd/issues/28538
> - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
>
> Jan Hendrik Farr (1):
>   x86/kexec: UKI support

Hi,

Thanks for working on this, it looks super useful and at Microsoft we
are definitely interested in it, as we rely heavily on kexec and we
are looking to switch to UKIs.

I had a quick look, two comments:

- the cmdline section is actually optional, just like it's optional to
pass it on a traditional kexec load, so it should be used if present,
but skipped if not
- the dtb section also is optional but supported, and given kexec
supports loading a new dtb I think this change should support it too
immediately. Moreover, we are adding support for multiple DTBs in a
single UKI (by simply having multiple .dtb sections, and picking the
one that matches the firmware), as a stretch goal would be nice to
support that too, but it can also come later
- also what would it take to support arm64, which is the other major UEFI arch?

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

> - the cmdline section is actually optional, just like it's optional to
> pass it on a traditional kexec load, so it should be used if present,
> but skipped if not

Should be an easy fix.

This should be updated in the UKI documentation as for other optional sections it's explicitly marked. I assumed that in the case of an empty cmdline the section would still be included but empty. Or is that semantically different: empty .cmdline section -> no cmdline allowed, no .cmdline section -> user can specify cmdline in bootloader?

> - the dtb section also is optional but supported, and given kexec
> supports loading a new dtb I think this change should support it too
> immediately.  Moreover, we are adding support for multiple DTBs in a
> single UKI (by simply having multiple .dtb sections, and picking the
> one that matches the firmware), as a stretch goal would be nice to
> support that too, but it can also come later

As far as I know dtb is not supported by kexec_file_load at all at the moment. Maybe someone here knows more about this. I'll look into it.

> - also what would it take to support arm64, which is the other major UEFI arch?

I'm not familiar with UEFI on arm64, but I can certainly look into it. I assume it's not gonna be that difficult.

Re: [PATCH 0/1] x86/kexec: UKI support

[Luca Boccassi]

On Sat, 9 Sept 2023 at 18:58, Jan Hendrik Farr <kernel@jfarr.cc> wrote:
>
> > - the cmdline section is actually optional, just like it's optional to
> > pass it on a traditional kexec load, so it should be used if present,
> > but skipped if not
>
> Should be an easy fix.
>
> This should be updated in the UKI documentation as for other optional sections it's explicitly marked. I assumed that in the case of an empty cmdline the section would still be included but empty. Or is that semantically different: empty .cmdline section -> no cmdline allowed, no .cmdline section -> user can specify cmdline in bootloader?

Yeah the doc is wrong, tools handle it as optional, fixing the docs:
https://github.com/uapi-group/specifications/pull/72

> > - the dtb section also is optional but supported, and given kexec
> > supports loading a new dtb I think this change should support it too
> > immediately.  Moreover, we are adding support for multiple DTBs in a
> > single UKI (by simply having multiple .dtb sections, and picking the
> > one that matches the firmware), as a stretch goal would be nice to
> > support that too, but it can also come later
>
> As far as I know dtb is not supported by kexec_file_load at all at the moment. Maybe someone here knows more about this. I'll look into it.

Not sure about the specific syscalls, but I definitely use kexec
--load --dtb on arm64 with existing released versions of
kernel/tooling

> > - also what would it take to support arm64, which is the other major UEFI arch?
>
> I'm not familiar with UEFI on arm64, but I can certainly look into it. I assume it's not gonna be that difficult.

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

>> > - the dtb section also is optional but supported, and given kexec
>> > supports loading a new dtb I think this change should support it too
>> > immediately.  Moreover, we are adding support for multiple DTBs in a
>> > single UKI (by simply having multiple .dtb sections, and picking the
>> > one that matches the firmware), as a stretch goal would be nice to
>> > support that too, but it can also come later
>>
>> As far as I know dtb is not supported by kexec_file_load at all at the moment. Maybe someone here knows more about this. I'll look into it.
>
> Not sure about the specific syscalls, but I definitely use kexec
> --load --dtb on arm64 with existing released versions of
> kernel/tooling

Ok, it looks like this is supported and should be doable (on arm64, not for x86). When I was looking around the source code yesterday I forgot that my LSP had indexed the kernel for x86 and not arm64.

>> > - also what would it take to support arm64, which is the other major UEFI arch?
>>
>> I'm not familiar with UEFI on arm64, but I can certainly look into it. I assume it's not gonna be that difficult.

I'll work on a separate patch for UKI support on arm64 (with .dtb section support). I think it's best to keep this x86 patch separate from that. I have a v2 patch for x86 put together with the small fix for the optional .cmdline section + a few other tweaks that I'll submit shortly.

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Sat Sep 9, 2023 at 7:18 PM EEST, Jan Hendrik Farr wrote:
> Hello,
>
> this patch implements UKI support for kexec_file_load. It will require support
> in the kexec-tools userspace utility. For testing purposes the following can be used:
> https://github.com/Cydox/kexec-test/
>
> There has been discussion on this topic in an issue on GitHub that is linked below
> for reference.
>
>
> Some links:
> - Related discussion: https://github.com/systemd/systemd/issues/28538
> - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
>
> Jan Hendrik Farr (1):
>   x86/kexec: UKI support
>
>  arch/x86/include/asm/kexec-uki.h       |   7 ++
>  arch/x86/include/asm/parse_pefile.h    |  32 +++++++
>  arch/x86/kernel/Makefile               |   2 +
>  arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
>  arch/x86/kernel/machine_kexec_64.c     |   2 +
>  arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
>  crypto/asymmetric_keys/mscode_parser.c |   2 +-
>  crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
>  crypto/asymmetric_keys/verify_pefile.h |  16 ----
>  9 files changed, 278 insertions(+), 116 deletions(-)
>  create mode 100644 arch/x86/include/asm/kexec-uki.h
>  create mode 100644 arch/x86/include/asm/parse_pefile.h
>  create mode 100644 arch/x86/kernel/kexec-uki.c
>  create mode 100644 arch/x86/kernel/parse_pefile.c
>
> -- 
> 2.40.1

What the heck is UKI?

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

> What the heck is UKI?

UKI (Unified Kernel Image) is the kernel image + initrd + cmdline (+ some other optional stuff) all packaged up together as one EFI application.

This EFI application can then be launched directly by the UEFI without the need for any additional stuff (or by systemd-boot). It's all self contained. One benefit is that this is a convenient way to distribute kernels all in one file. Another benefit is that the whole combination of kernel image, initrd, and cmdline can all be signed together so only that particular combination can be executed if you are using secure boot.

The format itself is rather simple. It's just a PE file (as required by the UEFI spec) that contains a small stub application in the .text, .data, etc sections that is responsible for invoking the contained kernel and initrd with the contained cmdline. The kernel image is placed into a .kernel section, the initrd into a .initrd section, and the cmdline into a .cmdline section in the PE executable.

If we want to kexec a UKI we could obviously just have userspace pick it apart and kexec it like normal. However in lockdown mode this will only work if you sign the kernel image that is contained inside the UKI. The problem with that is that anybody can then grab that signed kernel and launch it with any initrd or cmdline. So instead this patch makes the kernel do the work instead. The kernel verifies the signature on the entire UKI and then passes its components on to the normal kexec bzimage loader.

Useful Links:
UKI format documentation: https://uapi-group.org/specifications/specs/unified_kernel_image/
Arch wiki: https://wiki.archlinux.org/title/Unified_kernel_image
Fedora UKI support: https://fedoraproject.org/wiki/Changes/Unified_Kernel_Support_Phase_1

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Tue Sep 12, 2023 at 1:54 AM EEST, Jan Hendrik Farr wrote:
> > What the heck is UKI?
>
> UKI (Unified Kernel Image) is the kernel image + initrd + cmdline (+
> some other optional stuff) all packaged up together as one EFI
> application.
>
> This EFI application can then be launched directly by the UEFI without
> the need for any additional stuff (or by systemd-boot). It's all self
> contained. One benefit is that this is a convenient way to distribute
> kernels all in one file. Another benefit is that the whole combination
> of kernel image, initrd, and cmdline can all be signed together so
> only that particular combination can be executed if you are using
> secure boot.

Is this also for generic purpose distributions? I mean it is not
uncommon having to tweak the command-line in a workstation.

> The format itself is rather simple. It's just a PE file (as required
> by the UEFI spec) that contains a small stub application in the .text,
> .data, etc sections that is responsible for invoking the contained
> kernel and initrd with the contained cmdline. The kernel image is
> placed into a .kernel section, the initrd into a .initrd section, and
> the cmdline into a .cmdline section in the PE executable.

How does this interact with the existing EFI stub support in linux?

> If we want to kexec a UKI we could obviously just have userspace pick
> it apart and kexec it like normal. However in lockdown mode this will
> only work if you sign the kernel image that is contained inside the
> UKI. The problem with that is that anybody can then grab that signed
> kernel and launch it with any initrd or cmdline. So instead this patch
> makes the kernel do the work instead. The kernel verifies the
> signature on the entire UKI and then passes its components on to the
> normal kexec bzimage loader.
>
> Useful Links:
> UKI format documentation: https://uapi-group.org/specifications/specs/unified_kernel_image/
> Arch wiki: https://wiki.archlinux.org/title/Unified_kernel_image
> Fedora UKI support: https://fedoraproject.org/wiki/Changes/Unified_Kernel_Support_Phase_1

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

On Tue, Sep 12, 2023, at 12:33 PM, Jarkko Sakkinen wrote:
> On Tue Sep 12, 2023 at 1:54 AM EEST, Jan Hendrik Farr wrote:
>> > What the heck is UKI?
>>
>> UKI (Unified Kernel Image) is the kernel image + initrd + cmdline (+
>> some other optional stuff) all packaged up together as one EFI
>> application.
>>
>> This EFI application can then be launched directly by the UEFI without
>> the need for any additional stuff (or by systemd-boot). It's all self
>> contained. One benefit is that this is a convenient way to distribute
>> kernels all in one file. Another benefit is that the whole combination
>> of kernel image, initrd, and cmdline can all be signed together so
>> only that particular combination can be executed if you are using
>> secure boot.
>
> Is this also for generic purpose distributions? I mean it is not
> uncommon having to tweak the command-line in a workstation.

This is for generic purpose distributions. See fedora's planned rollout: https://fedoraproject.org/wiki/Changes/Unified_Kernel_Support_Phase_1
Or Arch: https://wiki.archlinux.org/title/Unified_kernel_image

There are UKI addons that help you achieve this. These are additional PE files that contain for example additional cmdline parameters. On a generic Linux distro doing secure boot you'd generally use shim, could enroll MOK and use that to sign an addon for your machine.

This patch currently does not support addons. The plan would be to support them in the future though.

I personally always run my own compiled kernel and build a UKI from that so I can obviously tweak the cmdline that way and sign the UKI with my own secure boot key.

>> The format itself is rather simple. It's just a PE file (as required
>> by the UEFI spec) that contains a small stub application in the .text,
>> .data, etc sections that is responsible for invoking the contained
>> kernel and initrd with the contained cmdline. The kernel image is
>> placed into a .kernel section, the initrd into a .initrd section, and
>> the cmdline into a .cmdline section in the PE executable.
>
> How does this interact with the existing EFI stub support in linux?

It doesn't. During normal boot of a UKI the stub in it is used (systemd-stub, see: https://www.freedesktop.org/software/systemd/man/systemd-stub.html). The kernel's own EFI stub will still be in the binary inside the .linux section but not used.

Now in this patch (also see v2 I already posted) obviously non of the stubs are used.

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Tue Sep 12, 2023 at 6:32 PM EEST, Jan Hendrik Farr wrote:
> >> The format itself is rather simple. It's just a PE file (as required
> >> by the UEFI spec) that contains a small stub application in the .text,
> >> .data, etc sections that is responsible for invoking the contained
> >> kernel and initrd with the contained cmdline. The kernel image is
> >> placed into a .kernel section, the initrd into a .initrd section, and
> >> the cmdline into a .cmdline section in the PE executable.
> >
> > How does this interact with the existing EFI stub support in linux?
>
> It doesn't. During normal boot of a UKI the stub in it is used
> (systemd-stub, see:
> https://www.freedesktop.org/software/systemd/man/systemd-stub.html).
> The kernel's own EFI stub will still be in the binary inside the
> .linux section but not used.

What sort of bottleneck does the EFI stub have so that we need yet
another envelope?

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

> What sort of bottleneck does the EFI stub have so that we need yet
> another envelope?

Well I can come up with a few advantages of UKI compared to normal bzImage with builtin initrd and cmdline.

1. You already identified this one. Using addons to adjust your cmdline
2. I can use my normal initramfs generation tooling. Just install my compiled kernel, my distros install script will generate the initramfs. Then I package it up as a UKI. This will be a lot more awkward with a builtin initrd.
3. Measured boot. You can place PCR signatures in the UKI using systemd-measure. This will sign the expected PCR values for booting this UKI. I think with normal bzImage this will be a lot more difficult. If you place those PCR signatures in the builtin initrd this will change the kernel image which means now the values you signed no longer match (depending on how you measure the kernel; I don't think the normal EFI stub even measures the kernel in first place, but I could be mistaken about this)
4. UKIs are automatically recognized by systemd-boot

There's probably more reasons. The main reason for me to go with UKIs initially was the good tooling around them.

You could probably overcome some of these drawbacks in the default kernel EFI stub. For example it could also get a place to put signed PCR values. And it could also do TPM measurements. However in the process all you're doing is rebuilding what already exists today in systemd-stub and the tooling around UKIs.

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Tue Sep 12, 2023 at 9:56 PM EEST, Jan Hendrik Farr wrote:
> > What sort of bottleneck does the EFI stub have so that we need yet
> > another envelope?
>
> Well I can come up with a few advantages of UKI compared to normal bzImage with builtin initrd and cmdline.
>
> 1. You already identified this one. Using addons to adjust your cmdline

It is not a benefit as this is already possible today.

> 2. I can use my normal initramfs generation tooling. Just install my
> compiled kernel, my distros install script will generate the
> initramfs. Then I package it up as a UKI. This will be a lot more
> awkward with a builtin initrd.
> 3. Measured boot. You can place PCR signatures in the UKI using
> systemd-measure. This will sign the expected PCR values for booting
> this UKI. I think with normal bzImage this will be a lot more
> difficult. If you place those PCR signatures in the builtin initrd
> this will change the kernel image which means now the values you
> signed no longer match (depending on how you measure the kernel; I
> don't think the normal EFI stub even measures the kernel in first
> place, but I could be mistaken about this)
> 4. UKIs are automatically recognized by systemd-boot

These are sort of "tautological" arguments. There must be some
objective reasons why this architecture was chosen instead of
other (i.e. using what already pre-exists).

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

> These are sort of "tautological" arguments. There must be some
> objective reasons why this architecture was chosen instead of
> other (i.e. using what already pre-exists).

You mean like your argument that the same can already be achieved with the normal EFI stub and builin initrd/cmdline? ;)

I think only reasons #4 and the last paragraph in me response relate to it being pre-existing. The other reasons are actual limitations with the normal EFI stub setup. Doesn't mean that they can't be overcome, but UKIs work.

I'm not sure what the initial reasons where for coming up with this architecture were, I was not involved.

What I can tell you is that right now it is a format that has practical advantages and that there are generic mainstream distros looking to adopt it. So having the capability to kexec them is gonna come in handy.

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

> These are sort of "tautological" arguments. There must be some
> objective reasons why this architecture was chosen instead of
> other (i.e. using what already pre-exists).

I think I misunderstood you in my earlier reply. I do not understand in what way you think my arguments are tautological. Can you elaborate?

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Tue Sep 12, 2023 at 11:49 PM EEST, Jan Hendrik Farr wrote:
>
> > These are sort of "tautological" arguments. There must be some
> > objective reasons why this architecture was chosen instead of
> > other (i.e. using what already pre-exists).
>
> I think I misunderstood you in my earlier reply. I do not understand
> in what way you think my arguments are tautological. Can you
> elaborate?

current Linux kernel has these features *already* in
place:

1. CONFIG_EFI_STUB
2. CONFIG_CMDLINE
3. CONFIG_INITRAMFS_SOURCE
4. Secure boot with MOK keys and .machine keyring to manage them.

Given that every single feature in IKU does exists in some form
in the Linux kernel, I think it is fair to ask why scrape away
this all existing science and reinvent the wheel?

If your reponse is "systemd", it is a tautological answerk, i.e.
same as sayig that "it is good because it is good". Not very
motivating.

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Jan Hendrik Farr]

On Wed, Sep 13, 2023, at 4:45 PM, Jarkko Sakkinen wrote:
> On Tue Sep 12, 2023 at 11:49 PM EEST, Jan Hendrik Farr wrote:
>>
>> > These are sort of "tautological" arguments. There must be some
>> > objective reasons why this architecture was chosen instead of other
>> > (i.e. using what already pre-exists).
>>
>> I think I misunderstood you in my earlier reply. I do not understand
>> in what way you think my arguments are tautological. Can you
>> elaborate?
>
> current Linux kernel has these features *already* in place:
>
> 1. CONFIG_EFI_STUB
> 2. CONFIG_CMDLINE
> 3. CONFIG_INITRAMFS_SOURCE
> 4. Secure boot with MOK keys and .machine keyring to manage them.

Well, you also have to add
5. CONFIG_CMDLINE_OVERRIDE
6. CONFIG_INITRAMFS_FORCE

Otherwise the bootloader can supply an unsinged initramfs/cmdline and
the kernel will use them.

And then you do not get all the features. One of your earlier responses
asks how a user might change the cmdline with UKIs. With UKIs all they
have to do is create a small addon file and sign that (with their MOK if
they are using a generic distro with shim, instead of using their own
secure boot keys). With the bzImage alternative they would have to
recompile the kernel. This was reason #1.

Also what about #3? How would you pass PCR signatures using the normal
EFI stub / bzImage?

I can see how #4 is kinda tautological, but please don't dismiss the
other arguments without even responding.

> Given that every single feature in IKU does exists in some form in the
> Linux kernel, I think it is fair to ask why scrape away this all
> existing science and reinvent the wheel?

No, not every single feature of *UKI*s exist in the linux kernel today.

> If your reponse is "systemd", it is a tautological answerk, i.e. same
> as sayig that "it is good because it is good". Not very motivating.

Again for #4 I see the point, but what about #1 , and #3 (#2 is not that
important, tooling can be fixed)? Also, do you see how your argument is
basically: "The current way is better because it is the current way."
I'm not trying to be snarky. But I'm coming from the perspective of a
user that actually experienced a problem (not being able to kexec my
UKIs) and trying to come up with a fix. I'm not trying to push something
for the heck of it.

Also the UKI spec at this point is not ready for the kernel, so this is
not going in anyways right now. See the discussion on v2:
https://lore.kernel.org/kexec/ZP+41JvEFjsnEG19@MiWiFi-R3L-srv/T/#t

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Wed Sep 13, 2023 at 6:07 PM EEST, Jan Hendrik Farr wrote:
> On Wed, Sep 13, 2023, at 4:45 PM, Jarkko Sakkinen wrote:
> > On Tue Sep 12, 2023 at 11:49 PM EEST, Jan Hendrik Farr wrote:
> >>
> >> > These are sort of "tautological" arguments. There must be some
> >> > objective reasons why this architecture was chosen instead of other
> >> > (i.e. using what already pre-exists).
> >>
> >> I think I misunderstood you in my earlier reply. I do not understand
> >> in what way you think my arguments are tautological. Can you
> >> elaborate?
> >
> > current Linux kernel has these features *already* in place:
> >
> > 1. CONFIG_EFI_STUB
> > 2. CONFIG_CMDLINE
> > 3. CONFIG_INITRAMFS_SOURCE
> > 4. Secure boot with MOK keys and .machine keyring to manage them.
>
> Well, you also have to add
> 5. CONFIG_CMDLINE_OVERRIDE
> 6. CONFIG_INITRAMFS_FORCE
>
> Otherwise the bootloader can supply an unsinged initramfs/cmdline and
> the kernel will use them.
>
> And then you do not get all the features. One of your earlier responses
> asks how a user might change the cmdline with UKIs. With UKIs all they
> have to do is create a small addon file and sign that (with their MOK if
> they are using a generic distro with shim, instead of using their own
> secure boot keys). With the bzImage alternative they would have to
> recompile the kernel. This was reason #1.
>
> Also what about #3? How would you pass PCR signatures using the normal
> EFI stub / bzImage?

I did not notice anything in the description about PCRs but this
counter-question does really enlighten what I've been trying to say. You
should take time to explain where's the gain, and why it makes sense for
the users.

It does not really make sense for me to "defend" anything if I'm against
something that I don't understand what it is and is capable of and such
and so forth. I don't really care if e.g. systemd is using it, if it
does not make me understand the topic better. It is then just argument
by authority, which not a real argument in the first place.

Linking URL to the specification is good enough for *details* but it
does not save you from trouble of painting the picture what we are
looking at.

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Lennart Poettering]

On Mi, 13.09.23 17:45, Jarkko Sakkinen (jarkko@kernel.org) wrote:

> On Tue Sep 12, 2023 at 11:49 PM EEST, Jan Hendrik Farr wrote:
> >
> > > These are sort of "tautological" arguments. There must be some
> > > objective reasons why this architecture was chosen instead of
> > > other (i.e. using what already pre-exists).
> >
> > I think I misunderstood you in my earlier reply. I do not understand
> > in what way you think my arguments are tautological. Can you
> > elaborate?
>
> current Linux kernel has these features *already* in
> place:
>
> 1. CONFIG_EFI_STUB
> 2. CONFIG_CMDLINE
> 3. CONFIG_INITRAMFS_SOURCE
> 4. Secure boot with MOK keys and .machine keyring to manage them.
>
> Given that every single feature in IKU does exists in some form
> in the Linux kernel, I think it is fair to ask why scrape away
> this all existing science and reinvent the wheel?

Nah, systemd-stub does considerably more than what you list above.

1. It measures the components of the UKI separately into PCR 11, 12,
   13, which makes the mesaurements predictable, and allows vendors to
   provide a signed PCR policy with can be used to unlock TPM2 secrets
   that ause a PolicyAuthorize policy. This is a fundamental
   improvement over mechanisms that bind to literal PCR values, since
   the "brittleness" goes away.
2. That said signed PCR policy is included in the UKI in another PE
   section, that is made available to userspace.
3. If you like it brings a boot splash to screen before passing
   control off to the kernel, which is also contained
4. It can contain a devicetree blob, which it will setup for the
   kernel it spawns
5. There's a random seed maintained by systemd-stub in the ESP that is
   updated and passed to the kernel, which includes in in the pool.
6. It picks up "credentials" (which are TPM protected, encrypted,
   authenticated supported by systemd) that can be used to securely
   parameterize the invoked system from the backing fs (i.e. the
   ESP). Similar it can pick up sysext images (which is another
   systemd thing, i.e. dm-verity protected, signed disk images which
   can extend the initrd and the host, by being overlayed on /usr).
7. It picks up "add-ons" -- which are PE binaries that actually contain
   no code, but are SecureBoot signed/shim signed "mules" for carrying
   addition kernel cmdlines, devictree blobs (and maybe in future
   initrds) that allow some form of modularity in the UKI model.

And there's more. This is just off the top of my head.

Now, I can totally see you personally might not need any of this
stuff, fine, but a claim that this stuff is redundant is just bogus.

Afaics all big distributions are preparing to providing UKIs
soonishly. It would be fantastic if kexec would just work with this
too, and the dissection would be done on the kernel side instead of
userspace.

Lennart

--
Lennart Poettering, Berlin

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Thu Sep 14, 2023 at 12:11 PM EEST, Lennart Poettering wrote:
> On Mi, 13.09.23 17:45, Jarkko Sakkinen (jarkko@kernel.org) wrote:
>
> > On Tue Sep 12, 2023 at 11:49 PM EEST, Jan Hendrik Farr wrote:
> > >
> > > > These are sort of "tautological" arguments. There must be some
> > > > objective reasons why this architecture was chosen instead of
> > > > other (i.e. using what already pre-exists).
> > >
> > > I think I misunderstood you in my earlier reply. I do not understand
> > > in what way you think my arguments are tautological. Can you
> > > elaborate?
> >
> > current Linux kernel has these features *already* in
> > place:
> >
> > 1. CONFIG_EFI_STUB
> > 2. CONFIG_CMDLINE
> > 3. CONFIG_INITRAMFS_SOURCE
> > 4. Secure boot with MOK keys and .machine keyring to manage them.
> >
> > Given that every single feature in IKU does exists in some form
> > in the Linux kernel, I think it is fair to ask why scrape away
> > this all existing science and reinvent the wheel?
>
> Nah, systemd-stub does considerably more than what you list above.
>
> 1. It measures the components of the UKI separately into PCR 11, 12,
>    13, which makes the mesaurements predictable, and allows vendors to
>    provide a signed PCR policy with can be used to unlock TPM2 secrets
>    that ause a PolicyAuthorize policy. This is a fundamental
>    improvement over mechanisms that bind to literal PCR values, since
>    the "brittleness" goes away.

I guess this is an appropriate reference:

https://uapi-group.org/specifications/specs/linux_tpm_pcr_registry/

I quickly checked what sort of use cases we have for PCRs in the
kernel. I could spot one:

https://www.kernel.org/doc/html/v6.5/security/keys/trusted-encrypted.html

Generally, my only concern here is potential conflicts with user space
extending the same PCRs as systemd does.

Since this all is only used for boot phase I guess this should not be
an issue, right?

> 2. That said signed PCR policy is included in the UKI in another PE
>    section, that is made available to userspace.
> 3. If you like it brings a boot splash to screen before passing
>    control off to the kernel, which is also contained
> 4. It can contain a devicetree blob, which it will setup for the
>    kernel it spawns
> 5. There's a random seed maintained by systemd-stub in the ESP that is
>    updated and passed to the kernel, which includes in in the pool.
> 6. It picks up "credentials" (which are TPM protected, encrypted,
>    authenticated supported by systemd) that can be used to securely
>    parameterize the invoked system from the backing fs (i.e. the
>    ESP). Similar it can pick up sysext images (which is another
>    systemd thing, i.e. dm-verity protected, signed disk images which
>    can extend the initrd and the host, by being overlayed on /usr).
> 7. It picks up "add-ons" -- which are PE binaries that actually contain
>    no code, but are SecureBoot signed/shim signed "mules" for carrying
>    addition kernel cmdlines, devictree blobs (and maybe in future
>    initrds) that allow some form of modularity in the UKI model.
>
> And there's more. This is just off the top of my head.
>
> Now, I can totally see you personally might not need any of this
> stuff, fine, but a claim that this stuff is redundant is just bogus.

Backing story was missing completely. Please add this reasoning to the
patch set in some form (cover letter and possibly some patch
descriptions). It is bogus without proper context, which is totally
different than my personal use. The response that I received was more
related to personal use.

It took quite many emails to learn about PCR usage. IMHO that should
have been told here so that we can then e.g. inspect possible conflicts
etc.

> Afaics all big distributions are preparing to providing UKIs
> soonishly. It would be fantastic if kexec would just work with this
> too, and the dissection would be done on the kernel side instead of
> userspace.

I don't see any existential reasons anymore not to include this to the
mainline but it takes what it takes in terms of time span of course.

>
> Lennart
>
> --
> Lennart Poettering, Berlin

BR, Jarkko

Re: [PATCH 0/1] x86/kexec: UKI support

[Lennart Poettering]

On Di, 12.09.23 17:32, Jan Hendrik Farr (kernel@jfarr.cc) wrote:

> >> The format itself is rather simple. It's just a PE file (as required
> >> by the UEFI spec) that contains a small stub application in the .text,
> >> .data, etc sections that is responsible for invoking the contained
> >> kernel and initrd with the contained cmdline. The kernel image is
> >> placed into a .kernel section, the initrd into a .initrd section, and
> >> the cmdline into a .cmdline section in the PE executable.
> >
> > How does this interact with the existing EFI stub support in
> > linux?
>
> It doesn't. During normal boot of a UKI the stub in it is used
> (systemd-stub, see:
> https://www.freedesktop.org/software/systemd/man/systemd-stub.html). The
> kernel's own EFI stub will still be in the binary inside the .linux
> section but not used.

That's not true actually, if the inner kernel supports the EFI stub
then systemd-stub actually defers to that for kernel execution. It's
more portable that way, since the kernel then deals with the
differences in the boot protocol on different architectures.

Lennart

--
Lennart Poettering, Berlin

Re: [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Thu Sep 14, 2023 at 11:48 AM EEST, Lennart Poettering wrote:
> On Di, 12.09.23 17:32, Jan Hendrik Farr (kernel@jfarr.cc) wrote:
>
> > >> The format itself is rather simple. It's just a PE file (as required
> > >> by the UEFI spec) that contains a small stub application in the .text,
> > >> .data, etc sections that is responsible for invoking the contained
> > >> kernel and initrd with the contained cmdline. The kernel image is
> > >> placed into a .kernel section, the initrd into a .initrd section, and
> > >> the cmdline into a .cmdline section in the PE executable.
> > >
> > > How does this interact with the existing EFI stub support in
> > > linux?
> >
> > It doesn't. During normal boot of a UKI the stub in it is used
> > (systemd-stub, see:
> > https://www.freedesktop.org/software/systemd/man/systemd-stub.html). The
> > kernel's own EFI stub will still be in the binary inside the .linux
> > section but not used.
>
> That's not true actually, if the inner kernel supports the EFI stub
> then systemd-stub actually defers to that for kernel execution. It's
> more portable that way, since the kernel then deals with the
> differences in the boot protocol on different architectures.

OK, that's nice.

> Lennart
>
> --
> Lennart Poettering, Berlin

BR, Jarkko

Re: [systemd-devel] [PATCH 0/1] x86/kexec: UKI support

[Neal Gompa]

On Mon, Sep 11, 2023 at 7:15 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Sat Sep 9, 2023 at 7:18 PM EEST, Jan Hendrik Farr wrote:
> > Hello,
> >
> > this patch implements UKI support for kexec_file_load. It will require support
> > in the kexec-tools userspace utility. For testing purposes the following can be used:
> > https://github.com/Cydox/kexec-test/
> >
> > There has been discussion on this topic in an issue on GitHub that is linked below
> > for reference.
> >
> >
> > Some links:
> > - Related discussion: https://github.com/systemd/systemd/issues/28538
> > - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
> >
> > Jan Hendrik Farr (1):
> >   x86/kexec: UKI support
> >
> >  arch/x86/include/asm/kexec-uki.h       |   7 ++
> >  arch/x86/include/asm/parse_pefile.h    |  32 +++++++
> >  arch/x86/kernel/Makefile               |   2 +
> >  arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
> >  arch/x86/kernel/machine_kexec_64.c     |   2 +
> >  arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
> >  crypto/asymmetric_keys/mscode_parser.c |   2 +-
> >  crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
> >  crypto/asymmetric_keys/verify_pefile.h |  16 ----
> >  9 files changed, 278 insertions(+), 116 deletions(-)
> >  create mode 100644 arch/x86/include/asm/kexec-uki.h
> >  create mode 100644 arch/x86/include/asm/parse_pefile.h
> >  create mode 100644 arch/x86/kernel/kexec-uki.c
> >  create mode 100644 arch/x86/kernel/parse_pefile.c
> >
> > --
> > 2.40.1
>
> What the heck is UKI?

Unified Kernel Images. More details available here:
https://uapi-group.org/specifications/specs/unified_kernel_image/

It's a way of creating initramfs-style images as fully generic,
reproducible images that can be built server-side.

It is a requirement for creating locked down Linux devices for
appliances that can be tamper-resistant too.




--
真実はいつも一つ！/ Always, there's only one truth!

Re: [systemd-devel] [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Tue Sep 12, 2023 at 2:20 AM EEST, Neal Gompa wrote: > On Mon, Sep 11, 2023 at 7:15 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > On Sat Sep 9, 2023 at 7:18 PM EEST, Jan Hendrik Farr wrote:
> > > Hello,
> > >
> > > this patch implements UKI support for kexec_file_load. It will require support
> > > in the kexec-tools userspace utility. For testing purposes the following can be used:
> > > https://github.com/Cydox/kexec-test/
> > >
> > > There has been discussion on this topic in an issue on GitHub that is linked below
> > > for reference.
> > >
> > >
> > > Some links:
> > > - Related discussion: https://github.com/systemd/systemd/issues/28538
> > > - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
> > >
> > > Jan Hendrik Farr (1):
> > >   x86/kexec: UKI support
> > >
> > >  arch/x86/include/asm/kexec-uki.h       |   7 ++
> > >  arch/x86/include/asm/parse_pefile.h    |  32 +++++++
> > >  arch/x86/kernel/Makefile               |   2 +
> > >  arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
> > >  arch/x86/kernel/machine_kexec_64.c     |   2 +
> > >  arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
> > >  crypto/asymmetric_keys/mscode_parser.c |   2 +-
> > >  crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
> > >  crypto/asymmetric_keys/verify_pefile.h |  16 ----
> > >  9 files changed, 278 insertions(+), 116 deletions(-)
> > >  create mode 100644 arch/x86/include/asm/kexec-uki.h
> > >  create mode 100644 arch/x86/include/asm/parse_pefile.h
> > >  create mode 100644 arch/x86/kernel/kexec-uki.c
> > >  create mode 100644 arch/x86/kernel/parse_pefile.c
> > >
> > > --
> > > 2.40.1
> >
> > What the heck is UKI?
>
> Unified Kernel Images. More details available here:
> https://uapi-group.org/specifications/specs/unified_kernel_image/
>
> It's a way of creating initramfs-style images as fully generic,
> reproducible images that can be built server-side.

You can build today a kernel with these compiled in:

1. EFI stub
2. initeramfs
3. cmdline

Why another way (and label 'UKI') for a pre-existing feature?

BR, Jarkko

Re: [systemd-devel] [PATCH 0/1] x86/kexec: UKI support

[Dimitri John Ledkov]

On Tue, 12 Sept 2023 at 11:38, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Tue Sep 12, 2023 at 2:20 AM EEST, Neal Gompa wrote: > On Mon, Sep 11, 2023 at 7:15 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > On Sat Sep 9, 2023 at 7:18 PM EEST, Jan Hendrik Farr wrote:
> > > > Hello,
> > > >
> > > > this patch implements UKI support for kexec_file_load. It will require support
> > > > in the kexec-tools userspace utility. For testing purposes the following can be used:
> > > > https://github.com/Cydox/kexec-test/
> > > >
> > > > There has been discussion on this topic in an issue on GitHub that is linked below
> > > > for reference.
> > > >
> > > >
> > > > Some links:
> > > > - Related discussion: https://github.com/systemd/systemd/issues/28538
> > > > - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
> > > >
> > > > Jan Hendrik Farr (1):
> > > >   x86/kexec: UKI support
> > > >
> > > >  arch/x86/include/asm/kexec-uki.h       |   7 ++
> > > >  arch/x86/include/asm/parse_pefile.h    |  32 +++++++
> > > >  arch/x86/kernel/Makefile               |   2 +
> > > >  arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
> > > >  arch/x86/kernel/machine_kexec_64.c     |   2 +
> > > >  arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
> > > >  crypto/asymmetric_keys/mscode_parser.c |   2 +-
> > > >  crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
> > > >  crypto/asymmetric_keys/verify_pefile.h |  16 ----
> > > >  9 files changed, 278 insertions(+), 116 deletions(-)
> > > >  create mode 100644 arch/x86/include/asm/kexec-uki.h
> > > >  create mode 100644 arch/x86/include/asm/parse_pefile.h
> > > >  create mode 100644 arch/x86/kernel/kexec-uki.c
> > > >  create mode 100644 arch/x86/kernel/parse_pefile.c
> > > >
> > > > --
> > > > 2.40.1
> > >
> > > What the heck is UKI?
> >
> > Unified Kernel Images. More details available here:
> > https://uapi-group.org/specifications/specs/unified_kernel_image/
> >
> > It's a way of creating initramfs-style images as fully generic,
> > reproducible images that can be built server-side.
>
> You can build today a kernel with these compiled in:
>
> 1. EFI stub
> 2. initeramfs
> 3. cmdline
>
> Why another way (and label 'UKI') for a pre-existing feature?
>

In Ubuntu, we have considered to use the existing kernel features
before going off to use UKI. Here are some of the reasons why we
didn't opt to use the kernel builtin things:
1) we wanted to have ability to have TPM measured kernel commandline
performed before kernel is being executed, which is what sd-stub
provides us
2) we wanted to have ability to update / regenerate initrd, without
rebuilding kernel. Thus whenever userspace in the initrd needs
updating, we can generate new initrd for existing kernel build, create
new kernel.efi, whilst using existing .linux / vmlinuz build. I don't
believe it is currently trivial to relink vmlinuz with builtin initrd.
3) licensing wise it was not clear if initrd has to be GPLv2
compatible when linked inside vmlinuz, or if it can contain GPLv3 /
LGPLv3 userspace code - with UKI it is believed unambigiously true,
because vmlinuz boots by itself standalone and is compiled separately
of the UKI.
4) we wanted to have ability to override cmdline via kernel args
without secureboot, and use stock cmdline args under secureboot, to
allow debugging & production behaviour from a single signed kernel.efi
(that was custom development, and could be done in the stock vmlinuz
too).
5) obvious mention, the intention here is to have TPM PCR measurements
and Secureboot signature for vmlinuz and initrd and cmdline and dtb.
There is otherwise no support for standalone signed initrd, cmdline,
dtb today. Nor does vendoring it into vmlinuz achieves this to the
same extent (and ease of predicting for sealing / resealing purposes).
6) in Ubuntu kernel.efi also has sbat section for targeted revocations
(discussed separately elsewhere)

Overall, it is mostly about flexibility to be able to reuse the same
initrd against multiple kernel builds, or update use multiple initrd
against the same kernel build. This is imho the biggest issue with
using initrd built-into the vmlinuz itself.
Resource wise, the initrd passed in via kernel.efi can be freed, as
far as I understand. I don't know if the one built-into the vmlinuz is
freeable.

Improving design to do something else instead of UKI would be
welcomed. Or for example improving the zimg linus upstream format to
be a partial or a valid UKI would help as well. For example, building
the kernel built-in initrd as a .initrd section that is replacable
would be nice, or allowing to preload zimg with .dtb or .cmdline
sections would help, and appropriately improve the linux efi stab to
do measurements and loading of .dtb / .initrd from itself would be
nice. Because then all the benefits / requirements described above
could be made available out of the box and be trivially updatable. The
biggest one being splitting out things into sections that can be
updated with objcopy.

> BR, Jarkko

-- 
okurrr,

Dimitri

Re: [systemd-devel] [PATCH 0/1] x86/kexec: UKI support

[Jarkko Sakkinen]

On Mon Sep 18, 2023 at 6:41 PM EEST, Dimitri John Ledkov wrote:
> On Tue, 12 Sept 2023 at 11:38, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Tue Sep 12, 2023 at 2:20 AM EEST, Neal Gompa wrote: > On Mon, Sep 11, 2023 at 7:15 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > On Sat Sep 9, 2023 at 7:18 PM EEST, Jan Hendrik Farr wrote:
> > > > > Hello,
> > > > >
> > > > > this patch implements UKI support for kexec_file_load. It will require support
> > > > > in the kexec-tools userspace utility. For testing purposes the following can be used:
> > > > > https://github.com/Cydox/kexec-test/
> > > > >
> > > > > There has been discussion on this topic in an issue on GitHub that is linked below
> > > > > for reference.
> > > > >
> > > > >
> > > > > Some links:
> > > > > - Related discussion: https://github.com/systemd/systemd/issues/28538
> > > > > - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
> > > > >
> > > > > Jan Hendrik Farr (1):
> > > > >   x86/kexec: UKI support
> > > > >
> > > > >  arch/x86/include/asm/kexec-uki.h       |   7 ++
> > > > >  arch/x86/include/asm/parse_pefile.h    |  32 +++++++
> > > > >  arch/x86/kernel/Makefile               |   2 +
> > > > >  arch/x86/kernel/kexec-uki.c            | 113 +++++++++++++++++++++++++
> > > > >  arch/x86/kernel/machine_kexec_64.c     |   2 +
> > > > >  arch/x86/kernel/parse_pefile.c         | 110 ++++++++++++++++++++++++
> > > > >  crypto/asymmetric_keys/mscode_parser.c |   2 +-
> > > > >  crypto/asymmetric_keys/verify_pefile.c | 110 +++---------------------
> > > > >  crypto/asymmetric_keys/verify_pefile.h |  16 ----
> > > > >  9 files changed, 278 insertions(+), 116 deletions(-)
> > > > >  create mode 100644 arch/x86/include/asm/kexec-uki.h
> > > > >  create mode 100644 arch/x86/include/asm/parse_pefile.h
> > > > >  create mode 100644 arch/x86/kernel/kexec-uki.c
> > > > >  create mode 100644 arch/x86/kernel/parse_pefile.c
> > > > >
> > > > > --
> > > > > 2.40.1
> > > >
> > > > What the heck is UKI?
> > >
> > > Unified Kernel Images. More details available here:
> > > https://uapi-group.org/specifications/specs/unified_kernel_image/
> > >
> > > It's a way of creating initramfs-style images as fully generic,
> > > reproducible images that can be built server-side.
> >
> > You can build today a kernel with these compiled in:
> >
> > 1. EFI stub
> > 2. initeramfs
> > 3. cmdline
> >
> > Why another way (and label 'UKI') for a pre-existing feature?
> >
>
> In Ubuntu, we have considered to use the existing kernel features
> before going off to use UKI. Here are some of the reasons why we
> didn't opt to use the kernel builtin things:
> 1) we wanted to have ability to have TPM measured kernel commandline
> performed before kernel is being executed, which is what sd-stub
> provides us

OK this does make a lot of sense.

> 2) we wanted to have ability to update / regenerate initrd, without
> rebuilding kernel. Thus whenever userspace in the initrd needs
> updating, we can generate new initrd for existing kernel build, create
> new kernel.efi, whilst using existing .linux / vmlinuz build. I don't
> believe it is currently trivial to relink vmlinuz with builtin initrd.
> 3) licensing wise it was not clear if initrd has to be GPLv2
> compatible when linked inside vmlinuz, or if it can contain GPLv3 /
> LGPLv3 userspace code - with UKI it is believed unambigiously true,
> because vmlinuz boots by itself standalone and is compiled separately
> of the UKI.

Right UKI wraps kernel and kernel is a "leaf object".

> 4) we wanted to have ability to override cmdline via kernel args
> without secureboot, and use stock cmdline args under secureboot, to
> allow debugging & production behaviour from a single signed kernel.efi
> (that was custom development, and could be done in the stock vmlinuz
> too).
> 5) obvious mention, the intention here is to have TPM PCR measurements
> and Secureboot signature for vmlinuz and initrd and cmdline and dtb.
> There is otherwise no support for standalone signed initrd, cmdline,
> dtb today. Nor does vendoring it into vmlinuz achieves this to the
> same extent (and ease of predicting for sealing / resealing purposes).

ok

> 6) in Ubuntu kernel.efi also has sbat section for targeted revocations
> (discussed separately elsewhere)
>
> Overall, it is mostly about flexibility to be able to reuse the same
> initrd against multiple kernel builds, or update use multiple initrd
> against the same kernel build. This is imho the biggest issue with
> using initrd built-into the vmlinuz itself.
> Resource wise, the initrd passed in via kernel.efi can be freed, as
> far as I understand. I don't know if the one built-into the vmlinuz is
> freeable.
>
> Improving design to do something else instead of UKI would be
> welcomed. Or for example improving the zimg linus upstream format to
> be a partial or a valid UKI would help as well. For example, building
> the kernel built-in initrd as a .initrd section that is replacable
> would be nice, or allowing to preload zimg with .dtb or .cmdline
> sections would help, and appropriately improve the linux efi stab to
> do measurements and loading of .dtb / .initrd from itself would be
> nice. Because then all the benefits / requirements described above
> could be made available out of the box and be trivially updatable. The
> biggest one being splitting out things into sections that can be
> updated with objcopy.

BR, Jarkko