Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got

[Max Tottenham <mtottenh@akamai.com>]

On 10/25, Javier Martinez Canillas wrote:
> Hello Mathieu,
> 
> On 10/25/19 4:48 PM, Mathieu Trudel-Lapierre wrote:
> > On Fri, Oct 25, 2019 at 10:28 AM Mathieu Trudel-Lapierre
> > <mathieu.tl@gmail.com> wrote:
> >>
> >> Signed-off-by: Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
> >> Patch-Name: ubuntu-tpm-unknown-error-non-fatal.patch
> >> ---
> >>  grub-core/commands/efi/tpm.c | 12 ++++++++----
> >>  1 file changed, 8 insertions(+), 4 deletions(-)
> >>
> > 
> > I see I omitted to explain why I'm proposing this.
> > 
> > I've seen a couple of reports so far of issues with booting with TPM
> > measurement enabled, when the firmware has TPM enabled, on some
> > hardware.
> > 
> > In particular, this has happened on a Dell laptop at Plumbers this
> > year (an older model XPS15 IIRC), and a few different models of
> > laptops/motherboards. Some report having a TPM, and some do not:
> > 
> > HP EliteBook 820 G4 (Infineon SLB9670?)
> > ASUS M32CD4-K motherboard (unknown)
> > ASUS ROG GL553VE Laptop (unknown)
> > ASUS ZenBook 3 UX390UA (unknown)
> > ASUS Zenbook UX305FA (unspecified TPM)
> > ASUS ZenBook UX303UA (unknown)
> > ASUS 2O7HSV6 ??
> > 
> > See https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1848892.
> >
> 
> Yes, we also got similar reports for Fedora, i.e:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1645903
>  
> > Unfortunately the reports are not of great quality, but I'm starting
> > to worry about what exactly is wrong, if it's really a firmware / TPM
> > issue or a bug in the TPM code.
> > 
> > For now, it seems like the best is to get more information as to what
> > exactly the failure is (hence grub_dprintf()), and treating these
> 
> Agreed. It would be good to get know the exact EFI status code returned by
> the firmware in the case of a failure.
> 
> > errors as non-fatal so people can still boot.
> >
> 
> I think that we should go even further and make all the TPM measurement
> errors to be non-fatal. For example something like the following patch [0].
> 

This poses a slight problem. For folks who rely on TPM sealed values
this would potentially make the issue harder to address. 

Maybe a compile time (or install time) option that allows a strictness
policy to be set - those who don't care about TPM capability can let it
default to printing warnings, those who rely on keying material sealed
to TPM state can explicitly configure GRUB to halt the boot process on
error?

> > After briefly discussing this with others, it's not clear whether all
> > the affected systems really do have a TPM, but they might still report
> > in firmware that they do. Are we running into a case where the
> > firmware wrongly reports there is a TPM, but fails to do any
> > measurements?
> > 
> 
> That's interesting. I see that EFI_TCG2_PROTOCOL.GetCapability() is called
> and the EFI_TCG2_BOOT_SERVICE_CAPABILITY.TPMPresentFlag checked to know if
> a TPM is present or not. So would be very weird that the firmware reported
> that a TPM is present but that's not the case.
> 
> Maybe the machines did have a TPM but the reporter just didn't know?
> 

It's possible that they do have a TPM but measurements are failing for a
different reason, I see that the TPM patches use HashLogExtendEvent -
Looking at the EDKII source it looks like this could succeed in
extending a PCR but fail to record the event in the event log (for
example if the structure is full). This would provide a different return
code (EFI_VOLUME_FULL), to any of the return codes that are being
checked currently.

In line with the above about allowing configurable policy around fatal
measurement errors, it might be an idea to allow said flexibility to
extend to which errors are deemed fatal (e.g. Measurement failure could
be fatal but a failure to log the subsequent event could be dealt with a
simple warning).

-- 
Max Tottenham       | mtottenh@akamai.com
Senior Software Engineer, Server Platform Engineering
/(* Akamai Technologies