[v8,14/15] x86: Secure Launch late initcall platform module

From: "Daniel P. Smith" <dpsmith@apertussolutions.com>

The Secure Launch platform module is a late init module. During the
init call, the TPM event log is read and measurements taken in the
early boot stub code are located. These measurements are extended
into the TPM PCRs using the mainline TPM kernel driver.

The platform module also registers the securityfs nodes to allow
access to TXT register fields on Intel along with the fetching of
and writing events to the late launch TPM log.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: garnetgrimm <grimmg@ainfosec.com>
Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
---
 arch/x86/kernel/Makefile   |   1 +
 arch/x86/kernel/slmodule.c | 511 +++++++++++++++++++++++++++++++++++++
 2 files changed, 512 insertions(+)
 create mode 100644 arch/x86/kernel/slmodule.c

On Wed, 14 Feb 2024 at 23:32, Ross Philipson <ross.philipson@oracle.com> wrote:
>
> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>
> The Secure Launch platform module is a late init module. During the
> init call, the TPM event log is read and measurements taken in the
> early boot stub code are located. These measurements are extended
> into the TPM PCRs using the mainline TPM kernel driver.
>
> The platform module also registers the securityfs nodes to allow
> access to TXT register fields on Intel along with the fetching of
> and writing events to the late launch TPM log.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: garnetgrimm <grimmg@ainfosec.com>
> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>

There is an awful amount of code that executes between the point where
the measurements are taken and the point where they are loaded into
the PCRs. All of this code could subvert the boot flow and hide this
fact, by replacing the actual taken measurement values with the known
'blessed' ones that will unseal the keys and/or phone home to do a
successful remote attestation.

At the very least, this should be documented somewhere. And if at all
possible, it should also be documented why this is ok, and to what
extent it limits the provided guarantees compared to a true D-RTM boot
where the early boot code measures straight into the TPMs before
proceeding.


