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

[Daniel Kiper]

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. Anyway, I would extend the
GRUB documentation with a note saying that platform
initialization/installation should happen in well controlled
environment. Just in case...

Daniel