LinuxLists.cc - [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

2020-09-05 01:32:21

by Lenny Szubowicz

[permalink] [raw]

Subject: [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

2020-09-05 01:35:48

by Lenny Szubowicz

[permalink] [raw]

Subject: [PATCH V2 3/3] integrity: Load certs from the EFI MOK config table

2020-09-05 01:35:51

by Lenny Szubowicz

[permalink] [raw]

Subject: [PATCH V2 1/3] efi: Support for MOK variable config table

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a much more primitive mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

This patch adds initial kernel support to recognize, parse,
and validate the EFI MOK configuration table, where named
entries contain the same data that would otherwise be provided
in similarly named EFI variables.

Additionally, this patch creates a sysfs binary file for each
EFI MOK configuration table entry found. These files are read-only
to root and are provided for use by user space utilities such as
mokutil.

A subsequent patch will load MOK certs into the trusted platform
key ring using this infrastructure.

Signed-off-by: Lenny Szubowicz <[email protected]>
---
arch/x86/kernel/setup.c | 1 +
arch/x86/platform/efi/efi.c | 3 +
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/arm-init.c | 1 +
drivers/firmware/efi/efi.c | 6 +
drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
include/linux/efi.h | 34 +++
7 files changed, 406 insertions(+)
create mode 100644 drivers/firmware/efi/mokvar-table.c

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3511736fbc74..d41be0df72f8 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
efi_fake_memmap();
efi_find_mirror();
efi_esrt_init();
+ efi_mokvar_table_init();

/*
* The EFI specification says that boot service code won't be
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index d37ebe6e70d7..8a26e705cb06 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
&efi.tpm_log,
&efi.tpm_final_log,
&efi_rng_seed,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ &efi.mokvar_table,
+#endif
};

u64 efi_setup; /* efi setup_data physical address */
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 7a216984552b..03964e2d27c5 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o
obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o
obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o
obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += embedded-firmware.o
+obj-$(CONFIG_LOAD_UEFI_KEYS) += mokvar-table.o

fake_map-y += fake_mem.o
fake_map-$(CONFIG_X86) += x86_fake_mem.o
diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
index 71c445d20258..f55a92ff12c0 100644
--- a/drivers/firmware/efi/arm-init.c
+++ b/drivers/firmware/efi/arm-init.c
@@ -236,6 +236,7 @@ void __init efi_init(void)

reserve_regions();
efi_esrt_init();
+ efi_mokvar_table_init();

memblock_reserve(data.phys_map & PAGE_MASK,
PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 3aa07c3b5136..3d4daf215e19 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
.esrt = EFI_INVALID_TABLE_ADDR,
.tpm_log = EFI_INVALID_TABLE_ADDR,
.tpm_final_log = EFI_INVALID_TABLE_ADDR,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ .mokvar_table = EFI_INVALID_TABLE_ADDR,
+#endif
};
EXPORT_SYMBOL(efi);

@@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
{EFI_RT_PROPERTIES_TABLE_GUID, &rt_prop, "RTPROP" },
#ifdef CONFIG_EFI_RCI2_TABLE
{DELLEMC_EFI_RCI2_TABLE_GUID, &rci2_table_phys },
+#endif
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ {LINUX_EFI_MOK_VARIABLE_TABLE_GUID, &efi.mokvar_table, "MOKvar" },
#endif
{},
};
diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
new file mode 100644
index 000000000000..f12f1710f5d9
--- /dev/null
+++ b/drivers/firmware/efi/mokvar-table.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mokvar-table.c
+ *
+ * Copyright (c) 2020 Red Hat
+ * Author: Lenny Szubowicz <[email protected]>
+ *
+ * This module contains the kernel support for the Linux EFI Machine
+ * Owner Key (MOK) variable configuration table, which is identified by
+ * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
+ *
+ * This EFI configuration table provides a more robust alternative to
+ * EFI volatile variables by which an EFI boot loader can pass the
+ * contents of the Machine Owner Key (MOK) certificate stores to the
+ * kernel during boot. If both the EFI MOK config table and corresponding
+ * EFI MOK variables are present, the table should be considered as
+ * more authoritative.
+ *
+ * This module includes code that validates and maps the EFI MOK table,
+ * if it's presence was detected very early in boot.
+ *
+ * Kernel interface routines are provided to walk through all the
+ * entries in the MOK config table or to search for a specific named
+ * entry.
+ *
+ * The contents of the individual named MOK config table entries are
+ * made available to user space via read-only sysfs binary files under:
+ *
+ * /sys/firmware/efi/mok-variables/
+ *
+ */
+#define pr_fmt(fmt) "mokvar: " fmt
+
+#include <linux/capability.h>
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kobject.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
+ * sequence of struct efi_mokvar_table_entry, one for each named
+ * MOK variable. The sequence is terminated by an entry with a
+ * completely NULL name and 0 data size.
+ *
+ * efi_mokvar_table_size is set to the computed size of the
+ * MOK config table by efi_mokvar_table_init(). This will be
+ * non-zero if and only if the table if present and has been
+ * validated by efi_mokvar_table_init().
+ */
+static size_t efi_mokvar_table_size;
+
+/*
+ * efi_mokvar_table_va is the kernel virtual address at which the
+ * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
+ */
+static struct efi_mokvar_table_entry *efi_mokvar_table_va;
+
+/*
+ * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
+ * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
+ * bin_attr.private points to the associated EFI MOK config table entry.
+ *
+ * This list is created during boot and then remains unchanged.
+ * So no sychronization is currently required to walk the list.
+ */
+struct efi_mokvar_sysfs_attr {
+ struct bin_attribute bin_attr;
+ struct list_head node;
+};
+
+static LIST_HEAD(efi_mokvar_sysfs_list);
+static struct kobject *mokvar_kobj;
+
+/*
+ * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
+ *
+ * If present, validate and compute the size of the EFI MOK variable
+ * configuration table. This table may be provided by an EFI boot loader
+ * as an alternative to ordinary EFI variables, due to platform-dependent
+ * limitations. The memory occupied by this table is marked as reserved.
+ *
+ * This routine must be called before efi_free_boot_services() in order
+ * to guarantee that it can mark the table as reserved.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table: Physical address of EFI MOK variable config table
+ * or special value that indicates no such table.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ * The table is considered present and valid if this
+ * is non-zero.
+ */
+void __init efi_mokvar_table_init(void)
+{
+ efi_memory_desc_t md;
+ u64 end_pa;
+ void *va = NULL;
+ size_t cur_offset = 0;
+ size_t offset_limit;
+ size_t map_size = 0;
+ size_t map_size_needed = 0;
+ size_t size;
+ struct efi_mokvar_table_entry *mokvar_entry;
+ int err = -EINVAL;
+
+ if (!efi_enabled(EFI_MEMMAP))
+ return;
+
+ if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
+ return;
+ /*
+ * The EFI MOK config table must fit within a single EFI memory
+ * descriptor range.
+ */
+ err = efi_mem_desc_lookup(efi.mokvar_table, &md);
+ if (err) {
+ pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
+ return;
+ }
+ end_pa = efi_mem_desc_end(&md);
+ if (efi.mokvar_table >= end_pa) {
+ pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
+ return;
+ }
+ offset_limit = end_pa - efi.mokvar_table;
+ /*
+ * Validate the MOK config table. Since there is no table header
+ * from which we could get the total size of the MOK config table,
+ * we compute the total size as we validate each variably sized
+ * entry, remapping as necessary.
+ */
+ while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
+ mokvar_entry = va + cur_offset;
+ map_size_needed = cur_offset + sizeof(*mokvar_entry);
+ if (map_size_needed > map_size) {
+ if (va)
+ early_memunmap(va, map_size);
+ /*
+ * Map a little more than the fixed size entry
+ * header, anticipating some data. It's safe to
+ * do so as long as we stay within current memory
+ * descriptor.
+ */
+ map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
+ offset_limit);
+ va = early_memremap(efi.mokvar_table, map_size);
+ if (!va) {
+ pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
+ efi.mokvar_table, map_size);
+ return;
+ }
+ mokvar_entry = va + cur_offset;
+ }
+
+ /* Check for last sentinel entry */
+ if (mokvar_entry->name[0] == '\0') {
+ if (mokvar_entry->data_size != 0)
+ break;
+ err = 0;
+ break;
+ }
+
+ /* Sanity check that the name is null terminated */
+ size = strnlen(mokvar_entry->name,
+ sizeof(mokvar_entry->name));
+ if (size >= sizeof(mokvar_entry->name))
+ break;
+
+ /* Advance to the next entry */
+ cur_offset = map_size_needed + mokvar_entry->data_size;
+ }
+
+ if (va)
+ early_memunmap(va, map_size);
+ if (err) {
+ pr_err("EFI MOKvar config table is not valid\n");
+ return;
+ }
+ efi_mem_reserve(efi.mokvar_table, map_size_needed);
+ efi_mokvar_table_size = map_size_needed;
+}
+
+/*
+ * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
+ *
+ * mokvar_entry: Pointer to current EFI MOK config table entry
+ * or null. Null indicates get first entry.
+ * Passed by reference. This is updated to the
+ * same value as the return value.
+ *
+ * Returns: Pointer to next EFI MOK config table entry
+ * or null, if there are no more entries.
+ * Same value is returned in the mokvar_entry
+ * parameter.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry)
+{
+ struct efi_mokvar_table_entry *mokvar_cur;
+ struct efi_mokvar_table_entry *mokvar_next;
+ size_t size_cur;
+
+ mokvar_cur = *mokvar_entry;
+ *mokvar_entry = NULL;
+
+ if (efi_mokvar_table_va == NULL)
+ return NULL;
+
+ if (mokvar_cur == NULL) {
+ mokvar_next = efi_mokvar_table_va;
+ } else {
+ if (mokvar_cur->name[0] == '\0')
+ return NULL;
+ size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
+ mokvar_next = (void *)mokvar_cur + size_cur;
+ }
+
+ if (mokvar_next->name[0] == '\0')
+ return NULL;
+
+ *mokvar_entry = mokvar_next;
+ return mokvar_next;
+}
+
+/*
+ * efi_mokvar_entry_find() - Find EFI MOK config entry by name
+ *
+ * name: Name of the entry to look for.
+ *
+ * Returns: Pointer to EFI MOK config table entry if found;
+ * null otherwise.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
+{
+ struct efi_mokvar_table_entry *mokvar_entry = NULL;
+
+ while (efi_mokvar_entry_next(&mokvar_entry)) {
+ if (!strncmp(name, mokvar_entry->name,
+ sizeof(mokvar_entry->name)))
+ return mokvar_entry;
+ }
+ return NULL;
+}
+
+/*
+ * efi_mokvar_sysfs_read() - sysfs binary file read routine
+ *
+ * Returns: Count of bytes read.
+ *
+ * Copy EFI MOK config table entry data for this mokvar sysfs binary file
+ * to the supplied buffer, starting at the specified offset into mokvar table
+ * entry data, for the specified count bytes. The copy is limited by the
+ * amount of data in this mokvar config table entry.
+ */
+static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return 0;
+
+ if (off >= mokvar_entry->data_size)
+ return 0;
+ if (count > mokvar_entry->data_size - off)
+ count = mokvar_entry->data_size - off;
+
+ memcpy(buf, mokvar_entry->data + off, count);
+ return count;
+}
+
+/*
+ * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
+ *
+ * Map the EFI MOK variable config table for run-time use by the kernel
+ * and create the sysfs entries in /sys/firmware/efi/mok-variables/
+ *
+ * This routine just returns if a valid EFI MOK variable config table
+ * was not found earlier during boot.
+ *
+ * This routine must be called during a "middle" initcall phase, i.e.
+ * after efi_mokvar_table_init() but before UEFI certs are loaded
+ * during late init.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table: Physical address of EFI MOK variable config table
+ * or special value that indicates no such table.
+ *
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ * The table is considered present and valid if this
+ * is non-zero.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_va: Start virtual address of the EFI MOK config table.
+ */
+static int __init efi_mokvar_sysfs_init(void)
+{
+ void *config_va;
+ struct efi_mokvar_table_entry *mokvar_entry = NULL;
+ struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
+ int err = 0;
+
+ if (efi_mokvar_table_size == 0)
+ return -ENOENT;
+
+ config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
+ MEMREMAP_WB);
+ if (!config_va) {
+ pr_err("Failed to map EFI MOKvar config table\n");
+ return -ENOMEM;
+ }
+ efi_mokvar_table_va = config_va;
+
+ mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
+ if (!mokvar_kobj) {
+ pr_err("Failed to create EFI mok-variables sysfs entry\n");
+ return -ENOMEM;
+ }
+
+ while (efi_mokvar_entry_next(&mokvar_entry)) {
+ mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
+ if (!mokvar_sysfs) {
+ err = -ENOMEM;
+ break;
+ }
+
+ sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
+ mokvar_sysfs->bin_attr.private = mokvar_entry;
+ mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
+ mokvar_sysfs->bin_attr.attr.mode = 0400;
+ mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
+ mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
+
+ err = sysfs_create_bin_file(mokvar_kobj,
+ &mokvar_sysfs->bin_attr);
+ if (err)
+ break;
+
+ list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
+ }
+
+ if (err) {
+ pr_err("Failed to create some EFI mok-variables sysfs entries\n");
+ kfree(mokvar_sysfs);
+ }
+ return err;
+}
+device_initcall(efi_mokvar_sysfs_init);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 73db1ae04cef..4a2332f146eb 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
#define LINUX_EFI_TPM_FINAL_LOG_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
#define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
+#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)