> ---
>  arch/x86/kernel/Makefile   |   1 +
>  arch/x86/kernel/slmodule.c | 511 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 512 insertions(+)
>  create mode 100644 arch/x86/kernel/slmodule.c
>
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 5848ea310175..948346ff4595 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -75,6 +75,7 @@ obj-$(CONFIG_IA32_EMULATION)  += tls.o
>  obj-y                          += step.o
>  obj-$(CONFIG_INTEL_TXT)                += tboot.o
>  obj-$(CONFIG_SECURE_LAUNCH)    += slaunch.o
> +obj-$(CONFIG_SECURE_LAUNCH)    += slmodule.o
>  obj-$(CONFIG_ISA_DMA_API)      += i8237.o
>  obj-y                          += stacktrace.o
>  obj-y                          += cpu/
> diff --git a/arch/x86/kernel/slmodule.c b/arch/x86/kernel/slmodule.c
> new file mode 100644
> index 000000000000..52269f24902e
> --- /dev/null
> +++ b/arch/x86/kernel/slmodule.c
> @@ -0,0 +1,511 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Secure Launch late validation/setup, securityfs exposure and finalization.
> + *
> + * Copyright (c) 2022 Apertus Solutions, LLC
> + * Copyright (c) 2021 Assured Information Security, Inc.
> + * Copyright (c) 2022, Oracle and/or its affiliates.
> + *
> + * Co-developed-by: Garnet T. Grimm <grimmg@ainfosec.com>
> + * Signed-off-by: Garnet T. Grimm <grimmg@ainfosec.com>
> + * Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/linkage.h>
> +#include <linux/mm.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +#include <linux/security.h>
> +#include <linux/memblock.h>
> +#include <asm/segment.h>
> +#include <asm/sections.h>
> +#include <crypto/sha2.h>
> +#include <linux/slr_table.h>
> +#include <linux/slaunch.h>
> +
> +/*
> + * The macro DECLARE_TXT_PUB_READ_U is used to read values from the TXT
> + * public registers as unsigned values.
> + */
> +#define DECLARE_TXT_PUB_READ_U(size, fmt, msg_size)                    \
> +static ssize_t txt_pub_read_u##size(unsigned int offset,               \
> +               loff_t *read_offset,                                    \
> +               size_t read_len,                                        \
> +               char __user *buf)                                       \
> +{                                                                      \
> +       char msg_buffer[msg_size];                                      \
> +       u##size reg_value = 0;                                          \
> +       void __iomem *txt;                                              \
> +                                                                       \
> +       txt = ioremap(TXT_PUB_CONFIG_REGS_BASE,                         \
> +                       TXT_NR_CONFIG_PAGES * PAGE_SIZE);               \
> +       if (!txt)                                                       \
> +               return -EFAULT;                                         \
> +       memcpy_fromio(&reg_value, txt + offset, sizeof(u##size));       \
> +       iounmap(txt);                                                   \
> +       snprintf(msg_buffer, msg_size, fmt, reg_value);                 \
> +       return simple_read_from_buffer(buf, read_len, read_offset,      \
> +                       &msg_buffer, msg_size);                         \
> +}
> +
> +DECLARE_TXT_PUB_READ_U(8, "%#04x\n", 6);
> +DECLARE_TXT_PUB_READ_U(32, "%#010x\n", 12);
> +DECLARE_TXT_PUB_READ_U(64, "%#018llx\n", 20);
> +
> +#define DECLARE_TXT_FOPS(reg_name, reg_offset, reg_size)               \
> +static ssize_t txt_##reg_name##_read(struct file *flip,                        \
> +               char __user *buf, size_t read_len, loff_t *read_offset) \
> +{                                                                      \
> +       return txt_pub_read_u##reg_size(reg_offset, read_offset,        \
> +                       read_len, buf);                                 \
> +}                                                                      \
> +static const struct file_operations reg_name##_ops = {                 \
> +       .read = txt_##reg_name##_read,                                  \
> +}
> +
> +DECLARE_TXT_FOPS(sts, TXT_CR_STS, 64);
> +DECLARE_TXT_FOPS(ests, TXT_CR_ESTS, 8);
> +DECLARE_TXT_FOPS(errorcode, TXT_CR_ERRORCODE, 32);
> +DECLARE_TXT_FOPS(didvid, TXT_CR_DIDVID, 64);
> +DECLARE_TXT_FOPS(e2sts, TXT_CR_E2STS, 64);
> +DECLARE_TXT_FOPS(ver_emif, TXT_CR_VER_EMIF, 32);
> +DECLARE_TXT_FOPS(scratchpad, TXT_CR_SCRATCHPAD, 64);
> +
> +/*
> + * Securityfs exposure
> + */
> +struct memfile {
> +       char *name;
> +       void *addr;
> +       size_t size;
> +};
> +
> +static struct memfile sl_evtlog = {"eventlog", NULL, 0};
> +static void *txt_heap;
> +static struct txt_heap_event_log_pointer2_1_element *evtlog20;
> +static DEFINE_MUTEX(sl_evt_log_mutex);
> +
> +static ssize_t sl_evtlog_read(struct file *file, char __user *buf,
> +                             size_t count, loff_t *pos)
> +{
> +       ssize_t size;
> +
> +       if (!sl_evtlog.addr)
> +               return 0;
> +
> +       mutex_lock(&sl_evt_log_mutex);
> +       size = simple_read_from_buffer(buf, count, pos, sl_evtlog.addr,
> +                                      sl_evtlog.size);
> +       mutex_unlock(&sl_evt_log_mutex);
> +
> +       return size;
> +}
> +
> +static ssize_t sl_evtlog_write(struct file *file, const char __user *buf,
> +                              size_t datalen, loff_t *ppos)
> +{
> +       ssize_t result;
> +       char *data;
> +
> +       if (!sl_evtlog.addr)
> +               return 0;
> +
> +       /* No partial writes. */
> +       result = -EINVAL;
> +       if (*ppos != 0)
> +               goto out;
> +
> +       data = memdup_user(buf, datalen);
> +       if (IS_ERR(data)) {
> +               result = PTR_ERR(data);
> +               goto out;
> +       }
> +
> +       mutex_lock(&sl_evt_log_mutex);
> +       if (evtlog20)
> +               result = tpm20_log_event(evtlog20, sl_evtlog.addr,
> +                                        sl_evtlog.size, datalen, data);
> +       else
> +               result = tpm12_log_event(sl_evtlog.addr, sl_evtlog.size,
> +                                        datalen, data);
> +       mutex_unlock(&sl_evt_log_mutex);
> +
> +       kfree(data);
> +out:
> +       return result;
> +}
> +
> +static const struct file_operations sl_evtlog_ops = {
> +       .read = sl_evtlog_read,
> +       .write = sl_evtlog_write,
> +       .llseek = default_llseek,
> +};
> +
> +struct sfs_file {
> +       const char *name;
> +       const struct file_operations *fops;
> +};
> +
> +#define SL_TXT_ENTRY_COUNT     7
> +static const struct sfs_file sl_txt_files[] = {
> +       { "sts", &sts_ops },
> +       { "ests", &ests_ops },
> +       { "errorcode", &errorcode_ops },
> +       { "didvid", &didvid_ops },
> +       { "ver_emif", &ver_emif_ops },
> +       { "scratchpad", &scratchpad_ops },
> +       { "e2sts", &e2sts_ops }
> +};
> +
> +/* sysfs file handles */
> +static struct dentry *slaunch_dir;
> +static struct dentry *event_file;
> +static struct dentry *txt_dir;
> +static struct dentry *txt_entries[SL_TXT_ENTRY_COUNT];
> +
> +static long slaunch_expose_securityfs(void)
> +{
> +       long ret = 0;
> +       int i;
> +
> +       slaunch_dir = securityfs_create_dir("slaunch", NULL);
> +       if (IS_ERR(slaunch_dir))
> +               return PTR_ERR(slaunch_dir);
> +
> +       if (slaunch_get_flags() & SL_FLAG_ARCH_TXT) {
> +               txt_dir = securityfs_create_dir("txt", slaunch_dir);
> +               if (IS_ERR(txt_dir)) {
> +                       ret = PTR_ERR(txt_dir);
> +                       goto remove_slaunch;
> +               }
> +
> +               for (i = 0; i < ARRAY_SIZE(sl_txt_files); i++) {
> +                       txt_entries[i] = securityfs_create_file(
> +                                               sl_txt_files[i].name, 0440,
> +                                               txt_dir, NULL,
> +                                               sl_txt_files[i].fops);
> +                       if (IS_ERR(txt_entries[i])) {
> +                               ret = PTR_ERR(txt_entries[i]);
> +                               goto remove_files;
> +                       }
> +               }
> +       }
> +
> +       if (sl_evtlog.addr) {
> +               event_file = securityfs_create_file(sl_evtlog.name, 0440,
> +                                                   slaunch_dir, NULL,
> +                                                   &sl_evtlog_ops);
> +               if (IS_ERR(event_file)) {
> +                       ret = PTR_ERR(event_file);
> +                       goto remove_files;
> +               }
> +       }
> +
> +       return 0;
> +
> +remove_files:
> +       if (slaunch_get_flags() & SL_FLAG_ARCH_TXT) {
> +               while (--i >= 0)
> +                       securityfs_remove(txt_entries[i]);
> +               securityfs_remove(txt_dir);
> +       }
> +
> +remove_slaunch:
> +       securityfs_remove(slaunch_dir);
> +
> +       return ret;
> +}
> +
> +static void slaunch_teardown_securityfs(void)
> +{
> +       int i;
> +
> +       securityfs_remove(event_file);
> +       if (sl_evtlog.addr) {
> +               memunmap(sl_evtlog.addr);
> +               sl_evtlog.addr = NULL;
> +       }
> +       sl_evtlog.size = 0;
> +
> +       if (slaunch_get_flags() & SL_FLAG_ARCH_TXT) {
> +               for (i = 0; i < ARRAY_SIZE(sl_txt_files); i++)
> +                       securityfs_remove(txt_entries[i]);
> +
> +               securityfs_remove(txt_dir);
> +
> +               if (txt_heap) {
> +                       memunmap(txt_heap);
> +                       txt_heap = NULL;
> +               }
> +       }
> +
> +       securityfs_remove(slaunch_dir);
> +}
> +
> +static void slaunch_intel_evtlog(void __iomem *txt)
> +{
> +       struct slr_entry_log_info *log_info;
> +       struct txt_os_mle_data *params;
> +       struct slr_table *slrt;
> +       void *os_sinit_data;
> +       u64 base, size;
> +
> +       memcpy_fromio(&base, txt + TXT_CR_HEAP_BASE, sizeof(base));
> +       memcpy_fromio(&size, txt + TXT_CR_HEAP_SIZE, sizeof(size));
> +
> +       /* now map TXT heap */
> +       txt_heap = memremap(base, size, MEMREMAP_WB);
> +       if (!txt_heap)
> +               slaunch_txt_reset(txt, "Error failed to memremap TXT heap\n",
> +                                 SL_ERROR_HEAP_MAP);
> +
> +       params = (struct txt_os_mle_data *)txt_os_mle_data_start(txt_heap);
> +
> +       /* Get the SLRT and remap it */
> +       slrt = memremap(params->slrt, sizeof(*slrt), MEMREMAP_WB);
> +       if (!slrt)
> +               slaunch_txt_reset(txt, "Error failed to memremap SLR Table\n",
> +                                 SL_ERROR_SLRT_MAP);
> +       size = slrt->size;
> +       memunmap(slrt);
> +
> +       slrt = memremap(params->slrt, size, MEMREMAP_WB);
> +       if (!slrt)
> +               slaunch_txt_reset(txt, "Error failed to memremap SLR Table\n",
> +                                 SL_ERROR_SLRT_MAP);
> +
> +       log_info = (struct slr_entry_log_info *)slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_LOG_INFO);
> +       if (!log_info)
> +               slaunch_txt_reset(txt, "Error failed to memremap SLR Table\n",
> +                                 SL_ERROR_SLRT_MISSING_ENTRY);
> +
> +       sl_evtlog.size = log_info->size;
> +       sl_evtlog.addr = memremap(log_info->addr, log_info->size,
> +                                 MEMREMAP_WB);
> +       if (!sl_evtlog.addr)
> +               slaunch_txt_reset(txt, "Error failed to memremap TPM event log\n",
> +                                 SL_ERROR_EVENTLOG_MAP);
> +
> +       memunmap(slrt);
> +
> +       /* Determine if this is TPM 1.2 or 2.0 event log */
> +       if (memcmp(sl_evtlog.addr + sizeof(struct tcg_pcr_event),
> +                   TCG_SPECID_SIG, sizeof(TCG_SPECID_SIG)))
> +               return; /* looks like it is not 2.0 */
> +
> +       /* For TPM 2.0 logs, the extended heap element must be located */
> +       os_sinit_data = txt_os_sinit_data_start(txt_heap);
> +
> +       evtlog20 = tpm20_find_log2_1_element(os_sinit_data);
> +
> +       /*
> +        * If this fails, things are in really bad shape. Any attempt to write
> +        * events to the log will fail.
> +        */
> +       if (!evtlog20)
> +               slaunch_txt_reset(txt, "Error failed to find TPM20 event log element\n",
> +                                 SL_ERROR_TPM_INVALID_LOG20);
> +}
> +
> +static void slaunch_tpm20_extend_event(struct tpm_chip *tpm, void __iomem *txt,
> +                                      struct tcg_pcr_event2_head *event)
> +{
> +       u16 *alg_id_field = (u16 *)((u8 *)event + sizeof(struct tcg_pcr_event2_head));
> +       struct tpm_digest *digests;
> +       u8 *dptr;
> +       u32 i, j;
> +       int ret;
> +
> +       digests = kcalloc(tpm->nr_allocated_banks, sizeof(*digests),
> +                         GFP_KERNEL);
> +       if (!digests)
> +               slaunch_txt_reset(txt, "Failed to allocate array of digests\n",
> +                                 SL_ERROR_GENERIC);
> +
> +       for (i = 0; i < tpm->nr_allocated_banks; i++)
> +               digests[i].alg_id = tpm->allocated_banks[i].alg_id;
> +
> +       /* Early SL code ensured there was a max count of 2 digests */
> +       for (i = 0; i < event->count; i++) {
> +               dptr = (u8 *)alg_id_field + sizeof(u16);
> +
> +               for (j = 0; j < tpm->nr_allocated_banks; j++) {
> +                       if (digests[j].alg_id != *alg_id_field)
> +                               continue;
> +
> +                       switch (digests[j].alg_id) {
> +                       case TPM_ALG_SHA256:
> +                               memcpy(&digests[j].digest[0], dptr,
> +                                      SHA256_DIGEST_SIZE);
> +                               alg_id_field = (u16 *)((u8 *)alg_id_field +
> +                                       SHA256_DIGEST_SIZE + sizeof(u16));
> +                               break;
> +                       case TPM_ALG_SHA1:
> +                               memcpy(&digests[j].digest[0], dptr,
> +                                      SHA1_DIGEST_SIZE);
> +                               alg_id_field = (u16 *)((u8 *)alg_id_field +
> +                                       SHA1_DIGEST_SIZE + sizeof(u16));
> +                       default:
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       ret = tpm_pcr_extend(tpm, event->pcr_idx, digests);
> +       if (ret) {
> +               pr_err("Error extending TPM20 PCR, result: %d\n", ret);
> +               slaunch_txt_reset(txt, "Failed to extend TPM20 PCR\n",
> +                                 SL_ERROR_TPM_EXTEND);
> +       }
> +
> +       kfree(digests);
> +}
> +
> +static void slaunch_tpm20_extend(struct tpm_chip *tpm, void __iomem *txt)
> +{
> +       struct tcg_pcr_event *event_header;
> +       struct tcg_pcr_event2_head *event;
> +       int start = 0, end = 0, size;
> +
> +       event_header = (struct tcg_pcr_event *)(sl_evtlog.addr +
> +                                               evtlog20->first_record_offset);
> +
> +       /* Skip first TPM 1.2 event to get to first TPM 2.0 event */
> +       event = (struct tcg_pcr_event2_head *)((u8 *)event_header +
> +                                               sizeof(struct tcg_pcr_event) +
> +                                               event_header->event_size);
> +
> +       while ((void  *)event < sl_evtlog.addr + evtlog20->next_record_offset) {
> +               size = __calc_tpm2_event_size(event, event_header, false);
> +               if (!size)
> +                       slaunch_txt_reset(txt, "TPM20 invalid event in event log\n",
> +                                         SL_ERROR_TPM_INVALID_EVENT);
> +
> +               /*
> +                * Marker events indicate where the Secure Launch early stub
> +                * started and ended adding post launch events.
> +                */
> +               if (event->event_type == TXT_EVTYPE_SLAUNCH_END) {
> +                       end = 1;
> +                       break;
> +               } else if (event->event_type == TXT_EVTYPE_SLAUNCH_START) {
> +                       start = 1;
> +                       goto next;
> +               }
> +
> +               if (start)
> +                       slaunch_tpm20_extend_event(tpm, txt, event);
> +
> +next:
> +               event = (struct tcg_pcr_event2_head *)((u8 *)event + size);
> +       }
> +
> +       if (!start || !end)
> +               slaunch_txt_reset(txt, "Missing start or end events for extending TPM20 PCRs\n",
> +                                 SL_ERROR_TPM_EXTEND);
> +}
> +
> +static void slaunch_tpm12_extend(struct tpm_chip *tpm, void __iomem *txt)
> +{
> +       struct tpm12_event_log_header *event_header;
> +       struct tcg_pcr_event *event;
> +       struct tpm_digest digest;
> +       int start = 0, end = 0;
> +       int size, ret;
> +
> +       event_header = (struct tpm12_event_log_header *)sl_evtlog.addr;
> +       event = (struct tcg_pcr_event *)((u8 *)event_header +
> +                               sizeof(struct tpm12_event_log_header));
> +
> +       while ((void  *)event < sl_evtlog.addr + event_header->next_event_offset) {
> +               size = sizeof(struct tcg_pcr_event) + event->event_size;
> +
> +               /*
> +                * Marker events indicate where the Secure Launch early stub
> +                * started and ended adding post launch events.
> +                */
> +               if (event->event_type == TXT_EVTYPE_SLAUNCH_END) {
> +                       end = 1;
> +                       break;
> +               } else if (event->event_type == TXT_EVTYPE_SLAUNCH_START) {
> +                       start = 1;
> +                       goto next;
> +               }
> +
> +               if (start) {
> +                       memset(&digest.digest[0], 0, TPM_MAX_DIGEST_SIZE);
> +                       digest.alg_id = TPM_ALG_SHA1;
> +                       memcpy(&digest.digest[0], &event->digest[0],
> +                              SHA1_DIGEST_SIZE);
> +
> +                       ret = tpm_pcr_extend(tpm, event->pcr_idx, &digest);
> +                       if (ret) {
> +                               pr_err("Error extending TPM12 PCR, result: %d\n", ret);
> +                               slaunch_txt_reset(txt, "Failed to extend TPM12 PCR\n",
> +                                                 SL_ERROR_TPM_EXTEND);
> +                       }
> +               }
> +
> +next:
> +               event = (struct tcg_pcr_event *)((u8 *)event + size);
> +       }
> +
> +       if (!start || !end)
> +               slaunch_txt_reset(txt, "Missing start or end events for extending TPM12 PCRs\n",
> +                                 SL_ERROR_TPM_EXTEND);
> +}
> +
> +static void slaunch_pcr_extend(void __iomem *txt)
> +{
> +       struct tpm_chip *tpm;
> +
> +       tpm = tpm_default_chip();
> +       if (!tpm)
> +               slaunch_txt_reset(txt, "Could not get default TPM chip\n",
> +                                 SL_ERROR_TPM_INIT);
> +
> +       if (!tpm_preferred_locality(tpm, 2))
> +               slaunch_txt_reset(txt, "Could not set TPM chip locality 2\n",
> +                                 SL_ERROR_TPM_INIT);
> +
> +       if (evtlog20)
> +               slaunch_tpm20_extend(tpm, txt);
> +       else
> +               slaunch_tpm12_extend(tpm, txt);
> +
> +       tpm_preferred_locality(tpm, 0);
> +}
> +
> +static int __init slaunch_module_init(void)
> +{
> +       void __iomem *txt;
> +
> +       /* Check to see if Secure Launch happened */
> +       if ((slaunch_get_flags() & (SL_FLAG_ACTIVE|SL_FLAG_ARCH_TXT)) !=
> +           (SL_FLAG_ACTIVE | SL_FLAG_ARCH_TXT))
> +               return 0;
> +
> +       txt = ioremap(TXT_PRIV_CONFIG_REGS_BASE, TXT_NR_CONFIG_PAGES *
> +                     PAGE_SIZE);
> +       if (!txt)
> +               panic("Error ioremap of TXT priv registers\n");
> +
> +       /* Only Intel TXT is supported at this point */
> +       slaunch_intel_evtlog(txt);
> +       slaunch_pcr_extend(txt);
> +       iounmap(txt);
> +
> +       return slaunch_expose_securityfs();
> +}
> +
> +static void __exit slaunch_module_exit(void)
> +{
> +       slaunch_teardown_securityfs();
> +}
> +
> +late_initcall(slaunch_module_init);
> +__exitcall(slaunch_module_exit);
> --
> 2.39.3
>

Hi Ross,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master herbert-crypto-2.6/master linus/master v6.8-rc4 next-20240215]
[cannot apply to tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/x86-boot-Place-kernel_info-at-a-fixed-offset/20240215-064712
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20240214221847.2066632-15-ross.philipson%40oracle.com
patch subject: [PATCH v8 14/15] x86: Secure Launch late initcall platform module
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20240216/202402160909.BRTtBK7T-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240216/202402160909.BRTtBK7T-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402160909.BRTtBK7T-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from arch/x86/kernel/slmodule.c:28:
   In file included from include/linux/slaunch.h:185:
   include/linux/tpm_eventlog.h:167:6: warning: variable 'mapping_size' set but not used [-Wunused-but-set-variable]
     167 |         int mapping_size;
         |             ^
>> arch/x86/kernel/slmodule.c:352:4: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
     352 |                         default:
         |                         ^
   arch/x86/kernel/slmodule.c:352:4: note: insert 'break;' to avoid fall-through
     352 |                         default:
         |                         ^
         |                         break; 
   2 warnings generated.


vim +352 arch/x86/kernel/slmodule.c

   313	
   314	static void slaunch_tpm20_extend_event(struct tpm_chip *tpm, void __iomem *txt,
   315					       struct tcg_pcr_event2_head *event)
   316	{
   317		u16 *alg_id_field = (u16 *)((u8 *)event + sizeof(struct tcg_pcr_event2_head));
   318		struct tpm_digest *digests;
   319		u8 *dptr;
   320		u32 i, j;
   321		int ret;
   322	
   323		digests = kcalloc(tpm->nr_allocated_banks, sizeof(*digests),
   324				  GFP_KERNEL);
   325		if (!digests)
   326			slaunch_txt_reset(txt, "Failed to allocate array of digests\n",
   327					  SL_ERROR_GENERIC);
   328	
   329		for (i = 0; i < tpm->nr_allocated_banks; i++)
   330			digests[i].alg_id = tpm->allocated_banks[i].alg_id;
   331	
   332		/* Early SL code ensured there was a max count of 2 digests */
   333		for (i = 0; i < event->count; i++) {
   334			dptr = (u8 *)alg_id_field + sizeof(u16);
   335	
   336			for (j = 0; j < tpm->nr_allocated_banks; j++) {
   337				if (digests[j].alg_id != *alg_id_field)
   338					continue;
   339	
   340				switch (digests[j].alg_id) {
   341				case TPM_ALG_SHA256:
   342					memcpy(&digests[j].digest[0], dptr,
   343					       SHA256_DIGEST_SIZE);
   344					alg_id_field = (u16 *)((u8 *)alg_id_field +
   345						SHA256_DIGEST_SIZE + sizeof(u16));
   346					break;
   347				case TPM_ALG_SHA1:
   348					memcpy(&digests[j].digest[0], dptr,
   349					       SHA1_DIGEST_SIZE);
   350					alg_id_field = (u16 *)((u8 *)alg_id_field +
   351						SHA1_DIGEST_SIZE + sizeof(u16));
 > 352				default:
   353					break;
   354				}
   355			}
   356		}
   357	
   358		ret = tpm_pcr_extend(tpm, event->pcr_idx, digests);
   359		if (ret) {
   360			pr_err("Error extending TPM20 PCR, result: %d\n", ret);
   361			slaunch_txt_reset(txt, "Failed to extend TPM20 PCR\n",
   362					  SL_ERROR_TPM_EXTEND);
   363		}
   364	
   365		kfree(digests);
   366	}
   367

Hi Ross,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus herbert-cryptodev-2.6/master herbert-crypto-2.6/master linus/master v6.8-rc4 next-20240216]
[cannot apply to tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ross-Philipson/x86-boot-Place-kernel_info-at-a-fixed-offset/20240215-064712
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20240214221847.2066632-15-ross.philipson%40oracle.com
patch subject: [PATCH v8 14/15] x86: Secure Launch late initcall platform module
config: x86_64-randconfig-r061-20240216 (https://download.01.org/0day-ci/archive/20240217/202402171536.dNnLrhB1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240217/202402171536.dNnLrhB1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402171536.dNnLrhB1-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/x86/kernel/slmodule.c: In function 'slaunch_pcr_extend':
>> arch/x86/kernel/slmodule.c:471:14: error: implicit declaration of function 'tpm_preferred_locality' [-Werror=implicit-function-declaration]
     471 |         if (!tpm_preferred_locality(tpm, 2))
         |              ^~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/tpm_preferred_locality +471 arch/x86/kernel/slmodule.c

   461	
   462	static void slaunch_pcr_extend(void __iomem *txt)
   463	{
   464		struct tpm_chip *tpm;
   465	
   466		tpm = tpm_default_chip();
   467		if (!tpm)
   468			slaunch_txt_reset(txt, "Could not get default TPM chip\n",
   469					  SL_ERROR_TPM_INIT);
   470	
 > 471		if (!tpm_preferred_locality(tpm, 2))
   472			slaunch_txt_reset(txt, "Could not set TPM chip locality 2\n",
   473					  SL_ERROR_TPM_INIT);
   474	
   475		if (evtlog20)
   476			slaunch_tpm20_extend(tpm, txt);
   477		else
   478			slaunch_tpm12_extend(tpm, txt);
   479	
   480		tpm_preferred_locality(tpm, 0);
   481	}
   482

On 2/15/24 03:40, Ard Biesheuvel wrote:
> On Wed, 14 Feb 2024 at 23:32, Ross Philipson <ross.philipson@oracle.com> wrote:
>>
>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>>
>> The Secure Launch platform module is a late init module. During the
>> init call, the TPM event log is read and measurements taken in the
>> early boot stub code are located. These measurements are extended
>> into the TPM PCRs using the mainline TPM kernel driver.
>>
>> The platform module also registers the securityfs nodes to allow
>> access to TXT register fields on Intel along with the fetching of
>> and writing events to the late launch TPM log.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> Signed-off-by: garnetgrimm <grimmg@ainfosec.com>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> 
> There is an awful amount of code that executes between the point where
> the measurements are taken and the point where they are loaded into
> the PCRs. All of this code could subvert the boot flow and hide this
> fact, by replacing the actual taken measurement values with the known
> 'blessed' ones that will unseal the keys and/or phone home to do a
> successful remote attestation.

To set context, in general the motivation to employ an RTM, Static or 
Dynamic, integrity solution is to enable external platform validation, 
aka attestation. These trust chains are constructed from the principle 
of measure and execute that rely on the presence of a RoT for Storage 
(RTS) and a RoT for Reporting (RTR). Under the TCG architecture adopted 
by x86 vendors and now recently by Arm, those roles are fulfilled by the 
TPM. With this context, lets layout the assumptive trusts being made here,
   1. The CPU GETSEC instruction functions correctly
   2. The IOMMU, and by extension the PMRs, functions correctly
   2. The ACM authentication process functions correctly
   3. The ACM functions correctly
   4. The TPM interactions function correctly
   5. The TPM functions correctly

With this basis, let's explore your assertion here. The assertion breaks 
down into two scenarios. The first is that the at-rest kernel binary is 
corrupt, unintentionally (bug) or maliciously, either of which does not 
matter for the situation. For the sake of simplicity, corruption of the 
Linux kernel during loading or before the DRTM Event is considered an 
equivalent to corruption of the kernel at-rest. The second is that the 
kernel binary was corrupted in memory at some point after the DRTM event 
occurs.

For both scenarios, the ACM will correctly configure the IOMMU PMRs to 
ensure the kernel can no longer be tampered with in memory. After which, 
the ACM will then accurately measure the kernel (bzImage) and safely 
store the measurement in the TPM.

In the first scenario, the TPM will accurately report the kernel 
measurement in the attestation. The attestation authority will be able 
to detect if an invalid kernel was started and can take whatever 
remediation actions it may employ.

In the second scenario, any attempt to corrupt the binary after the ACM 
has configured the IOMMU PMR will fail.


> At the very least, this should be documented somewhere. And if at all
> possible, it should also be documented why this is ok, and to what
> extent it limits the provided guarantees compared to a true D-RTM boot
> where the early boot code measures straight into the TPMs before
> proceeding.

I can add a rendition of the above into the existing section of the 
documentation patch that already discusses separation of the measurement 
from the TPM recording code. As to the limits it incurs on the DRTM 
integrity, as explained above, I submit there are none.

v/r,
dps

Hi Ard,

On 2/23/24 04:36, Ard Biesheuvel wrote:
> On Thu, 22 Feb 2024 at 14:58, Daniel P. Smith
> <dpsmith@apertussolutions.com> wrote:
>>
>> On 2/15/24 03:40, Ard Biesheuvel wrote:
>>> On Wed, 14 Feb 2024 at 23:32, Ross Philipson <ross.philipson@oracle.com> wrote:
>>>>
>>>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>>>>
>>>> The Secure Launch platform module is a late init module. During the
>>>> init call, the TPM event log is read and measurements taken in the
>>>> early boot stub code are located. These measurements are extended
>>>> into the TPM PCRs using the mainline TPM kernel driver.
>>>>
>>>> The platform module also registers the securityfs nodes to allow
>>>> access to TXT register fields on Intel along with the fetching of
>>>> and writing events to the late launch TPM log.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>>> Signed-off-by: garnetgrimm <grimmg@ainfosec.com>
>>>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>>>
>>> There is an awful amount of code that executes between the point where
>>> the measurements are taken and the point where they are loaded into
>>> the PCRs. All of this code could subvert the boot flow and hide this
>>> fact, by replacing the actual taken measurement values with the known
>>> 'blessed' ones that will unseal the keys and/or phone home to do a
>>> successful remote attestation.
>>
>> To set context, in general the motivation to employ an RTM, Static or
>> Dynamic, integrity solution is to enable external platform validation,
>> aka attestation. These trust chains are constructed from the principle
>> of measure and execute that rely on the presence of a RoT for Storage
>> (RTS) and a RoT for Reporting (RTR). Under the TCG architecture adopted
>> by x86 vendors and now recently by Arm, those roles are fulfilled by the
>> TPM. With this context, lets layout the assumptive trusts being made here,
>>     1. The CPU GETSEC instruction functions correctly
>>     2. The IOMMU, and by extension the PMRs, functions correctly
>>     2. The ACM authentication process functions correctly
>>     3. The ACM functions correctly
>>     4. The TPM interactions function correctly
>>     5. The TPM functions correctly
>>
>> With this basis, let's explore your assertion here. The assertion breaks
>> down into two scenarios. The first is that the at-rest kernel binary is
>> corrupt, unintentionally (bug) or maliciously, either of which does not
>> matter for the situation. For the sake of simplicity, corruption of the
>> Linux kernel during loading or before the DRTM Event is considered an
>> equivalent to corruption of the kernel at-rest. The second is that the
>> kernel binary was corrupted in memory at some point after the DRTM event
>> occurs.
>>
>> For both scenarios, the ACM will correctly configure the IOMMU PMRs to
>> ensure the kernel can no longer be tampered with in memory. After which,
>> the ACM will then accurately measure the kernel (bzImage) and safely
>> store the measurement in the TPM.
>>
>> In the first scenario, the TPM will accurately report the kernel
>> measurement in the attestation. The attestation authority will be able
>> to detect if an invalid kernel was started and can take whatever
>> remediation actions it may employ.
>>
>> In the second scenario, any attempt to corrupt the binary after the ACM
>> has configured the IOMMU PMR will fail.
>>
>>
> 
> This protects the memory image from external masters after the
> measurement has been taken.

It blocks access before the measurement is taken.

> So any external influences in the time window between taking the
> measurements and loading them into the PCRs are out of scope here, I
> guess?

Correct, as long as the assumption that the user configured the kernel 
to program the IOMMU correctly after gaining control. In early versions 
of this series the correct IOMMU configuration was enforced. This was 
changed due to objections that the user should be free to configure the 
system how they see fit, even if it results in an insecure system.

> Maybe it would help (or if I missed it - apologies) to include a
> threat model here. I suppose physical tampering is out of scope?

I can take a look at what other security capabilities have documented in 
this area and provide a similar level of explanation.

I would not say physical tampering is out, I would say that it is 
supported to the degree to which the TPM was designed to mitigate it.

>>> At the very least, this should be documented somewhere. And if at all
>>> possible, it should also be documented why this is ok, and to what
>>> extent it limits the provided guarantees compared to a true D-RTM boot
>>> where the early boot code measures straight into the TPMs before
>>> proceeding.
>>
>> I can add a rendition of the above into the existing section of the
>> documentation patch that already discusses separation of the measurement
>> from the TPM recording code. As to the limits it incurs on the DRTM
>> integrity, as explained above, I submit there are none.
>>
> 
> Thanks for the elaborate explananation. And yes, please document this
> with the changes.

Ack.

V/r,
Daniel