[PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Javier Martinez Canillas]

Currently if an EFI firmware fails to do a TPM measurement for a file, the
error will be propagated to the verifiers framework which will prevent it
to be opened.

This mean that buggy firmwares will lead to the system not booting because
files won't be allowed to be loaded. But a failure to do a TPM measurement
isn't expected to be a fatal error that causes the system to be unbootable.

To avoid this, don't return errors from .write and .verify_string callbacks
and just print a debug message in the case of a TPM measurement failure.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 grub-core/commands/tpm.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm.c
index 2052c36eaba..24874ffacbc 100644
--- a/grub-core/commands/tpm.c
+++ b/grub-core/commands/tpm.c
@@ -42,7 +42,11 @@ grub_tpm_verify_init (grub_file_t io,
 static grub_err_t
 grub_tpm_verify_write (void *context, void *buf, grub_size_t size)
 {
-  return grub_tpm_measure (buf, size, GRUB_BINARY_PCR, context);
+  grub_err_t status = grub_tpm_measure (buf, size, GRUB_BINARY_PCR, context);
+
+  if (status)
+    grub_dprintf ("tpm", "Measuring buffer failed: %d\n", status);
+  return GRUB_ERR_NONE;
 }
 
 static grub_err_t
@@ -66,15 +70,17 @@ grub_tpm_verify_string (char *str, enum grub_verify_string_type type)
     }
   description = grub_malloc (grub_strlen (str) + grub_strlen (prefix) + 1);
   if (!description)
-    return grub_errno;
+    return GRUB_ERR_NONE;
   grub_memcpy (description, prefix, grub_strlen (prefix));
   grub_memcpy (description + grub_strlen (prefix), str,
 	       grub_strlen (str) + 1);
   status =
     grub_tpm_measure ((unsigned char *) str, grub_strlen (str),
 		      GRUB_STRING_PCR, description);
+  if (status)
+    grub_dprintf ("tpm", "Measuring string %s failed: %d\n", str, status);
   grub_free (description);
-  return status;
+  return GRUB_ERR_NONE;
 }
 
 struct grub_file_verifier grub_tpm_verifier = {
-- 
2.29.2

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[James Bottomley]

On Sun, 2021-02-28 at 00:05 +0100, Javier Martinez Canillas wrote:
> Currently if an EFI firmware fails to do a TPM measurement for a
> file, the error will be propagated to the verifiers framework which
> will prevent it to be opened.
> 
> This mean that buggy firmwares will lead to the system not booting
> because files won't be allowed to be loaded. But a failure to do a
> TPM measurement isn't expected to be a fatal error that causes the
> system to be unbootable.
> 
> To avoid this, don't return errors from .write and .verify_string
> callbacks and just print a debug message in the case of a TPM
> measurement failure.

This does seem somewhat the wrong response.  For everyone who is doing
measured boot, this will cause a complete break of the verification
chain.   It looks like this is likely the fault of the bios vendor, so
why not push it back to source by failing the boot rather than
deferring the problem so it lands on the measured boot implementors.

This looks like a simple fault in the UEFI vendor (likely insufficient
log space), can't you simply force the motherboard OEM to issue a fix
rather than adding a work around to the open source project that first
detects it.

James

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Javier Martinez Canillas]

Hello James,

On 2/28/21 8:10 AM, James Bottomley wrote:
> On Sun, 2021-02-28 at 00:05 +0100, Javier Martinez Canillas wrote:
>> Currently if an EFI firmware fails to do a TPM measurement for a
>> file, the error will be propagated to the verifiers framework which
>> will prevent it to be opened.
>>
>> This mean that buggy firmwares will lead to the system not booting
>> because files won't be allowed to be loaded. But a failure to do a
>> TPM measurement isn't expected to be a fatal error that causes the
>> system to be unbootable.
>>
>> To avoid this, don't return errors from .write and .verify_string
>> callbacks and just print a debug message in the case of a TPM
>> measurement failure.
> 
> This does seem somewhat the wrong response.  For everyone who is doing
> measured boot, this will cause a complete break of the verification
> chain.   It looks like this is likely the fault of the bios vendor, so

I don't see how that would be the case. For anyone doing measured boot,
they can check the TPM event log to verify what has been measured during
the boot path. It's up to attestation software to check that and not be
enforced by the bootloaders in my opinion.

