Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
From: Tyler Hicks <tyhicks@linux.microsoft.com>
To: Peter Jones <pjones@redhat.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Matthew Garrett <mjg59@google.com>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Peter Huewe <peterhuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
	Petr Vandrovec <petr@vmware.com>,
	Nayna Jain <nayna@linux.ibm.com>,
	Thirupathaiah Annapureddy <thiruan@microsoft.com>,
	linux-integrity <linux-integrity@vger.kernel.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tpm: Require that all digests are present in TCG_PCR_EVENT2 structures
Date: Tue, 30 Jun 2020 14:23:22 -0500
Message-ID: <20200630192322.GF4694@sequoia> (raw)
In-Reply-To: <20200630185327.pasrylg7og7rlno3@redhat.com>

On 2020-06-30 14:53:28, Peter Jones wrote:
> On Tue, Jun 16, 2020 at 11:08:38AM +0200, Ard Biesheuvel wrote:
> > (cc Matthew and Peter)
> > 
> > On Tue, 16 Jun 2020 at 01:28, Tyler Hicks <tyhicks@linux.microsoft.com> wrote:
> > >
> > > Require that the TCG_PCR_EVENT2.digests.count value strictly matches the
> > > value of TCG_EfiSpecIdEvent.numberOfAlgorithms in the event field of the
> > > TCG_PCClientPCREvent event log header. Also require that
> > > TCG_EfiSpecIdEvent.numberOfAlgorithms is non-zero.
> > >
> > > The TCG PC Client Platform Firmware Profile Specification section 9.1
> > > (Family "2.0", Level 00 Revision 1.04) states:
> > >
> > >  For each Hash algorithm enumerated in the TCG_PCClientPCREvent entry,
> > >  there SHALL be a corresponding digest in all TCG_PCR_EVENT2 structures.
> > >  Note: This includes EV_NO_ACTION events which do not extend the PCR.
> > >
> > > Section 9.4.5.1 provides this description of
> > > TCG_EfiSpecIdEvent.numberOfAlgorithms:
> > >
> > >  The number of Hash algorithms in the digestSizes field. This field MUST
> > >  be set to a value of 0x01 or greater.
> > >
> > > Enforce these restrictions, as required by the above specification, in
> > > order to better identify and ignore invalid sequences of bytes at the
> > > end of an otherwise valid TPM2 event log. Firmware doesn't always have
> > > the means necessary to inform the kernel of the actual event log size so
> > > the kernel's event log parsing code should be stringent when parsing the
> > > event log for resiliency against firmware bugs. This is true, for
> > > example, when firmware passes the event log to the kernel via a reserved
> > > memory region described in device tree.
> > >
> > 
> > When does this happen? Do we have code in mainline that does this?
> > 
> > > Prior to this patch, a single bit set in the offset corresponding to
> > > either the TCG_PCR_EVENT2.eventType or TCG_PCR_EVENT2.eventSize fields,
> > > after the last valid event log entry, could confuse the parser into
> > > thinking that an additional entry is present in the event log. This
> > > patch raises the bar on how difficult it is for stale memory to confuse
> > > the kernel's event log parser but there's still a reliance on firmware
> > > to properly initialize the remainder of the memory region reserved for
> > > the event log as the parser cannot be expected to detect a stale but
> > > otherwise properly formatted firmware event log entry.
> > >
> > > Fixes: fd5c78694f3f ("tpm: fix handling of the TPM 2.0 event logs")
> > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> > > ---
> > 
> > I am all for stringent checks, but this could potentially break
> > measured boot on systems that are working fine today, right?
> 
> Seems like in that case our measurement is unreliable and can't really
> be trusted.  That said, having things that were using the measurements
> before this suddenly stop being able to access sealed secrets is not a
> great experience for the user who unwittingly bought the junk hardware.

I haven't seen where anyone has suggested that such junk hardware
exists. Do you know of or expect any firmware that has mismatched
TCG_PCR_EVENT2.digests.count and TCG_EfiSpecIdEvent.numberOfAlgorithms
values?

I would think that the userspace code that's parsing
/sys/kernel/security/tpm0/binary_bios_measurements would also have
issues with such an event log.

> Same with the zero-supported-hashes case.

Small but important correction: it is a zero-hashes case, not a
zero-supported-hashes case

There's no handshake involved or anything like that. This would only
cause problems if the firmware provided no hashes, which means the
firmware event log is unusable, anyways.

Tyler

> It would be nice to at log it and have a way for them to opt-in to
> allowing the old measurement to go through, so they can recover their
> data, though I don't know what that method would be if the measured
> command line is one of their dependencies.
> 
> -- 
>         Peter

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-15 23:25 Tyler Hicks
2020-06-16  9:08 ` Ard Biesheuvel
2020-06-16 15:22   ` Tyler Hicks
2020-06-17 23:09   ` Jarkko Sakkinen
2020-06-30 18:33     ` Tyler Hicks
2020-07-02 23:57       ` Jarkko Sakkinen
2020-06-30 18:53   ` Peter Jones
2020-06-30 19:10     ` Ard Biesheuvel
2020-06-30 19:23     ` Tyler Hicks [this message]
2020-06-30 19:48       ` Peter Jones

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-all 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=20200630192322.GF4694@sequoia \
    --to=tyhicks@linux.microsoft.com \
    --cc=ardb@kernel.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jgg@ziepe.ca \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg59@google.com \
    --cc=nayna@linux.ibm.com \
    --cc=peterhuewe@gmx.de \
    --cc=petr@vmware.com \
    --cc=pjones@redhat.com \
    --cc=thiruan@microsoft.com \
    /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

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git