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

[Daniel Kiper]

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.

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