/* OEM GUIDs */
#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
@@ -546,6 +547,7 @@ extern struct efi {
unsigned long esrt; /* ESRT table */
unsigned long tpm_log; /* TPM2 Event Log table */
unsigned long tpm_final_log; /* TPM2 Final Events Log table */
+ unsigned long mokvar_table; /* MOK variable config table */

efi_get_time_t *get_time;
efi_set_time_t *set_time;
@@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);

char *efi_systab_show_arch(char *str);

+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
+ * to the kernel by an EFI boot loader. The table contains a packed
+ * sequence of these entries, one for each named MOK variable.
+ * The sequence is terminated by an entry with a completely NULL
+ * name and 0 data size.
+ */
+struct efi_mokvar_table_entry {
+ char name[256];
+ u64 data_size;
+ u8 data[];
+} __attribute((packed));
+
+#ifdef CONFIG_LOAD_UEFI_KEYS
+extern void __init efi_mokvar_table_init(void);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
+#else
+static inline void efi_mokvar_table_init(void) { }
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry)
+{
+ return NULL;
+}
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
+ const char *name)
+{
+ return NULL;
+}
+#endif
+
#endif /* _LINUX_EFI_H */
--
2.27.0

2020-09-11 16:09:13

by Mimi Zohar

[permalink] [raw]

Subject: Re: [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

On Fri, 2020-09-11 at 18:17 +0300, Ard Biesheuvel wrote:
> On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <[email protected]> wrote:
> >
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a simpler and more robust mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > Entries in the MOK variable configuration table are named key/value
> > pairs. Therefore the shim boot loader can create a MokListRT named
> > entry in the MOK configuration table that contains exactly the same
> > data as the MokListRT UEFI variable does or would otherwise contain.
> > As such, the kernel can load certs from the data in the MokListRT
> > configuration table entry data in the same way that it loads certs
> > from the data returned by the EFI GetVariable() runtime call for the
> > MokListRT variable.
> >
> > This patch set does not remove the support for loading certs from the
> > EFI MOK variables into the platform key ring. However, if both the EFI
> > MOK configuration table and corresponding EFI MOK variables are present,
> > the MOK table is used as the source of MOK certs.
> >
> > The contents of the individual named MOK config table entries are
> > made available to user space as individual sysfs binary files,
> > which are read-only to root, under:
> >
> > /sys/firmware/efi/mok-variables/
> >
> > This enables an updated mokutil to provide support for:
> >
> > mokutil --list-enrolled
> >
> > such that it can provide accurate information regardless of whether
> > the MOK configuration table or MOK EFI variables were the source
> > for certs. Note that all modifications of MOK related state are still
> > initiated by mokutil via EFI variables.
> >
> > V2: Incorporate feedback from V1
> > Patch 01: efi: Support for MOK variable config table
> > - Minor update to change log; no code changes
> > Patch 02: integrity: Move import of MokListRT certs to a separate routine
> > - Clean up code flow in code moved to load_moklist_certs()
> > - Remove some unnecessary initialization of variables
> > Patch 03: integrity: Load certs from the EFI MOK config table
> > - Update required due to changes in patch 02.
> > - Remove unnecessary init of mokvar_entry in load_moklist_certs()
> >
> > V1:
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Lenny Szubowicz (3):
> > efi: Support for MOK variable config table
> > integrity: Move import of MokListRT certs to a separate routine
> > integrity: Load certs from the EFI MOK config table
> >
> > arch/x86/kernel/setup.c | 1 +
> > arch/x86/platform/efi/efi.c | 3 +
> > drivers/firmware/efi/Makefile | 1 +
> > drivers/firmware/efi/arm-init.c | 1 +
> > drivers/firmware/efi/efi.c | 6 +
> > drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++
> > include/linux/efi.h | 34 ++
> > security/integrity/platform_certs/load_uefi.c | 85 ++++-
> > 8 files changed, 472 insertions(+), 19 deletions(-)
> > create mode 100644 drivers/firmware/efi/mokvar-table.c
> >
>
> Thanks. I have tentatively queued these up in efi/next.
>
> Mimi, please let me know if you have any thoughts on 3/3, and whether
> your R-b on 2/3 [v1] implies that you are ok with the series going
> through the EFI tree.

Yes, Ard, that was the intent. I haven't reviewed the most recent
version.

Mimi

2020-09-11 16:29:32

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH V2 0/3] integrity: Load certs from EFI MOK config table

On Sat, 5 Sep 2020 at 04:31, Lenny Szubowicz <[email protected]> wrote:
>
> Because of system-specific EFI firmware limitations, EFI volatile
> variables may not be capable of holding the required contents of
> the Machine Owner Key (MOK) certificate store when the certificate
> list grows above some size. Therefore, an EFI boot loader may pass
> the MOK certs via a EFI configuration table created specifically for
> this purpose to avoid this firmware limitation.
>
> An EFI configuration table is a simpler and more robust mechanism
> compared to EFI variables and is well suited for one-way passage
> of static information from a pre-OS environment to the kernel.
>
> Entries in the MOK variable configuration table are named key/value
> pairs. Therefore the shim boot loader can create a MokListRT named
> entry in the MOK configuration table that contains exactly the same
> data as the MokListRT UEFI variable does or would otherwise contain.
> As such, the kernel can load certs from the data in the MokListRT
> configuration table entry data in the same way that it loads certs
> from the data returned by the EFI GetVariable() runtime call for the
> MokListRT variable.
>
> This patch set does not remove the support for loading certs from the
> EFI MOK variables into the platform key ring. However, if both the EFI
> MOK configuration table and corresponding EFI MOK variables are present,
> the MOK table is used as the source of MOK certs.
>
> The contents of the individual named MOK config table entries are
> made available to user space as individual sysfs binary files,
> which are read-only to root, under:
>
> /sys/firmware/efi/mok-variables/
>
> This enables an updated mokutil to provide support for:
>
> mokutil --list-enrolled
>
> such that it can provide accurate information regardless of whether
> the MOK configuration table or MOK EFI variables were the source
> for certs. Note that all modifications of MOK related state are still
> initiated by mokutil via EFI variables.
>
> V2: Incorporate feedback from V1
> Patch 01: efi: Support for MOK variable config table
> - Minor update to change log; no code changes
> Patch 02: integrity: Move import of MokListRT certs to a separate routine
> - Clean up code flow in code moved to load_moklist_certs()
> - Remove some unnecessary initialization of variables
> Patch 03: integrity: Load certs from the EFI MOK config table
> - Update required due to changes in patch 02.
> - Remove unnecessary init of mokvar_entry in load_moklist_certs()
>
> V1:
> https://lore.kernel.org/lkml/[email protected]/
>
> Lenny Szubowicz (3):
> efi: Support for MOK variable config table
> integrity: Move import of MokListRT certs to a separate routine
> integrity: Load certs from the EFI MOK config table
>
> arch/x86/kernel/setup.c | 1 +
> arch/x86/platform/efi/efi.c | 3 +
> drivers/firmware/efi/Makefile | 1 +
> drivers/firmware/efi/arm-init.c | 1 +
> drivers/firmware/efi/efi.c | 6 +
> drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++
> include/linux/efi.h | 34 ++
> security/integrity/platform_certs/load_uefi.c | 85 ++++-
> 8 files changed, 472 insertions(+), 19 deletions(-)
> create mode 100644 drivers/firmware/efi/mokvar-table.c
>

Thanks. I have tentatively queued these up in efi/next.

Mimi, please let me know if you have any thoughts on 3/3, and whether
your R-b on 2/3 [v1] implies that you are ok with the series going
through the EFI tree.

--
Ard.

2020-09-18 08:32:45

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: efi/core] integrity: Load certs from the EFI MOK config table

The following commit has been merged into the efi/core branch of tip:

Commit-ID: 726bd8965a5f112d9601f7ce68effa1e46e02bf2
Gitweb: https://git.kernel.org/tip/726bd8965a5f112d9601f7ce68effa1e46e02bf2
Author: Lenny Szubowicz <[email protected]>
AuthorDate: Fri, 04 Sep 2020 21:31:07 -04:00
Committer: Ard Biesheuvel <[email protected]>
CommitterDate: Wed, 16 Sep 2020 18:53:42 +03:00

integrity: Load certs from the EFI MOK config table

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a much more primitive mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

This patch adds the support to load certs from the MokListRT
entry in the MOK variable configuration table, if it's present.
The pre-existing support to load certs from the MokListRT EFI
variable remains and is used if the EFI MOK configuration table
isn't present or can't be successfully used.

Signed-off-by: Lenny Szubowicz <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Ard Biesheuvel <[email protected]>
---
security/integrity/platform_certs/load_uefi.c | 22 ++++++++++++++++++-
1 file changed, 22 insertions(+)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index c1c622b..ee4b4c6 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -71,16 +71,38 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
* Load the certs contained in the UEFI MokListRT database into the
* platform trusted keyring.
*
+ * This routine checks the EFI MOK config table first. If and only if
+ * that fails, this routine uses the MokListRT ordinary UEFI variable.
+ *
* Return: Status
*/
static int __init load_moklist_certs(void)
{
+ struct efi_mokvar_table_entry *mokvar_entry;
efi_guid_t mok_var = EFI_SHIM_LOCK_GUID;
void *mok;
unsigned long moksize;
efi_status_t status;
int rc;

+ /* First try to load certs from the EFI MOKvar config table.
+ * It's not an error if the MOKvar config table doesn't exist
+ * or the MokListRT entry is not found in it.
+ */
+ mokvar_entry = efi_mokvar_entry_find("MokListRT");
+ if (mokvar_entry) {
+ rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
+ mokvar_entry->data,
+ mokvar_entry->data_size,
+ get_handler_for_db);
+ /* All done if that worked. */
+ if (!rc)
+ return rc;
+
+ pr_err("Couldn't parse MokListRT signatures from EFI MOKvar config table: %d\n",
+ rc);
+ }
+
/* Get MokListRT. It might not exist, so it isn't an error
* if we can't get it.
*/

