[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v2 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0 |
Date: |
Wed, 28 Jul 2021 15:25:56 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Tue, Jul 20, 2021 at 05:14:49PM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Add support for trusted boot using a vTPM 2.0 on the IBM IEEE1275
> PowerPC platform. With this patch grub now measures text and binary data
> into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
> does.
>
> Cc: Eric Snowberg <eric.snowberg@oracle.com>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
> docs/grub.texi | 3 +-
> grub-core/Makefile.core.def | 7 ++
> grub-core/commands/ieee1275/ibmvtpm.c | 169 ++++++++++++++++++++++++++
> 3 files changed, 178 insertions(+), 1 deletion(-)
> create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index f8b4b3b21..23d2f2d90 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -6023,7 +6023,8 @@ tpm module is loaded. As such it is recommended that
> the tpm module be built
> into @file{core.img} in order to avoid a potential gap in measurement between
> @file{core.img} being loaded and the tpm module being loaded.
>
> -Measured boot is currently only supported on EFI platforms.
> +Measured boot is currently only supported on EFI and IBM IEEE1275 PowerPC
> +platforms.
>
> @node Lockdown
> @section Lockdown when booting on a secure setup
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 8022e1c0a..e6ac50704 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1118,6 +1118,13 @@ module = {
> enable = powerpc_ieee1275;
> };
>
> +module = {
> + name = tpm;
> + common = commands/tpm.c;
> + ieee1275 = commands/ieee1275/ibmvtpm.c;
> + enable = powerpc_ieee1275;
> +};
> +
> module = {
> name = terminal;
> common = commands/terminal.c;
> diff --git a/grub-core/commands/ieee1275/ibmvtpm.c
> b/grub-core/commands/ieee1275/ibmvtpm.c
> new file mode 100644
> index 000000000..d61cb111a
> --- /dev/null
> +++ b/grub-core/commands/ieee1275/ibmvtpm.c
> @@ -0,0 +1,169 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2021 Free Software Foundation, Inc.
> + * Copyright (C) 2021 IBM Corporation
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * IBM vTPM support code.
> + */
> +
> +#include <grub/err.h>
> +#include <grub/types.h>
> +#include <grub/tpm.h>
> +#include <grub/ieee1275/ieee1275.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +
> +struct tpm_2hash_ext_log
> +{
> + struct grub_ieee1275_common_hdr common;
> + grub_ieee1275_cell_t method;
> + grub_ieee1275_cell_t ihandle;
> + grub_ieee1275_cell_t size;
> + grub_ieee1275_cell_t buf;
> + grub_ieee1275_cell_t description_size;
> + grub_ieee1275_cell_t description;
> + grub_ieee1275_cell_t eventtype;
> + grub_ieee1275_cell_t pcrindex;
> + grub_ieee1275_cell_t catch_result;
> + grub_ieee1275_cell_t rc;
> +};
Huh! It looks that the struct should be defined in ibmvtpm_2hash_ext_log()
as it is done in the similar GRUB IEEE1275 code. Please move it back there.
Sorry for the confusion.
> +#define IEEE1275_CELL_TRUE ((grub_ieee1275_cell_t) -1)
This smells like global constant. Does not it? If yes could you define it
in a global header and use it? Maybe even replace existing comparisons
in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then
s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/...
> +static grub_ieee1275_ihandle_t tpm_ihandle;
> +static grub_uint8_t tpm_version;
> +
> +static grub_ieee1275_ihandle_t
> +tpm_init (void)
> +{
> + static int init_called = 0;
> +
> + if (!init_called)
> + {
> + init_called = 1;
I would move this behind grub_ieee1275_open() call.
> + grub_ieee1275_open ("/vdevice/vtpm", &tpm_ihandle);
Why do you ignore status returned by grub_ieee1275_open()?
> + }
> +
> + return tpm_ihandle;
> +}
> +
> +static void
> +tpm_get_tpm_version (void)
> +{
> + static int version_probed = 0;
> + grub_ieee1275_phandle_t vtpm;
> + char buffer[20];
> + grub_ssize_t buffer_size;
> +
> + if (!version_probed)
> + {
> + version_probed = 1;
I would move version_probed behind this if.
> + if (!grub_ieee1275_finddevice ("/vdevice/vtpm", &vtpm) &&
> + !grub_ieee1275_get_property (vtpm, "compatible", buffer,
> + sizeof buffer, &buffer_size) &&
s/sizeof buffer/sizeof (buffer)/
And you can use NULL instead of buffer_size.
> + !grub_strcmp (buffer, "IBM,vtpm20"))
> + {
> + tpm_version = 2;
> + }
Please drop these curly brackets.
> + }
> +}
> +
> +static grub_err_t
> +tpm_handle_find (grub_ieee1275_ihandle_t *tpm_handle)
> +{
> + *tpm_handle = tpm_init ();
> + if (*tpm_handle == 0)
> + return GRUB_ERR_UNKNOWN_DEVICE;
> +
> + tpm_get_tpm_version ();
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +static int
> +ibmvtpm_2hash_ext_log (grub_uint32_t ihandle,
> + grub_uint8_t pcrindex,
> + grub_uint32_t eventtype,
> + const char *description,
> + grub_size_t description_size,
> + void *buf, grub_size_t size)
> +{
> + struct tpm_2hash_ext_log args;
> +
> + INIT_IEEE1275_COMMON (&args.common, "call-method", 8, 2);
> + args.method = (grub_ieee1275_cell_t) "2hash-ext-log";
> + args.ihandle = ihandle;
> + args.pcrindex = pcrindex;
> + args.eventtype = eventtype;
> + args.description = (grub_ieee1275_cell_t) description;
> + args.description_size = description_size;
> + args.buf = (grub_ieee1275_cell_t) buf;
> + args.size = (grub_ieee1275_cell_t) size;
> +
> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> + return -1;
Incorrect alignment of return.
> + /*
> + * catch_result is set if firmware does not support 2hash-ext-log
> + * rc is TRUE (-1) on success
> + */
> + if ((args.catch_result) || args.rc != IEEE1275_CELL_TRUE)
> + return -1;
Ditto.
> + return 0;
> +}
> +
> +static grub_err_t
> +tpm2_log_event (grub_ieee1275_ihandle_t tpm_handle, unsigned char *buf,
> + grub_size_t size, grub_uint8_t pcr,
> + const char *description)
> +{
> + static int error_displayed;
static int error_displayed = 0;
> + int err;
> +
> + err = ibmvtpm_2hash_ext_log (tpm_handle,
> + pcr, EV_IPL,
> + description,
> + grub_strlen(description) + 1,
> + buf, size);
> + if (err && error_displayed < 1)
s/error_displayed < 1/!error_displayed/
> + {
> + error_displayed++;
> + grub_error (GRUB_ERR_BAD_DEVICE,
s/grub_error/return grub_error/
> + "2HASH-EXT-LOG failed: Firmware is likely too old.\n");
> + }
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +grub_err_t
> +grub_tpm_measure (unsigned char *buf, grub_size_t size, grub_uint8_t pcr,
> + const char *description)
> +{
> + grub_ieee1275_ihandle_t tpm_handle;
> +
> + /* Absence of a TPM isn't a failure. */
> + if (tpm_handle_find (&tpm_handle) != GRUB_ERR_NONE)
> + return GRUB_ERR_NONE;
Incorrect alignment of return.
> + grub_dprintf ("tpm", "log_event, pcr = %d, size = 0x%" PRIxGRUB_SIZE ",
> %s\n",
> + pcr, size, description);
> +
> + if (tpm_version == 2)
> + return tpm2_log_event (tpm_handle, buf, size, pcr, description);
Ditto.
Daniel
- [PATCH v2 0/4] Add support for trusted boot on IBM PPC platform, Stefan Berger, 2021/07/20
- [PATCH v2 1/4] ieee1275: drop HEAP_MAX_ADDR, HEAP_MIN_SIZE, Stefan Berger, 2021/07/20
- [PATCH v2 2/4] ieee1275: claim more memory, Stefan Berger, 2021/07/20
- [PATCH v2 3/4] ieee1275: request memory with ibm, client-architecture-support, Stefan Berger, 2021/07/20
- [PATCH v2 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0, Stefan Berger, 2021/07/20
- Re: [PATCH v2 4/4] ibmvtpm: Add support for trusted boot using a vTPM 2.0,
Daniel Kiper <=