Date:   Wed, 8 Feb 2023 01:20:11 +0200
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     William Roberts <bill.c.roberts@gmail.com>
Cc:     James Bottomley <jejb@linux.ibm.com>,
        Matthew Garrett <mgarrett@aurora.tech>,
        Evan Green <evgreen@chromium.org>,
        linux-kernel@vger.kernel.org, corbet@lwn.net,
        linux-integrity@vger.kernel.org,
        Eric Biggers <ebiggers@kernel.org>, gwendal@chromium.org,
        dianders@chromium.org, apronin@chromium.org,
        Pavel Machek <pavel@ucw.cz>, Ben Boeckel <me@benboeckel.net>,
        rjw@rjwysocki.net, Kees Cook <keescook@chromium.org>,
        dlunev@google.com, zohar@linux.ibm.com, linux-pm@vger.kernel.org,
        Matthew Garrett <mjg59@google.com>,
        Jason Gunthorpe <jgg@ziepe.ca>, Peter Huewe <peterhuewe@gmx.de>
Subject: Re: [PATCH v5 03/11] tpm: Allow PCR 23 to be restricted to
 kernel-only use
Message-ID: <Y+Lcq0u6mUWxxgzd@kernel.org>
References: <CAFftDdqUOiysgrAC4wPUXRaEWz4j9V6na3u4bm29AfxE8TAyXw@mail.gmail.com>
 <CAHSSk04asd_ac8KLJYNRyR1Z+fD+iUb+UxjUu0U=HbT1-2R7Ag@mail.gmail.com>
 <08302ed1c056da86a71aa2e6ca19111075383e75.camel@linux.ibm.com>
 <Y8tcEtr8Kl3p4qtA@kernel.org>
 <CAFftDdoVraQVKLZGc6gMpZRyyK+LEO3cwjLhKM61qbp8ZSRYrg@mail.gmail.com>
 <5fb9193be57d22131feecf8b39dffbb03af3f60a.camel@linux.ibm.com>
 <Y9K2mOsmB1+CFk9l@kernel.org>
 <CAFftDdqq-eeryycv_11m=-1+aR=cgCUU7C_BFDrmYRwFF13i5w@mail.gmail.com>
 <Y9LxDYYEp0qTfhqN@kernel.org>
 <CAFftDdoSg-kF6fd2Jyz5UuP1pG_bUe0kYy+G9Y1_nfoUkkpW4g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFftDdoSg-kF6fd2Jyz5UuP1pG_bUe0kYy+G9Y1_nfoUkkpW4g@mail.gmail.com>
Precedence: bulk