2020-09-18 08:34:29

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: efi/core] efi: Support for MOK variable config table

The following commit has been merged into the efi/core branch of tip:

Commit-ID: 58c909022a5a56cd1d9e89c8c5461fd1f6a27bb5
Gitweb: https://git.kernel.org/tip/58c909022a5a56cd1d9e89c8c5461fd1f6a27bb5
Author: Lenny Szubowicz <[email protected]>
AuthorDate: Fri, 04 Sep 2020 21:31:05 -04:00
Committer: Ard Biesheuvel <[email protected]>
CommitterDate: Wed, 16 Sep 2020 18:53:42 +03:00

efi: Support for MOK variable config table

Because of system-specific EFI firmware limitations, EFI volatile
variables may not be capable of holding the required contents of
the Machine Owner Key (MOK) certificate store when the certificate
list grows above some size. Therefore, an EFI boot loader may pass
the MOK certs via a EFI configuration table created specifically for
this purpose to avoid this firmware limitation.

An EFI configuration table is a much more primitive mechanism
compared to EFI variables and is well suited for one-way passage
of static information from a pre-OS environment to the kernel.

This patch adds initial kernel support to recognize, parse,
and validate the EFI MOK configuration table, where named
entries contain the same data that would otherwise be provided
in similarly named EFI variables.

Additionally, this patch creates a sysfs binary file for each
EFI MOK configuration table entry found. These files are read-only
to root and are provided for use by user space utilities such as
mokutil.

A subsequent patch will load MOK certs into the trusted platform
key ring using this infrastructure.

Signed-off-by: Lenny Szubowicz <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/kernel/setup.c | 1 +-
arch/x86/platform/efi/efi.c | 3 +-
drivers/firmware/efi/Makefile | 1 +-
drivers/firmware/efi/efi-init.c | 1 +-
drivers/firmware/efi/efi.c | 6 +-
drivers/firmware/efi/mokvar-table.c | 360 +++++++++++++++++++++++++++-
include/linux/efi.h | 34 +++-
7 files changed, 406 insertions(+)
create mode 100644 drivers/firmware/efi/mokvar-table.c

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3511736..d41be0d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
efi_fake_memmap();
efi_find_mirror();
efi_esrt_init();
+ efi_mokvar_table_init();

/*
* The EFI specification says that boot service code won't be
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index f6ea8f1..6961bed 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -91,6 +91,9 @@ static const unsigned long * const efi_tables[] = {
&efi.tpm_log,
&efi.tpm_final_log,
&efi_rng_seed,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ &efi.mokvar_table,
+#endif
};

u64 efi_setup; /* efi setup_data physical address */
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 61fd1e8..e8da782 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o
obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o
obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o
obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += embedded-firmware.o
+obj-$(CONFIG_LOAD_UEFI_KEYS) += mokvar-table.o

fake_map-y += fake_mem.o
fake_map-$(CONFIG_X86) += x86_fake_mem.o
diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c
index 71c445d..f55a92f 100644
--- a/drivers/firmware/efi/efi-init.c
+++ b/drivers/firmware/efi/efi-init.c
@@ -236,6 +236,7 @@ void __init efi_init(void)

reserve_regions();
efi_esrt_init();
+ efi_mokvar_table_init();

memblock_reserve(data.phys_map & PAGE_MASK,
PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index fdd1db0..820f5b1 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
.esrt = EFI_INVALID_TABLE_ADDR,
.tpm_log = EFI_INVALID_TABLE_ADDR,
.tpm_final_log = EFI_INVALID_TABLE_ADDR,
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ .mokvar_table = EFI_INVALID_TABLE_ADDR,
+#endif
};
EXPORT_SYMBOL(efi);

@@ -517,6 +520,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
#ifdef CONFIG_EFI_RCI2_TABLE
{DELLEMC_EFI_RCI2_TABLE_GUID, &rci2_table_phys },
#endif
+#ifdef CONFIG_LOAD_UEFI_KEYS
+ {LINUX_EFI_MOK_VARIABLE_TABLE_GUID, &efi.mokvar_table, "MOKvar" },
+#endif
{},
};

diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
new file mode 100644
index 0000000..b1cd498
--- /dev/null
+++ b/drivers/firmware/efi/mokvar-table.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mokvar-table.c
+ *
+ * Copyright (c) 2020 Red Hat
+ * Author: Lenny Szubowicz <[email protected]>
+ *
+ * This module contains the kernel support for the Linux EFI Machine
+ * Owner Key (MOK) variable configuration table, which is identified by
+ * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
+ *
+ * This EFI configuration table provides a more robust alternative to
+ * EFI volatile variables by which an EFI boot loader can pass the
+ * contents of the Machine Owner Key (MOK) certificate stores to the
+ * kernel during boot. If both the EFI MOK config table and corresponding
+ * EFI MOK variables are present, the table should be considered as
+ * more authoritative.
+ *
+ * This module includes code that validates and maps the EFI MOK table,
+ * if it's presence was detected very early in boot.
+ *
+ * Kernel interface routines are provided to walk through all the
+ * entries in the MOK config table or to search for a specific named
+ * entry.
+ *
+ * The contents of the individual named MOK config table entries are
+ * made available to user space via read-only sysfs binary files under:
+ *
+ * /sys/firmware/efi/mok-variables/
+ *
+ */
+#define pr_fmt(fmt) "mokvar: " fmt
+
+#include <linux/capability.h>
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kobject.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
+ * sequence of struct efi_mokvar_table_entry, one for each named
+ * MOK variable. The sequence is terminated by an entry with a
+ * completely NULL name and 0 data size.
+ *
+ * efi_mokvar_table_size is set to the computed size of the
+ * MOK config table by efi_mokvar_table_init(). This will be
+ * non-zero if and only if the table if present and has been
+ * validated by efi_mokvar_table_init().
+ */
+static size_t efi_mokvar_table_size;
+
+/*
+ * efi_mokvar_table_va is the kernel virtual address at which the
+ * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
+ */
+static struct efi_mokvar_table_entry *efi_mokvar_table_va;
+
+/*
+ * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
+ * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
+ * bin_attr.private points to the associated EFI MOK config table entry.
+ *
+ * This list is created during boot and then remains unchanged.
+ * So no synchronization is currently required to walk the list.
+ */
+struct efi_mokvar_sysfs_attr {
+ struct bin_attribute bin_attr;
+ struct list_head node;
+};
+
+static LIST_HEAD(efi_mokvar_sysfs_list);
+static struct kobject *mokvar_kobj;
+
+/*
+ * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
+ *
+ * If present, validate and compute the size of the EFI MOK variable
+ * configuration table. This table may be provided by an EFI boot loader
+ * as an alternative to ordinary EFI variables, due to platform-dependent
+ * limitations. The memory occupied by this table is marked as reserved.
+ *
+ * This routine must be called before efi_free_boot_services() in order
+ * to guarantee that it can mark the table as reserved.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table: Physical address of EFI MOK variable config table
+ * or special value that indicates no such table.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ * The table is considered present and valid if this
+ * is non-zero.
+ */
+void __init efi_mokvar_table_init(void)
+{
+ efi_memory_desc_t md;
+ u64 end_pa;
+ void *va = NULL;
+ size_t cur_offset = 0;
+ size_t offset_limit;
+ size_t map_size = 0;
+ size_t map_size_needed = 0;
+ size_t size;
+ struct efi_mokvar_table_entry *mokvar_entry;
+ int err = -EINVAL;
+
+ if (!efi_enabled(EFI_MEMMAP))
+ return;
+
+ if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
+ return;
+ /*
+ * The EFI MOK config table must fit within a single EFI memory
+ * descriptor range.
+ */
+ err = efi_mem_desc_lookup(efi.mokvar_table, &md);
+ if (err) {
+ pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
+ return;
+ }
+ end_pa = efi_mem_desc_end(&md);
+ if (efi.mokvar_table >= end_pa) {
+ pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
+ return;
+ }
+ offset_limit = end_pa - efi.mokvar_table;
+ /*
+ * Validate the MOK config table. Since there is no table header
+ * from which we could get the total size of the MOK config table,
+ * we compute the total size as we validate each variably sized
+ * entry, remapping as necessary.
+ */
+ while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
+ mokvar_entry = va + cur_offset;
+ map_size_needed = cur_offset + sizeof(*mokvar_entry);
+ if (map_size_needed > map_size) {
+ if (va)
+ early_memunmap(va, map_size);
+ /*
+ * Map a little more than the fixed size entry
+ * header, anticipating some data. It's safe to
+ * do so as long as we stay within current memory
+ * descriptor.
+ */
+ map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
+ offset_limit);
+ va = early_memremap(efi.mokvar_table, map_size);
+ if (!va) {
+ pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
+ efi.mokvar_table, map_size);
+ return;
+ }
+ mokvar_entry = va + cur_offset;
+ }
+
+ /* Check for last sentinel entry */
+ if (mokvar_entry->name[0] == '\0') {
+ if (mokvar_entry->data_size != 0)
+ break;
+ err = 0;
+ break;
+ }
+
+ /* Sanity check that the name is null terminated */
+ size = strnlen(mokvar_entry->name,
+ sizeof(mokvar_entry->name));
+ if (size >= sizeof(mokvar_entry->name))
+ break;
+
+ /* Advance to the next entry */
+ cur_offset = map_size_needed + mokvar_entry->data_size;
+ }
+
+ if (va)
+ early_memunmap(va, map_size);
+ if (err) {
+ pr_err("EFI MOKvar config table is not valid\n");
+ return;
+ }
+ efi_mem_reserve(efi.mokvar_table, map_size_needed);
+ efi_mokvar_table_size = map_size_needed;
+}
+
+/*
+ * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
+ *
+ * mokvar_entry: Pointer to current EFI MOK config table entry
+ * or null. Null indicates get first entry.
+ * Passed by reference. This is updated to the
+ * same value as the return value.
+ *
+ * Returns: Pointer to next EFI MOK config table entry
+ * or null, if there are no more entries.
+ * Same value is returned in the mokvar_entry
+ * parameter.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry)
+{
+ struct efi_mokvar_table_entry *mokvar_cur;
+ struct efi_mokvar_table_entry *mokvar_next;
+ size_t size_cur;
+
+ mokvar_cur = *mokvar_entry;
+ *mokvar_entry = NULL;
+
+ if (efi_mokvar_table_va == NULL)
+ return NULL;
+
+ if (mokvar_cur == NULL) {
+ mokvar_next = efi_mokvar_table_va;
+ } else {
+ if (mokvar_cur->name[0] == '\0')
+ return NULL;
+ size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
+ mokvar_next = (void *)mokvar_cur + size_cur;
+ }
+
+ if (mokvar_next->name[0] == '\0')
+ return NULL;
+
+ *mokvar_entry = mokvar_next;
+ return mokvar_next;
+}
+
+/*
+ * efi_mokvar_entry_find() - Find EFI MOK config entry by name
+ *
+ * name: Name of the entry to look for.
+ *
+ * Returns: Pointer to EFI MOK config table entry if found;
+ * null otherwise.
+ *
+ * This routine depends on the EFI MOK config table being entirely
+ * mapped with it's starting virtual address in efi_mokvar_table_va.
+ */
+struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
+{
+ struct efi_mokvar_table_entry *mokvar_entry = NULL;
+
+ while (efi_mokvar_entry_next(&mokvar_entry)) {
+ if (!strncmp(name, mokvar_entry->name,
+ sizeof(mokvar_entry->name)))
+ return mokvar_entry;
+ }
+ return NULL;
+}
+
+/*
+ * efi_mokvar_sysfs_read() - sysfs binary file read routine
+ *
+ * Returns: Count of bytes read.
+ *
+ * Copy EFI MOK config table entry data for this mokvar sysfs binary file
+ * to the supplied buffer, starting at the specified offset into mokvar table
+ * entry data, for the specified count bytes. The copy is limited by the
+ * amount of data in this mokvar config table entry.
+ */
+static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return 0;
+
+ if (off >= mokvar_entry->data_size)
+ return 0;
+ if (count > mokvar_entry->data_size - off)
+ count = mokvar_entry->data_size - off;
+
+ memcpy(buf, mokvar_entry->data + off, count);
+ return count;
+}
+
+/*
+ * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
+ *
+ * Map the EFI MOK variable config table for run-time use by the kernel
+ * and create the sysfs entries in /sys/firmware/efi/mok-variables/
+ *
+ * This routine just returns if a valid EFI MOK variable config table
+ * was not found earlier during boot.
+ *
+ * This routine must be called during a "middle" initcall phase, i.e.
+ * after efi_mokvar_table_init() but before UEFI certs are loaded
+ * during late init.
+ *
+ * Implicit inputs:
+ * efi.mokvar_table: Physical address of EFI MOK variable config table
+ * or special value that indicates no such table.
+ *
+ * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
+ * The table is considered present and valid if this
+ * is non-zero.
+ *
+ * Implicit outputs:
+ * efi_mokvar_table_va: Start virtual address of the EFI MOK config table.
+ */
+static int __init efi_mokvar_sysfs_init(void)
+{
+ void *config_va;
+ struct efi_mokvar_table_entry *mokvar_entry = NULL;
+ struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
+ int err = 0;
+
+ if (efi_mokvar_table_size == 0)
+ return -ENOENT;
+
+ config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
+ MEMREMAP_WB);
+ if (!config_va) {
+ pr_err("Failed to map EFI MOKvar config table\n");
+ return -ENOMEM;
+ }
+ efi_mokvar_table_va = config_va;
+
+ mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
+ if (!mokvar_kobj) {
+ pr_err("Failed to create EFI mok-variables sysfs entry\n");
+ return -ENOMEM;
+ }
+
+ while (efi_mokvar_entry_next(&mokvar_entry)) {
+ mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
+ if (!mokvar_sysfs) {
+ err = -ENOMEM;
+ break;
+ }
+
+ sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
+ mokvar_sysfs->bin_attr.private = mokvar_entry;
+ mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
+ mokvar_sysfs->bin_attr.attr.mode = 0400;
+ mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
+ mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
+
+ err = sysfs_create_bin_file(mokvar_kobj,
+ &mokvar_sysfs->bin_attr);
+ if (err)
+ break;
+
+ list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
+ }
+
+ if (err) {
+ pr_err("Failed to create some EFI mok-variables sysfs entries\n");
+ kfree(mokvar_sysfs);
+ }
+ return err;
+}
+device_initcall(efi_mokvar_sysfs_init);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 73db1ae..4a2332f 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
#define LINUX_EFI_TPM_FINAL_LOG_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
#define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
+#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)

