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,
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 version 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.
The reason I'm not too concerned about that is twofold:
- Systems with fTPM all have RDRAND anyway, so there's no entropy problem.
- fTPM *probably* uses the same random source as RDRAND -- the
TRNG_OUT MMIO register -- so it's not really doing much more than what we already have available.
Yeah I have conversations ongoing about this topic, but also I concluded your suspicion is correct. They both get their values from the integrated CCP HW IP.
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:
- 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 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 in the first place or just use TPM for boot time entropy.
- 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 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 notable changes:
- It only disables RNG generation in the case of having RDRAND or RDSEED.
- It also matches Intel PTT.
I still do also think Linus' idea of TPMs only providing boot time 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