Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present

[Michael Chang]

On Thu, Mar 02, 2023 at 07:59:00PM +0100, Daniel Kiper wrote:
> On Tue, Feb 28, 2023 at 11:22:14AM +0800, Michael Chang via Grub-devel wrote:
> > On Thu, Feb 23, 2023 at 02:22:27PM +0100, Daniel Kiper wrote:
> > > Adding James, Stefan and Robbie...
> > >
> > > On Mon, Feb 20, 2023 at 12:57:01PM +0800, Michael Chang via Grub-devel 
> > > wrote:
> > > > On Tue, Nov 29, 2022 at 04:11:48PM +0100, Daniel Kiper wrote:
> > > > > On Fri, Nov 25, 2022 at 03:00:48PM +0800, Michael Chang via 
> > > > > Grub-devel wrote:
> > > > > > On Thu, Nov 24, 2022 at 05:04:48PM +0100, Daniel Kiper wrote:
> > > > > > > On Mon, Oct 17, 2022 at 01:19:08PM +0800, Michael Chang via 
> > > > > > > Grub-devel wrote:
> > > > > > > > On Fri, Oct 14, 2022 at 11:40:01AM +0200, Daniel Kiper wrote:
> > > > > > > > > On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via 
> > > > > > > > > Grub-devel wrote:
> > > > > > > > > > This helps to prevent out of memory error when reading 
> > > > > > > > > > large files via disabling
> > > > > > > > > > tpm device as verifier has to read all content into memory 
> > > > > > > > > > in one chunk to
> > > > > > > > > > measure the hash and extend to tpm.
> > > > > > > > >
> > > > > > > > > How does this patch help when the TPM is present in the 
> > > > > > > > > system?
> > > > > > > >
> > > > > > > > If the firmware menu offers option to disable TPM device, then 
> > > > > > > > this
> > > > > > > > patch can be useful to get around 'out of memory error' through
> > > > > > > > disabling TPM device from firmware in order to make tpm 
> > > > > > > > verifier won't
> > > > > > > > be in the way of reading huge files.
> > > > > > > >
> > > > > > > > This is essentially a compromised solution as long as tpm 
> > > > > > > > module can be
> > > > > > > > a built-in module in signed image and at the same time user may 
> > > > > > > > come
> > > > > > > > across the need to open huge files, for eg, loopback mount in 
> > > > > > > > grub for
> > > > > > > > the rescue image. In this case they could be opted in to 
> > > > > > > > disable tpm
> > > > > > > > device from firmware to proceed if they run into out of memory 
> > > > > > > > or other
> > > > > > > > (slow) reading issues.
> > > > > > >
> > > > > > > I think I would prefer something similar to this [1] patch. Of 
> > > > > > > course
> > > > > > > if [1] is not enough...
> > > > > >
> > > > > > The tpm verifier attempts to set GRUB_VERIFY_FLAGS_SINGLE_CHUNK for 
> > > > > > all
> > > > > > incoming files, which gets loaded into memory in its entirety as an
> > > > > > duplicated copy to disk files. The overhead is too huge to some low
> > > > > > profile hardwares with smaller memory or when the boot path has to 
> > > > > > cover
> > > > > > very large files, hence the out of memory error.
> > > > > >
> > > > > > I think it inevitable to use GRUB_VERIFY_FLAGS_SINGLE_CHUNK as tpm
> > > > > > measures and extends file intergrity. But we ought to avoid the 
> > > > > > overhead
> > > > > > when TPM device is not present or disabled by the user.
> > > > > >
> > > > > > The patch [1] seems to deal with the tpm error which prevents a file
> > > > > > from being opened, which is orthogonal to the memory allocation 
> > > > > > issue in
> > > > > > the common verifier before tpm doing measurement.
> > > > >
> > > > > This is what I expected. So, that is why I said "Of course if [1] is 
> > > > > not
> > > > > enough..."... :-) Now I think it would be nice if TPM verifier is
> > > > > disabled when the TPM device is broken/disabled/... and/or somebody 
> > > > > sets
> > > > > a separate environemnt variable in the GRUB. WDYT?
> > > >
> > > > I'm not sure if a separate environment a good idea, because it would
> > > > expose the funtion to disable verifier in a way much readily accessible
> > > > through one of grub command line interface, grub.cfg and grubenv file so
> > > > that the intention have to be very carefully reviewed here.
> > >
> > > I think it should be safe because even if somebody is doing nasty things
> > > with disabling the TPM verifier they can be easily detected or will lead
> > > to early boot failures when the TPM is used to store secrets. Of course
> > > there is small chance somebody disables the TPM verifier during platform
> > > initialization/installation. However, this should be also easily to
> > > detect due to, e.g., lack of measurements.
> >
> > IMHO it is hardly be detectable, there's no clear distinction to it is a
> > result of lack of measurement or a compromised system if we know that
> > tpm is active and working ...
> 
> Sudden lack of measurements should be a red flag and machine should be
> treated initially as a compromised. Additionally, how the proposed
> variable/switch/... in the GRUB differs from the switch in the BIOS
> setup which you mentioned above? The result of their use will be
> the same.

The off switch in bios may completely shut down the tpm device, so it
won't be usable at all to the user who wants to actively disable it
there. On the other hand the swich in grub is in a context where tpm is
still in action, the missing measurement could lead to a comprehension
of compromised system and it can break root of trust, killing the most
idea of using a tpm to attest integrity of a booted system. 

So instead of making grub bypass tpm become possible, I think user
should always go for bios to disable tpm if running into OOM issue upon
executing memory consuming commands like loopback, initrd etc, in
particular after grub upgrade to add tpm module in small memory systems. 

Thanks,
Michael

> 
> > > Anyway, I would extend the
> > > GRUB documentation with a note saying that platform
> > > initialization/installation should happen in well controlled
> > > environment. Just in case...
> >
> > Yes the initial bootstrapping environemt or process is important, but
> > also there's system update which should happen in a well controlled
> > environment as well.
> 
> Yes, and we can document this as well...
> 
> Daniel