/* OEM GUIDs */
#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
@@ -546,6 +547,7 @@ extern struct efi {
unsigned long esrt; /* ESRT table */
unsigned long tpm_log; /* TPM2 Event Log table */
unsigned long tpm_final_log; /* TPM2 Final Events Log table */
+ unsigned long mokvar_table; /* MOK variable config table */

efi_get_time_t *get_time;
efi_set_time_t *set_time;
@@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);

char *efi_systab_show_arch(char *str);

+/*
+ * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
+ * to the kernel by an EFI boot loader. The table contains a packed
+ * sequence of these entries, one for each named MOK variable.
+ * The sequence is terminated by an entry with a completely NULL
+ * name and 0 data size.
+ */
+struct efi_mokvar_table_entry {
+ char name[256];
+ u64 data_size;
+ u8 data[];
+} __attribute((packed));
+
+#ifdef CONFIG_LOAD_UEFI_KEYS
+extern void __init efi_mokvar_table_init(void);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry);
+extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
+#else
+static inline void efi_mokvar_table_init(void) { }
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
+ struct efi_mokvar_table_entry **mokvar_entry)
+{
+ return NULL;
+}
+static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
+ const char *name)
+{
+ return NULL;
+}
+#endif
+
#endif /* _LINUX_EFI_H */

2020-09-21 15:57:49

by Borislav Petkov

[permalink] [raw]

Subject: Re: [tip: efi/core] efi: Support for MOK variable config table

On Fri, Sep 18, 2020 at 08:30:54AM -0000, tip-bot2 for Lenny Szubowicz wrote:
> +void __init efi_mokvar_table_init(void)
> +{
> + efi_memory_desc_t md;
> + u64 end_pa;
> + void *va = NULL;
> + size_t cur_offset = 0;
> + size_t offset_limit;
> + size_t map_size = 0;
> + size_t map_size_needed = 0;
> + size_t size;
> + struct efi_mokvar_table_entry *mokvar_entry;
> + int err = -EINVAL;
> +
> + if (!efi_enabled(EFI_MEMMAP))
> + return;
> +
> + if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
> + return;
> + /*
> + * The EFI MOK config table must fit within a single EFI memory
> + * descriptor range.
> + */
> + err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> + if (err) {
> + pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> + return;
> + }
> + end_pa = efi_mem_desc_end(&md);
> + if (efi.mokvar_table >= end_pa) {
> + pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> + return;
> + }
> + offset_limit = end_pa - efi.mokvar_table;
> + /*
> + * Validate the MOK config table. Since there is no table header
> + * from which we could get the total size of the MOK config table,
> + * we compute the total size as we validate each variably sized
> + * entry, remapping as necessary.
> + */
> + while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> + mokvar_entry = va + cur_offset;
> + map_size_needed = cur_offset + sizeof(*mokvar_entry);
> + if (map_size_needed > map_size) {
> + if (va)
> + early_memunmap(va, map_size);
> + /*
> + * Map a little more than the fixed size entry
> + * header, anticipating some data. It's safe to
> + * do so as long as we stay within current memory
> + * descriptor.
> + */
> + map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> + offset_limit);

i386 allmodconfig build gives here:

In file included from ./arch/x86/include/asm/percpu.h:27:0,
from ./arch/x86/include/asm/current.h:6,
from ./arch/x86/include/asm/processor.h:17,
from ./arch/x86/include/asm/timex.h:5,
from ./include/linux/timex.h:65,
from ./include/linux/time32.h:13,
from ./include/linux/time.h:73,
from ./include/linux/efi.h:17,
from drivers/firmware/efi/mokvar-table.c:35:
drivers/firmware/efi/mokvar-table.c: In function ‘efi_mokvar_table_init’:
./include/linux/kernel.h:850:29: warning: comparison of distinct pointer types lacks a cast
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
^
./include/linux/kernel.h:864:4: note: in expansion of macro ‘__typecheck’
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~
./include/linux/kernel.h:874:24: note: in expansion of macro ‘__safe_cmp’
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~
./include/linux/kernel.h:883:19: note: in expansion of macro ‘__careful_cmp’
#define min(x, y) __careful_cmp(x, y, <)
^~~~~~~~~~~~~
drivers/firmware/efi/mokvar-table.c:149:15: note: in expansion of macro ‘min’
map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
^~~

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-09-21 16:20:36

by Arvind Sankar

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> Because of system-specific EFI firmware limitations, EFI volatile
> variables may not be capable of holding the required contents of
> the Machine Owner Key (MOK) certificate store when the certificate
> list grows above some size. Therefore, an EFI boot loader may pass
> the MOK certs via a EFI configuration table created specifically for
> this purpose to avoid this firmware limitation.
>
> An EFI configuration table is a much more primitive mechanism
> compared to EFI variables and is well suited for one-way passage
> of static information from a pre-OS environment to the kernel.
>
> This patch adds initial kernel support to recognize, parse,
> and validate the EFI MOK configuration table, where named
> entries contain the same data that would otherwise be provided
> in similarly named EFI variables.
>
> Additionally, this patch creates a sysfs binary file for each
> EFI MOK configuration table entry found. These files are read-only
> to root and are provided for use by user space utilities such as
> mokutil.
>
> A subsequent patch will load MOK certs into the trusted platform
> key ring using this infrastructure.
>
> Signed-off-by: Lenny Szubowicz <[email protected]>
> ---
> arch/x86/kernel/setup.c | 1 +
> arch/x86/platform/efi/efi.c | 3 +
> drivers/firmware/efi/Makefile | 1 +
> drivers/firmware/efi/arm-init.c | 1 +
> drivers/firmware/efi/efi.c | 6 +
> drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
> include/linux/efi.h | 34 +++
> 7 files changed, 406 insertions(+)
> create mode 100644 drivers/firmware/efi/mokvar-table.c
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 3511736fbc74..d41be0df72f8 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
> efi_fake_memmap();
> efi_find_mirror();
> efi_esrt_init();
> + efi_mokvar_table_init();
>
> /*
> * The EFI specification says that boot service code won't be
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index d37ebe6e70d7..8a26e705cb06 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
> &efi.tpm_log,
> &efi.tpm_final_log,
> &efi_rng_seed,
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> + &efi.mokvar_table,
> +#endif
> };
>
> u64 efi_setup; /* efi setup_data physical address */
> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index 7a216984552b..03964e2d27c5 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o
> obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o
> obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o
> obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += embedded-firmware.o
> +obj-$(CONFIG_LOAD_UEFI_KEYS) += mokvar-table.o
>
> fake_map-y += fake_mem.o
> fake_map-$(CONFIG_X86) += x86_fake_mem.o
> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> index 71c445d20258..f55a92ff12c0 100644
> --- a/drivers/firmware/efi/arm-init.c
> +++ b/drivers/firmware/efi/arm-init.c
> @@ -236,6 +236,7 @@ void __init efi_init(void)
>
> reserve_regions();
> efi_esrt_init();
> + efi_mokvar_table_init();
>
> memblock_reserve(data.phys_map & PAGE_MASK,
> PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 3aa07c3b5136..3d4daf215e19 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
> .esrt = EFI_INVALID_TABLE_ADDR,
> .tpm_log = EFI_INVALID_TABLE_ADDR,
> .tpm_final_log = EFI_INVALID_TABLE_ADDR,
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> + .mokvar_table = EFI_INVALID_TABLE_ADDR,
> +#endif
> };
> EXPORT_SYMBOL(efi);
>
> @@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
> {EFI_RT_PROPERTIES_TABLE_GUID, &rt_prop, "RTPROP" },
> #ifdef CONFIG_EFI_RCI2_TABLE
> {DELLEMC_EFI_RCI2_TABLE_GUID, &rci2_table_phys },
> +#endif
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> + {LINUX_EFI_MOK_VARIABLE_TABLE_GUID, &efi.mokvar_table, "MOKvar" },
> #endif
> {},
> };
> diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> new file mode 100644
> index 000000000000..f12f1710f5d9
> --- /dev/null
> +++ b/drivers/firmware/efi/mokvar-table.c
> @@ -0,0 +1,360 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mokvar-table.c
> + *
> + * Copyright (c) 2020 Red Hat
> + * Author: Lenny Szubowicz <[email protected]>
> + *
> + * This module contains the kernel support for the Linux EFI Machine
> + * Owner Key (MOK) variable configuration table, which is identified by
> + * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
> + *
> + * This EFI configuration table provides a more robust alternative to
> + * EFI volatile variables by which an EFI boot loader can pass the
> + * contents of the Machine Owner Key (MOK) certificate stores to the
> + * kernel during boot. If both the EFI MOK config table and corresponding
> + * EFI MOK variables are present, the table should be considered as
> + * more authoritative.
> + *
> + * This module includes code that validates and maps the EFI MOK table,
> + * if it's presence was detected very early in boot.
> + *
> + * Kernel interface routines are provided to walk through all the
> + * entries in the MOK config table or to search for a specific named
> + * entry.
> + *
> + * The contents of the individual named MOK config table entries are
> + * made available to user space via read-only sysfs binary files under:
> + *
> + * /sys/firmware/efi/mok-variables/
> + *
> + */
> +#define pr_fmt(fmt) "mokvar: " fmt
> +
> +#include <linux/capability.h>
> +#include <linux/efi.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +/*
> + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
> + * sequence of struct efi_mokvar_table_entry, one for each named
> + * MOK variable. The sequence is terminated by an entry with a
> + * completely NULL name and 0 data size.
> + *
> + * efi_mokvar_table_size is set to the computed size of the
> + * MOK config table by efi_mokvar_table_init(). This will be
> + * non-zero if and only if the table if present and has been
> + * validated by efi_mokvar_table_init().
> + */
> +static size_t efi_mokvar_table_size;
> +
> +/*
> + * efi_mokvar_table_va is the kernel virtual address at which the
> + * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
> + */
> +static struct efi_mokvar_table_entry *efi_mokvar_table_va;
> +
> +/*
> + * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
> + * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
> + * bin_attr.private points to the associated EFI MOK config table entry.
> + *
> + * This list is created during boot and then remains unchanged.
> + * So no sychronization is currently required to walk the list.
> + */
> +struct efi_mokvar_sysfs_attr {
> + struct bin_attribute bin_attr;
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(efi_mokvar_sysfs_list);
> +static struct kobject *mokvar_kobj;
> +
> +/*
> + * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
> + *
> + * If present, validate and compute the size of the EFI MOK variable
> + * configuration table. This table may be provided by an EFI boot loader
> + * as an alternative to ordinary EFI variables, due to platform-dependent
> + * limitations. The memory occupied by this table is marked as reserved.
> + *
> + * This routine must be called before efi_free_boot_services() in order
> + * to guarantee that it can mark the table as reserved.
> + *
> + * Implicit inputs:
> + * efi.mokvar_table: Physical address of EFI MOK variable config table
> + * or special value that indicates no such table.
> + *
> + * Implicit outputs:
> + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> + * The table is considered present and valid if this
> + * is non-zero.
> + */
> +void __init efi_mokvar_table_init(void)
> +{
> + efi_memory_desc_t md;
> + u64 end_pa;
> + void *va = NULL;
> + size_t cur_offset = 0;
> + size_t offset_limit;
> + size_t map_size = 0;
> + size_t map_size_needed = 0;
> + size_t size;
> + struct efi_mokvar_table_entry *mokvar_entry;
> + int err = -EINVAL;
> +
> + if (!efi_enabled(EFI_MEMMAP))
> + return;
> +
> + if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
> + return;
> + /*
> + * The EFI MOK config table must fit within a single EFI memory
> + * descriptor range.
> + */
> + err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> + if (err) {
> + pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> + return;
> + }
> + end_pa = efi_mem_desc_end(&md);
> + if (efi.mokvar_table >= end_pa) {
> + pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> + return;
> + }

efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
why check it again?

> + offset_limit = end_pa - efi.mokvar_table;
> + /*
> + * Validate the MOK config table. Since there is no table header
> + * from which we could get the total size of the MOK config table,
> + * we compute the total size as we validate each variably sized
> + * entry, remapping as necessary.
> + */
> + while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> + mokvar_entry = va + cur_offset;
> + map_size_needed = cur_offset + sizeof(*mokvar_entry);
> + if (map_size_needed > map_size) {
> + if (va)
> + early_memunmap(va, map_size);
> + /*
> + * Map a little more than the fixed size entry
> + * header, anticipating some data. It's safe to
> + * do so as long as we stay within current memory
> + * descriptor.
> + */
> + map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> + offset_limit);
> + va = early_memremap(efi.mokvar_table, map_size);

Can't we just map the entire region from efi.mokvar_table to end_pa in
one early_memremap call before the loop and avoid all the remapping
logic?

> + if (!va) {
> + pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
> + efi.mokvar_table, map_size);
> + return;
> + }
> + mokvar_entry = va + cur_offset;
> + }
> +
> + /* Check for last sentinel entry */
> + if (mokvar_entry->name[0] == '\0') {
> + if (mokvar_entry->data_size != 0)
> + break;
> + err = 0;
> + break;
> + }
> +
> + /* Sanity check that the name is null terminated */
> + size = strnlen(mokvar_entry->name,
> + sizeof(mokvar_entry->name));
> + if (size >= sizeof(mokvar_entry->name))
> + break;
> +
> + /* Advance to the next entry */
> + cur_offset = map_size_needed + mokvar_entry->data_size;
> + }
> +
> + if (va)
> + early_memunmap(va, map_size);
> + if (err) {
> + pr_err("EFI MOKvar config table is not valid\n");
> + return;
> + }

