public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "James Bottomley" <jejb@linux.ibm.com>
To: Stefan Berger <stefanb@linux.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>
Cc: devel@edk2.groups.io, "Min Xu" <min.m.xu@intel.com>,
	"Jordan Justen" <jordan.l.justen@intel.com>,
	"Erdem Aktas" <erdemaktas@google.com>,
	"Ard Biesheuvel" <ardb+tianocore@kernel.org>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Jiewen Yao" <jiewen.yao@intel.com>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Brijesh Singh" <brijesh.singh@amd.com>
Subject: Re: [PATCH 4/4] OvmfPkg: add TPM2_SHA1_ENABLE build option
Date: Fri, 22 Oct 2021 10:17:11 -0400	[thread overview]
Message-ID: <bdd573f45d59425244626bbf9935f4c948edde9b.camel@linux.ibm.com> (raw)
In-Reply-To: <bb3715bb-5034-cd74-80b0-5f79fa0b2470@linux.ibm.com>

On Fri, 2021-10-22 at 09:13 -0400, Stefan Berger wrote:
> On 10/22/21 8:40 AM, James Bottomley wrote:
> 
> > On Fri, 2021-10-22 at 07:57 -0400, Stefan Berger wrote:
> > > On 10/22/21 7:49 AM, James Bottomley wrote:
> > > > On Fri, 2021-10-22 at 06:50 -0400, Stefan Berger wrote:
> > > > [...]
> > > > > I see this also but when I get into Linux and run
> > > > > tpm2_pcrread I see the SHA1 bank active but not having
> > > > > received any PCR extensions from the firmware, which is not
> > > > > supposed to happen.
> > > > 
> > > > That's not entirely correct: the TCG firmware profile just
> > > > requires us to log through at least one bank; it doesn't
> > > > require that all active banks be logged.  I've got several
> > > > physical systems with three active banks but only one or two
> > > > measured through.
> > >   
> > > The problem with this is that you can then fake measured boot on
> > > that system using it's unused SHA1 bank and extend into it
> > > whatever you want and create a fake log along with it and the
> > > quote is going to look alright.
> > 
> > I don't think you can.  The measured boot PCRs in unused banks
> > should always be their default values and the measurement software
> > should check for this.  So on a system that only uses the sha256
> > bank, the sha1 bank PCR0-7 should be all zeros ... if they aren't
> > this should be a measurement failure.
> > 
> > That means that if you try to replace the sha256 agile log with one
> > containing fake sha1 entries, the attestation still fails because
> > the sha256 bank doesn't have default entries.
> 
> You can still pretend that your system only has an active SHA1 bank
> and serve the fake log.

Which "You" can fake a TPM quote?  The whole design of the TPM system
is supposed to be that what goes into the TPM can't be erased, only
updated and we can get definitive proof of the values using a quote. 
You can fake the log to be sha1 only but you can't make it match the
quote that includes the sha256 banks.

> at that trusted boot log, SHA1 PCR 0-7 state, and quote then?

You don't just quote the bank you think is being logged ... you should
quote all banks of the TPM; that way you can't be duped in this
fashion.

> > > > >    So I think you should drop this patch and I'll change the
> > > > > set of active PCR banks on the swtpm_setup level.
> > > >   
> > > > Even if the firmware deactivated the sha1 bank, the kernel
> > > > expectation problem is still going to exist.
> > >  
> > > Is that older Linux kernels or which part still requires sha1? A
> > > pointer would be good. I would have to revert the change to not
> > > activat ethe SHA1 bank from swtpm_setup if that's going to create
> > > headaches. I thought some hardware TPM 2's today are only
> > > providing a SHA256 bank and so it shouldn't be a problem.
> >  
> > The problem is IMA: it's hash is a kernel config parameter which
> > defaults to sha1.  It then tries to calculate the boot aggregate
> > over the configured hash bank and doesn't check if it's unused.
> > 
> > What IMA should probably be doing is working out which bank the
> > bios is logging through and using that as the hash instead of
> > having it as a Kconfig parameter.
> 
> I think IMA is doing the right thing and extending into SHA1 and
> SHA256 PCRs if the banks are active and with the boot aggregate puts
> a lid on top of the PCRs 0-7(,8-9). IMA may help raise the suspicion
> about abuse of an unused PCR bank by the firmware but looking at the
> measured boot log etc. alone I think is not enough.

The problem is not where IMA extends, it's where it gets the boot
aggregate from.  If the IMA hash is sha1 and a sha1 bank exists, it
will use it alone for the boot aggregate.

> At least a test with a recent kernel seems to work out alright when
> only the SHA256 bank is active.

Well, yes, if IMA is configured as sha1 and no sha1 bank exists, it
will fall back to sha256, but that doesn't cover the boot aggregate
problem above.

James



  reply	other threads:[~2021-10-22 14:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-21 12:19 [PATCH 0/4] OvmfPkg: rework TPM configuration Gerd Hoffmann
2021-10-21 12:20 ` [PATCH 1/4] OvmfPkg: move tcg configuration to dsc and fdf include files Gerd Hoffmann
2021-10-21 14:12   ` [edk2-devel] " Stefan Berger
2021-10-21 12:20 ` [PATCH 2/4] OvmfPkg: create Tcg2ConfigPeiCompat12.inf Gerd Hoffmann
2021-10-21 14:46   ` [edk2-devel] " Stefan Berger
2021-10-22  6:31     ` Gerd Hoffmann
2021-10-22 13:29       ` Stefan Berger
2021-10-21 12:20 ` [PATCH 3/4] OvmfPkg: rework TPM configuration Gerd Hoffmann
2021-10-21 15:44   ` Stefan Berger
2021-10-22  6:30     ` Gerd Hoffmann
2021-10-21 12:20 ` [PATCH 4/4] OvmfPkg: add TPM2_SHA1_ENABLE build option Gerd Hoffmann
2021-10-21 13:24   ` Stefan Berger
2021-10-22  6:39     ` Gerd Hoffmann
2021-10-22 10:50       ` Stefan Berger
2021-10-22 11:37         ` Gerd Hoffmann
2021-10-22 11:49         ` James Bottomley
2021-10-22 11:57           ` Stefan Berger
2021-10-22 12:40             ` James Bottomley
2021-10-22 13:13               ` Stefan Berger
2021-10-22 14:17                 ` James Bottomley [this message]
2021-10-22 14:52                   ` [edk2-devel] " Stefan Berger
2021-10-22 15:01                     ` James Bottomley
2021-10-22 15:48                       ` Stefan Berger
2021-10-22 16:50                         ` James Bottomley
2021-10-21 16:13 ` [PATCH 0/4] OvmfPkg: rework TPM configuration Stefan Berger
2021-10-22  7:01   ` Gerd Hoffmann
2021-10-22 10:46     ` [edk2-devel] " Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bdd573f45d59425244626bbf9935f4c948edde9b.camel@linux.ibm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox