All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
@ 2019-10-25 14:27 Mathieu Trudel-Lapierre
  2019-10-25 14:48 ` Mathieu Trudel-Lapierre
  0 siblings, 1 reply; 8+ messages in thread
From: Mathieu Trudel-Lapierre @ 2019-10-25 14:27 UTC (permalink / raw)
  To: grub-devel; +Cc: Mathieu Trudel-Lapierre

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(-)

diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
index 32909c192..fdbaaee19 100644
--- a/grub-core/commands/efi/tpm.c
+++ b/grub-core/commands/efi/tpm.c
@@ -155,7 +155,8 @@ grub_tpm1_execute (grub_efi_handle_t tpm_handle,
     case GRUB_EFI_NOT_FOUND:
       return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("TPM unavailable"));
     default:
-      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("Unknown TPM error"));
+      grub_dprintf("tpm", "Unknown TPM error: %" PRIdGRUB_SSIZE, status);
+      return 0;
     }
 }
 
@@ -195,7 +196,8 @@ grub_tpm2_execute (grub_efi_handle_t tpm_handle,
     case GRUB_EFI_NOT_FOUND:
       return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("TPM unavailable"));
     default:
-      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("Unknown TPM error"));
+      grub_dprintf("tpm", "Unknown TPM error: %" PRIdGRUB_SSIZE, status);
+      return 0;
     }
 }
 
@@ -262,7 +264,8 @@ grub_tpm1_log_event (grub_efi_handle_t tpm_handle, unsigned char *buf,
     case GRUB_EFI_NOT_FOUND:
       return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("TPM unavailable"));
     default:
-      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("Unknown TPM error"));
+      grub_dprintf("tpm", "Unknown TPM error: %" PRIdGRUB_SSIZE, status);
+      return 0;
     }
 }
 
@@ -312,7 +315,8 @@ grub_tpm2_log_event (grub_efi_handle_t tpm_handle, unsigned char *buf,
     case GRUB_EFI_NOT_FOUND:
       return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("TPM unavailable"));
     default:
-      return grub_error (GRUB_ERR_UNKNOWN_DEVICE, N_("Unknown TPM error"));
+      grub_dprintf("tpm", "Unknown TPM error: %" PRIdGRUB_SSIZE, status);
+      return 0;
     }
 }
 
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-25 14:27 [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got Mathieu Trudel-Lapierre
@ 2019-10-25 14:48 ` Mathieu Trudel-Lapierre
  2019-10-25 17:36   ` Javier Martinez Canillas
  2019-10-28 16:53   ` Daniel Kiper
  0 siblings, 2 replies; 8+ messages in thread
From: Mathieu Trudel-Lapierre @ 2019-10-25 14:48 UTC (permalink / raw)
  To: Mathieu Trudel-Lapierre; +Cc: The development of GNU GRUB

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.

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
errors as non-fatal so people can still boot.

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?

Kindly,

-- 

Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
4096R/65B58DA1 818A D123 0992 275B 23C2  CF89 C67B B4D6 65B5 8DA1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-25 14:48 ` Mathieu Trudel-Lapierre
@ 2019-10-25 17:36   ` Javier Martinez Canillas
  2019-10-29 10:49     ` Max Tottenham
  2019-10-28 16:53   ` Daniel Kiper
  1 sibling, 1 reply; 8+ messages in thread
From: Javier Martinez Canillas @ 2019-10-25 17:36 UTC (permalink / raw)
  To: The development of GNU GRUB, Mathieu Trudel-Lapierre,
	Mathieu Trudel-Lapierre

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].

> 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?

[0]:
From 0d404b65cddcf92e96d3cfa6e23b82e336d2535b Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Fri, 25 Oct 2019 19:27:26 +0200
Subject: [RFC PATCH] tpm: Don't propagate TPM measurement failures to the
 verifiers layer

Currently if the EFI firmware fails to do a TPM measurement for a file,
the error will be propagated to the verifiers framework and so opening
the file will not succeed.

This mean that buggy firmwares will prevent an operating system to boot
since the loader won't be able to open the kernel binaries. But failing
to do TPM measurements shouldn't be a fatal error and the system should
still be able to boot.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
 grub-core/commands/tpm.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git grub-core/commands/tpm.c grub-core/commands/tpm.c
index 1441c494d81..dbaeae46dfa 100644
--- grub-core/commands/tpm.c
+++ grub-core/commands/tpm.c
@@ -49,7 +49,8 @@ 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_tpm_measure (buf, size, GRUB_BINARY_PCR, context);
+  return GRUB_ERR_NONE;
 }
 
 static grub_err_t