err will never be non-zero here: it was cleared when the
efi_mem_desc_lookup() was done. I think the initialization of err to
-EINVAL needs to be moved just prior to the loop.

> + efi_mem_reserve(efi.mokvar_table, map_size_needed);
> + efi_mokvar_table_size = map_size_needed;
> +}
> +
> +/*
> + * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
> + *
> + * mokvar_entry: Pointer to current EFI MOK config table entry
> + * or null. Null indicates get first entry.
> + * Passed by reference. This is updated to the
> + * same value as the return value.
> + *
> + * Returns: Pointer to next EFI MOK config table entry
> + * or null, if there are no more entries.
> + * Same value is returned in the mokvar_entry
> + * parameter.
> + *
> + * This routine depends on the EFI MOK config table being entirely
> + * mapped with it's starting virtual address in efi_mokvar_table_va.
> + */
> +struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> + struct efi_mokvar_table_entry **mokvar_entry)
> +{
> + struct efi_mokvar_table_entry *mokvar_cur;
> + struct efi_mokvar_table_entry *mokvar_next;
> + size_t size_cur;
> +
> + mokvar_cur = *mokvar_entry;
> + *mokvar_entry = NULL;
> +
> + if (efi_mokvar_table_va == NULL)
> + return NULL;
> +
> + if (mokvar_cur == NULL) {
> + mokvar_next = efi_mokvar_table_va;
> + } else {
> + if (mokvar_cur->name[0] == '\0')
> + return NULL;
> + size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
> + mokvar_next = (void *)mokvar_cur + size_cur;
> + }
> +
> + if (mokvar_next->name[0] == '\0')
> + return NULL;
> +
> + *mokvar_entry = mokvar_next;
> + return mokvar_next;
> +}
> +
> +/*
> + * efi_mokvar_entry_find() - Find EFI MOK config entry by name
> + *
> + * name: Name of the entry to look for.
> + *
> + * Returns: Pointer to EFI MOK config table entry if found;
> + * null otherwise.
> + *
> + * This routine depends on the EFI MOK config table being entirely
> + * mapped with it's starting virtual address in efi_mokvar_table_va.
> + */
> +struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
> +{
> + struct efi_mokvar_table_entry *mokvar_entry = NULL;
> +
> + while (efi_mokvar_entry_next(&mokvar_entry)) {
> + if (!strncmp(name, mokvar_entry->name,
> + sizeof(mokvar_entry->name)))
> + return mokvar_entry;
> + }
> + return NULL;
> +}
> +
> +/*
> + * efi_mokvar_sysfs_read() - sysfs binary file read routine
> + *
> + * Returns: Count of bytes read.
> + *
> + * Copy EFI MOK config table entry data for this mokvar sysfs binary file
> + * to the supplied buffer, starting at the specified offset into mokvar table
> + * entry data, for the specified count bytes. The copy is limited by the
> + * amount of data in this mokvar config table entry.
> + */
> +static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
> + struct bin_attribute *bin_attr, char *buf,
> + loff_t off, size_t count)
> +{
> + struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return 0;
> +
> + if (off >= mokvar_entry->data_size)
> + return 0;
> + if (count > mokvar_entry->data_size - off)
> + count = mokvar_entry->data_size - off;
> +
> + memcpy(buf, mokvar_entry->data + off, count);
> + return count;
> +}
> +
> +/*
> + * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
> + *
> + * Map the EFI MOK variable config table for run-time use by the kernel
> + * and create the sysfs entries in /sys/firmware/efi/mok-variables/
> + *
> + * This routine just returns if a valid EFI MOK variable config table
> + * was not found earlier during boot.
> + *
> + * This routine must be called during a "middle" initcall phase, i.e.
> + * after efi_mokvar_table_init() but before UEFI certs are loaded
> + * during late init.
> + *
> + * Implicit inputs:
> + * efi.mokvar_table: Physical address of EFI MOK variable config table
> + * or special value that indicates no such table.
> + *
> + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> + * The table is considered present and valid if this
> + * is non-zero.
> + *
> + * Implicit outputs:
> + * efi_mokvar_table_va: Start virtual address of the EFI MOK config table.
> + */
> +static int __init efi_mokvar_sysfs_init(void)
> +{
> + void *config_va;
> + struct efi_mokvar_table_entry *mokvar_entry = NULL;
> + struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
> + int err = 0;
> +
> + if (efi_mokvar_table_size == 0)
> + return -ENOENT;
> +
> + config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
> + MEMREMAP_WB);
> + if (!config_va) {
> + pr_err("Failed to map EFI MOKvar config table\n");
> + return -ENOMEM;
> + }
> + efi_mokvar_table_va = config_va;
> +
> + mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
> + if (!mokvar_kobj) {
> + pr_err("Failed to create EFI mok-variables sysfs entry\n");
> + return -ENOMEM;
> + }
> +
> + while (efi_mokvar_entry_next(&mokvar_entry)) {
> + mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
> + if (!mokvar_sysfs) {
> + err = -ENOMEM;
> + break;
> + }
> +
> + sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
> + mokvar_sysfs->bin_attr.private = mokvar_entry;
> + mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
> + mokvar_sysfs->bin_attr.attr.mode = 0400;
> + mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
> + mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
> +
> + err = sysfs_create_bin_file(mokvar_kobj,
> + &mokvar_sysfs->bin_attr);
> + if (err)
> + break;
> +
> + list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
> + }
> +
> + if (err) {
> + pr_err("Failed to create some EFI mok-variables sysfs entries\n");
> + kfree(mokvar_sysfs);
> + }
> + return err;
> +}
> +device_initcall(efi_mokvar_sysfs_init);
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 73db1ae04cef..4a2332f146eb 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
> #define LINUX_EFI_TPM_FINAL_LOG_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
> #define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
> #define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> +#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
>
> /* OEM GUIDs */
> #define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
> @@ -546,6 +547,7 @@ extern struct efi {
> unsigned long esrt; /* ESRT table */
> unsigned long tpm_log; /* TPM2 Event Log table */
> unsigned long tpm_final_log; /* TPM2 Final Events Log table */
> + unsigned long mokvar_table; /* MOK variable config table */
>
> efi_get_time_t *get_time;
> efi_set_time_t *set_time;
> @@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
>
> char *efi_systab_show_arch(char *str);
>
> +/*
> + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
> + * to the kernel by an EFI boot loader. The table contains a packed
> + * sequence of these entries, one for each named MOK variable.
> + * The sequence is terminated by an entry with a completely NULL
> + * name and 0 data size.
> + */
> +struct efi_mokvar_table_entry {
> + char name[256];
> + u64 data_size;
> + u8 data[];
> +} __attribute((packed));
> +
> +#ifdef CONFIG_LOAD_UEFI_KEYS
> +extern void __init efi_mokvar_table_init(void);
> +extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> + struct efi_mokvar_table_entry **mokvar_entry);
> +extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
> +#else
> +static inline void efi_mokvar_table_init(void) { }
> +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> + struct efi_mokvar_table_entry **mokvar_entry)
> +{
> + return NULL;
> +}
> +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
> + const char *name)
> +{
> + return NULL;
> +}
> +#endif
> +
> #endif /* _LINUX_EFI_H */
> --
> 2.27.0
>