As far as I can tell there is nothing in the TCG specs that mention that
a failure to measure should prevent a system to boot.

For me this is just a by-product of using the verifier framework as a way
to hook the TPM measurements into the file open path. It's not really a
verification, buffers passed aren't validated like with other verifiers.

> why not push it back to source by failing the boot rather than
> deferring the problem so it lands on the measured boot implementors.
> 
> This looks like a simple fault in the UEFI vendor (likely insufficient
> log space), can't you simply force the motherboard OEM to issue a fix
> rather than adding a work around to the open source project that first
> detects it.
>

You are looking through the lenses of the people doing trusted boot but
what about those who are not? For them, they just updated GRUB and now
the machine doesn't boot. They don't care or might even know what a TPM
is, they just want to boot their OS.

And in many cases the faulty firmware is from a vendor that doesn't even
provide firmware updates or at least doesn't provide them through LVFS.

So now the user (who can't boot their machine) has to report the issue,
wait for a firmware fix and figure out a way to update it. For my this is
not acceptable and quite antisocial for users.
 
> James
> 
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Javier Martinez Canillas]

[re-sending since I got a 'Recipient server unavailable or busy' error,
 sorry if someone gets this email twice]

Hello James,

On 2/28/21 8:10 AM, James Bottomley wrote:
> On Sun, 2021-02-28 at 00:05 +0100, Javier Martinez Canillas wrote:
>> Currently if an EFI firmware fails to do a TPM measurement for a
>> file, the error will be propagated to the verifiers framework which
>> will prevent it to be opened.
>>
>> This mean that buggy firmwares will lead to the system not booting
>> because files won't be allowed to be loaded. But a failure to do a
>> TPM measurement isn't expected to be a fatal error that causes the
>> system to be unbootable.
>>
>> To avoid this, don't return errors from .write and .verify_string
>> callbacks and just print a debug message in the case of a TPM
>> measurement failure.
> 
> This does seem somewhat the wrong response.  For everyone who is doing
> measured boot, this will cause a complete break of the verification
> chain.   It looks like this is likely the fault of the bios vendor, so

I don't see how that would be the case. For anyone doing measured boot,
they can check the TPM event log to verify what has been measured during
the boot path. It's up to attestation software to check that and not be
enforced by the bootloaders in my opinion.

As far as I can tell there is nothing in the TCG specs that mention that
a failure to measure should prevent a system to boot.

For me this is just a by-product of using the verifier framework as a way
to hook the TPM measurements into the file open path. It's not really a
verification, buffers passed aren't validated like with other verifiers.

> why not push it back to source by failing the boot rather than
> deferring the problem so it lands on the measured boot implementors.
> 
> This looks like a simple fault in the UEFI vendor (likely insufficient
> log space), can't you simply force the motherboard OEM to issue a fix
> rather than adding a work around to the open source project that first
> detects it.
>

You are looking through the lenses of the people doing trusted boot but
what about those who are not? For them, they just updated GRUB and now
the machine doesn't boot. They don't care or might even know what a TPM
is, they just want to boot their OS.

And in many cases the faulty firmware is from a vendor that doesn't even
provide firmware updates or at least doesn't provide them through LVFS.