@@ -57,7 +58,6 @@ grub_tpm_verify_string (char *str, enum grub_verify_string_type type)
 {
   const char *prefix = NULL;
   char *description;
-  grub_err_t status;
 
   switch (type)
     {
@@ -73,15 +73,15 @@ 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);
+
+  grub_tpm_measure ((unsigned char *) str, grub_strlen (str), GRUB_STRING_PCR,
+                    description);
   grub_free (description);
-  return status;
+  return GRUB_ERR_NONE;
 }
 
 struct grub_file_verifier grub_tpm_verifier = {
-- 
2.21.0

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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-25 14:48 ` Mathieu Trudel-Lapierre
  2019-10-25 17:36   ` Javier Martinez Canillas
@ 2019-10-28 16:53   ` Daniel Kiper
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel Kiper @ 2019-10-28 16:53 UTC (permalink / raw)
  To: Mathieu Trudel-Lapierre, mjg59
  Cc: Mathieu Trudel-Lapierre, The development of GNU GRUB

Adding Matthew...

Matthew, whole thread is at [1].

Daniel

[1] https://lists.gnu.org/archive/html/grub-devel/2019-10/msg00103.html

On Fri, Oct 25, 2019 at 10:48:43AM -0400, 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.
>
> 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
> errors as non-fatal so people can still boot.
>
> 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?
>
> Kindly,
>
> --
>
> Mathieu Trudel-Lapierre <mathieu.trudel-lapierre@canonical.com>
> Freenode: cyphermox, Jabber: mathieu.tl@gmail.com
> 4096R/65B58DA1 818A D123 0992 275B 23C2  CF89 C67B B4D6 65B5 8DA1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-25 17:36   ` Javier Martinez Canillas
@ 2019-10-29 10:49     ` Max Tottenham
  2019-10-29 12:12       ` Javier Martinez Canillas
  0 siblings, 1 reply; 8+ messages in thread
From: Max Tottenham @ 2019-10-29 10:49 UTC (permalink / raw)
  To: The development of GNU GRUB
  Cc: Mathieu Trudel-Lapierre, Mathieu Trudel-Lapierre

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-29 10:49     ` Max Tottenham
@ 2019-10-29 12:12       ` Javier Martinez Canillas
  2019-11-06 11:37         ` Daniel Kiper
  0 siblings, 1 reply; 8+ messages in thread
From: Javier Martinez Canillas @ 2019-10-29 12:12 UTC (permalink / raw)
  To: The development of GNU GRUB
  Cc: Max Tottenham, Mathieu Trudel-Lapierre, Mathieu Trudel-Lapierre

Hello Max,

On 10/29/19 11:49 AM, Max Tottenham via Grub-devel wrote:
> On 10/25, Javier Martinez Canillas wrote:

[snip]

>>>
>>
>> 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. 
>

I fail to see how making it a non-fatal error would make this harder for users
that rely on TPM sealed values.

If the HashLogExtendEvent failed and the PCR were not extended, then the sealed
values won't be unsealed by the TPM. So it won't be less secure for users while
still allowing the system to boot for users that aren't relying on PCR values.

And even for users that rely on it, I think that halting the boot is too extreme.
For example a user could have a LUKS volume key sealed with a TPM but still have
another key slot with a passphrase as fallback in case the PCR measurements fail.

Even for the case you mentioned that EFI firmware could return an EFI_VOLUME_FULL
meaning that the extend operation occurred but the event could not be written to
the event log, then attestation software that not only check the PCR values but
also the event logs will determine that the logs are not correct and report that
the system is not healthy.

Then you could reboot your machine enabling debug logs for grub and check if the
call to HashLogExtendEvent is failing and what error code is returning to address
the issue and troubleshoot.

In other words, preventing the system from booting should be the last option in
my opinion and only for situations where there is really no other choice.

> 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?
>

Yes, having a compile time or runtime option to choose this could work. But
I'm still not convinced that halting the boot process due a TPM measurement
failure is the correct thing to do.

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-10-29 12:12       ` Javier Martinez Canillas
@ 2019-11-06 11:37         ` Daniel Kiper
  2019-11-06 14:04           ` Max Tottenham
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Kiper @ 2019-11-06 11:37 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: The development of GNU GRUB, Max Tottenham,
	Mathieu Trudel-Lapierre, Mathieu Trudel-Lapierre, mjg59

CC-ing Matthew...

On Tue, Oct 29, 2019 at 01:12:34PM +0100, Javier Martinez Canillas wrote:
> Hello Max,
>
> On 10/29/19 11:49 AM, Max Tottenham via Grub-devel wrote:
> > On 10/25, Javier Martinez Canillas wrote:
>
> [snip]
>
> >>>
> >>
> >> 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.
> >
>
> I fail to see how making it a non-fatal error would make this harder for users
> that rely on TPM sealed values.
>
> If the HashLogExtendEvent failed and the PCR were not extended, then the sealed
> values won't be unsealed by the TPM. So it won't be less secure for users while
> still allowing the system to boot for users that aren't relying on PCR values.
>
> And even for users that rely on it, I think that halting the boot is too extreme.
> For example a user could have a LUKS volume key sealed with a TPM but still have
> another key slot with a passphrase as fallback in case the PCR measurements fail.
>
> Even for the case you mentioned that EFI firmware could return an EFI_VOLUME_FULL
> meaning that the extend operation occurred but the event could not be written to
> the event log, then attestation software that not only check the PCR values but
> also the event logs will determine that the logs are not correct and report that
> the system is not healthy.
>
> Then you could reboot your machine enabling debug logs for grub and check if the
> call to HashLogExtendEvent is failing and what error code is returning to address
> the issue and troubleshoot.
>
> In other words, preventing the system from booting should be the last option in
> my opinion and only for situations where there is really no other choice.
>
> > 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?
>
> Yes, having a compile time or runtime option to choose this could work. But
> I'm still not convinced that halting the boot process due a TPM measurement
> failure is the correct thing to do.

I full agree with Javier. So, I think that by default GRUB should print
warning about TPM failures and continue booting. Though user should have
a choice during installation to disable that behavior and enforce halt
at boot. Maybe even he/she should be able to choose which kind of errors
(numeric values) he/she is going to ignore if any. Of course we can
suggest in the docs which errors are pretty safe to ignore at boot
phase. Or even we can set a reasonable default in the GRUB config.

Daniel


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got
  2019-11-06 11:37         ` Daniel Kiper
@ 2019-11-06 14:04           ` Max Tottenham
  0 siblings, 0 replies; 8+ messages in thread
From: Max Tottenham @ 2019-11-06 14:04 UTC (permalink / raw)
  To: Daniel Kiper
  Cc: Javier Martinez Canillas, The development of GNU GRUB,
	Mathieu Trudel-Lapierre, Mathieu Trudel-Lapierre, mjg59

On 11/06, Daniel Kiper wrote:
> CC-ing Matthew...
> 
> On Tue, Oct 29, 2019 at 01:12:34PM +0100, Javier Martinez Canillas wrote:
> > Hello Max,
> >
> > On 10/29/19 11:49 AM, Max Tottenham via Grub-devel wrote:
> > > On 10/25, Javier Martinez Canillas wrote:
> >
> > [snip]
> >
> > >>>
> > >>
> > >> 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.
> > >
> >
> > I fail to see how making it a non-fatal error would make this harder for users
> > that rely on TPM sealed values.
> >
> > If the HashLogExtendEvent failed and the PCR were not extended, then the sealed
> > values won't be unsealed by the TPM. So it won't be less secure for users while
> > still allowing the system to boot for users that aren't relying on PCR values.
> >
> > And even for users that rely on it, I think that halting the boot is too extreme.
> > For example a user could have a LUKS volume key sealed with a TPM but still have
> > another key slot with a passphrase as fallback in case the PCR measurements fail.
> >
> > Even for the case you mentioned that EFI firmware could return an EFI_VOLUME_FULL
> > meaning that the extend operation occurred but the event could not be written to
> > the event log, then attestation software that not only check the PCR values but
> > also the event logs will determine that the logs are not correct and report that
> > the system is not healthy.
> >
> > Then you could reboot your machine enabling debug logs for grub and check if the
> > call to HashLogExtendEvent is failing and what error code is returning to address
> > the issue and troubleshoot.
> >
> > In other words, preventing the system from booting should be the last option in
> > my opinion and only for situations where there is really no other choice.
> >
> > > 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?
> >
> > Yes, having a compile time or runtime option to choose this could work. But
> > I'm still not convinced that halting the boot process due a TPM measurement
> > failure is the correct thing to do.
> 
> I full agree with Javier. So, I think that by default GRUB should print
> warning about TPM failures and continue booting. Though user should have
> a choice during installation to disable that behavior and enforce halt
> at boot. Maybe even he/she should be able to choose which kind of errors
> (numeric values) he/she is going to ignore if any. Of course we can
> suggest in the docs which errors are pretty safe to ignore at boot
> phase. Or even we can set a reasonable default in the GRUB config.
> 
> Daniel

I think that makes sense, I think it's correct to assume that in the
general case this should not prevent the system from booting. 

As long as there is some sort of compile time ./configure option that
would allow halting behavior (which by default is disabled). It can be
tricky to read the error messages as they flash by before you get stuck
at a later prompt.


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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-11-06 14:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25 14:27 [PATCH] tpm: Pass unknown error as non-fatal, but debug print the error we got Mathieu Trudel-Lapierre
2019-10-25 14:48 ` Mathieu Trudel-Lapierre
2019-10-25 17:36   ` Javier Martinez Canillas
2019-10-29 10:49     ` Max Tottenham
2019-10-29 12:12       ` Javier Martinez Canillas
2019-11-06 11:37         ` Daniel Kiper
2019-11-06 14:04           ` Max Tottenham
2019-10-28 16:53   ` Daniel Kiper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.