2020-09-21 16:30:02

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <[email protected]> wrote:
>
> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a much more primitive mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > This patch adds initial kernel support to recognize, parse,
> > and validate the EFI MOK configuration table, where named
> > entries contain the same data that would otherwise be provided
> > in similarly named EFI variables.
> >
> > Additionally, this patch creates a sysfs binary file for each
> > EFI MOK configuration table entry found. These files are read-only
> > to root and are provided for use by user space utilities such as
> > mokutil.
> >
> > A subsequent patch will load MOK certs into the trusted platform
> > key ring using this infrastructure.
> >
> > Signed-off-by: Lenny Szubowicz <[email protected]>
> > ---
> > arch/x86/kernel/setup.c | 1 +
> > arch/x86/platform/efi/efi.c | 3 +
> > drivers/firmware/efi/Makefile | 1 +
> > drivers/firmware/efi/arm-init.c | 1 +
> > drivers/firmware/efi/efi.c | 6 +
> > drivers/firmware/efi/mokvar-table.c | 360 ++++++++++++++++++++++++++++
> > include/linux/efi.h | 34 +++
> > 7 files changed, 406 insertions(+)
> > create mode 100644 drivers/firmware/efi/mokvar-table.c
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 3511736fbc74..d41be0df72f8 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -1077,6 +1077,7 @@ void __init setup_arch(char **cmdline_p)
> > efi_fake_memmap();
> > efi_find_mirror();
> > efi_esrt_init();
> > + efi_mokvar_table_init();
> >
> > /*
> > * The EFI specification says that boot service code won't be
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index d37ebe6e70d7..8a26e705cb06 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -90,6 +90,9 @@ static const unsigned long * const efi_tables[] = {
> > &efi.tpm_log,
> > &efi.tpm_final_log,
> > &efi_rng_seed,
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > + &efi.mokvar_table,
> > +#endif
> > };
> >
> > u64 efi_setup; /* efi setup_data physical address */
> > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> > index 7a216984552b..03964e2d27c5 100644
> > --- a/drivers/firmware/efi/Makefile
> > +++ b/drivers/firmware/efi/Makefile
> > @@ -28,6 +28,7 @@ obj-$(CONFIG_EFI_DEV_PATH_PARSER) += dev-path-parser.o
> > obj-$(CONFIG_APPLE_PROPERTIES) += apple-properties.o
> > obj-$(CONFIG_EFI_RCI2_TABLE) += rci2-table.o
> > obj-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += embedded-firmware.o
> > +obj-$(CONFIG_LOAD_UEFI_KEYS) += mokvar-table.o
> >
> > fake_map-y += fake_mem.o
> > fake_map-$(CONFIG_X86) += x86_fake_mem.o
> > diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c
> > index 71c445d20258..f55a92ff12c0 100644
> > --- a/drivers/firmware/efi/arm-init.c
> > +++ b/drivers/firmware/efi/arm-init.c
> > @@ -236,6 +236,7 @@ void __init efi_init(void)
> >
> > reserve_regions();
> > efi_esrt_init();
> > + efi_mokvar_table_init();
> >
> > memblock_reserve(data.phys_map & PAGE_MASK,
> > PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index 3aa07c3b5136..3d4daf215e19 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -43,6 +43,9 @@ struct efi __read_mostly efi = {
> > .esrt = EFI_INVALID_TABLE_ADDR,
> > .tpm_log = EFI_INVALID_TABLE_ADDR,
> > .tpm_final_log = EFI_INVALID_TABLE_ADDR,
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > + .mokvar_table = EFI_INVALID_TABLE_ADDR,
> > +#endif
> > };
> > EXPORT_SYMBOL(efi);
> >
> > @@ -518,6 +521,9 @@ static const efi_config_table_type_t common_tables[] __initconst = {
> > {EFI_RT_PROPERTIES_TABLE_GUID, &rt_prop, "RTPROP" },
> > #ifdef CONFIG_EFI_RCI2_TABLE
> > {DELLEMC_EFI_RCI2_TABLE_GUID, &rci2_table_phys },
> > +#endif
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > + {LINUX_EFI_MOK_VARIABLE_TABLE_GUID, &efi.mokvar_table, "MOKvar" },
> > #endif
> > {},
> > };
> > diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> > new file mode 100644
> > index 000000000000..f12f1710f5d9
> > --- /dev/null
> > +++ b/drivers/firmware/efi/mokvar-table.c
> > @@ -0,0 +1,360 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mokvar-table.c
> > + *
> > + * Copyright (c) 2020 Red Hat
> > + * Author: Lenny Szubowicz <[email protected]>
> > + *
> > + * This module contains the kernel support for the Linux EFI Machine
> > + * Owner Key (MOK) variable configuration table, which is identified by
> > + * the LINUX_EFI_MOK_VARIABLE_TABLE_GUID.
> > + *
> > + * This EFI configuration table provides a more robust alternative to
> > + * EFI volatile variables by which an EFI boot loader can pass the
> > + * contents of the Machine Owner Key (MOK) certificate stores to the
> > + * kernel during boot. If both the EFI MOK config table and corresponding
> > + * EFI MOK variables are present, the table should be considered as
> > + * more authoritative.
> > + *
> > + * This module includes code that validates and maps the EFI MOK table,
> > + * if it's presence was detected very early in boot.
> > + *
> > + * Kernel interface routines are provided to walk through all the
> > + * entries in the MOK config table or to search for a specific named
> > + * entry.
> > + *
> > + * The contents of the individual named MOK config table entries are
> > + * made available to user space via read-only sysfs binary files under:
> > + *
> > + * /sys/firmware/efi/mok-variables/
> > + *
> > + */
> > +#define pr_fmt(fmt) "mokvar: " fmt
> > +
> > +#include <linux/capability.h>
> > +#include <linux/efi.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/kobject.h>
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table is a packed
> > + * sequence of struct efi_mokvar_table_entry, one for each named
> > + * MOK variable. The sequence is terminated by an entry with a
> > + * completely NULL name and 0 data size.
> > + *
> > + * efi_mokvar_table_size is set to the computed size of the
> > + * MOK config table by efi_mokvar_table_init(). This will be
> > + * non-zero if and only if the table if present and has been
> > + * validated by efi_mokvar_table_init().
> > + */
> > +static size_t efi_mokvar_table_size;
> > +
> > +/*
> > + * efi_mokvar_table_va is the kernel virtual address at which the
> > + * EFI MOK config table has been mapped by efi_mokvar_sysfs_init().
> > + */
> > +static struct efi_mokvar_table_entry *efi_mokvar_table_va;
> > +
> > +/*
> > + * Each /sys/firmware/efi/mok-variables/ sysfs file is represented by
> > + * an instance of struct efi_mokvar_sysfs_attr on efi_mokvar_sysfs_list.
> > + * bin_attr.private points to the associated EFI MOK config table entry.
> > + *
> > + * This list is created during boot and then remains unchanged.
> > + * So no sychronization is currently required to walk the list.
> > + */
> > +struct efi_mokvar_sysfs_attr {
> > + struct bin_attribute bin_attr;
> > + struct list_head node;
> > +};
> > +
> > +static LIST_HEAD(efi_mokvar_sysfs_list);
> > +static struct kobject *mokvar_kobj;
> > +
> > +/*
> > + * efi_mokvar_table_init() - Early boot validation of EFI MOK config table
> > + *
> > + * If present, validate and compute the size of the EFI MOK variable
> > + * configuration table. This table may be provided by an EFI boot loader
> > + * as an alternative to ordinary EFI variables, due to platform-dependent
> > + * limitations. The memory occupied by this table is marked as reserved.
> > + *
> > + * This routine must be called before efi_free_boot_services() in order
> > + * to guarantee that it can mark the table as reserved.
> > + *
> > + * Implicit inputs:
> > + * efi.mokvar_table: Physical address of EFI MOK variable config table
> > + * or special value that indicates no such table.
> > + *
> > + * Implicit outputs:
> > + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> > + * The table is considered present and valid if this
> > + * is non-zero.
> > + */
> > +void __init efi_mokvar_table_init(void)
> > +{
> > + efi_memory_desc_t md;
> > + u64 end_pa;
> > + void *va = NULL;
> > + size_t cur_offset = 0;
> > + size_t offset_limit;
> > + size_t map_size = 0;
> > + size_t map_size_needed = 0;
> > + size_t size;
> > + struct efi_mokvar_table_entry *mokvar_entry;
> > + int err = -EINVAL;
> > +
> > + if (!efi_enabled(EFI_MEMMAP))
> > + return;
> > +
> > + if (efi.mokvar_table == EFI_INVALID_TABLE_ADDR)
> > + return;
> > + /*
> > + * The EFI MOK config table must fit within a single EFI memory
> > + * descriptor range.
> > + */
> > + err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> > + if (err) {
> > + pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> > + return;
> > + }
> > + end_pa = efi_mem_desc_end(&md);
> > + if (efi.mokvar_table >= end_pa) {
> > + pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> > + return;
> > + }
>
> efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
> why check it again?
>
> > + offset_limit = end_pa - efi.mokvar_table;
> > + /*
> > + * Validate the MOK config table. Since there is no table header
> > + * from which we could get the total size of the MOK config table,
> > + * we compute the total size as we validate each variably sized
> > + * entry, remapping as necessary.
> > + */
> > + while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> > + mokvar_entry = va + cur_offset;
> > + map_size_needed = cur_offset + sizeof(*mokvar_entry);
> > + if (map_size_needed > map_size) {
> > + if (va)
> > + early_memunmap(va, map_size);
> > + /*
> > + * Map a little more than the fixed size entry
> > + * header, anticipating some data. It's safe to
> > + * do so as long as we stay within current memory
> > + * descriptor.
> > + */
> > + map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> > + offset_limit);
> > + va = early_memremap(efi.mokvar_table, map_size);
>
> Can't we just map the entire region from efi.mokvar_table to end_pa in
> one early_memremap call before the loop and avoid all the remapping
> logic?
>

I suppose that depends on whether there is a reasonable upper bound on
the size which is guaranteed to be mappable using early_memremap()
(e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)

> > + if (!va) {
> > + pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
> > + efi.mokvar_table, map_size);
> > + return;
> > + }
> > + mokvar_entry = va + cur_offset;
> > + }
> > +
> > + /* Check for last sentinel entry */
> > + if (mokvar_entry->name[0] == '\0') {
> > + if (mokvar_entry->data_size != 0)
> > + break;
> > + err = 0;
> > + break;
> > + }
> > +
> > + /* Sanity check that the name is null terminated */
> > + size = strnlen(mokvar_entry->name,
> > + sizeof(mokvar_entry->name));
> > + if (size >= sizeof(mokvar_entry->name))
> > + break;
> > +
> > + /* Advance to the next entry */
> > + cur_offset = map_size_needed + mokvar_entry->data_size;
> > + }
> > +
> > + if (va)
> > + early_memunmap(va, map_size);
> > + if (err) {
> > + pr_err("EFI MOKvar config table is not valid\n");
> > + return;
> > + }
>
> err will never be non-zero here: it was cleared when the
> efi_mem_desc_lookup() was done. I think the initialization of err to
> -EINVAL needs to be moved just prior to the loop.
>
> > + efi_mem_reserve(efi.mokvar_table, map_size_needed);
> > + efi_mokvar_table_size = map_size_needed;
> > +}
> > +
> > +/*
> > + * efi_mokvar_entry_next() - Get next entry in the EFI MOK config table
> > + *
> > + * mokvar_entry: Pointer to current EFI MOK config table entry
> > + * or null. Null indicates get first entry.
> > + * Passed by reference. This is updated to the
> > + * same value as the return value.
> > + *
> > + * Returns: Pointer to next EFI MOK config table entry
> > + * or null, if there are no more entries.
> > + * Same value is returned in the mokvar_entry
> > + * parameter.
> > + *
> > + * This routine depends on the EFI MOK config table being entirely
> > + * mapped with it's starting virtual address in efi_mokvar_table_va.
> > + */
> > +struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > + struct efi_mokvar_table_entry **mokvar_entry)
> > +{
> > + struct efi_mokvar_table_entry *mokvar_cur;
> > + struct efi_mokvar_table_entry *mokvar_next;
> > + size_t size_cur;
> > +
> > + mokvar_cur = *mokvar_entry;
> > + *mokvar_entry = NULL;
> > +
> > + if (efi_mokvar_table_va == NULL)
> > + return NULL;
> > +
> > + if (mokvar_cur == NULL) {
> > + mokvar_next = efi_mokvar_table_va;
> > + } else {
> > + if (mokvar_cur->name[0] == '\0')
> > + return NULL;
> > + size_cur = sizeof(*mokvar_cur) + mokvar_cur->data_size;
> > + mokvar_next = (void *)mokvar_cur + size_cur;
> > + }
> > +
> > + if (mokvar_next->name[0] == '\0')
> > + return NULL;
> > +
> > + *mokvar_entry = mokvar_next;
> > + return mokvar_next;
> > +}
> > +
> > +/*
> > + * efi_mokvar_entry_find() - Find EFI MOK config entry by name
> > + *
> > + * name: Name of the entry to look for.
> > + *
> > + * Returns: Pointer to EFI MOK config table entry if found;
> > + * null otherwise.
> > + *
> > + * This routine depends on the EFI MOK config table being entirely
> > + * mapped with it's starting virtual address in efi_mokvar_table_va.
> > + */
> > +struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name)
> > +{
> > + struct efi_mokvar_table_entry *mokvar_entry = NULL;
> > +
> > + while (efi_mokvar_entry_next(&mokvar_entry)) {
> > + if (!strncmp(name, mokvar_entry->name,
> > + sizeof(mokvar_entry->name)))
> > + return mokvar_entry;
> > + }
> > + return NULL;
> > +}
> > +
> > +/*
> > + * efi_mokvar_sysfs_read() - sysfs binary file read routine
> > + *
> > + * Returns: Count of bytes read.
> > + *
> > + * Copy EFI MOK config table entry data for this mokvar sysfs binary file
> > + * to the supplied buffer, starting at the specified offset into mokvar table
> > + * entry data, for the specified count bytes. The copy is limited by the
> > + * amount of data in this mokvar config table entry.
> > + */
> > +static ssize_t efi_mokvar_sysfs_read(struct file *file, struct kobject *kobj,
> > + struct bin_attribute *bin_attr, char *buf,
> > + loff_t off, size_t count)
> > +{
> > + struct efi_mokvar_table_entry *mokvar_entry = bin_attr->private;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return 0;
> > +
> > + if (off >= mokvar_entry->data_size)
> > + return 0;
> > + if (count > mokvar_entry->data_size - off)
> > + count = mokvar_entry->data_size - off;
> > +
> > + memcpy(buf, mokvar_entry->data + off, count);
> > + return count;
> > +}
> > +
> > +/*
> > + * efi_mokvar_sysfs_init() - Map EFI MOK config table and create sysfs
> > + *
> > + * Map the EFI MOK variable config table for run-time use by the kernel
> > + * and create the sysfs entries in /sys/firmware/efi/mok-variables/
> > + *
> > + * This routine just returns if a valid EFI MOK variable config table
> > + * was not found earlier during boot.
> > + *
> > + * This routine must be called during a "middle" initcall phase, i.e.
> > + * after efi_mokvar_table_init() but before UEFI certs are loaded
> > + * during late init.
> > + *
> > + * Implicit inputs:
> > + * efi.mokvar_table: Physical address of EFI MOK variable config table
> > + * or special value that indicates no such table.
> > + *
> > + * efi_mokvar_table_size: Computed size of EFI MOK variable config table.
> > + * The table is considered present and valid if this
> > + * is non-zero.
> > + *
> > + * Implicit outputs:
> > + * efi_mokvar_table_va: Start virtual address of the EFI MOK config table.
> > + */
> > +static int __init efi_mokvar_sysfs_init(void)
> > +{
> > + void *config_va;
> > + struct efi_mokvar_table_entry *mokvar_entry = NULL;
> > + struct efi_mokvar_sysfs_attr *mokvar_sysfs = NULL;
> > + int err = 0;
> > +
> > + if (efi_mokvar_table_size == 0)
> > + return -ENOENT;
> > +
> > + config_va = memremap(efi.mokvar_table, efi_mokvar_table_size,
> > + MEMREMAP_WB);
> > + if (!config_va) {
> > + pr_err("Failed to map EFI MOKvar config table\n");
> > + return -ENOMEM;
> > + }
> > + efi_mokvar_table_va = config_va;
> > +
> > + mokvar_kobj = kobject_create_and_add("mok-variables", efi_kobj);
> > + if (!mokvar_kobj) {
> > + pr_err("Failed to create EFI mok-variables sysfs entry\n");
> > + return -ENOMEM;
> > + }
> > +
> > + while (efi_mokvar_entry_next(&mokvar_entry)) {
> > + mokvar_sysfs = kzalloc(sizeof(*mokvar_sysfs), GFP_KERNEL);
> > + if (!mokvar_sysfs) {
> > + err = -ENOMEM;
> > + break;
> > + }
> > +
> > + sysfs_bin_attr_init(&mokvar_sysfs->bin_attr);
> > + mokvar_sysfs->bin_attr.private = mokvar_entry;
> > + mokvar_sysfs->bin_attr.attr.name = mokvar_entry->name;
> > + mokvar_sysfs->bin_attr.attr.mode = 0400;
> > + mokvar_sysfs->bin_attr.size = mokvar_entry->data_size;
> > + mokvar_sysfs->bin_attr.read = efi_mokvar_sysfs_read;
> > +
> > + err = sysfs_create_bin_file(mokvar_kobj,
> > + &mokvar_sysfs->bin_attr);
> > + if (err)
> > + break;
> > +
> > + list_add_tail(&mokvar_sysfs->node, &efi_mokvar_sysfs_list);
> > + }
> > +
> > + if (err) {
> > + pr_err("Failed to create some EFI mok-variables sysfs entries\n");
> > + kfree(mokvar_sysfs);
> > + }
> > + return err;
> > +}
> > +device_initcall(efi_mokvar_sysfs_init);
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 73db1ae04cef..4a2332f146eb 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -357,6 +357,7 @@ void efi_native_runtime_setup(void);
> > #define LINUX_EFI_TPM_FINAL_LOG_GUID EFI_GUID(0x1e2ed096, 0x30e2, 0x4254, 0xbd, 0x89, 0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25)
> > #define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
> > #define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
> > +#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
> >
> > /* OEM GUIDs */
> > #define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
> > @@ -546,6 +547,7 @@ extern struct efi {
> > unsigned long esrt; /* ESRT table */
> > unsigned long tpm_log; /* TPM2 Event Log table */
> > unsigned long tpm_final_log; /* TPM2 Final Events Log table */
> > + unsigned long mokvar_table; /* MOK variable config table */
> >
> > efi_get_time_t *get_time;
> > efi_set_time_t *set_time;
> > @@ -1252,4 +1254,36 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size);
> >
> > char *efi_systab_show_arch(char *str);
> >
> > +/*
> > + * The LINUX_EFI_MOK_VARIABLE_TABLE_GUID config table can be provided
> > + * to the kernel by an EFI boot loader. The table contains a packed
> > + * sequence of these entries, one for each named MOK variable.
> > + * The sequence is terminated by an entry with a completely NULL
> > + * name and 0 data size.
> > + */
> > +struct efi_mokvar_table_entry {
> > + char name[256];
> > + u64 data_size;
> > + u8 data[];
> > +} __attribute((packed));
> > +
> > +#ifdef CONFIG_LOAD_UEFI_KEYS
> > +extern void __init efi_mokvar_table_init(void);
> > +extern struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > + struct efi_mokvar_table_entry **mokvar_entry);
> > +extern struct efi_mokvar_table_entry *efi_mokvar_entry_find(const char *name);
> > +#else
> > +static inline void efi_mokvar_table_init(void) { }
> > +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_next(
> > + struct efi_mokvar_table_entry **mokvar_entry)
> > +{
> > + return NULL;
> > +}
> > +static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
> > + const char *name)
> > +{
> > + return NULL;
> > +}
> > +#endif
> > +
> > #endif /* _LINUX_EFI_H */
> > --
> > 2.27.0
> >

