On Thu, Oct 17, 2024 at 07:57:11PM +0200, Daniel Kiper wrote:
> On Fri, Sep 06, 2024 at 05:11:22PM +0800, Gary Lin via Grub-devel wrote:
> > As a preparation to test tpm2_key_protector with grub-emu, the new
> > option, --tpm-device, is introduced to specify the TPM device for
> > grub-emu so that grub-emu can share the emulated TPM device with
> > the host.
>
> s/can share the emulated TPM device with/can access an emulated TPM device
> from/?
>
> > Since grub-emu can directly access the device node on host, it's easy to
>
> s/device node/device/
>
Will fix them in the next version.
> > implement the essential TCG2 command submission function with the
> > read/write functions and enable tpm2_key_protector module for grub-emu,
> > so that we can further test TPM2 key unsealing with grub-emu.
> >
> > Signed-off-by: Gary Lin <g...@suse.com>
> > Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
>
> Otherwise Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>...
>
> ... and three nits below...
>
> > ---
> > grub-core/Makefile.core.def | 3 ++
> > grub-core/kern/emu/main.c | 11 ++++++-
> > grub-core/kern/emu/misc.c | 51 +++++++++++++++++++++++++++++++++
> > grub-core/lib/tss2/tcg2_emu.c | 54 +++++++++++++++++++++++++++++++++++
> > include/grub/emu/misc.h | 5 ++++
> > 5 files changed, 123 insertions(+), 1 deletion(-)
> > create mode 100644 grub-core/lib/tss2/tcg2_emu.c
> >
> > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > index 97ae4e49b..40427165e 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -2574,7 +2574,9 @@ module = {
> > common = lib/tss2/tpm2_cmd.c;
> > common = lib/tss2/tss2.c;
> > efi = lib/efi/tcg2.c;
> > + emu = lib/tss2/tcg2_emu.c;
> > enable = efi;
> > + enable = emu;
> > cppflags = '-I$(srcdir)/lib/tss2';
> > };
> >
> > @@ -2586,6 +2588,7 @@ module = {
> > common = commands/tpm2_key_protector/tpm2key_asn1_tab.c;
> > /* The plaform support of tpm2_key_protector depends on the tcg2
> > implementation in tss2. */
> > enable = efi;
> > + enable = emu;
> > cppflags = '-I$(srcdir)/lib/tss2 -I$(srcdir)/lib/libtasn1-grub';
> > };
> >
> > diff --git a/grub-core/kern/emu/main.c b/grub-core/kern/emu/main.c
> > index 855b11c3d..c10838613 100644
> > --- a/grub-core/kern/emu/main.c
> > +++ b/grub-core/kern/emu/main.c
> > @@ -55,7 +55,7 @@
> > static jmp_buf main_env;
> >
> > /* Store the prefix specified by an argument. */
> > -static char *root_dev = NULL, *dir = NULL;
> > +static char *root_dev = NULL, *dir = NULL, *tpm_dev = NULL;
> >
> > grub_addr_t grub_modbase = 0;
> >
> > @@ -108,6 +108,7 @@ static struct argp_option options[] = {
> > {"verbose", 'v', 0, 0, N_("print verbose messages."), 0},
> > {"hold", 'H', N_("SECS"), OPTION_ARG_OPTIONAL, N_("wait until a
> > debugger will attach"), 0},
> > {"kexec", 'X', 0, 0, N_("use kexec to boot Linux kernels via
> > systemctl (pass twice to enable dangerous fallback to non-systemctl)."), 0},
> > + {"tpm-device", 't', N_("DEV"), 0, N_("Set TPM device."), 0},
>
> s/Set/set/? It looks all messages start with lowercase...
>
Oh, yes, it should start with lowercase.
> > { 0, 0, 0, 0, 0, 0 }
> > };
> >
> > @@ -168,6 +169,10 @@ argp_parser (int key, char *arg, struct argp_state
> > *state)
> > case 'X':
> > grub_util_set_kexecute ();
> > break;
> > + case 't':
> > + free (tpm_dev);
> > + tpm_dev = xstrdup (arg);
> > + break;
> >
> > case ARGP_KEY_ARG:
> > {
> > @@ -276,6 +281,9 @@ main (int argc, char *argv[])
> >
> > dir = xstrdup (dir);
> >
> > + if (tpm_dev)
> > + grub_util_tpm_open (tpm_dev);
> > +
> > /* Start GRUB! */
> > if (setjmp (main_env) == 0)
> > grub_main ();
> > @@ -283,6 +291,7 @@ main (int argc, char *argv[])
> > grub_fini_all ();
> > grub_hostfs_fini ();
> > grub_host_fini ();
> > + grub_util_tpm_close ();
> >
> > grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
> >
> > diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
> > index 521220b49..1db24fde7 100644
> > --- a/grub-core/kern/emu/misc.c
> > +++ b/grub-core/kern/emu/misc.c
> > @@ -28,6 +28,8 @@
> > #include <string.h>
> > #include <sys/time.h>
> > #include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> >
> > #include <grub/mm.h>
> > #include <grub/err.h>
> > @@ -41,6 +43,8 @@
> > int verbosity;
> > int kexecute;
> >
> > +static int grub_util_tpm_fd = -1;
> > +
> > void
> > grub_util_warn (const char *fmt, ...)
> > {
> > @@ -230,3 +234,50 @@ grub_util_get_kexecute (void)
> > {
> > return kexecute;
> > }
> > +
> > +grub_err_t
> > +grub_util_tpm_open (const char *tpm_dev)
> > +{
> > + if (grub_util_tpm_fd != -1)
> > + return GRUB_ERR_NONE;
> > +
> > + grub_util_tpm_fd = open (tpm_dev, O_RDWR);
> > + if (grub_util_tpm_fd == -1)
> > + grub_util_error (_("cannot open TPM device '%s': %s"), tpm_dev,
> > strerror (errno));
> > +
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > +grub_err_t
> > +grub_util_tpm_close (void)
> > +{
> > + int err;
> > +
> > + if (grub_util_tpm_fd == -1)
> > + return GRUB_ERR_NONE;
> > +
> > + err = close (grub_util_tpm_fd);
> > + if (err != GRUB_ERR_NONE)
> > + grub_util_error (_("cannot close TPM device: %s"), strerror (errno));
> > +
> > + grub_util_tpm_fd = -1;
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > +grub_size_t
> > +grub_util_tpm_read (void *output, grub_size_t size)
> > +{
> > + if (grub_util_tpm_fd == -1)
> > + return -1;
> > +
> > + return read (grub_util_tpm_fd, output, size);
> > +}
> > +
> > +grub_size_t
> > +grub_util_tpm_write (const void *input, grub_size_t size)
> > +{
> > + if (grub_util_tpm_fd == -1)
> > + return -1;
> > +
> > + return write (grub_util_tpm_fd, input, size);
> > +}
> > diff --git a/grub-core/lib/tss2/tcg2_emu.c b/grub-core/lib/tss2/tcg2_emu.c
> > new file mode 100644
> > index 000000000..6bf4195d7
> > --- /dev/null
> > +++ b/grub-core/lib/tss2/tcg2_emu.c
> > @@ -0,0 +1,54 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2024 SUSE LLC
> > + * Copyright (C) 2024 Free Software Foundation, Inc.
> > + *
> > + * 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/>.
> > + */
> > +
> > +#include <grub/efi/api.h>
> > +#include <grub/efi/efi.h>
> > +#include <grub/efi/tpm.h>
>
> Hmmm... Do we really need that stuff for grub-emu?
>
Ah, those headers are not necessary.
> > +#include <grub/mm.h>
> > +#include <grub/emu/misc.h>
> > +
> > +#include <tss2_buffer.h>
> > +#include <tcg2.h>
> > +
> > +grub_err_t
> > +grub_tcg2_get_max_output_size (grub_size_t *size)
> > +{
> > + if (size == NULL)
> > + return GRUB_ERR_BAD_ARGUMENT;
> > +
> > + *size = GRUB_TPM2_BUFFER_CAPACITY;
> > +
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > +grub_err_t
> > +grub_tcg2_submit_command (grub_size_t input_size, grub_uint8_t *input,
> > + grub_size_t output_size, grub_uint8_t *output)
> > +{
> > + static const grub_size_t header_size = sizeof (grub_uint16_t) +
> > + (2 * sizeof(grub_uint32_t));
>
> Missing space after "sizeof"...
>
Will fix them in the next version.
Gary Lin
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
