Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Jerry Snitselaar <jsnitsel@redhat.com>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	David Safford <david.safford@ge.com>,
	linux-integrity@vger.kernel.org, stable@vger.kernel.org,
	David Howells <dhowells@redhat.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	"open list:ASYMMETRIC KEYS" <keyrings@vger.kernel.org>,
	"open list:CRYPTO API" <linux-crypto@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes()
Date: Fri, 04 Oct 2019 15:11:08 -0700
Message-ID: <1570227068.17537.4.camel@HansenPartnership.com> (raw)
In-Reply-To: <20191004201134.nuesk6hxtxajnxh2@cantor>

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)
 {

  reply index

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-26 17:16 Jarkko Sakkinen
2019-09-28 18:05 ` Jerry Snitselaar
2019-10-01 20:54   ` Jarkko Sakkinen
2019-10-02 14:00 ` Mimi Zohar
2019-10-03 11:41   ` Jarkko Sakkinen
2019-10-03 11:43     ` Jarkko Sakkinen
2019-10-03 13:02     ` Mimi Zohar
2019-10-03 17:58       ` Jarkko Sakkinen
2019-10-03 18:53         ` Mimi Zohar
2019-10-03 21:51           ` Jarkko Sakkinen
2019-10-03 21:57             ` Jarkko Sakkinen
2019-10-03 22:08               ` Mimi Zohar
2019-10-03 23:59                 ` James Bottomley
2019-10-04 18:22                   ` Jarkko Sakkinen
2019-10-04 18:24                     ` James Bottomley
2019-10-04 18:33                       ` Jerry Snitselaar
2019-10-04 18:42                         ` James Bottomley
2019-10-04 20:07                           ` Jerry Snitselaar
2019-10-04 20:11                             ` Jerry Snitselaar
2019-10-04 22:11                               ` James Bottomley [this message]
2019-10-06  0:38                                 ` Mimi Zohar
2019-10-06 23:52                                   ` Jarkko Sakkinen
2019-10-07 18:08                                     ` Mimi Zohar
2019-10-04 18:20                 ` Jarkko Sakkinen
2019-10-03 22:10               ` Jarkko Sakkinen
2019-10-04 13:26           ` Safford, David (GE Global Research, US)
2019-10-04 18:27             ` Jarkko Sakkinen
2019-10-04 18:30               ` Jarkko Sakkinen
2019-10-04 19:56               ` Safford, David (GE Global Research, US)
2019-10-07  0:05                 ` Jarkko Sakkinen
2019-10-07 22:13                   ` Ken Goldman
2019-10-08 23:49                     ` Jarkko Sakkinen
2019-10-08 23:53                       ` Jarkko Sakkinen
2019-10-09  7:10                         ` Pascal Van Leeuwen
2019-10-09  7:33                         ` Jarkko Sakkinen
2019-10-09  7:41                           ` Jarkko Sakkinen
2019-10-09  8:09                             ` Pascal Van Leeuwen
2019-10-14 19:11                               ` Jarkko Sakkinen
2019-10-09  8:02                           ` Pascal Van Leeuwen
2019-10-09 12:11                         ` Safford, David (GE Global Research, US)
2019-10-14 19:00                           ` Jarkko Sakkinen
2019-10-14 19:29                             ` Jarkko Sakkinen
2019-10-14 19:29                             ` James Bottomley
2019-10-16 11:00                               ` Jarkko Sakkinen
2019-10-16 12:34                                 ` James Bottomley
2019-10-16 16:25                                   ` Jarkko Sakkinen
2019-10-16 19:10                                     ` James Bottomley
2019-10-17 12:52                                       ` Sumit Garg
2019-10-17 12:58                                         ` James Bottomley
2019-10-17 18:04                                       ` Jarkko Sakkinen
2019-10-21 11:39                                         ` Jarkko Sakkinen
2019-10-29  8:42                                           ` Jarkko Sakkinen
2019-10-29 14:58                                             ` James Bottomley
2019-10-31 21:03                                               ` Jarkko Sakkinen
2019-10-18  7:32                                   ` Janne Karhunen
2019-10-03 18:02       ` Jarkko Sakkinen
2019-10-03 18:15         ` Jarkko Sakkinen
2019-10-07 10:33     ` Janne Karhunen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1570227068.17537.4.camel@HansenPartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=davem@davemloft.net \
    --cc=david.safford@ge.com \
    --cc=dhowells@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jsnitsel@redhat.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git