2020-09-21 16:58:50

by Arvind Sankar

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On Mon, Sep 21, 2020 at 06:27:17PM +0200, Ard Biesheuvel wrote:
> On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <[email protected]> wrote:
> >
> > On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > > + /*
> > > + * The EFI MOK config table must fit within a single EFI memory
> > > + * descriptor range.
> > > + */
> > > + err = efi_mem_desc_lookup(efi.mokvar_table, &md);
> > > + if (err) {
> > > + pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
> > > + return;
> > > + }
> > > + end_pa = efi_mem_desc_end(&md);
> > > + if (efi.mokvar_table >= end_pa) {
> > > + pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> > > + return;
> > > + }
> >
> > efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
> > why check it again?
> >
> > > + offset_limit = end_pa - efi.mokvar_table;
> > > + /*
> > > + * Validate the MOK config table. Since there is no table header
> > > + * from which we could get the total size of the MOK config table,
> > > + * we compute the total size as we validate each variably sized
> > > + * entry, remapping as necessary.
> > > + */
> > > + while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
> > > + mokvar_entry = va + cur_offset;
> > > + map_size_needed = cur_offset + sizeof(*mokvar_entry);
> > > + if (map_size_needed > map_size) {
> > > + if (va)
> > > + early_memunmap(va, map_size);
> > > + /*
> > > + * Map a little more than the fixed size entry
> > > + * header, anticipating some data. It's safe to
> > > + * do so as long as we stay within current memory
> > > + * descriptor.
> > > + */
> > > + map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
> > > + offset_limit);
> > > + va = early_memremap(efi.mokvar_table, map_size);
> >
> > Can't we just map the entire region from efi.mokvar_table to end_pa in
> > one early_memremap call before the loop and avoid all the remapping
> > logic?
> >
>
> I suppose that depends on whether there is a reasonable upper bound on
> the size which is guaranteed to be mappable using early_memremap()
> (e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)

