On Fri, 2019-10-04 at 13:11 -0700, Jerry Snitselaar wrote:
On Fri Oct 04 19, Jerry Snitselaar wrote:
On Fri Oct 04 19, James Bottomley wrote:
On Fri, 2019-10-04 at 11:33 -0700, Jerry Snitselaar wrote:
On Fri Oct 04 19, James Bottomley wrote:
On Fri, 2019-10-04 at 21:22 +0300, Jarkko Sakkinen wrote:
On Thu, Oct 03, 2019 at 04:59:37PM -0700, James Bottomley wrote: > I think the principle of using multiple RNG sources for > strong keys is a sound one, so could I propose a > compromise: We have a tpm subsystem random number > generator that, when asked for <n> random bytes first > extracts <n> bytes from the TPM RNG and places it into > the kernel entropy pool and then asks for <n> random > bytes from the kernel RNG? That way, it will always have > the entropy to satisfy the request and in the worst case, > where the kernel has picked up no other entropy sources > at all it will be equivalent to what we have now (single > entropy source) but usually it will be a much better > mixed entropy source.
I think we should rely the existing architecture where TPM is contributing to the entropy pool as hwrng.
That doesn't seem to work: when I trace what happens I see us inject 32 bytes of entropy at boot time, but never again. I think the problem is the kernel entropy pool is push not pull and we have no triggering event in the TPM to get us to push. I suppose we could set a timer to do this or perhaps there is a pull hook and we haven't wired it up correctly?
James
Shouldn't hwrng_fillfn be pulling from it?
It should, but the problem seems to be it only polls the "current" hw rng ... it doesn't seem to have a concept that there may be more than one. What happens, according to a brief reading of the code, is when multiple are registered, it determines what the "best" one is and then only pulls from that. What I think it should be doing is filling from all of them using the entropy quality to adjust how many bits we get.
James
Most of them don't even set quality, including the tpm, so they end up at the end of the list. For the ones that do I'm not sure how they determined the value. For example virtio-rng sets quality to 1000.
I should have added that I like that idea though.
OK, so I looked at how others implement this. It turns out there's only one other: the atheros rng and this is what it does:
drivers/net/wireless/ath/ath9k/rng.c
so rather than redoing the entirety of the TPM rng like this, I thought it's easier to keep what we have (direct hwrng device) and plug our tpm_get_random() function into the kernel rng like the below.
James
---
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 3d6d394a8661..0794521c0784 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -536,7 +536,7 @@ 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);
- return tpm_get_random(chip, data, max); + return __tpm_get_random(chip, data, max); }
static int tpm_add_hwrng(struct tpm_chip *chip) diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index d7a3888ad80f..14631cba000c 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -24,6 +24,7 @@ #include <linux/mutex.h> #include <linux/spinlock.h> #include <linux/freezer.h> +#include <linux/random.h> #include <linux/tpm_eventlog.h>
#include "tpm.h" @@ -424,15 +425,11 @@ int tpm_pm_resume(struct device *dev) } EXPORT_SYMBOL_GPL(tpm_pm_resume);
-/** - * tpm_get_random() - get random bytes from the TPM's RNG - * @chip: a &struct tpm_chip instance, %NULL for the default chip - * @out: destination buffer for the random bytes - * @max: the max number of bytes to write to @out - * - * Return: number of random bytes read or a negative error value. +/* + * Internal interface for tpm_get_random(): gets the random string + * directly from the TPM without mixing into the linux rng. */ -int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max) +int __tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max) { int rc;
@@ -451,6 +448,38 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max) tpm_put_ops(chip); return rc; } + +/** + * tpm_get_random() - get random bytes influenced by the TPM's RNG + * @chip: a &struct tpm_chip instance, %NULL for the default chip + * @out: destination buffer for the random bytes + * @max: the max number of bytes to write to @out + * + * Uses the TPM as a source of input to the kernel random number + * generator and then takes @max bytes directly from the kernel. In + * the worst (no other entropy) case, this will return the pure TPM + * random number, but if the kernel RNG has any entropy at all it will + * return a mixed entropy output which doesn't rely on a single + * source. + * + * Return: number of random bytes read or a negative error value. + */ +int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max) +{ + int rc; + + rc = __tpm_get_random(chip, out, max); + if (rc <= 0) + return rc; + /* + * assume the TPM produces pure randomness, so the amount of + * entropy is the number of bits returned + */ + add_hwgenerator_randomness(out, rc, rc * 8); + get_random_bytes(out, rc); + + return rc; +} EXPORT_SYMBOL_GPL(tpm_get_random);
/** diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index a7fea3e0ca86..25f6b347b194 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -398,6 +398,7 @@ int tpm1_get_pcr_allocation(struct tpm_chip *chip); unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal); int tpm_pm_suspend(struct device *dev); int tpm_pm_resume(struct device *dev); +int __tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
static inline void tpm_msleep(unsigned int delay_msec) {