| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <543c2e9b-6d82-7ee0-9a1f-e3176aba16e2@amd.com>
Date: Thu, 9 Feb 2023 09:41:48 -0600
From: "Limonciello, Mario" <mario.limonciello@....com>
To: "Jason A. Donenfeld" <Jason@...c4.com>,
regressions@...ts.linux.dev, linux-integrity@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Jarkko Sakkinen <jarkko@...nel.org>,
Thorsten Leemhuis <regressions@...mhuis.info>,
James Bottomley <James.Bottomley@...senpartnership.com>
Subject: Re: [PATCH RFC] tpm: disable hwrng for known-defective AMD RNGs
On 2/9/2023 09:31, Jason A. Donenfeld wrote:
> Do not register a hwrng for certain AMD TPMs that are running an old
> known-buggy revision. Do this by probing the TPM2_PT_MANUFACTURER,
> TPM2_PT_FIRMWARE_VERSION_1, and TPM2_PT_FIRMWARE_VERSION_2 properties,
> and failing when an "AMD"-manufactured TPM2 chip is below a threshold.
>
> BROKEN BROKEN BROKEN - I just made the version numbers up and haven't
> tested this because I don't actually have hardware for it. I'm posting
> this so that Mario can take over its development and submit a v2 himself
> once he has confirmed the versioning info from inside AMD.
Thanks, happy to do that. As I'll take over some comments are inline
mostly for my future self when this is resubmitted.
Perhaps one of the people who can reproduce this right now can confirm
this works? The version number string should at least trigger one of
the reported systems to avoid doing this.
>
> Suggested-by: Mario Limonciello <mario.limonciello@....com>
> Cc: Jarkko Sakkinen <jarkko@...nel.org>
> Cc: Thorsten Leemhuis <regressions@...mhuis.info>
> Cc: James Bottomley <James.Bottomley@...senpartnership.com>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
At least as a note for myself this needs:
1. 'Link' tag to the public advisory for this fTPM issue
2. A 'Fixes' tag to the commit that enabled fTPM as a hwrng source by
default and exposed the issue.
3. A 'Link' tag to the bugzilla.
> ---
> drivers/char/tpm/tpm-chip.c | 34 ++++++++++++++++-
> drivers/char/tpm/tpm.h | 73 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 741d8f3e8fb3..e0f8134d31a3 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -512,6 +512,37 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> return 0;
> }
>
> +static bool tpm_is_rng_defective(struct tpm_chip *chip)
> +{
> + int ret;
> + u32 val1, val2;
> + u64 version;
> +
> + /* No known-broken TPM1 chips. */
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + return false;
> +
> + /* Only known-broken are AMD. */
> + ret = tpm2_get_tpm_pt(chip, TPM2_PT_MANUFACTURER, &val1, NULL);
> + if (ret < 0 || val1 != 0x414D4400U /* AMD */)
> + return false;
> +
> + /* Grab and concat the version values. */
> + ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_1, &val1, NULL);
> + if (ret < 0)
> + return false;
> + ret = tpm2_get_tpm_pt(chip, TPM2_PT_FIRMWARE_VERSION_2, &val2, NULL);
> + if (ret < 0)
> + return false;
> + version = ((u64)val1 << 32) | val2;
> +
> + /* Versions below 3.4e.2.9 are broken. */
> + if (version < 0x0003004E0002009ULL)
> + return true;
We probably want to also do a 'dev_info_once' or 'dev_warn_once' here to
let people know that they're not getting a fTPM RNG so it's not a surprise.
> +
> + return false;
> +}
> +
> static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> {
> struct tpm_chip *chip = container_of(rng, struct tpm_chip, hwrng);
> @@ -521,7 +552,8 @@ static int tpm_hwrng_read(struct hwrng *rng, void *data, size_t max, bool wait)
>
> static int tpm_add_hwrng(struct tpm_chip *chip)
> {
> - if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip))
> + if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
> + tpm_is_rng_defective(chip))
> return 0;
>
> snprintf(chip->hwrng_name, sizeof(chip->hwrng_name),
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 24ee4e1cc452..830014a26609 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -150,6 +150,79 @@ enum tpm_sub_capabilities {
> TPM_CAP_PROP_TIS_DURATION = 0x120,
> };
>
> +enum tpm2_pt_props {
> + TPM2_PT_NONE = 0x00000000,
> + TPM2_PT_GROUP = 0x00000100,
> + TPM2_PT_FIXED = TPM2_PT_GROUP * 1,
> + TPM2_PT_FAMILY_INDICATOR = TPM2_PT_FIXED + 0,
> + TPM2_PT_LEVEL = TPM2_PT_FIXED + 1,
> + TPM2_PT_REVISION = TPM2_PT_FIXED + 2,
> + TPM2_PT_DAY_OF_YEAR = TPM2_PT_FIXED + 3,
> + TPM2_PT_YEAR = TPM2_PT_FIXED + 4,
> + TPM2_PT_MANUFACTURER = TPM2_PT_FIXED + 5,
> + TPM2_PT_VENDOR_STRING_1 = TPM2_PT_FIXED + 6,
> + TPM2_PT_VENDOR_STRING_2 = TPM2_PT_FIXED + 7,
> + TPM2_PT_VENDOR_STRING_3 = TPM2_PT_FIXED + 8,
> + TPM2_PT_VENDOR_STRING_4 = TPM2_PT_FIXED + 9,
> + TPM2_PT_VENDOR_TPM_TYPE = TPM2_PT_FIXED + 10,
> + TPM2_PT_FIRMWARE_VERSION_1 = TPM2_PT_FIXED + 11,
> + TPM2_PT_FIRMWARE_VERSION_2 = TPM2_PT_FIXED + 12,
> + TPM2_PT_INPUT_BUFFER = TPM2_PT_FIXED + 13,
> + TPM2_PT_HR_TRANSIENT_MIN = TPM2_PT_FIXED + 14,
> + TPM2_PT_HR_PERSISTENT_MIN = TPM2_PT_FIXED + 15,
> + TPM2_PT_HR_LOADED_MIN = TPM2_PT_FIXED + 16,
> + TPM2_PT_ACTIVE_SESSIONS_MAX = TPM2_PT_FIXED + 17,
> + TPM2_PT_PCR_COUNT = TPM2_PT_FIXED + 18,
> + TPM2_PT_PCR_SELECT_MIN = TPM2_PT_FIXED + 19,
> + TPM2_PT_CONTEXT_GAP_MAX = TPM2_PT_FIXED + 20,
> + TPM2_PT_NV_COUNTERS_MAX = TPM2_PT_FIXED + 22,
> + TPM2_PT_NV_INDEX_MAX = TPM2_PT_FIXED + 23,
> + TPM2_PT_MEMORY = TPM2_PT_FIXED + 24,
> + TPM2_PT_CLOCK_UPDATE = TPM2_PT_FIXED + 25,
> + TPM2_PT_CONTEXT_HASH = TPM2_PT_FIXED + 26,
> + TPM2_PT_CONTEXT_SYM = TPM2_PT_FIXED + 27,
> + TPM2_PT_CONTEXT_SYM_SIZE = TPM2_PT_FIXED + 28,
> + TPM2_PT_ORDERLY_COUNT = TPM2_PT_FIXED + 29,
> + TPM2_PT_MAX_COMMAND_SIZE = TPM2_PT_FIXED + 30,
> + TPM2_PT_MAX_RESPONSE_SIZE = TPM2_PT_FIXED + 31,
> + TPM2_PT_MAX_DIGEST = TPM2_PT_FIXED + 32,
> + TPM2_PT_MAX_OBJECT_CONTEXT = TPM2_PT_FIXED + 33,
> + TPM2_PT_MAX_SESSION_CONTEXT = TPM2_PT_FIXED + 34,
> + TPM2_PT_PS_FAMILY_INDICATOR = TPM2_PT_FIXED + 35,
> + TPM2_PT_PS_LEVEL = TPM2_PT_FIXED + 36,
> + TPM2_PT_PS_REVISION = TPM2_PT_FIXED + 37,
> + TPM2_PT_PS_DAY_OF_YEAR = TPM2_PT_FIXED + 38,
> + TPM2_PT_PS_YEAR = TPM2_PT_FIXED + 39,
> + TPM2_PT_SPLIT_MAX = TPM2_PT_FIXED + 40,
> + TPM2_PT_TOTAL_COMMANDS = TPM2_PT_FIXED + 41,
> + TPM2_PT_LIBRARY_COMMANDS = TPM2_PT_FIXED + 42,
> + TPM2_PT_VENDOR_COMMANDS = TPM2_PT_FIXED + 43,
> + TPM2_PT_NV_BUFFER_MAX = TPM2_PT_FIXED + 44,
> + TPM2_PT_MODES = TPM2_PT_FIXED + 45,
> + TPM2_PT_MAX_CAP_BUFFER = TPM2_PT_FIXED + 46,
> + TPM2_PT_VAR = TPM2_PT_GROUP * 2,
> + TPM2_PT_PERMANENT = TPM2_PT_VAR + 0,
> + TPM2_PT_STARTUP_CLEAR = TPM2_PT_VAR + 1,
> + TPM2_PT_HR_NV_INDEX = TPM2_PT_VAR + 2,
> + TPM2_PT_HR_LOADED = TPM2_PT_VAR + 3,
> + TPM2_PT_HR_LOADED_AVAIL = TPM2_PT_VAR + 4,
> + TPM2_PT_HR_ACTIVE = TPM2_PT_VAR + 5,
> + TPM2_PT_HR_ACTIVE_AVAIL = TPM2_PT_VAR + 6,
> + TPM2_PT_HR_TRANSIENT_AVAIL = TPM2_PT_VAR + 7,
> + TPM2_PT_HR_PERSISTENT = TPM2_PT_VAR + 8,
> + TPM2_PT_HR_PERSISTENT_AVAIL = TPM2_PT_VAR + 9,
> + TPM2_PT_NV_COUNTERS = TPM2_PT_VAR + 10,
> + TPM2_PT_NV_COUNTERS_AVAIL = TPM2_PT_VAR + 11,
> + TPM2_PT_ALGORITHM_SET = TPM2_PT_VAR + 12,
> + TPM2_PT_LOADED_CURVES = TPM2_PT_VAR + 13,
> + TPM2_PT_LOCKOUT_COUNTER = TPM2_PT_VAR + 14,
> + TPM2_PT_MAX_AUTH_FAIL = TPM2_PT_VAR + 15,
> + TPM2_PT_LOCKOUT_INTERVAL = TPM2_PT_VAR + 16,
> + TPM2_PT_LOCKOUT_RECOVERY = TPM2_PT_VAR + 17,
> + TPM2_PT_NV_WRITE_RECOVERY = TPM2_PT_VAR + 18,
> + TPM2_PT_AUDIT_COUNTER_0 = TPM2_PT_VAR + 19,
> + TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
> +};
>
I guess these properties should be split out to a separate patch in a
series.
> /* 128 bytes is an arbitrary cap. This could be as large as TPM_BUFSIZE - 18
> * bytes, but 128 is still a relatively large number of random bytes and
Powered by blists - more mailing lists