Ah, sorry, I thought only the number of early mappings was limited, not
the size as well. We could still just map the maximum possible
(NR_FIX_BTMAPS * PAGE_SIZE), since it will fail anyway if the config
table turns out to be bigger than that?

>
>
> > > + if (va)
> > > + early_memunmap(va, map_size);
> > > + if (err) {
> > > + pr_err("EFI MOKvar config table is not valid\n");
> > > + return;
> > > + }
> >
> > err will never be non-zero here: it was cleared when the
> > efi_mem_desc_lookup() was done. I think the initialization of err to
> > -EINVAL needs to be moved just prior to the loop.
> >
> > > + efi_mem_reserve(efi.mokvar_table, map_size_needed);
> > > + efi_mokvar_table_size = map_size_needed;
> > > +}

2020-09-24 19:11:31

by Lenny Szubowicz

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On 9/21/20 12:55 PM, Arvind Sankar wrote:
> On Mon, Sep 21, 2020 at 06:27:17PM +0200, Ard Biesheuvel wrote:
>> On Mon, 21 Sep 2020 at 18:19, Arvind Sankar <[email protected]> wrote:
>>>
>>> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
>>>> + /*
>>>> + * The EFI MOK config table must fit within a single EFI memory
>>>> + * descriptor range.
>>>> + */
>>>> + err = efi_mem_desc_lookup(efi.mokvar_table, &md);
>>>> + if (err) {
>>>> + pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
>>>> + return;
>>>> + }
>>>> + end_pa = efi_mem_desc_end(&md);
>>>> + if (efi.mokvar_table >= end_pa) {
>>>> + pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
>>>> + return;
>>>> + }
>>>
>>> efi_mem_desc_lookup() can't return success if efi.mokvar_table >= end_pa,
>>> why check it again?

I agree it's unnecessary and I see that Ard has addressed this in a patch.

>>>
>>>> + offset_limit = end_pa - efi.mokvar_table;
>>>> + /*
>>>> + * Validate the MOK config table. Since there is no table header
>>>> + * from which we could get the total size of the MOK config table,
>>>> + * we compute the total size as we validate each variably sized
>>>> + * entry, remapping as necessary.
>>>> + */
>>>> + while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
>>>> + mokvar_entry = va + cur_offset;
>>>> + map_size_needed = cur_offset + sizeof(*mokvar_entry);
>>>> + if (map_size_needed > map_size) {
>>>> + if (va)
>>>> + early_memunmap(va, map_size);
>>>> + /*
>>>> + * Map a little more than the fixed size entry
>>>> + * header, anticipating some data. It's safe to
>>>> + * do so as long as we stay within current memory
>>>> + * descriptor.
>>>> + */
>>>> + map_size = min(map_size_needed + 2*EFI_PAGE_SIZE,
>>>> + offset_limit);
>>>> + va = early_memremap(efi.mokvar_table, map_size);
>>>
>>> Can't we just map the entire region from efi.mokvar_table to end_pa in
>>> one early_memremap call before the loop and avoid all the remapping
>>> logic?
>>>
>>
>> I suppose that depends on whether there is a reasonable upper bound on
>> the size which is guaranteed to be mappable using early_memremap()
>> (e.g., 128 KB on 32-bit ARM, or 256 KB on other architectures)
>
> Ah, sorry, I thought only the number of early mappings was limited, not
> the size as well. We could still just map the maximum possible
> (NR_FIX_BTMAPS * PAGE_SIZE), since it will fail anyway if the config
> table turns out to be bigger than that?

In practice, the loop will only do one or two mappings. That's because of
two factors.

First the code attempts to map a little more than it needs (2*EFI_PAGE_SIZE).
Secondly, right now only one entry in the MOKvar config table might be quite
large, i.e. the entry named MoKListRT. It's extremely likely that the header
of MokListRT entry will be encountered on the first mapping. If that entry
goes beyond what was originally mapped, then a second mapping will cover all
the data in that large entry as well as the remaining small entries that might
follow it.

>
>>
>>
>>>> + if (va)
>>>> + early_memunmap(va, map_size);
>>>> + if (err) {
>>>> + pr_err("EFI MOKvar config table is not valid\n");
>>>> + return;
>>>> + }
>>>
>>> err will never be non-zero here: it was cleared when the
>>> efi_mem_desc_lookup() was done. I think the initialization of err to
>>> -EINVAL needs to be moved just prior to the loop.
>>>
>>>> + efi_mem_reserve(efi.mokvar_table, map_size_needed);
>>>> + efi_mokvar_table_size = map_size_needed;
>>>> +}
>

2020-10-01 17:48:01

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

2020-10-01 20:59:08

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On Thu, 1 Oct 2020 at 19:44, Nathan Chancellor <[email protected]> wrote:
>
> On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > Because of system-specific EFI firmware limitations, EFI volatile
> > variables may not be capable of holding the required contents of
> > the Machine Owner Key (MOK) certificate store when the certificate
> > list grows above some size. Therefore, an EFI boot loader may pass
> > the MOK certs via a EFI configuration table created specifically for
> > this purpose to avoid this firmware limitation.
> >
> > An EFI configuration table is a much more primitive mechanism
> > compared to EFI variables and is well suited for one-way passage
> > of static information from a pre-OS environment to the kernel.
> >
> > This patch adds initial kernel support to recognize, parse,
> > and validate the EFI MOK configuration table, where named
> > entries contain the same data that would otherwise be provided
> > in similarly named EFI variables.
> >
> > Additionally, this patch creates a sysfs binary file for each
> > EFI MOK configuration table entry found. These files are read-only
> > to root and are provided for use by user space utilities such as
> > mokutil.
> >
> > A subsequent patch will load MOK certs into the trusted platform
> > key ring using this infrastructure.
> >
> > Signed-off-by: Lenny Szubowicz <[email protected]>
>
> I have not seen this reported yet but this breaks arm allyesconfig and
> allmodconfig when CPU_LITTLE_ENDIAN is force selected (because CONFIG_EFI
> will actually be enabled):
>
> $ cat le.config
> CONFIG_CPU_BIG_ENDIAN=n
>
> $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=le.config allyesconfig drivers/firmware/efi/mokvar-table.o
> drivers/firmware/efi/mokvar-table.c: In function 'efi_mokvar_table_init':
> drivers/firmware/efi/mokvar-table.c:139:5: error: implicit declaration of function 'early_memunmap' [-Werror=implicit-function-declaration]
> 139 | early_memunmap(va, map_size);
> | ^~~~~~~~~~~~~~
> drivers/firmware/efi/mokvar-table.c:148:9: error: implicit declaration of function 'early_memremap' [-Werror=implicit-function-declaration]
> 148 | va = early_memremap(efi.mokvar_table, map_size);
> | ^~~~~~~~~~~~~~
> drivers/firmware/efi/mokvar-table.c:148:7: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> 148 | va = early_memremap(efi.mokvar_table, map_size);
> | ^
> cc1: some warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:283: drivers/firmware/efi/mokvar-table.o] Error 1
>
> Cheers,
> Nathan

Hi Nathan,

Does adding

#include <asm/early_ioremap.h>

to drivers/firmware/efi/mokvar-table.c fix the issue?

2020-10-01 21:08:55

by Nathan Chancellor

[permalink] [raw]

Subject: Re: [PATCH V2 1/3] efi: Support for MOK variable config table

On Thu, Oct 01, 2020 at 10:57:07PM +0200, Ard Biesheuvel wrote:
> On Thu, 1 Oct 2020 at 19:44, Nathan Chancellor <[email protected]> wrote:
> >
> > On Fri, Sep 04, 2020 at 09:31:05PM -0400, Lenny Szubowicz wrote:
> > > Because of system-specific EFI firmware limitations, EFI volatile
> > > variables may not be capable of holding the required contents of
> > > the Machine Owner Key (MOK) certificate store when the certificate
> > > list grows above some size. Therefore, an EFI boot loader may pass
> > > the MOK certs via a EFI configuration table created specifically for
> > > this purpose to avoid this firmware limitation.
> > >
> > > An EFI configuration table is a much more primitive mechanism
> > > compared to EFI variables and is well suited for one-way passage
> > > of static information from a pre-OS environment to the kernel.
> > >
> > > This patch adds initial kernel support to recognize, parse,
> > > and validate the EFI MOK configuration table, where named
> > > entries contain the same data that would otherwise be provided
> > > in similarly named EFI variables.
> > >
> > > Additionally, this patch creates a sysfs binary file for each
> > > EFI MOK configuration table entry found. These files are read-only
> > > to root and are provided for use by user space utilities such as
> > > mokutil.
> > >
> > > A subsequent patch will load MOK certs into the trusted platform
> > > key ring using this infrastructure.
> > >
> > > Signed-off-by: Lenny Szubowicz <[email protected]>
> >
> > I have not seen this reported yet but this breaks arm allyesconfig and
> > allmodconfig when CPU_LITTLE_ENDIAN is force selected (because CONFIG_EFI
> > will actually be enabled):
> >
> > $ cat le.config
> > CONFIG_CPU_BIG_ENDIAN=n
> >
> > $ make -skj"$(nproc)" ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KCONFIG_ALLCONFIG=le.config allyesconfig drivers/firmware/efi/mokvar-table.o
> > drivers/firmware/efi/mokvar-table.c: In function 'efi_mokvar_table_init':
> > drivers/firmware/efi/mokvar-table.c:139:5: error: implicit declaration of function 'early_memunmap' [-Werror=implicit-function-declaration]
> > 139 | early_memunmap(va, map_size);
> > | ^~~~~~~~~~~~~~
> > drivers/firmware/efi/mokvar-table.c:148:9: error: implicit declaration of function 'early_memremap' [-Werror=implicit-function-declaration]
> > 148 | va = early_memremap(efi.mokvar_table, map_size);
> > | ^~~~~~~~~~~~~~
> > drivers/firmware/efi/mokvar-table.c:148:7: warning: assignment to 'void *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
> > 148 | va = early_memremap(efi.mokvar_table, map_size);
> > | ^
> > cc1: some warnings being treated as errors
> > make[4]: *** [scripts/Makefile.build:283: drivers/firmware/efi/mokvar-table.o] Error 1
> >
> > Cheers,
> > Nathan
>
> Hi Nathan,
>
> Does adding
>
> #include <asm/early_ioremap.h>
>
> to drivers/firmware/efi/mokvar-table.c fix the issue?

Indeed, that was much simpler than I thought it would be... If you send
or apply a patch, feel free to add:

Tested-by: Nathan Chancellor <[email protected]>

Cheers,
Nathan