Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Tue, 01 Aug 2023 21:55:11 +0300
Message-Id: <CUHFQHNPO3PG.OKPKFTYYC8V9@suppilovahvero>
Cc:     <linux@dominikbrodowski.net>,
        "Daniil Stas" <daniil.stas@posteo.net>,
        <James.Bottomley@hansenpartnership.com>,
        <linux-integrity@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <regressions@leemhuis.info>, <stable@vger.kernel.org>
Subject: Re: [PATCH 1/1] tpm: disable hwrng for fTPM on some AMD designs
From:   "Jarkko Sakkinen" <jarkko@kernel.org>
To:     "Jarkko Sakkinen" <jarkko@kernel.org>,
        "Mario Limonciello" <mario.limonciello@amd.com>,
        "Jason A. Donenfeld" <Jason@zx2c4.com>,
        "Linus Torvalds" <torvalds@linux-foundation.org>
References: <20230727183805.69c36d6e@g14>
 <b1dd27df-744b-3977-0a86-f5dde8e24288@amd.com>
 <20230727193949.55c18805@g14>
 <65a1c307-826d-4ca3-0336-07a185684e5d@amd.com>
 <20230727195019.41abb48d@g14>
 <67eefe98-e6df-e152-3169-44329e22478d@amd.com>
 <20230727200527.4080c595@g14>
 <CAHk-=whqT0PxBazwfjWwoHQQFzZt50tV6Jfgq3iYceKMJtyuUg@mail.gmail.com>
 <CUGAV1Y993FB.1O2Q691015Z2C@seitikki>
 <CAHk-=whphk8Jp=NYmnm7Qv+vZ6ScYCz+rV8a2G1nD-AQY3z+mQ@mail.gmail.com>
 <105b9d13-cedd-7d3c-1f29-2c65199f1de7@amd.com>
 <CAHk-=why64j-K4e1VxKwx7o6FiGjcXEnu1Pz+1QnNTBCv9AGyA@mail.gmail.com>
 <fd3cc87a-97ec-00ea-e480-f6597664c13a@amd.com>
 <CAHk-=whPvSJH=H9eHZbJQ+sxC-AVDvrgJ+M14fD3K5A+5s=zVA@mail.gmail.com>
 <CAHmME9r9DC+EMfxQy+HQAkf4AkAQnXNeBmOfCWnBUyiFQh2t6Q@mail.gmail.com>
 <61b363bc-286a-535c-27da-0f52673768ad@amd.com>
 <CUHFOCLWR4VO.OLBGKIXELSIN@suppilovahvero>
In-Reply-To: <CUHFOCLWR4VO.OLBGKIXELSIN@suppilovahvero>
Precedence: bulk

On Tue Aug 1, 2023 at 9:52 PM EEST, Jarkko Sakkinen wrote:
> On Tue Aug 1, 2023 at 6:04 AM EEST, Mario Limonciello wrote:
> > On 7/31/23 18:40, Jason A. Donenfeld wrote:
> > > Hi all,
> > >=20
> > > I've been tracking this issue with Mario on various threads and
> > > bugzilla for a while now. My suggestion over at bugzilla was to just
> > > disable all current AMD fTPMs by bumping the check for a major versio=
n
> > > number, so that the hardware people can reenable it i it's ever fixed=
,
> > > but only if this is something that the hardware people would actually
> > > respect. As I understand it, Mario was going to check into it and see=
.
> > > Failing that, yea, just disabling hwrng on fTPM seems like a fine
> > > enough thing to do.
> > >=20
> > > The reason I'm not too concerned about that is twofold:
> > > - Systems with fTPM all have RDRAND anyway, so there's no entropy pro=
blem.
> > > - fTPM *probably* uses the same random source as RDRAND -- the
> > > TRNG_OUT MMIO register -- so it's not really doing much more than wha=
t
> > > we already have available.
> >
> > Yeah I have conversations ongoing about this topic, but also I conclude=
d
> > your suspicion is correct.  They both get their values from the=20
> > integrated CCP HW IP.
> >
> > >=20
> > > So this all seems fine. And Jarkko's patch seems more or less the
> > > straight forward way of disabling it. But with that said, in order of
> > > priority, maybe we should first try these:
> > >=20
> > > 1) Adjust the version check to a major-place fTPM version that AMD's
> > > hardware team pinky swears will have this bug fixed. (Though, I can
> > > already imagine somebody on the list shouting, "we don't trust
> > > hardware teams to do anything with unreleased stuff!", which could be
> > > valid.)
> >
> > I find it very likely the actual root cause is similar to what Linus=20
> > suggested.  If that's the case I don't think the bug can be fixed
> > by just an fTPM fix but would rather require a BIOS fix.
> >
> > This to me strengthens the argument to either not register fTPM as RNG=
=20
> > in the first place or just use TPM for boot time entropy.
> >
> > > 2) Remove the version check, but add some other query to detect AMD
> > > fTPM vs realTPM, and ban fTPM.
> >
> > AMD doesn't make dTPMs, only fTPMs.  It's tempting to try to use=20
> > TPM2_PT_VENDOR_TPM_TYPE, but this actually is a vendor specific value.
> >
> > I don't see a reliable way in the spec to do this.
> >
> > > - Remove the version check, and just check for AMD; this is Jarrko's =
patch.
> >
> > I have a counter-proposal to Jarkko's patch attached.  This has two=20
> > notable changes:
> >
> > 1) It only disables RNG generation in the case of having RDRAND or RDSE=
ED.
> > 2) It also matches Intel PTT.
> >
> > I still do also think Linus' idea of TPMs only providing boot time=20
> > entropy is worth weighing out.
>
> You should add something like TPM_CHIP_HWRNG_DISABLED instead and
> set this in tpm_crb before calling tpm_chip_register().
>
> Nothing else concerning AMD hardware should be done in tpm-chip.c.
> It should only check TPM_CHIP_HWRNG_DISABLED in the beginning of
> tpm_add_hwrng().

In English: I think adding the function to tpm-chip.c was a really
bad idea in the first place, so let's revert that decisions and do
this correctly in tpm_crb.c.

BR, Jarkko