So now the user (who can't boot their machine) has to report the issue,
wait for a firmware fix and figure out a way to update it. For my this is
not acceptable and quite antisocial for users.
 
> James
> 
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[James Bottomley]

On Sun, 2021-02-28 at 18:58 +0100, Javier Martinez Canillas wrote:
> [re-sending since I got a 'Recipient server unavailable or busy'
> error,  sorry if someone gets this email twice]
> 
> Hello James,
> 
> On 2/28/21 8:10 AM, James Bottomley wrote:
> > On Sun, 2021-02-28 at 00:05 +0100, Javier Martinez Canillas wrote:
> > > Currently if an EFI firmware fails to do a TPM measurement for a
> > > file, the error will be propagated to the verifiers framework
> > > which will prevent it to be opened.
> > > 
> > > This mean that buggy firmwares will lead to the system not
> > > booting because files won't be allowed to be loaded. But a
> > > failure to do a TPM measurement isn't expected to be a fatal
> > > error that causes the system to be unbootable.
> > > 
> > > To avoid this, don't return errors from .write and .verify_string
> > > callbacks and just print a debug message in the case of a TPM
> > > measurement failure.
> > 
> > This does seem somewhat the wrong response.  For everyone who is
> > doing measured boot, this will cause a complete break of the
> > verification chain.   It looks like this is likely the fault of the
> > bios vendor, so
> 
> I don't see how that would be the case. For anyone doing measured
> boot, they can check the TPM event log to verify what has been
> measured during the boot path.

No, they can't ... that's the point, the log will no longer verify
because of the failure.  That's why this is a break in the measurement
chain.  If you're requesting verified boot, which is an opt-in by
inserting the TPM grub module, this is an unrecoverable failure.

>  It's up to attestation software to check that and not
> be enforced by the bootloaders in my opinion.
> 
> As far as I can tell there is nothing in the TCG specs that mention
> that a failure to measure should prevent a system to boot.

The point I'm making are that the specs are somewhat badly written so
as not to contemplate failure like this.  The reason a UEFI TCG event
write fails in this case is probably because there is insufficient log
space (if it were a TPM failure, it would have failed and been disabled
early in the boot sequence).

The UEFI BIOS vendor is supposed to ensure enough space in the logs and
if they don't the problem needs to be reported back to them.

> For me this is just a by-product of using the verifier framework as a
> way to hook the TPM measurements into the file open path. It's not
> really a verification, buffers passed aren't validated like with
> other verifiers.
> 
> > why not push it back to source by failing the boot rather than
> > deferring the problem so it lands on the measured boot
> > implementors.
> > 
> > This looks like a simple fault in the UEFI vendor (likely
> > insufficient log space), can't you simply force the motherboard OEM
> > to issue a fix rather than adding a work around to the open source
> > project that first detects it.
> > 
> 
> You are looking through the lenses of the people doing trusted boot
> but what about those who are not? For them, they just updated GRUB
> and now the machine doesn't boot.

The verifiers are optional ... if they just updated grub but don't
intend to do a measured boot, why did they insert the grub tpm verifier
module?

>  They don't care or might even know what a TPM is, they just want to
> boot their OS.

Then, as I said, why would they insert the TPM grub module?

> And in many cases the faulty firmware is from a vendor that doesn't
> even provide firmware updates or at least doesn't provide them
> through LVFS.
> 
> So now the user (who can't boot their machine) has to report the
> issue, wait for a firmware fix and figure out a way to update it. For
> my this is not acceptable and quite antisocial for users.

Isn't the fix for the user to report the problem either directly or to
you for onward transmission to the UEFI vendor and then stop their
current boot from inserting the TPM module, which will remove the
failing verifier? That is assuming they don't want or are OK not doing
measured boot ... if it's a requirement, then I agree they have no
choice but to wait for the BIOS to be fixed.

James

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Javier Martinez Canillas]

On 2/28/21 7:25 PM, James Bottomley wrote:
> On Sun, 2021-02-28 at 18:58 +0100, Javier Martinez Canillas wrote:

[snip]

>> I don't see how that would be the case. For anyone doing measured
>> boot, they can check the TPM event log to verify what has been
>> measured during the boot path.
> 
> No, they can't ... that's the point, the log will no longer verify
> because of the failure.  That's why this is a break in the measurement

Yes, my point was that the attestation software can verify if there are
any gaps in the TPM logs due the failure to measure the boot components.

> chain.  If you're requesting verified boot, which is an opt-in by
> inserting the TPM grub module, this is an unrecoverable failure.
>

But it's not really optional, since most distros disable module loading
in GRUB if Secure Boot is enabled. Instead, a curated list of modules
are built-in the signed EFI binary. The tpm module is one of these that
we include in Fedora.

The other option would be to not include the tpm module, but that would
mean that users wanting to do measured boot will have to disable Secure
Boot. And usually people who care about TPM measurements are security
minded people who want to use it in conjunction with Secure Boot.
 
>>  It's up to attestation software to check that and not
>> be enforced by the bootloaders in my opinion.
>>
>> As far as I can tell there is nothing in the TCG specs that mention
>> that a failure to measure should prevent a system to boot.
> 
> The point I'm making are that the specs are somewhat badly written so
> as not to contemplate failure like this.  The reason a UEFI TCG event
> write fails in this case is probably because there is insufficient log
> space (if it were a TPM failure, it would have failed and been disabled
> early in the boot sequence).
> 
> The UEFI BIOS vendor is supposed to ensure enough space in the logs and
> if they don't the problem needs to be reported back to them.
>

I'm not arguing that EFI firmware vendors don't have to get reports about
broken TPM support and fix it. I'm just arguing that uses shouldn't be left
with a system that fails to boot due that.

Since we are logging the errors, users can verify if there were failures
with the TPM measurements and report back any issues that are found.

[snip] 

>> You are looking through the lenses of the people doing trusted boot
>> but what about those who are not? For them, they just updated GRUB
>> and now the machine doesn't boot.
> 
> The verifiers are optional ... if they just updated grub but don't
> intend to do a measured boot, why did they insert the grub tpm verifier
> module?
> 

It's not optional mentioned. By the way, is not only Fedora that uses this
approach but also other distros are carrying similar patches. For example
Ubuntu [0]. Their patch is slightly different in the sense that they don't
propagate any errors the that happens when calling the EFI firmware, while
we don't propagate any errors that happen when measuring, regardless of the
firmware interface.

Anyways, mainline is free to disagree with this approach and not take this
patch. But I don't think that any distro would trade preventing machines to
boot over the possibility of breaking the trusted boot chain.

So in that case, it would just be yet another patch that Fedora, Ubuntu (and
possibly others distros) will have to carry downstream in their packages.

[0]: https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/commit/?h=ubuntu&id=2460a6e237a

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[James Bottomley]

On Sun, 2021-02-28 at 23:42 +0100, Javier Martinez Canillas wrote:
> On 2/28/21 7:25 PM, James Bottomley wrote:
> > On Sun, 2021-02-28 at 18:58 +0100, Javier Martinez Canillas wrote:
> 
> [snip]
> 
> > > I don't see how that would be the case. For anyone doing measured
> > > boot, they can check the TPM event log to verify what has been
> > > measured during the boot path.
> > 
> > No, they can't ... that's the point, the log will no longer verify
> > because of the failure.  That's why this is a break in the
> > measurement
> 
> Yes, my point was that the attestation software can verify if there
> are any gaps in the TPM logs due the failure to measure the boot
> components.

Right, but there's no way to match the log pieces to any value in the
PCRs: that's why verification fails.

> > chain.  If you're requesting verified boot, which is an opt-in by
> > inserting the TPM grub module, this is an unrecoverable failure.
> > 
> 
> But it's not really optional, since most distros disable module
> loading in GRUB if Secure Boot is enabled. Instead, a curated list of
> modules are built-in the signed EFI binary. The tpm module is one of
> these that we include in Fedora.

Heh, well, don't get me started on how the different distributions
package grub differently ... it would be really nice if everyone could
agree on the right, or at least reasonable, way of doing it.  However,
my understanding, from having to do an annoying amount of boot
debugging on various systems, is that you all package the grub elements
to somewhere in the host root fileystem.  Then you have a grub
installer which picks the necessary elements out of the root filesystem
and installs them in the EFI partition, which may or may not be
permanently mounted.

The problem, as you see it, is that the EFI version of grub-mkimage
creates a unitary PE/COFF binary which consists of the core plus the
curated modules and which must be secure boot signed in its entirety. 
This signed unitary binary is usually what is installed by the grub
installer.

The reason you don't like modules for secure boot is that the grub
module system, at present, is unsigned and thus violates the secure
boot trust model which requires all code executed in the boot sequence
to be approved either by the machine owner or the software supplier. 
Incorporating a module signature system, like the kernel has, is orders
of magnitude more effort than you want to invest in external patching,
and likely would take several developer lifetimes to get upstream.

Hopefully, I have all this right so far?

How about a more simple solution: you sign two grub unitary EFI
binaries, one of which does measured boot and one of which doesn't. 
Your installer is already config file driven, so by default it would
install the measured boot one, but if there's a failure you can tell
the user to add the config option to install the unmeasured boot one
... this could also be useful for various other situations where you
want secure but not measured boot?  I'm fairly certain you could design
a distro installer test for the problem and thus always install a
working system.  There's no security issue because anyone who does
attested measured boot will instantly detect someone booting via the
signed unmeasured boot grub.

Note: I'm certainly not presenting this as the optimal solution, merely
the least effort solution that looks like it will work with the current
grub upstream.

James

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Daniel Kiper]