On Thu, Jan 26, 2023 at 04:01:55PM -0600, William Roberts wrote:
> On Thu, Jan 26, 2023 at 3:30 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Thu, Jan 26, 2023 at 11:32:22AM -0600, William Roberts wrote:
> > > On Thu, Jan 26, 2023 at 11:21 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > >
> > > > On Tue, Jan 24, 2023 at 07:38:04AM -0500, James Bottomley wrote:
> > > > > On Mon, 2023-01-23 at 11:48 -0600, William Roberts wrote:
> > > > > > On Fri, Jan 20, 2023 at 9:29 PM Jarkko Sakkinen <jarkko@kernel.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Sat, Jan 14, 2023 at 09:55:37AM -0500, James Bottomley wrote:
> > > > > > > > On Tue, 2023-01-03 at 13:10 -0800, Matthew Garrett wrote:
> > > > > > > > > On Tue, Jan 3, 2023 at 1:05 PM William Roberts
> > > > > > > > > <bill.c.roberts@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > > What's the use case of using the creation data and ticket in
> > > > > > > > > > this context? Who gets the creationData and the ticket?
> > > > > > > > > > Could a user supplied outsideInfo work? IIRC I saw some
> > > > > > > > > > patches flying around where the sessions will get encrypted
> > > > > > > > > > and presumably correctly as well. This would allow the
> > > > > > > > > > transfer of that outsideInfo, like the NV Index PCR value to
> > > > > > > > > > be included and integrity protected by the session HMAC.
> > > > > > > > >
> > > > > > > > > The goal is to ensure that the key was generated by the kernel.
> > > > > > > > > In the absence of the creation data, an attacker could generate
> > > > > > > > > a hibernation image using their own key and trick the kernel
> > > > > > > > > into resuming arbitrary code. We don't have any way to pass
> > > > > > > > > secret data from the hibernate kernel to the resume kernel, so
> > > > > > > > > I don't think there's any easy way to do it with outsideinfo.
> > > > > > > >
> > > > > > > > Can we go back again to why you can't use locality?  It's exactly
> > > > > > > > designed for this since locality is part of creation data.
> > > > > > > > Currently everything only uses locality 0, so it's impossible for
> > > > > > > > anyone on Linux to produce a key with anything other than 0 in
> > > > > > > > the creation data for locality.  However, the dynamic launch
> > > > > > > > people are proposing that the Kernel should use Locality 2 for
> > > > > > > > all its operations, which would allow you to distinguish a key
> > > > > > > > created by the kernel from one created by a user by locality.
> > > > > > > >
> > > > > > > > I think the previous objection was that not all TPMs implement
> > > > > > > > locality, but then not all laptops have TPMs either, so if you
> > > > > > > > ever come across one which has a TPM but no locality, it's in a
> > > > > > > > very similar security boat to one which has no TPM.
> > > > > > >
> > > > > > > Kernel could try to use locality 2 and use locality 0 as fallback.
> > > > > >
> > > > > > I don't think that would work for Matthew, they need something
> > > > > > reliable to indicate key provenance.
> > > > >
> > > > > No, I think it would be good enough: locality 0 means anyone (including
> > > > > the kernel on a machine which doesn't function correctly) could have
> > > > > created this key.  Locality 2 would mean only the kernel could have
> > > > > created this key.
> > > > >
> > > > > By the time the kernel boots and before it loads the hibernation image
> > > > > it will know the answer to the question "does my TPM support locality
> > > > > 2", so it can use that in its security assessment: if the kernel
> > > > > supports locality 2 and the key wasn't created in locality 2 then
> > > > > assume an attack.  Obviously, if the kernel doesn't support locality 2
> > > > > then the hibernation resume has to accept any old key, but that's the
> > > > > same as the situation today.
> > > >
> > > > This sounds otherwise great to me but why bother even allowing a
> > > > machine with no-locality TPM to be involved with hibernate? Simply
> > > > detect locality support during driver initialization and disallow
> > > > sealed hibernation (or whatever the feature was called) if localities
> > > > were not detected.
> > > >
> > > > I get supporting old hardware with old features but it does not make
> > > > sense to maintain new features with hardware, which clearly does not
> > > > scale, right?
> > > >
> > > > BR, Jarkko
> > >
> > > Here's a thought, what if we had a static/cmd line configurable
> > > no-auth NV Index and writelocked it with the expected key information,
> > > name or something. I guess the problem is atomicity with write/lock,
> > > but can't the kernel lock out all other users?
> > >
> > > An attacker would need to issue tpm2_startup, which in this case would DOS
> > > the kernel in both scenarios. If an attacker already wrote and locked the NV
> > > index, that would also be a DOS. If they already wrote it, the kernel simply
> > > writes whatever they want. Is there an attack I am missing?
> > >
> > > I guess the issue here would be setup, since creating the NV index requires
> > > hierarchy auth, does the kernel have platform auth or is that already shut down
> > > by firmware (I can't recall)? A null hierarchy volatile lockable index would be
> > > nice for this, too bad that doesn't exist.
> >
> > How do you see this would better when compared to finding a way to use
> > locality, which could potentially be made to somewhat simple to setup
> > (practically zero config)?
> >
> 
> I never said it was better, I said here is a thought for discussion.
> If we had to support older hardware (I could care less about things
> that don't support localities, but some might not), this could be an
> avenue to support them without walling off a PCR. I pointed out the
> downsides, and argument could be made that when localities is not
> supported then walling off PCR23 is the better approach if older
> hardware is an issue. This all hinges on do we care about things
> that don't support multiple localities. I don't, im for if you have locality
> support you get the feature else you don't.

Probably does not make much sense to care for this feature.

BR, Jarkko