Re: [PATCH v13 20/20] tests: Add tpm2_test

[Gary Lin]

On Mon, Apr 29, 2024 at 04:10:05PM +0800, Gary Lin wrote:
> On Fri, Apr 26, 2024 at 05:18:04PM -0500, Glenn Washburn wrote:
> > On Thu, 25 Apr 2024 16:02:06 +0800
> > Gary Lin <glin@suse.com> wrote:
> > 
> > > For the tpm2 module, the TCG2 command submission function is the only
> > > difference between the a QEMU instance and grub-emu. To test TPM key
> > > unsealing with a QEMU instance, it requires an extra OS image to invoke
> > > grub-protect to seal the LUKS key, rather than a simple grub-shell rescue
> > > CD image. On the other hand, grub-emu can share the emulated TPM device
> > > with the host, so that we can seal the LUKS key on host and test key
> > > unsealing with grub-emu.
> > 
> > I'm glad we're getting a test with this feature. Its also unfortunate
> > that the test only works on the emu platform, which I suspect is tested
> > less.
> > 
> Doing a full test with QEMU requires a more powerful testsuite such as
> openQA to manipulate the OS to seal the disk secret to the TPM of the
> VM. With grub-emu, at least we can test the TPM2 stack and verify the
> result of tpm2_key_protector_init.
> 
> > > 
> > > This test script firstly creates a simple LUKS image to be loaded as a
> > > loopback device in grub-emu. Then an emulated TPM device is created by
> > > swtpm_cuse and PCR 0 and 1 are extended.
> > > 
> > > There are several test cases in the script to test various settings. Each
> > > test case uses grub-protect or tpm2-tools to seal the LUKS password
> > > against PCR 0 and PCR 1. Then grub-emu is launched to load the LUKS image,
> > > try to mount the image with tpm2_key_protector_init and cryptomount, and
> > > verify the result.
> > > 
> > > Based on the idea from Michael Chang.
> > > 
> > > Cc: Michael Chang <mchang@suse.com>
> > > Cc: Stefan Berger <stefanb@linux.ibm.com>
> > > Signed-off-by: Gary Lin <glin@suse.com>
> > > ---
[...]
> > > diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
> > > index 496e1bab3..f8642543d 100644
> > > --- a/tests/util/grub-shell.in
> > > +++ b/tests/util/grub-shell.in
> > > @@ -75,6 +75,7 @@ work_directory=${WORKDIR:-`mktemp -d 
> > > "${TMPDIR:-/tmp}/grub-shell.XXXXXXXXXX"`} |
> > >  
> > >  . "${builddir}/grub-core/modinfo.sh"
> > >  qemuopts=
> > > +grubemuopts=
> > >  serial_port=com0
> > >  serial_null=
> > >  halt_cmd=halt
> > > @@ -281,6 +282,9 @@ for option in "$@"; do
> > >      --qemu-opts=*)
> > >   qs=`echo "$option" | sed -e 's/--qemu-opts=//'`
> > >   qemuopts="$qemuopts $qs" ;;
> > > +    --grub-emu-opts=*)
> > > + qs=`echo "$option" | sed -e 's/--grub-emu-opts=//'`
> > > + grubemuopts="$grubemuopts $qs" ;;
> > 
> > I'm on the fence on this. '--qemu-opts' could just be reused, although
> > it might be a little confusing, as it would be misnamed. It would be
> > nice to think of a way to combine --qemu-opts and --grub-emu-opts into
> > one appropriately named argument (perhaps just --emu-opts?). They are
> > mutually exclusive in the sense that they will both never be in effect
> > for a given target. Also, prefixing with 'grub' goes against the
> > existing naming scheme, so regardless should not be done.
> > 
> Then I'd just append ${qemuopts} to the grub-emu command since renaming
> the '--qemu-opts' will affect other test scripts. This has to be done in
> a separate patch set.
> 
It's actually not feasible to append ${qemuopts} to grub-emu since the
following line will be executed when $debug is not specified:

qemuopts="${qemuopts} -nographic -monitor file:/dev/null"

Thus, a separate variable for grub-emu is necessary. I'll change the
option to '--emu-opts' and the variable to '$emuopts'.

Gary Lin

> Gary Lin
> 
> > Glenn
> > 
> > >      --disk=*)
> > >   dsk=`echo "$option" | sed -e 's/--disk=//'`
> > >   if [ ${grub_modinfo_platform} = emu ]; then
> > > @@ -576,7 +580,7 @@ elif [ x$boot = xemu ]; then
> > >      cat >"$work_directory/run.sh" <<EOF
> > >  #! @BUILD_SHEBANG@
> > >  SDIR=\$(realpath -e \${0%/*})
> > > -exec "$(realpath -e "${builddir}")/grub-core/grub-emu" -m 
> > > "\$SDIR/${device_map##*/}" --memdisk "\$SDIR/${roottar##*/}" -r memdisk 
> > > -d "/boot/grub"
> > > +exec "$(realpath -e "${builddir}")/grub-core/grub-emu" -m 
> > > "\$SDIR/${device_map##*/}" --memdisk "\$SDIR/${roottar##*/}" -r memdisk 
> > > -d "/boot/grub" ${grubemuopts}
> > >  EOF
> > >  else
> > >      cat >"$work_directory/run.sh" <<EOF