On Sun, Feb 28, 2021 at 03:25:04PM -0800, James Bottomley wrote:

[...]

> How about a more simple solution: you sign two grub unitary EFI
> binaries, one of which does measured boot and one of which doesn't.
> Your installer is already config file driven, so by default it would
> install the measured boot one, but if there's a failure you can tell
> the user to add the config option to install the unmeasured boot one
> ... this could also be useful for various other situations where you
> want secure but not measured boot?  I'm fairly certain you could design
> a distro installer test for the problem and thus always install a
> working system.  There's no security issue because anyone who does
> attested measured boot will instantly detect someone booting via the
> signed unmeasured boot grub.
>
> Note: I'm certainly not presenting this as the optimal solution, merely
> the least effort solution that looks like it will work with the current
> grub upstream.

I think we can do this in much simpler way. Let's use one GRUB Secure
Boot signed image which contains the tpm module embedded. By default the
tpm verifier will ignore UEFI errors and always return GRUB_ERR_NONE.
However, if somebody cares about these errors they can set, e.g.,
tpm_err_ignore environment variable in grub.cfg to false. Then if the
TPM UEFI calls fail for any reason machine boot fails. Does it work for
you guys?

Daniel

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[Javier Martinez Canillas]

Hello Daniel,

On 3/3/21 6:28 PM, Daniel Kiper wrote:
> On Sun, Feb 28, 2021 at 03:25:04PM -0800, James Bottomley wrote:
> 
> [...]
> 
>> How about a more simple solution: you sign two grub unitary EFI
>> binaries, one of which does measured boot and one of which doesn't.
>> Your installer is already config file driven, so by default it would
>> install the measured boot one, but if there's a failure you can tell
>> the user to add the config option to install the unmeasured boot one
>> ... this could also be useful for various other situations where you
>> want secure but not measured boot?  I'm fairly certain you could design
>> a distro installer test for the problem and thus always install a
>> working system.  There's no security issue because anyone who does
>> attested measured boot will instantly detect someone booting via the
>> signed unmeasured boot grub.
>>
>> Note: I'm certainly not presenting this as the optimal solution, merely
>> the least effort solution that looks like it will work with the current
>> grub upstream.
> 
> I think we can do this in much simpler way. Let's use one GRUB Secure
> Boot signed image which contains the tpm module embedded. By default the
> tpm verifier will ignore UEFI errors and always return GRUB_ERR_NONE.
> However, if somebody cares about these errors they can set, e.g.,
> tpm_err_ignore environment variable in grub.cfg to false. Then if the
> TPM UEFI calls fail for any reason machine boot fails. Does it work for
> you guys?
>

