Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 14 Dec 2020 10:42:22 -0600
From:   Tyler Hicks <tyhicks@linux.microsoft.com>
To:     Mimi Zohar <zohar@linux.ibm.com>
Cc:     Sasha Levin <sashal@kernel.org>, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org, Maurizio Drocco <maurizio.drocco@ibm.com>,
        Bruno Meneguele <bmeneg@redhat.com>,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org
Subject: Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel
 measurements
Message-ID: <20201214164222.GK4951@sequoia>
References: <20200708154116.3199728-1-sashal@kernel.org>
 <20200708154116.3199728-3-sashal@kernel.org>
 <1594224793.23056.251.camel@linux.ibm.com>
 <20200709012735.GX2722994@sasha-vm>
 <5b8dcdaf66fbe2a39631833b03772a11613fbbbf.camel@linux.ibm.com>
 <20201211031008.GN489768@sequoia>
 <659c09673affe9637a5d1391c12af3aa710ba78a.camel@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <659c09673affe9637a5d1391c12af3aa710ba78a.camel@linux.ibm.com>
Precedence: bulk

On 2020-12-11 06:01:54, Mimi Zohar wrote:
> On Thu, 2020-12-10 at 21:10 -0600, Tyler Hicks wrote:
> > On 2020-11-29 08:17:38, Mimi Zohar wrote:
> > > Hi Sasha,
> > > 
> > > On Wed, 2020-07-08 at 21:27 -0400, Sasha Levin wrote:
> > > > On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
> > > > >Hi Sasha,
> > > > >
> > > > >On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
> > > > >> From: Maurizio Drocco <maurizio.drocco@ibm.com>
> > > > >>
> > > > >> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
> > > > >>
> > > > >> Registers 8-9 are used to store measurements of the kernel and its
> > > > >> command line (e.g., grub2 bootloader with tpm module enabled). IMA
> > > > >> should include them in the boot aggregate. Registers 8-9 should be
> > > > >> only included in non-SHA1 digests to avoid ambiguity.
> > > > >
> > > > >Prior to Linux 5.8, the SHA1 template data hashes were padded before
> > > > >being extended into the TPM.  Support for calculating and extending
> > > > >the per TPM bank template data digests is only being upstreamed in
> > > > >Linux 5.8.
> > > > >
> > > > >How will attestation servers know whether to include PCRs 8 & 9 in the
> > > > >the boot_aggregate calculation?  Now, there is a direct relationship
> > > > >between the template data SHA1 padded digest not including PCRs 8 & 9,
> > > > >and the new per TPM bank template data digest including them.
> > > > 
> > > > Got it, I'll drop it then, thank you!
> > > 
> > > After re-thinking this over, I realized that the attestation server can
> > > verify the "boot_aggregate" based on the quoted PCRs without knowing
> > > whether padded SHA1 hashes or per TPM bank hash values were extended
> > > into the TPM[1], but non-SHA1 boot aggregate values [2] should always
> > > include PCRs 8 & 9.
> > 
> > I'm still not clear on how an attestation server would know to include
> > PCRs 8 and 9 after this change came through a stable kernel update. It
> > doesn't seem like something appropriate for stable since it requires
> > code changes to attestation servers to handle the change.
> > 
> > I know this has already been released in some stable releases, so I'm
> > too late, but perhaps I'm missing something.
> 
> The point of adding PCRs 8 & 9 only to non-SHA1 boot_aggregate values
> was to avoid affecting existing attestation servers.  The intention was
> when attestation servers added support for the non-sha1 boot_aggregate
> values, they'd also include PCRs 8 & 9.  The existing SHA1
> boot_aggregate value remains PCRs 0 - 7.

AFAIK, there's nothing that prevents the non-SHA1 TPM 2.0 PCR banks from
being used even before v5.8, albeit with zero padded SHA1 digests.
Existing attestation servers that already support that configuration are
broken by this stable backport.

> To prevent this or something similar from happening again, what should
> have been the proper way of including PCRs 8 & 9?

I don't think that commits like 6f1a1d103b48 ("ima: Switch to
ima_hash_algo for boot aggregate") and 20c59ce010f8 ("ima: extend
boot_aggregate with kernel measurements") should be backported to
stable.

Including PCRs 8 and 9 definitely makes sense to include in the
boot_aggregate value but limiting such a change to "starting in 5.8",
rather than "starting in 5.8 and 5.4.82", is the safer approach when
attestation server modifications are required.

Tyler

> 
> thanks,
> 
> Mimi
>