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

[Daniel Kiper]

On Fri, Mar 03, 2023 at 12:18:19PM +0800, Michael Chang wrote:
> 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.

I am still not convinced. Using one or another lead to the same result,
lack of measurements. Am I missing something?

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

Did you try latest GRUB upstream master branch? It has significant changes
in mm code which should solve your problems. Or at least in some cases...

Daniel