That sounds sensible to me. As long as the default is to ignore errors
as you suggested (i.e: leading to an unbootable system is opt-in and not
opt-out), I agree with the approach and can prepare a v2 that does this.
 
> Daniel
> 

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH] tpm: Don't propagate measurement failures to the verifiers layer

[James Bottomley]

On Wed, 2021-03-03 at 18:28 +0100, Daniel Kiper wrote:
> On Sun, Feb 28, 2021 at 03:25:04PM -0800, James Bottomley wrote:
> 
> [...]
> 
> > How about a more simple solution: you sign two grub unitary EFI
> > binaries, one of which does measured boot and one of which doesn't.
> > Your installer is already config file driven, so by default it
> > would install the measured boot one, but if there's a failure you
> > can tell the user to add the config option to install the
> > unmeasured boot one ... this could also be useful for various other
> > situations where you want secure but not measured boot?  I'm fairly
> > certain you could design a distro installer test for the problem
> > and thus always install a working system.  There's no security
> > issue because anyone who does attested measured boot will instantly
> > detect someone booting via the signed unmeasured boot grub.
> > 
> > Note: I'm certainly not presenting this as the optimal solution,
> > merely the least effort solution that looks like it will work with
> > the current grub upstream.
> 
> I think we can do this in much simpler way. Let's use one GRUB Secure
> Boot signed image which contains the tpm module embedded. By default
> the tpm verifier will ignore UEFI errors and always return
> GRUB_ERR_NONE. However, if somebody cares about these errors they can
> set, e.g., tpm_err_ignore environment variable in grub.cfg to false.
> Then if the TPM UEFI calls fail for any reason machine boot fails.
> Does it work for you guys?

It's certainly an acceptable solution.  However, I'd prefer the flag be
inverted so the boot will fail if the logging does because it means the
UEFI firmware in the system has a very unexpected failure that needs
reporting.  Then any possessor of a failing system can set a flag to
allow boot to proceed.

James