| Message ID | 20190926171601.30404-1-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | KEYS: asym_tpm: Switch to get_random_bytes() | expand |
On Thu Sep 26 19, Jarkko Sakkinen wrote: >Only the kernel random pool should be used for generating random numbers. >TPM contributes to that pool among the other sources of entropy. In here it >is not, agreed, absolutely critical because TPM is what is trusted anyway >but in order to remove tpm_get_random() we need to first remove all the >call sites. > >Cc: stable@vger.kernel.org >Fixes: 0c36264aa1d5 ("KEYS: asym_tpm: Add loadkey2 and flushspecific [ver #2]") >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >--- > crypto/asymmetric_keys/asym_tpm.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > >diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c >index 76d2ce3a1b5b..c14b8d186e93 100644 >--- a/crypto/asymmetric_keys/asym_tpm.c >+++ b/crypto/asymmetric_keys/asym_tpm.c >@@ -6,6 +6,7 @@ > #include <linux/kernel.h> > #include <linux/seq_file.h> > #include <linux/scatterlist.h> >+#include <linux/random.h> > #include <linux/tpm.h> > #include <linux/tpm_command.h> > #include <crypto/akcipher.h> >@@ -54,11 +55,7 @@ static int tpm_loadkey2(struct tpm_buf *tb, > } > > /* generate odd nonce */ >- ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE); >- if (ret < 0) { >- pr_info("tpm_get_random failed (%d)\n", ret); >- return ret; >- } >+ get_random_bytes(nonceodd, TPM_NONCE_SIZE); > > /* calculate authorization HMAC value */ > ret = TSS_authhmac(authdata, keyauth, SHA1_DIGEST_SIZE, enonce, >-- >2.20.1 > Should tpm_unbind and tpm_sign in asym_tpm.c be switched as well then?
On Sat, Sep 28, 2019 at 11:05:59AM -0700, Jerry Snitselaar wrote: > On Thu Sep 26 19, Jarkko Sakkinen wrote: > > Only the kernel random pool should be used for generating random numbers. > > TPM contributes to that pool among the other sources of entropy. In here it > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > but in order to remove tpm_get_random() we need to first remove all the > > call sites. > > > > Cc: stable@vger.kernel.org > > Fixes: 0c36264aa1d5 ("KEYS: asym_tpm: Add loadkey2 and flushspecific [ver #2]") > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > --- > > crypto/asymmetric_keys/asym_tpm.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c > > index 76d2ce3a1b5b..c14b8d186e93 100644 > > --- a/crypto/asymmetric_keys/asym_tpm.c > > +++ b/crypto/asymmetric_keys/asym_tpm.c > > @@ -6,6 +6,7 @@ > > #include <linux/kernel.h> > > #include <linux/seq_file.h> > > #include <linux/scatterlist.h> > > +#include <linux/random.h> > > #include <linux/tpm.h> > > #include <linux/tpm_command.h> > > #include <crypto/akcipher.h> > > @@ -54,11 +55,7 @@ static int tpm_loadkey2(struct tpm_buf *tb, > > } > > > > /* generate odd nonce */ > > - ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE); > > - if (ret < 0) { > > - pr_info("tpm_get_random failed (%d)\n", ret); > > - return ret; > > - } > > + get_random_bytes(nonceodd, TPM_NONCE_SIZE); > > > > /* calculate authorization HMAC value */ > > ret = TSS_authhmac(authdata, keyauth, SHA1_DIGEST_SIZE, enonce, > > -- > > 2.20.1 > > > > Should tpm_unbind and tpm_sign in asym_tpm.c be switched as well then? Without doubt. Thanks. I'll send an update soon. /Jarkko
On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > Only the kernel random pool should be used for generating random numbers. > TPM contributes to that pool among the other sources of entropy. In here it > is not, agreed, absolutely critical because TPM is what is trusted anyway > but in order to remove tpm_get_random() we need to first remove all the > call sites. At what point during boot is the kernel random pool available? Does this imply that you're planning on changing trusted keys as well? Mimi
On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > Only the kernel random pool should be used for generating random numbers. > > TPM contributes to that pool among the other sources of entropy. In here it > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > but in order to remove tpm_get_random() we need to first remove all the > > call sites. > > At what point during boot is the kernel random pool available? Does > this imply that you're planning on changing trusted keys as well? Well trusted keys *must* be changed to use it. It is not a choice because using a proprietary random number generator instead of defacto one in the kernel can be categorized as a *regression*. Also, TEE trusted keys cannot use the TPM option. If it was not initialized early enough we would need fix that too. I don't think there should be a problem anyway since encrypted keys is already using get_random_bytes(). /Jarkko
On Thu, Oct 03, 2019 at 02:41:19PM +0300, Jarkko Sakkinen wrote: > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > Only the kernel random pool should be used for generating random numbers. > > > TPM contributes to that pool among the other sources of entropy. In here it > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > but in order to remove tpm_get_random() we need to first remove all the > > > call sites. > > > > At what point during boot is the kernel random pool available? Does > > this imply that you're planning on changing trusted keys as well? > > Well trusted keys *must* be changed to use it. It is not a choice > because using a proprietary random number generator instead of defacto > one in the kernel can be categorized as a *regression*. > > Also, TEE trusted keys cannot use the TPM option. > > If it was not initialized early enough we would need fix that too. > > I don't think there should be a problem anyway since encrypted keys is > already using get_random_bytes(). Looking at asym_tpm.c the implementation copies all the anti-patterns from trusted keys, which is really unfortunate. I don't know how that has passed through all the filters. /Jarkko
On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > Only the kernel random pool should be used for generating random numbers. > > > TPM contributes to that pool among the other sources of entropy. In here it > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > but in order to remove tpm_get_random() we need to first remove all the > > > call sites. > > > > At what point during boot is the kernel random pool available? Does > > this imply that you're planning on changing trusted keys as well? > > Well trusted keys *must* be changed to use it. It is not a choice > because using a proprietary random number generator instead of defacto > one in the kernel can be categorized as a *regression*. I really don't see how using the TPM random number for TPM trusted keys would be considered a regression. That by definition is a trusted key. If anything, changing what is currently being done would be the regression. > Also, TEE trusted keys cannot use the TPM option. That isn't a valid justification for changing the original definition of trusted keys. Just as the kernel supports different methods of implementing the same function on different architectures, trusted keys will need to support different methods of generating a random number. > > If it was not initialized early enough we would need fix that too. Shouldn't this be determined and fixed, before making any changes? > > I don't think there should be a problem anyway since encrypted keys is > already using get_random_bytes(). Encrypted keys has no bearing on trusted keys. Mimi
On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > Only the kernel random pool should be used for generating random numbers. > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > call sites. > > > > > > At what point during boot is the kernel random pool available? Does > > > this imply that you're planning on changing trusted keys as well? > > > > Well trusted keys *must* be changed to use it. It is not a choice > > because using a proprietary random number generator instead of defacto > > one in the kernel can be categorized as a *regression*. > > I really don't see how using the TPM random number for TPM trusted > keys would be considered a regression. That by definition is a > trusted key. If anything, changing what is currently being done would > be the regression. It is really not a TPM trusted key. It trusted key that gets sealed with the TPM. The key itself is used in clear by kernel. The random number generator exists in the kernel to for a reason. It is without doubt a regression. /Jarkko
On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > That isn't a valid justification for changing the original definition > of trusted keys. Just as the kernel supports different methods of > implementing the same function on different architectures, trusted > keys will need to support different methods of generating a random > number. This is completely incorrect deduction. The random number generator inside the kernel is there to gather entropy from different sources. You would exploit trusted keys to potential weaknesses of a single entropy source by doing that. Of course in TEE platform, TEE can be one of the entropy sources but there is no reason to weaken the security by using it as the only sources. /Jarkko
On Thu, Oct 03, 2019 at 09:02:01PM +0300, Jarkko Sakkinen wrote: > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > That isn't a valid justification for changing the original definition > > of trusted keys. Just as the kernel supports different methods of > > implementing the same function on different architectures, trusted > > keys will need to support different methods of generating a random > > number. > > This is completely incorrect deduction. The random number generator > inside the kernel is there to gather entropy from different sources. > You would exploit trusted keys to potential weaknesses of a single > entropy source by doing that. > > Of course in TEE platform, TEE can be one of the entropy sources but > there is no reason to weaken the security by using it as the only > sources. I.e. where you go wrong is that you are inter mixing requirements for the payload and for sealing. They are disjoint assets. The rules for the payload should not be dependent on how you seal your trusted key. /Jarkko
[Cc'ing David Safford] On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > Only the kernel random pool should be used for generating random numbers. > > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > > call sites. > > > > > > > > At what point during boot is the kernel random pool available? Does > > > > this imply that you're planning on changing trusted keys as well? > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > because using a proprietary random number generator instead of defacto > > > one in the kernel can be categorized as a *regression*. > > > > I really don't see how using the TPM random number for TPM trusted > > keys would be considered a regression. That by definition is a > > trusted key. If anything, changing what is currently being done would > > be the regression. > > It is really not a TPM trusted key. It trusted key that gets sealed with > the TPM. The key itself is used in clear by kernel. The random number > generator exists in the kernel to for a reason. > > It is without doubt a regression. You're misusing the term "regression" here. A regression is something that previously worked and has stopped working. In this case, trusted keys has always been based on the TPM random number generator. Before changing this, there needs to be some guarantees that the kernel random number generator has a pool of random numbers early, on all systems including embedded devices, not just servers. Mimi
On Thu, Oct 03, 2019 at 02:53:47PM -0400, Mimi Zohar wrote: > [Cc'ing David Safford] > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > > Only the kernel random pool should be used for generating random numbers. > > > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > > > call sites. > > > > > > > > > > At what point during boot is the kernel random pool available? Does > > > > > this imply that you're planning on changing trusted keys as well? > > > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > > because using a proprietary random number generator instead of defacto > > > > one in the kernel can be categorized as a *regression*. > > > > > > I really don't see how using the TPM random number for TPM trusted > > > keys would be considered a regression. That by definition is a > > > trusted key. If anything, changing what is currently being done would > > > be the regression. > > > > It is really not a TPM trusted key. It trusted key that gets sealed with > > the TPM. The key itself is used in clear by kernel. The random number > > generator exists in the kernel to for a reason. > > > > It is without doubt a regression. > > You're misusing the term "regression" here. A regression is something > that previously worked and has stopped working. In this case, trusted > keys has always been based on the TPM random number generator. Before > changing this, there needs to be some guarantees that the kernel > random number generator has a pool of random numbers early, on all > systems including embedded devices, not just servers. I'm not using the term regression incorrectly here. Wrong function was used to generate random numbers for the payload here. It is an obvious bug. /Jarkko
On Fri, Oct 04, 2019 at 12:51:25AM +0300, Jarkko Sakkinen wrote: > On Thu, Oct 03, 2019 at 02:53:47PM -0400, Mimi Zohar wrote: > > [Cc'ing David Safford] > > > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > > > Only the kernel random pool should be used for generating random numbers. > > > > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > > > > call sites. > > > > > > > > > > > > At what point during boot is the kernel random pool available? Does > > > > > > this imply that you're planning on changing trusted keys as well? > > > > > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > > > because using a proprietary random number generator instead of defacto > > > > > one in the kernel can be categorized as a *regression*. > > > > > > > > I really don't see how using the TPM random number for TPM trusted > > > > keys would be considered a regression. That by definition is a > > > > trusted key. If anything, changing what is currently being done would > > > > be the regression. > > > > > > It is really not a TPM trusted key. It trusted key that gets sealed with > > > the TPM. The key itself is used in clear by kernel. The random number > > > generator exists in the kernel to for a reason. > > > > > > It is without doubt a regression. > > > > You're misusing the term "regression" here. A regression is something > > that previously worked and has stopped working. In this case, trusted > > keys has always been based on the TPM random number generator. Before > > changing this, there needs to be some guarantees that the kernel > > random number generator has a pool of random numbers early, on all > > systems including embedded devices, not just servers. > > I'm not using the term regression incorrectly here. Wrong function > was used to generate random numbers for the payload here. It is an > obvious bug. At the time when trusted keys was introduced I'd say that it was a wrong design decision and badly implemented code. But you are right in that as far that code is considered it would unfair to speak of a regression. asym-tpm.c on the other hand this is fresh new code. There has been *countless* of discussions over the years that random numbers should come from multiple sources of entropy. There is no other categorization than a bug for the tpm_get_random() there. /Jarkko
On Fri, 2019-10-04 at 00:57 +0300, Jarkko Sakkinen wrote: > On Fri, Oct 04, 2019 at 12:51:25AM +0300, Jarkko Sakkinen wrote: > > On Thu, Oct 03, 2019 at 02:53:47PM -0400, Mimi Zohar wrote: > > > [Cc'ing David Safford] > > > > > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > > > > Only the kernel random pool should be used for generating random numbers. > > > > > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > > > > > call sites. > > > > > > > > > > > > > > At what point during boot is the kernel random pool available? Does > > > > > > > this imply that you're planning on changing trusted keys as well? > > > > > > > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > > > > because using a proprietary random number generator instead of defacto > > > > > > one in the kernel can be categorized as a *regression*. > > > > > > > > > > I really don't see how using the TPM random number for TPM trusted > > > > > keys would be considered a regression. That by definition is a > > > > > trusted key. If anything, changing what is currently being done would > > > > > be the regression. > > > > > > > > It is really not a TPM trusted key. It trusted key that gets sealed with > > > > the TPM. The key itself is used in clear by kernel. The random number > > > > generator exists in the kernel to for a reason. > > > > > > > > It is without doubt a regression. > > > > > > You're misusing the term "regression" here. A regression is something > > > that previously worked and has stopped working. In this case, trusted > > > keys has always been based on the TPM random number generator. Before > > > changing this, there needs to be some guarantees that the kernel > > > random number generator has a pool of random numbers early, on all > > > systems including embedded devices, not just servers. > > > > I'm not using the term regression incorrectly here. Wrong function > > was used to generate random numbers for the payload here. It is an > > obvious bug. > > At the time when trusted keys was introduced I'd say that it was a wrong > design decision and badly implemented code. But you are right in that as > far that code is considered it would unfair to speak of a regression. > > asym-tpm.c on the other hand this is fresh new code. There has been > *countless* of discussions over the years that random numbers should > come from multiple sources of entropy. There is no other categorization > than a bug for the tpm_get_random() there. This week's LWN article on "5.4 Merge window, part 2" discusses "boot- time entropy". This article couldn't have been more perfectly timed. Mimi
On Fri, Oct 04, 2019 at 12:57:43AM +0300, Jarkko Sakkinen wrote: > On Fri, Oct 04, 2019 at 12:51:25AM +0300, Jarkko Sakkinen wrote: > > On Thu, Oct 03, 2019 at 02:53:47PM -0400, Mimi Zohar wrote: > > > [Cc'ing David Safford] > > > > > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > > > > Only the kernel random pool should be used for generating random numbers. > > > > > > > > TPM contributes to that pool among the other sources of entropy. In here it > > > > > > > > is not, agreed, absolutely critical because TPM is what is trusted anyway > > > > > > > > but in order to remove tpm_get_random() we need to first remove all the > > > > > > > > call sites. > > > > > > > > > > > > > > At what point during boot is the kernel random pool available? Does > > > > > > > this imply that you're planning on changing trusted keys as well? > > > > > > > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > > > > because using a proprietary random number generator instead of defacto > > > > > > one in the kernel can be categorized as a *regression*. > > > > > > > > > > I really don't see how using the TPM random number for TPM trusted > > > > > keys would be considered a regression. That by definition is a > > > > > trusted key. If anything, changing what is currently being done would > > > > > be the regression. > > > > > > > > It is really not a TPM trusted key. It trusted key that gets sealed with > > > > the TPM. The key itself is used in clear by kernel. The random number > > > > generator exists in the kernel to for a reason. > > > > > > > > It is without doubt a regression. > > > > > > You're misusing the term "regression" here. A regression is something > > > that previously worked and has stopped working. In this case, trusted > > > keys has always been based on the TPM random number generator. Before > > > changing this, there needs to be some guarantees that the kernel > > > random number generator has a pool of random numbers early, on all > > > systems including embedded devices, not just servers. > > > > I'm not using the term regression incorrectly here. Wrong function > > was used to generate random numbers for the payload here. It is an > > obvious bug. > > At the time when trusted keys was introduced I'd say that it was a wrong > design decision and badly implemented code. But you are right in that as > far that code is considered it would unfair to speak of a regression. > > asym-tpm.c on the other hand this is fresh new code. There has been > *countless* of discussions over the years that random numbers should > come from multiple sources of entropy. There is no other categorization > than a bug for the tpm_get_random() there. Saying that regression is something that "stopped working" is very blunt and naive definition of regression, and is not true. Any misbehaviour can be categorized as a regression. /Jarkko
On Thu, 2019-10-03 at 18:08 -0400, Mimi Zohar wrote: > On Fri, 2019-10-04 at 00:57 +0300, Jarkko Sakkinen wrote: > > On Fri, Oct 04, 2019 at 12:51:25AM +0300, Jarkko Sakkinen wrote: > > > On Thu, Oct 03, 2019 at 02:53:47PM -0400, Mimi Zohar wrote: > > > > [Cc'ing David Safford] > > > > > > > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > > > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar > > > > > > > wrote: > > > > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen > > > > > > > > wrote: > > > > > > > > > Only the kernel random pool should be used for > > > > > > > > > generating random numbers. > > > > > > > > > TPM contributes to that pool among the other sources > > > > > > > > > of entropy. In here it > > > > > > > > > is not, agreed, absolutely critical because TPM is > > > > > > > > > what is trusted anyway > > > > > > > > > but in order to remove tpm_get_random() we need to > > > > > > > > > first remove all the > > > > > > > > > call sites. > > > > > > > > > > > > > > > > At what point during boot is the kernel random pool > > > > > > > > available? Does this imply that you're planning on > > > > > > > > changing trusted keys as well? > > > > > > > > > > > > > > Well trusted keys *must* be changed to use it. It is not > > > > > > > a choice because using a proprietary random number > > > > > > > generator instead of defacto one in the kernel can be > > > > > > > categorized as a *regression*. > > > > > > > > > > > > I really don't see how using the TPM random number for TPM > > > > > > trusted keys would be considered a regression. That by > > > > > > definition is a trusted key. If anything, changing what is > > > > > > currently being done would be the regression. > > > > > > > > > > It is really not a TPM trusted key. It trusted key that gets > > > > > sealed with the TPM. The key itself is used in clear by > > > > > kernel. The random number generator exists in the kernel to > > > > > for a reason. > > > > > > > > > > It is without doubt a regression. > > > > > > > > You're misusing the term "regression" here. A regression is > > > > something that previously worked and has stopped working. In > > > > this case, trusted keys has always been based on the TPM random > > > > number generator. Before changing this, there needs to be some > > > > guarantees that the kernel random number generator has a pool > > > > of random numbers early, on all systems including embedded > > > > devices, not just servers. > > > > > > I'm not using the term regression incorrectly here. Wrong > > > function was used to generate random numbers for the payload > > > here. It is an obvious bug. > > > > At the time when trusted keys was introduced I'd say that it was a > > wrong design decision and badly implemented code. But you are right > > in that as far that code is considered it would unfair to speak of > > a regression. I think that's a bit unfair: when the code was written, a decade ago, the TPM was thought to be the state of the art in terms of random number generators. Of course, since then the kernel RNG has become way better and we've become more aware of nation state actors thinking that influencing the RNG is the best way to compromise strong crypto. > > asym-tpm.c on the other hand this is fresh new code. There has been > > *countless* of discussions over the years that random numbers > > should come from multiple sources of entropy. There is no other > > categorization than a bug for the tpm_get_random() there. > > This week's LWN article on "5.4 Merge window, part 2" discusses > "boot- time entropy". This article couldn't have been more perfectly > timed. 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. James
> From: Mimi Zohar <zohar@linux.ibm.com> > Sent: Thursday, October 3, 2019 2:54 PM > To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>; Safford, David (GE > Subject: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > [Cc'ing David Safford] > > On Thu, 2019-10-03 at 20:58 +0300, Jarkko Sakkinen wrote: > > On Thu, Oct 03, 2019 at 09:02:32AM -0400, Mimi Zohar wrote: > > > On Thu, 2019-10-03 at 14:41 +0300, Jarkko Sakkinen wrote: > > > > On Wed, Oct 02, 2019 at 10:00:19AM -0400, Mimi Zohar wrote: > > > > > On Thu, 2019-09-26 at 20:16 +0300, Jarkko Sakkinen wrote: > > > > > > Only the kernel random pool should be used for generating random > numbers. > > > > > > TPM contributes to that pool among the other sources of > > > > > > entropy. In here it is not, agreed, absolutely critical > > > > > > because TPM is what is trusted anyway but in order to remove > > > > > > tpm_get_random() we need to first remove all the call sites. > > > > > > > > > > At what point during boot is the kernel random pool available? > > > > > Does this imply that you're planning on changing trusted keys as well? > > > > > > > > Well trusted keys *must* be changed to use it. It is not a choice > > > > because using a proprietary random number generator instead of > > > > defacto one in the kernel can be categorized as a *regression*. > > > > > > I really don't see how using the TPM random number for TPM trusted > > > keys would be considered a regression. That by definition is a > > > trusted key. If anything, changing what is currently being done > > > would be the regression. > > > > It is really not a TPM trusted key. It trusted key that gets sealed > > with the TPM. The key itself is used in clear by kernel. The random > > number generator exists in the kernel to for a reason. > > > > It is without doubt a regression. > > You're misusing the term "regression" here. A regression is something that > previously worked and has stopped working. In this case, trusted keys has > always been based on the TPM random number generator. Before changing > this, there needs to be some guarantees that the kernel random number > generator has a pool of random numbers early, on all systems including > embedded devices, not just servers. > > Mimi As the original author of trusted keys, let me make a few comments. First, trusted keys were specifically implemented and *documented* to use the TPM to both generate and seal keys. Its kernel documentation specifically states this as a promise to user space. If you want to have a different key system that uses the random pool to generate the keys, fine, but don't change trusted keys, as that changes the existing promise to user space. There are many good reasons for wanting the keys to be based on the TPM generator. As the source for the kernel random number generator itself says, some systems lack good randomness at startup, and systems should preserve and reload the pool across shutdown and startup. There are use cases for trusted keys which need to generate keys before such scripts have run. Also, in some use cases, we need to show that trusted keys are FIPS compliant, which is possible with TPM generated keys. Second, the TPM is hardly a "proprietary random number generator". It is an open standard with multiple implementations, many of which are FIPS certified. Third, as Mimi states, using a TPM is not a "regression". It would be a regression to change trusted keys _not_ to use the TPM, because that is what trusted keys are documented to provide to user space. dave
On Thu, Oct 03, 2019 at 06:08:11PM -0400, Mimi Zohar wrote: > > At the time when trusted keys was introduced I'd say that it was a wrong > > design decision and badly implemented code. But you are right in that as > > far that code is considered it would unfair to speak of a regression. > > > > asym-tpm.c on the other hand this is fresh new code. There has been > > *countless* of discussions over the years that random numbers should > > come from multiple sources of entropy. There is no other categorization > > than a bug for the tpm_get_random() there. > > This week's LWN article on "5.4 Merge window, part 2" discusses "boot- > time entropy". This article couldn't have been more perfectly timed. Do not see any obvious relation to this dicussion. Are you saying that you should not use the defacto kernel API's but instead bake your own hacks because even defacto stuff bumps into issues from time to time? And BTW, at the time you call tpm_get_random(), TPM driver is already contributing to the entropy pool (registered as hwrng). /Jarkko
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. /Jarkko
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
On Fri, Oct 04, 2019 at 01:26:58PM +0000, Safford, David (GE Global Research, US) wrote: > As the original author of trusted keys, let me make a few comments. > First, trusted keys were specifically implemented and *documented* to > use the TPM to both generate and seal keys. Its kernel documentation > specifically states this as a promise to user space. If you want to have > a different key system that uses the random pool to generate the keys, > fine, but don't change trusted keys, as that changes the existing promise > to user space. TPM generating keys (i.e. the random number) would make sense if the key would never leave from TPM (that kind of trusted keys would not be a bad idea at all). > There are many good reasons for wanting the keys to be based on the > TPM generator. As the source for the kernel random number generator > itself says, some systems lack good randomness at startup, and systems > should preserve and reload the pool across shutdown and startup. > There are use cases for trusted keys which need to generate keys > before such scripts have run. Also, in some use cases, we need to show > that trusted keys are FIPS compliant, which is possible with TPM > generated keys. If you are able to call tpm_get_random(), the driver has already registered TPN as hwrng. With this solution you fail to follow the principle of defense in depth. If the TPM random number generator is compromissed (has a bug) using the entropy pool will decrease the collateral damage. > Second, the TPM is hardly a "proprietary random number generator". > It is an open standard with multiple implementations, many of which are > FIPS certified. > > Third, as Mimi states, using a TPM is not a "regression". It would be a > regression to change trusted keys _not_ to use the TPM, because that > is what trusted keys are documented to provide to user space. For asym-tpm.c it is without a question a regression because of the evolution that has happened after trusted keys. For trusted keys using kernel rng would be improvement. /Jarkko
> > There are many good reasons for wanting the keys to be based on the > > TPM generator. As the source for the kernel random number generator > > itself says, some systems lack good randomness at startup, and systems > > should preserve and reload the pool across shutdown and startup. > > There are use cases for trusted keys which need to generate keys > > before such scripts have run. Also, in some use cases, we need to show > > that trusted keys are FIPS compliant, which is possible with TPM > > generated keys. > > If you are able to call tpm_get_random(), the driver has already > registered TPN as hwrng. With this solution you fail to follow the > principle of defense in depth. If the TPM random number generator > is compromissed (has a bug) using the entropy pool will decrease > the collateral damage. I.e. you make everything depend on single point of failure instead of multiple (e.g. rdrand, TPM, whatnot). /Jarkko
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?
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
> From: linux-integrity-owner@vger.kernel.org <linux-integrity- > owner@vger.kernel.org> On Behalf Of Jarkko Sakkinen > Sent: Friday, October 4, 2019 2:27 PM > Subject: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > If you are able to call tpm_get_random(), the driver has already registered > TPN as hwrng. With this solution you fail to follow the principle of defense in > depth. If the TPM random number generator is compromissed (has a bug) > using the entropy pool will decrease the collateral damage. And if the entropy pool has a bug or is misconfigured, you lose everything. That does not sound like defense in depth to me. In the real world I am not aware of a single instance of RNG vulnerability on a TPM. I am directly aware of several published vulnerabilities in embedded systems due to a badly ported version of the kernel random pool. In addition, the random generator in a TPM is hardware isolated, and less likely to be vulnerable to side channel or memory manipulation errors. The TPM RNG is typically FIPS certified. The use of the TPM RNG was a deliberate design choice in trusted keys. > > Third, as Mimi states, using a TPM is not a "regression". It would be > > a regression to change trusted keys _not_ to use the TPM, because that > > is what trusted keys are documented to provide to user space. > > For asym-tpm.c it is without a question a regression because of the evolution > that has happened after trusted keys. For trusted keys using kernel rng > would be improvement. Perhaps this is a language issue, but you are not using "regression" correctly. Changing to the kernel pool would not only be a debatable "improvement", but also would certainly be a change to the documented trusted key behavior, which I thought was frowned upon. dave
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.
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.
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) {
On Fri, 2019-10-04 at 15:11 -0700, James Bottomley wrote: > + > +/** > + * 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); Using the TPM as a source of input to the kernel random number generator is fine, but please don't change the meaning of trusted keys. The trusted-encrypted keys documentation clearly states "Trusted Keys use a TPM both to generate and to seal the keys." If you really want to use a different random number source instead of the TPM, then define a new trusted key option (eg. rng=kernel), with the default being the TPM. Mimi > + > + return rc; > +} > EXPORT_SYMBOL_GPL(tpm_get_random);
On Sat, Oct 05, 2019 at 08:38:53PM -0400, Mimi Zohar wrote: > On Fri, 2019-10-04 at 15:11 -0700, James Bottomley wrote: > > > + > > +/** > > + * 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); > > Using the TPM as a source of input to the kernel random number > generator is fine, but please don't change the meaning of trusted > keys. The trusted-encrypted keys documentation clearly states > "Trusted Keys use a TPM both to generate and to seal the keys." > > If you really want to use a different random number source instead of > the TPM, then define a new trusted key option (eg. rng=kernel), with > the default being the TPM. I'll add a patch that updates the documentation because it is clearly not a good practice to use TPM to generate keys that are not keys residing inside the TPM. With TEE coming in, TPM is not the only hardware measure anymore sealing the keys and we don't want a mess where every hardware asset does their own proprietary key generation. The proprietary technology should only take care of the sealing part. /Jarkko
On Fri, Oct 04, 2019 at 07:56:01PM +0000, Safford, David (GE Global Research, US) wrote: > > > From: linux-integrity-owner@vger.kernel.org <linux-integrity- > > owner@vger.kernel.org> On Behalf Of Jarkko Sakkinen > > Sent: Friday, October 4, 2019 2:27 PM > > Subject: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > > > If you are able to call tpm_get_random(), the driver has already registered > > TPN as hwrng. With this solution you fail to follow the principle of defense in > > depth. If the TPM random number generator is compromissed (has a bug) > > using the entropy pool will decrease the collateral damage. > > And if the entropy pool has a bug or is misconfigured, you lose everything. > That does not sound like defense in depth to me. In the real world > I am not aware of a single instance of RNG vulnerability on a TPM. > I am directly aware of several published vulnerabilities in embedded systems > due to a badly ported version of the kernel random pool. In addition, > the random generator in a TPM is hardware isolated, and less likely to be > vulnerable to side channel or memory manipulation errors. The TPM > RNG is typically FIPS certified. The use of the TPM RNG was a deliberate > design choice in trusted keys. Hmm... so is RDRAND opcode FIPS certified. Kernel has the random number generator for two reasons: 1. To protect against bugs in hwrng's. 2. To protect against deliberate backdoors in hwrng's. How TPM RNG is guaranteed to protect against both 1 and 2? If I would agree what you say, that'd be argument against using kernel random number generator *anywhere* in the kernel. Even with the entropy issues it is least worst thing to use for key generations for better or worse. /Jarkko
On Thu, Oct 3, 2019 at 2:41 PM Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > At what point during boot is the kernel random pool available? Does > > this imply that you're planning on changing trusted keys as well? > > Well trusted keys *must* be changed to use it. It is not a choice > because using a proprietary random number generator instead of defacto > one in the kernel can be categorized as a *regression*. > > Also, TEE trusted keys cannot use the TPM option. > > If it was not initialized early enough we would need fix that too. Note that especially IMA and fs encryptions are pretty annoying in this sense. You probably want to keep your keys device specific and you really need the keys around the time when the filesystems mount for the first time. This is very early on.. -- Janne
On Mon, 2019-10-07 at 02:52 +0300, Jarkko Sakkinen wrote: > > With TEE coming in, TPM is not the only hardware measure anymore sealing > the keys and we don't want a mess where every hardware asset does their > own proprietary key generation. The proprietary technology should only > take care of the sealing part. I'm fine with the concept of "trusted" keys being extended beyond just TPM. But just as the VFS layer defines a set of callbacks and generic functions, which can be used in lieu of file system specific callback functions, a similar approach could be used here. Mimi
The TPM library specification states that the TPM must comply with NIST SP800-90 A. https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ shows that the TPMs get third party certification, Common Criteria EAL 4+. While it's theoretically possible that an attacker could compromise both the TPM vendors and the evaluation agencies, we do have EAL 4+ assurance against both 1 and 2. On 10/6/2019 8:05 PM, Jarkko Sakkinen wrote: > > Kernel has the random number generator for two reasons: > > 1. To protect against bugs in hwrng's. > 2. To protect against deliberate backdoors in hwrng's. > > How TPM RNG is guaranteed to protect against both 1 and 2?
On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > The TPM library specification states that the TPM must comply with NIST > SP800-90 A. > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > While it's theoretically possible that an attacker could compromise > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > assurance against both 1 and 2. Certifications do not equal to trust. /Jarkko
On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > The TPM library specification states that the TPM must comply with NIST > > SP800-90 A. > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > While it's theoretically possible that an attacker could compromise > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > assurance against both 1 and 2. > > Certifications do not equal to trust. And for trusted keys the least trust solution is to do generation with the kernel assets and sealing with TPM. With TEE the least trust solution is equivalent. Are you proposing that the kernel random number generation should be removed? That would be my conclusion of this discussion if I would agree any of this (I don't). /Jarkko
> -----Original Message----- > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of > Jarkko Sakkinen > Sent: Wednesday, October 9, 2019 1:54 AM > To: Ken Goldman <kgold@linux.ibm.com> > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi Zohar > <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; stable@vger.kernel.org; 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() > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > The TPM library specification states that the TPM must comply with NIST > > > SP800-90 A. > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > While it's theoretically possible that an attacker could compromise > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > assurance against both 1 and 2. > > > > Certifications do not equal to trust. > So having an implementation reviewed by a capable third party of your choosing (as that's how certification usually works) is less trustworthy than having random individuals hacking away at it? (and trust me, _most_ people are not going to review that by themselves - very few people on this planet are capable to do so) > And for trusted keys the least trust solution is to do generation > with the kernel assets and sealing with TPM. With TEE the least > trust solution is equivalent. > > Are you proposing that the kernel random number generation should > be removed? That would be my conclusion of this discussion if I > would agree any of this (I don't). > Life is not that black and white. If certification is _not_ a requirement you can use the kernel random number generator, especially if you don't trust, say, the TPM one. If you _do_ require certification - and in many industries this is _mandatory_, you simply _must_ follow the certification rules (which may impose restrictions how the random number generation is done), and this should not be made impossible for such _existing_ use cases. Having said all that, generating _true_ entropy (and, importantly, ensuring it cannot be manipulated) is a very complicated subject and considering how all encryption security ultimately depends on the quality of the random numbers used for key material, I would not trust any implementation that has not been certified or otherwise carefully scrutinized by people _proven_ to have the expertise. Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
On Wed, Oct 09, 2019 at 02:53:39AM +0300, Jarkko Sakkinen wrote: > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > The TPM library specification states that the TPM must comply with NIST > > > SP800-90 A. > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > While it's theoretically possible that an attacker could compromise > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > assurance against both 1 and 2. > > > > Certifications do not equal to trust. > > And for trusted keys the least trust solution is to do generation > with the kernel assets and sealing with TPM. With TEE the least > trust solution is equivalent. > > Are you proposing that the kernel random number generation should > be removed? That would be my conclusion of this discussion if I > would agree any of this (I don't). The whole point of rng in kernel has been to use multiple entropy sources in order to disclose the trust issue. Even with weaker entropy than TPM RNG it is still a better choice for *non-TPM* keys because of better trustworthiness. Using only TPM RNG is a design flaw that has existed probably because when trusted keys were introduced TPM was more niche than it is today. Please remember that a trusted key is not a TPM key. The reality distortion field is strong here it seems. /Jarkko
On Wed, Oct 09, 2019 at 10:33:15AM +0300, Jarkko Sakkinen wrote: > On Wed, Oct 09, 2019 at 02:53:39AM +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > The TPM library specification states that the TPM must comply with NIST > > > > SP800-90 A. > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > > > While it's theoretically possible that an attacker could compromise > > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > > assurance against both 1 and 2. > > > > > > Certifications do not equal to trust. > > > > And for trusted keys the least trust solution is to do generation > > with the kernel assets and sealing with TPM. With TEE the least > > trust solution is equivalent. > > > > Are you proposing that the kernel random number generation should > > be removed? That would be my conclusion of this discussion if I > > would agree any of this (I don't). > > The whole point of rng in kernel has been to use multiple entropy > sources in order to disclose the trust issue. > > Even with weaker entropy than TPM RNG it is still a better choice for > *non-TPM* keys because of better trustworthiness. Using only TPM RNG is > a design flaw that has existed probably because when trusted keys were > introduced TPM was more niche than it is today. > > Please remember that a trusted key is not a TPM key. The reality > distortion field is strong here it seems. And why not use RDRAND on x86 instead of TPM RNG here? It is also FIPS compliant and has less latency than TPM RNG. :-) If we go with this route, lets pick the HRNG that performs best. /Jarkko
> -----Original Message----- > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of > Jarkko Sakkinen > Sent: Wednesday, October 9, 2019 9:33 AM > To: Ken Goldman <kgold@linux.ibm.com> > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi Zohar > <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; stable@vger.kernel.org; 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() > > On Wed, Oct 09, 2019 at 02:53:39AM +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > The TPM library specification states that the TPM must comply with NIST > > > > SP800-90 A. > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > > > While it's theoretically possible that an attacker could compromise > > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > > assurance against both 1 and 2. > > > > > > Certifications do not equal to trust. > > > > And for trusted keys the least trust solution is to do generation > > with the kernel assets and sealing with TPM. With TEE the least > > trust solution is equivalent. > > > > Are you proposing that the kernel random number generation should > > be removed? That would be my conclusion of this discussion if I > > would agree any of this (I don't). > > The whole point of rng in kernel has been to use multiple entropy > sources in order to disclose the trust issue. > I do understand that, and combining multiple entropy sources, if you have them available to get _more_ entropy is a good idea, at least in theory. But ... How do I know the mixing of entropy happens properly? Especially if I'm not capable of judging this by myself. And how do I know the SW entropy pool and/or code cannot be influenced _somehow_? (either directly or indirectly by influencing one of the contributors). More code and/or HW involved means more attack vectors and complication of the review process. The point is, if you want to certify such an application, you would have to have _all_ contributors _plus_ the kernel rng code certified. And you would have to have it _recertified_ every time a _single_ component - including the kernel code itself! - changes. > Even with weaker entropy than TPM RNG it is still a better choice for > *non-TPM* keys because of better trustworthiness. > "Even with weaker entropy"? Now that's just silly. If you _know_ and _trust_ the TPM to have _better_ entropy, then obviously that is the better choice. I guess the key word being the trust you don't have. > Using only TPM RNG is > a design flaw that has existed probably because when trusted keys were > introduced TPM was more niche than it is today. > For non-TPM keys, possibly. Assuming the kernel RNG indeed adds (or at least does not weaken) entropy. And assuming I _can_ trust the kernel RNG implementation. Question is: why would I trust that more than the TPM implementation? Sure, I could look at the code, but would I truly and fully understand it? (so maybe _I_ would, but would Joe Random User?) > Please remember that a trusted key is not a TPM key. The reality > distortion field is strong here it seems. > Agree. But you should not mess with the possibility to generate keys based on _just_ the TPM RNG _where that is required_ (and perhaps _only_ where that is required, if possible) > /Jarkko Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
> -----Original Message----- > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of > Jarkko Sakkinen > Sent: Wednesday, October 9, 2019 9:42 AM > To: Ken Goldman <kgold@linux.ibm.com> > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi Zohar > <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; stable@vger.kernel.org; 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() > > On Wed, Oct 09, 2019 at 10:33:15AM +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 09, 2019 at 02:53:39AM +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > > The TPM library specification states that the TPM must comply with NIST > > > > > SP800-90 A. > > > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certified-products/ > > > > > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > > > > > While it's theoretically possible that an attacker could compromise > > > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > > > assurance against both 1 and 2. > > > > > > > > Certifications do not equal to trust. > > > > > > And for trusted keys the least trust solution is to do generation > > > with the kernel assets and sealing with TPM. With TEE the least > > > trust solution is equivalent. > > > > > > Are you proposing that the kernel random number generation should > > > be removed? That would be my conclusion of this discussion if I > > > would agree any of this (I don't). > > > > The whole point of rng in kernel has been to use multiple entropy > > sources in order to disclose the trust issue. > > > > Even with weaker entropy than TPM RNG it is still a better choice for > > *non-TPM* keys because of better trustworthiness. Using only TPM RNG is > > a design flaw that has existed probably because when trusted keys were > > introduced TPM was more niche than it is today. > > > > Please remember that a trusted key is not a TPM key. The reality > > distortion field is strong here it seems. > > And why not use RDRAND on x86 instead of TPM RNG here? It is also FIPS > compliant and has less latency than TPM RNG. :-) If we go with this > route, lets pick the HRNG that performs best. > There's certification and certification. Not all certificates are created equally. But if it matches your specific requirements, why not. There's a _lot_ of HW out there that's not x86 though ... And: is RDRAND certified for _all_ x86 processors? Or just Intel? Or perhaps even only _specific (server) models_ of CPU's? I also know for a fact that some older AMD processors had a broken RDRAND implementation ... So the choice really should be up to the application or user. Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
> From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Sent: Tuesday, October 8, 2019 7:54 PM > To: Ken Goldman <kgold@linux.ibm.com> > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi > Zohar <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; > stable@vger.kernel.org; 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: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > The TPM library specification states that the TPM must comply with > > > NIST > > > SP800-90 A. > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certi > > > fied-products/ > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > While it's theoretically possible that an attacker could compromise > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > assurance against both 1 and 2. > > > > Certifications do not equal to trust. > > And for trusted keys the least trust solution is to do generation with the kernel > assets and sealing with TPM. With TEE the least trust solution is equivalent. > > Are you proposing that the kernel random number generation should be > removed? That would be my conclusion of this discussion if I would agree any > of this (I don't). > > /Jarkko No one is suggesting that. You are suggesting changing the documented behavior of trusted keys, and that would cause problems for some of our use cases. While certification may not in your mind be equal to trust, it is equal to compliance with mandatory regulations. Perhaps rather than arguing past each other, we should look into providing users the ability to choose, as an argument to keyctl? dave
On Wed, Oct 09, 2019 at 12:11:06PM +0000, Safford, David (GE Global Research, US) wrote: > > > From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > Sent: Tuesday, October 8, 2019 7:54 PM > > To: Ken Goldman <kgold@linux.ibm.com> > > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi > > Zohar <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; > > stable@vger.kernel.org; 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: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > The TPM library specification states that the TPM must comply with > > > > NIST > > > > SP800-90 A. > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certi > > > > fied-products/ > > > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > > > While it's theoretically possible that an attacker could compromise > > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > > assurance against both 1 and 2. > > > > > > Certifications do not equal to trust. > > > > And for trusted keys the least trust solution is to do generation with the kernel > > assets and sealing with TPM. With TEE the least trust solution is equivalent. > > > > Are you proposing that the kernel random number generation should be > > removed? That would be my conclusion of this discussion if I would agree any > > of this (I don't). > > > > /Jarkko > > No one is suggesting that. > > You are suggesting changing the documented behavior of trusted keys, and > that would cause problems for some of our use cases. While certification > may not in your mind be equal to trust, it is equal to compliance with > mandatory regulations. > > Perhaps rather than arguing past each other, we should look into > providing users the ability to choose, as an argument to keyctl? > > dave I'm taking my words back in the regression part as regression would need really a failing system. Definitely the fixes tag should be removed from my patch. What is anyway the role of the kernel rng? Why does it exist and when exactly it should be used? This exactly where the whole review process throughout the "chain of command" failed misserably with tpm_asym.c. The commit message for tpm_asym.c does not document the design choice in any possible way and still was merged to the mainline. Before knowning the answer to the "existential" question we are somewhat paralyzed on moving forward with trusted keys (e.g. paralyzed to merge TEE backend). Your proposal might make sense but I don't really want to say anything since I'm completely cluesless of the role of the kernel rng. Looks like everyone who participated to the review process of tpm_asym.c, is too. /Jarkko
On Wed, Oct 09, 2019 at 08:09:29AM +0000, Pascal Van Leeuwen wrote: > There's certification and certification. Not all certificates are > created equally. But if it matches your specific requirements, why not. > There's a _lot_ of HW out there that's not x86 though ... > > And: is RDRAND certified for _all_ x86 processors? Or just Intel? > Or perhaps even only _specific (server) models_ of CPU's? > I also know for a fact that some older AMD processors had a broken > RDRAND implementation ... > > So the choice really should be up to the application or user. I'm not seriously suggesting to move rdrand here. I'm trying find the logic how to move forward with trusted keys with multiple backends (not only TPM). AKA we have a kernel rng in existence but no clear policy when it should be used and when not. This leads to throwing a dice with the design choices. Even it TPM RNG is the right choice in tpm_asym.c, the choice was not based on anything (otherwise it would have been documented). /Jarkko
On Mon, Oct 14, 2019 at 10:00:33PM +0300, Jarkko Sakkinen wrote: > On Wed, Oct 09, 2019 at 12:11:06PM +0000, Safford, David (GE Global Research, US) wrote: > > > > > From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > Sent: Tuesday, October 8, 2019 7:54 PM > > > To: Ken Goldman <kgold@linux.ibm.com> > > > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com>; Mimi > > > Zohar <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; > > > stable@vger.kernel.org; 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: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to get_random_bytes() > > > > > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > > The TPM library specification states that the TPM must comply with > > > > > NIST > > > > > SP800-90 A. > > > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tpm-certi > > > > > fied-products/ > > > > > > > > > > shows that the TPMs get third party certification, Common Criteria EAL 4+. > > > > > > > > > > While it's theoretically possible that an attacker could compromise > > > > > both the TPM vendors and the evaluation agencies, we do have EAL 4+ > > > > > assurance against both 1 and 2. > > > > > > > > Certifications do not equal to trust. > > > > > > And for trusted keys the least trust solution is to do generation with the kernel > > > assets and sealing with TPM. With TEE the least trust solution is equivalent. > > > > > > Are you proposing that the kernel random number generation should be > > > removed? That would be my conclusion of this discussion if I would agree any > > > of this (I don't). > > > > > > /Jarkko > > > > No one is suggesting that. > > > > You are suggesting changing the documented behavior of trusted keys, and > > that would cause problems for some of our use cases. While certification > > may not in your mind be equal to trust, it is equal to compliance with > > mandatory regulations. > > > > Perhaps rather than arguing past each other, we should look into > > providing users the ability to choose, as an argument to keyctl? > > > > dave > > I'm taking my words back in the regression part as regression would need > really a failing system. Definitely the fixes tag should be removed from > my patch. > > What is anyway the role of the kernel rng? Why does it exist and when > exactly it should be used? This exactly where the whole review process > throughout the "chain of command" failed misserably with tpm_asym.c. > > The commit message for tpm_asym.c does not document the design choice in > any possible way and still was merged to the mainline. > > Before knowning the answer to the "existential" question we are > somewhat paralyzed on moving forward with trusted keys (e.g. paralyzed > to merge TEE backend). > > Your proposal might make sense but I don't really want to say anything > since I'm completely cluesless of the role of the kernel rng. Looks like > everyone who participated to the review process of tpm_asym.c, is too. As a ABI backwards compatibility workaround I'd agree most likely agree with you. As a guideline for new features there should be a framework on how to decide what to do. /Jarkko
On Mon, 2019-10-14 at 22:00 +0300, Jarkko Sakkinen wrote: > On Wed, Oct 09, 2019 at 12:11:06PM +0000, Safford, David (GE Global > Research, US) wrote: > > > > > From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > Sent: Tuesday, October 8, 2019 7:54 PM > > > To: Ken Goldman <kgold@linux.ibm.com> > > > Cc: Safford, David (GE Global Research, US) <david.safford@ge.com > > > >; Mimi > > > Zohar <zohar@linux.ibm.com>; linux-integrity@vger.kernel.org; > > > stable@vger.kernel.org; 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: EXT: Re: [PATCH] KEYS: asym_tpm: Switch to > > > get_random_bytes() > > > > > > On Wed, Oct 09, 2019 at 02:49:35AM +0300, Jarkko Sakkinen wrote: > > > > On Mon, Oct 07, 2019 at 06:13:01PM -0400, Ken Goldman wrote: > > > > > The TPM library specification states that the TPM must comply > > > > > with NIST SP800-90 A. > > > > > > > > > > https://trustedcomputinggroup.org/membership/certification/tp > > > > > m-certified-products/ > > > > > > > > > > shows that the TPMs get third party certification, Common > > > > > Criteria EAL 4+. > > > > > > > > > > While it's theoretically possible that an attacker could > > > > > compromise both the TPM vendors and the evaluation agencies, > > > > > we do have EAL 4+ assurance against both 1 and 2. > > > > > > > > Certifications do not equal to trust. > > > > > > And for trusted keys the least trust solution is to do generation > > > with the kernel assets and sealing with TPM. With TEE the least > > > trust solution is equivalent. > > > > > > Are you proposing that the kernel random number generation should > > > be removed? That would be my conclusion of this discussion if I > > > would agree any of this (I don't). > > > > > > /Jarkko > > > > No one is suggesting that. > > > > You are suggesting changing the documented behavior of trusted > > keys, and that would cause problems for some of our use cases. > > While certification may not in your mind be equal to trust, it is > > equal to compliance with mandatory regulations. > > > > Perhaps rather than arguing past each other, we should look into > > providing users the ability to choose, as an argument to keyctl? > > > > dave > > I'm taking my words back in the regression part as regression would > need really a failing system. Definitely the fixes tag should be > removed from my patch. > > What is anyway the role of the kernel rng? Why does it exist and when > exactly it should be used? This exactly where the whole review > process throughout the "chain of command" failed misserably with > tpm_asym.c. > > The commit message for tpm_asym.c does not document the design choice > in any possible way and still was merged to the mainline. > > Before knowning the answer to the "existential" question we are > somewhat paralyzed on moving forward with trusted keys (e.g. > paralyzed to merge TEE backend). > > Your proposal might make sense but I don't really want to say > anything since I'm completely cluesless of the role of the kernel > rng. Looks like everyone who participated to the review process of > tpm_asym.c, is too. The job of the in-kernel rng is simply to produce a mixed entropy pool from which we can draw random numbers. The idea is that quite a few attackers have identified the rng as being a weak point in the security architecture of the kernel, so if we mix entropy from all the sources we have, you have to compromise most of them to gain some predictive power over the rng sequence. The point is not how certified the TPM RNG is, the point is that it's a single source and if we rely on it solely for some applications, like trusted keys, then it gives the attackers a single known point to go after. This may be impossible for script kiddies, but it won't be for nation states ... are you going to exclusively trust the random number you got from your chinese certified TPM? Remember also that the attack doesn't have to be to the TPM only, it could be the pathway by which we get the random number, which involves components outside of the TPM certification. James
On Mon, Oct 14, 2019 at 12:29:57PM -0700, James Bottomley wrote: > The job of the in-kernel rng is simply to produce a mixed entropy pool > from which we can draw random numbers. The idea is that quite a few > attackers have identified the rng as being a weak point in the security > architecture of the kernel, so if we mix entropy from all the sources > we have, you have to compromise most of them to gain some predictive > power over the rng sequence. The documentation says that krng is suitable for key generation. Should the documentation changed to state that it is unsuitable? > The point is not how certified the TPM RNG is, the point is that it's a > single source and if we rely on it solely for some applications, like > trusted keys, then it gives the attackers a single known point to go > after. This may be impossible for script kiddies, but it won't be for > nation states ... are you going to exclusively trust the random number > you got from your chinese certified TPM? I'd suggest approach where TPM RNG result is xored with krng result. > Remember also that the attack doesn't have to be to the TPM only, it > could be the pathway by which we get the random number, which involves > components outside of the TPM certification. Yeah, I do get this. /Jarkko
On Wed, 2019-10-16 at 14:00 +0300, Jarkko Sakkinen wrote: > On Mon, Oct 14, 2019 at 12:29:57PM -0700, James Bottomley wrote: > > The job of the in-kernel rng is simply to produce a mixed entropy > > pool from which we can draw random numbers. The idea is that quite > > a few attackers have identified the rng as being a weak point in > > the security architecture of the kernel, so if we mix entropy from > > all the sources we have, you have to compromise most of them to > > gain some predictive power over the rng sequence. > > The documentation says that krng is suitable for key generation. > Should the documentation changed to state that it is unsuitable? How do you get that from the argument above? The krng is about the best we have in terms of unpredictable key generation, so of course it is suitable ... provided you give the entropy enough time to have sufficient entropy. It's also not foolproof ... Bernstein did a speculation about how you could compromise all our input sources for entropy. However the more sources we have the more difficult the compromise becomes. > > The point is not how certified the TPM RNG is, the point is that > > it's a single source and if we rely on it solely for some > > applications, like trusted keys, then it gives the attackers a > > single known point to go after. This may be impossible for script > > kiddies, but it won't be for nation states ... are you going to > > exclusively trust the random number you got from your chinese > > certified TPM? > > I'd suggest approach where TPM RNG result is xored with krng result. reversible ciphers are generally frowned upon in random number generation, that's why the krng uses chacha20. In general I think we shouldn't try to code our own mixing and instead should get the krng to do it for us using whatever the algorithm du jour that the crypto guys have blessed is. That's why I proposed adding the TPM output to the krng as entropy input and then taking the output of the krng. James > > Remember also that the attack doesn't have to be to the TPM only, > > it could be the pathway by which we get the random number, which > > involves components outside of the TPM certification. > > Yeah, I do get this. > > /Jarkko >
On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > reversible ciphers are generally frowned upon in random number > generation, that's why the krng uses chacha20. In general I think we > shouldn't try to code our own mixing and instead should get the krng to > do it for us using whatever the algorithm du jour that the crypto guys > have blessed is. That's why I proposed adding the TPM output to the > krng as entropy input and then taking the output of the krng. It is already registered as hwrng. What else? Was the issue that it is only used as seed when the rng is init'd first? I haven't at this point gone to the internals of krng. /Jarkko
On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > reversible ciphers are generally frowned upon in random number > > generation, that's why the krng uses chacha20. In general I think > > we shouldn't try to code our own mixing and instead should get the > > krng to do it for us using whatever the algorithm du jour that the > > crypto guys have blessed is. That's why I proposed adding the TPM > > output to the krng as entropy input and then taking the output of > > the krng. > > It is already registered as hwrng. What else? It only contributes entropy once at start of OS. > Was the issue that it is only used as seed when the rng is init'd > first? I haven't at this point gone to the internals of krng. Basically it was similar to your xor patch except I got the kernel rng to do the mixing, so it would use the chacha20 cipher at the moment until they decide that's unsafe and change it to something else: https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@HansenPartnership.com/ It uses add_hwgenerator_randomness() to do the mixing. It also has an unmixed source so that read of the TPM hwrng device works as expected. James
On Thu, 17 Oct 2019 at 00:40, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > > reversible ciphers are generally frowned upon in random number > > > generation, that's why the krng uses chacha20. In general I think > > > we shouldn't try to code our own mixing and instead should get the > > > krng to do it for us using whatever the algorithm du jour that the > > > crypto guys have blessed is. That's why I proposed adding the TPM > > > output to the krng as entropy input and then taking the output of > > > the krng. > > > > It is already registered as hwrng. What else? > > It only contributes entropy once at start of OS. > Why not just configure quality parameter of TPM hwrng as follows? It would automatically initiate a kthread during hwrng_init() to feed entropy from TPM to kernel random numbers pool (see: drivers/char/hw_random/core.c +142). diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 3d6d394..fcc3817 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -548,6 +548,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip) "tpm-rng-%d", chip->dev_num); chip->hwrng.name = chip->hwrng_name; chip->hwrng.read = tpm_hwrng_read; + chip->hwrng.quality = 1024; /* Here we assume TPM provides full entropy */ return hwrng_register(&chip->hwrng); } > > Was the issue that it is only used as seed when the rng is init'd > > first? I haven't at this point gone to the internals of krng. > > Basically it was similar to your xor patch except I got the kernel rng > to do the mixing, so it would use the chacha20 cipher at the moment > until they decide that's unsafe and change it to something else: > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@HansenPartnership.com/ > > It uses add_hwgenerator_randomness() to do the mixing. It also has an > unmixed source so that read of the TPM hwrng device works as expected. Above suggestion is something similar to yours but utilizing the framework already provided via hwrng core. -Sumit > > James > > > > >
On Thu, 2019-10-17 at 18:22 +0530, Sumit Garg wrote: > On Thu, 17 Oct 2019 at 00:40, James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: > > > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > > > reversible ciphers are generally frowned upon in random number > > > > generation, that's why the krng uses chacha20. In general I > > > > think we shouldn't try to code our own mixing and instead > > > > should get the krng to do it for us using whatever the > > > > algorithm du jour that the crypto guys have blessed is. That's > > > > why I proposed adding the TPM output to the krng as entropy > > > > input and then taking the output of the krng. > > > > > > It is already registered as hwrng. What else? > > > > It only contributes entropy once at start of OS. > > > > Why not just configure quality parameter of TPM hwrng as follows? It > would automatically initiate a kthread during hwrng_init() to feed > entropy from TPM to kernel random numbers pool (see: > drivers/char/hw_random/core.c +142). The question was asked before by Jerry. The main reason is we still can't guarantee that at 1024 the hwrng will choose the TPM as the best source (the problem being it only chooses one) and the mixing is done periodically in a timer thread so it's still vulnerable to an entropy exhaustion attack. I think it's better to source the random number in the TPM when asked but mix it with whatever entropy we have in the pool using the crypto people's mixing algorithm. This definitely avoids exhaustion and provides some protection against single source rng compromises. James > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm- > chip.c > index 3d6d394..fcc3817 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -548,6 +548,7 @@ static int tpm_add_hwrng(struct tpm_chip *chip) > "tpm-rng-%d", chip->dev_num); > chip->hwrng.name = chip->hwrng_name; > chip->hwrng.read = tpm_hwrng_read; > + chip->hwrng.quality = 1024; /* Here we assume TPM provides > full entropy */ > return hwrng_register(&chip->hwrng); > > } > > > > Was the issue that it is only used as seed when the rng is > > > init'd > > > first? I haven't at this point gone to the internals of krng. > > > > Basically it was similar to your xor patch except I got the kernel > > rng > > to do the mixing, so it would use the chacha20 cipher at the moment > > until they decide that's unsafe and change it to something else: > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@Hanse > > nPartnership.com/ > > > > It uses add_hwgenerator_randomness() to do the mixing. It also has > > an > > unmixed source so that read of the TPM hwrng device works as > > expected. > > Above suggestion is something similar to yours but utilizing the > framework already provided via hwrng core. > > -Sumit > > > > > James > > > > > > > > > > > >
On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote: > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > > reversible ciphers are generally frowned upon in random number > > > generation, that's why the krng uses chacha20. In general I think > > > we shouldn't try to code our own mixing and instead should get the > > > krng to do it for us using whatever the algorithm du jour that the > > > crypto guys have blessed is. That's why I proposed adding the TPM > > > output to the krng as entropy input and then taking the output of > > > the krng. > > > > It is already registered as hwrng. What else? > > It only contributes entropy once at start of OS. Ok. > > Was the issue that it is only used as seed when the rng is init'd > > first? I haven't at this point gone to the internals of krng. > > Basically it was similar to your xor patch except I got the kernel rng > to do the mixing, so it would use the chacha20 cipher at the moment > until they decide that's unsafe and change it to something else: > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@HansenPartnership.com/ > > It uses add_hwgenerator_randomness() to do the mixing. It also has an > unmixed source so that read of the TPM hwrng device works as expected. Thinking that could this potentially racy? I.e. between the calls something else could eat the entropy added? /Jarkko
On Wed, Oct 16, 2019 at 6:35 PM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > The documentation says that krng is suitable for key generation. > > Should the documentation changed to state that it is unsuitable? > > How do you get that from the argument above? The krng is about the > best we have in terms of unpredictable key generation, so of course it > is suitable ... provided you give the entropy enough time to have > sufficient entropy. Yes, so it can be both the safest and the least safe option available. By default it's the worst one, but use it wisely and it can be the best source. Hence I was proposing that kconfig option + boot time printout to make this clear for everyone.. -- Janne
On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote: > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote: > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > > > reversible ciphers are generally frowned upon in random number > > > > generation, that's why the krng uses chacha20. In general I think > > > > we shouldn't try to code our own mixing and instead should get the > > > > krng to do it for us using whatever the algorithm du jour that the > > > > crypto guys have blessed is. That's why I proposed adding the TPM > > > > output to the krng as entropy input and then taking the output of > > > > the krng. > > > > > > It is already registered as hwrng. What else? > > > > It only contributes entropy once at start of OS. > > Ok. > > > > Was the issue that it is only used as seed when the rng is init'd > > > first? I haven't at this point gone to the internals of krng. > > > > Basically it was similar to your xor patch except I got the kernel rng > > to do the mixing, so it would use the chacha20 cipher at the moment > > until they decide that's unsafe and change it to something else: > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@HansenPartnership.com/ > > > > It uses add_hwgenerator_randomness() to do the mixing. It also has an > > unmixed source so that read of the TPM hwrng device works as expected. > > Thinking that could this potentially racy? I.e. between the calls > something else could eat the entropy added? Also, what is wrong just taking one value from krng and mixing it with a value from TPM RNG where needed? That would be non-racy too. /Jarkko
On Mon, Oct 21, 2019 at 02:39:39PM +0300, Jarkko Sakkinen wrote: > On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote: > > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote: > > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley wrote: > > > > > reversible ciphers are generally frowned upon in random number > > > > > generation, that's why the krng uses chacha20. In general I think > > > > > we shouldn't try to code our own mixing and instead should get the > > > > > krng to do it for us using whatever the algorithm du jour that the > > > > > crypto guys have blessed is. That's why I proposed adding the TPM > > > > > output to the krng as entropy input and then taking the output of > > > > > the krng. > > > > > > > > It is already registered as hwrng. What else? > > > > > > It only contributes entropy once at start of OS. > > > > Ok. > > > > > > Was the issue that it is only used as seed when the rng is init'd > > > > first? I haven't at this point gone to the internals of krng. > > > > > > Basically it was similar to your xor patch except I got the kernel rng > > > to do the mixing, so it would use the chacha20 cipher at the moment > > > until they decide that's unsafe and change it to something else: > > > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@HansenPartnership.com/ > > > > > > It uses add_hwgenerator_randomness() to do the mixing. It also has an > > > unmixed source so that read of the TPM hwrng device works as expected. > > > > Thinking that could this potentially racy? I.e. between the calls > > something else could eat the entropy added? > > Also, what is wrong just taking one value from krng and mixing > it with a value from TPM RNG where needed? That would be non-racy > too. I guess we can move forward with this? /Jarkko
On Tue, 2019-10-29 at 10:42 +0200, Jarkko Sakkinen wrote: > On Mon, Oct 21, 2019 at 02:39:39PM +0300, Jarkko Sakkinen wrote: > > On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote: > > > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote: > > > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley > > > > > wrote: > > > > > > reversible ciphers are generally frowned upon in random > > > > > > number > > > > > > generation, that's why the krng uses chacha20. In general > > > > > > I think > > > > > > we shouldn't try to code our own mixing and instead should > > > > > > get the > > > > > > krng to do it for us using whatever the algorithm du jour > > > > > > that the > > > > > > crypto guys have blessed is. That's why I proposed adding > > > > > > the TPM > > > > > > output to the krng as entropy input and then taking the > > > > > > output of > > > > > > the krng. > > > > > > > > > > It is already registered as hwrng. What else? > > > > > > > > It only contributes entropy once at start of OS. > > > > > > Ok. > > > > > > > > Was the issue that it is only used as seed when the rng is > > > > > init'd > > > > > first? I haven't at this point gone to the internals of krng. > > > > > > > > Basically it was similar to your xor patch except I got the > > > > kernel rng > > > > to do the mixing, so it would use the chacha20 cipher at the > > > > moment > > > > until they decide that's unsafe and change it to something > > > > else: > > > > > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@H > > > > ansenPartnership.com/ > > > > > > > > It uses add_hwgenerator_randomness() to do the mixing. It also > > > > has an > > > > unmixed source so that read of the TPM hwrng device works as > > > > expected. > > > > > > Thinking that could this potentially racy? I.e. between the calls > > > something else could eat the entropy added? > > > > Also, what is wrong just taking one value from krng and mixing > > it with a value from TPM RNG where needed? That would be non-racy > > too. > > I guess we can move forward with this? Sure I suppose; can we can figure out how to get the mixing function du jour exposed? James
On Tue, Oct 29, 2019 at 07:58:16AM -0700, James Bottomley wrote: > On Tue, 2019-10-29 at 10:42 +0200, Jarkko Sakkinen wrote: > > On Mon, Oct 21, 2019 at 02:39:39PM +0300, Jarkko Sakkinen wrote: > > > On Thu, Oct 17, 2019 at 09:04:40PM +0300, Jarkko Sakkinen wrote: > > > > On Wed, Oct 16, 2019 at 03:10:29PM -0400, James Bottomley wrote: > > > > > On Wed, 2019-10-16 at 19:25 +0300, Jarkko Sakkinen wrote: > > > > > > On Wed, Oct 16, 2019 at 08:34:12AM -0400, James Bottomley > > > > > > wrote: > > > > > > > reversible ciphers are generally frowned upon in random > > > > > > > number > > > > > > > generation, that's why the krng uses chacha20. In general > > > > > > > I think > > > > > > > we shouldn't try to code our own mixing and instead should > > > > > > > get the > > > > > > > krng to do it for us using whatever the algorithm du jour > > > > > > > that the > > > > > > > crypto guys have blessed is. That's why I proposed adding > > > > > > > the TPM > > > > > > > output to the krng as entropy input and then taking the > > > > > > > output of > > > > > > > the krng. > > > > > > > > > > > > It is already registered as hwrng. What else? > > > > > > > > > > It only contributes entropy once at start of OS. > > > > > > > > Ok. > > > > > > > > > > Was the issue that it is only used as seed when the rng is > > > > > > init'd > > > > > > first? I haven't at this point gone to the internals of krng. > > > > > > > > > > Basically it was similar to your xor patch except I got the > > > > > kernel rng > > > > > to do the mixing, so it would use the chacha20 cipher at the > > > > > moment > > > > > until they decide that's unsafe and change it to something > > > > > else: > > > > > > > > > > https://lore.kernel.org/linux-crypto/1570227068.17537.4.camel@H > > > > > ansenPartnership.com/ > > > > > > > > > > It uses add_hwgenerator_randomness() to do the mixing. It also > > > > > has an > > > > > unmixed source so that read of the TPM hwrng device works as > > > > > expected. > > > > > > > > Thinking that could this potentially racy? I.e. between the calls > > > > something else could eat the entropy added? > > > > > > Also, what is wrong just taking one value from krng and mixing > > > it with a value from TPM RNG where needed? That would be non-racy > > > too. > > > > I guess we can move forward with this? > > Sure I suppose; can we can figure out how to get the mixing function du > jour exposed? Maybe it is best to reflect the whole issue in the context of the Sumit's 2nd patch set, which adds ARM TEE support in order to move forward. /Jarkko
diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c index 76d2ce3a1b5b..c14b8d186e93 100644 --- a/crypto/asymmetric_keys/asym_tpm.c +++ b/crypto/asymmetric_keys/asym_tpm.c @@ -6,6 +6,7 @@ #include <linux/kernel.h> #include <linux/seq_file.h> #include <linux/scatterlist.h> +#include <linux/random.h> #include <linux/tpm.h> #include <linux/tpm_command.h> #include <crypto/akcipher.h> @@ -54,11 +55,7 @@ static int tpm_loadkey2(struct tpm_buf *tb, } /* generate odd nonce */ - ret = tpm_get_random(NULL, nonceodd, TPM_NONCE_SIZE); - if (ret < 0) { - pr_info("tpm_get_random failed (%d)\n", ret); - return ret; - } + get_random_bytes(nonceodd, TPM_NONCE_SIZE); /* calculate authorization HMAC value */ ret = TSS_authhmac(authdata, keyauth, SHA1_DIGEST_SIZE, enonce,
Only the kernel random pool should be used for generating random numbers. TPM contributes to that pool among the other sources of entropy. In here it is not, agreed, absolutely critical because TPM is what is trusted anyway but in order to remove tpm_get_random() we need to first remove all the call sites. Cc: stable@vger.kernel.org Fixes: 0c36264aa1d5 ("KEYS: asym_tpm: Add loadkey2 and flushspecific [ver #2]") Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- crypto/asymmetric_keys/asym_tpm.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)