| Message ID | 7b771da7b09a01c8b4da2ed21f05251ea797b2e8.1626885907.git-series.a.fatoum@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | KEYS: trusted: Introduce support for NXP CAAM-based trusted keys | expand |
On Wed, 21 Jul 2021 at 22:19, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > The two existing trusted key sources don't make use of the kernel RNG, > but instead let the hardware that does the sealing/unsealing also > generate the random key material. While a previous change offers users > the choice to use the kernel RNG instead for both, new trust sources > may want to unconditionally use the kernel RNG for generating key > material, like it's done elsewhere in the kernel. > > This is especially prudent for hardware that has proven-in-production > HWRNG drivers implemented, as otherwise code would have to be duplicated > only to arrive at a possibly worse result. > > Make this possible by turning struct trusted_key_ops::get_random > into an optional member. If a driver leaves it NULL, kernel RNG > will be used instead. > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > To: James Bottomley <jejb@linux.ibm.com> > To: Jarkko Sakkinen <jarkko@kernel.org> > To: Mimi Zohar <zohar@linux.ibm.com> > To: David Howells <dhowells@redhat.com> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: "Horia Geantă" <horia.geanta@nxp.com> > Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Udit Agarwal <udit.agarwal@nxp.com> > Cc: Eric Biggers <ebiggers@kernel.org> > Cc: Jan Luebbe <j.luebbe@pengutronix.de> > Cc: David Gstir <david@sigma-star.at> > Cc: Richard Weinberger <richard@nod.at> > Cc: Franck LENORMAND <franck.lenormand@nxp.com> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: keyrings@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > Cc: linux-integrity@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-security-module@vger.kernel.org > --- > include/keys/trusted-type.h | 2 +- > security/keys/trusted-keys/trusted_core.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h > index d89fa2579ac0..4eb64548a74f 100644 > --- a/include/keys/trusted-type.h > +++ b/include/keys/trusted-type.h > @@ -64,7 +64,7 @@ struct trusted_key_ops { > /* Unseal a key. */ > int (*unseal)(struct trusted_key_payload *p, char *datablob); > > - /* Get a randomized key. */ > + /* Optional: Get a randomized key. */ > int (*get_random)(unsigned char *key, size_t key_len); > > /* Exit key interface. */ > diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c > index 569af9af8df0..d2b7626cde8b 100644 > --- a/security/keys/trusted-keys/trusted_core.c > +++ b/security/keys/trusted-keys/trusted_core.c > @@ -334,7 +334,7 @@ static int __init init_trusted(void) > continue; > > get_random = trusted_key_sources[i].ops->get_random; > - if (trusted_kernel_rng) > + if (trusted_kernel_rng || !get_random) > get_random = kernel_get_random; > For ease of understanding, I would prefer to write it as: get_random = trusted_key_sources[i].ops->get_random ?: kernel_get_random; if (trusted_kernel_rng) get_random = kernel_get_random; With that: Acked-by: Sumit Garg <sumit.garg@linaro.org> -Sumit > static_call_update(trusted_key_init, > -- > git-series 0.9.1
Hello Sumit, On 22.07.21 08:31, Sumit Garg wrote: > On Wed, 21 Jul 2021 at 22:19, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: >> >> The two existing trusted key sources don't make use of the kernel RNG, >> but instead let the hardware that does the sealing/unsealing also >> generate the random key material. While a previous change offers users >> the choice to use the kernel RNG instead for both, new trust sources >> may want to unconditionally use the kernel RNG for generating key >> material, like it's done elsewhere in the kernel. >> >> This is especially prudent for hardware that has proven-in-production >> HWRNG drivers implemented, as otherwise code would have to be duplicated >> only to arrive at a possibly worse result. >> >> Make this possible by turning struct trusted_key_ops::get_random >> into an optional member. If a driver leaves it NULL, kernel RNG >> will be used instead. >> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> --- >> To: James Bottomley <jejb@linux.ibm.com> >> To: Jarkko Sakkinen <jarkko@kernel.org> >> To: Mimi Zohar <zohar@linux.ibm.com> >> To: David Howells <dhowells@redhat.com> >> Cc: James Morris <jmorris@namei.org> >> Cc: "Serge E. Hallyn" <serge@hallyn.com> >> Cc: "Horia Geantă" <horia.geanta@nxp.com> >> Cc: Aymen Sghaier <aymen.sghaier@nxp.com> >> Cc: Herbert Xu <herbert@gondor.apana.org.au> >> Cc: "David S. Miller" <davem@davemloft.net> >> Cc: Udit Agarwal <udit.agarwal@nxp.com> >> Cc: Eric Biggers <ebiggers@kernel.org> >> Cc: Jan Luebbe <j.luebbe@pengutronix.de> >> Cc: David Gstir <david@sigma-star.at> >> Cc: Richard Weinberger <richard@nod.at> >> Cc: Franck LENORMAND <franck.lenormand@nxp.com> >> Cc: Sumit Garg <sumit.garg@linaro.org> >> Cc: keyrings@vger.kernel.org >> Cc: linux-crypto@vger.kernel.org >> Cc: linux-integrity@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linux-security-module@vger.kernel.org >> --- >> include/keys/trusted-type.h | 2 +- >> security/keys/trusted-keys/trusted_core.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h >> index d89fa2579ac0..4eb64548a74f 100644 >> --- a/include/keys/trusted-type.h >> +++ b/include/keys/trusted-type.h >> @@ -64,7 +64,7 @@ struct trusted_key_ops { >> /* Unseal a key. */ >> int (*unseal)(struct trusted_key_payload *p, char *datablob); >> >> - /* Get a randomized key. */ >> + /* Optional: Get a randomized key. */ >> int (*get_random)(unsigned char *key, size_t key_len); >> >> /* Exit key interface. */ >> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c >> index 569af9af8df0..d2b7626cde8b 100644 >> --- a/security/keys/trusted-keys/trusted_core.c >> +++ b/security/keys/trusted-keys/trusted_core.c >> @@ -334,7 +334,7 @@ static int __init init_trusted(void) >> continue; >> >> get_random = trusted_key_sources[i].ops->get_random; >> - if (trusted_kernel_rng) >> + if (trusted_kernel_rng || !get_random) >> get_random = kernel_get_random; >> > > For ease of understanding, I would prefer to write it as: > > get_random = trusted_key_sources[i].ops->get_random ?: > kernel_get_random; > if (trusted_kernel_rng) > get_random = kernel_get_random; > > With that: > > Acked-by: Sumit Garg <sumit.garg@linaro.org> I don't think it improves readability to split up the conditional. At least I need to take a second pass over the code to understand the second conditional. Cheers, Ahmad > > -Sumit > >> static_call_update(trusted_key_init, >> -- >> git-series 0.9.1 >
On Mon, Aug 09, 2021 at 09:52:20AM +0200, Ahmad Fatoum wrote: > Hello Sumit, > > On 22.07.21 08:31, Sumit Garg wrote: > > On Wed, 21 Jul 2021 at 22:19, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > >> > >> The two existing trusted key sources don't make use of the kernel RNG, > >> but instead let the hardware that does the sealing/unsealing also > >> generate the random key material. While a previous change offers users > >> the choice to use the kernel RNG instead for both, new trust sources > >> may want to unconditionally use the kernel RNG for generating key > >> material, like it's done elsewhere in the kernel. > >> > >> This is especially prudent for hardware that has proven-in-production > >> HWRNG drivers implemented, as otherwise code would have to be duplicated > >> only to arrive at a possibly worse result. > >> > >> Make this possible by turning struct trusted_key_ops::get_random > >> into an optional member. If a driver leaves it NULL, kernel RNG > >> will be used instead. > >> > >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > >> --- > >> To: James Bottomley <jejb@linux.ibm.com> > >> To: Jarkko Sakkinen <jarkko@kernel.org> > >> To: Mimi Zohar <zohar@linux.ibm.com> > >> To: David Howells <dhowells@redhat.com> > >> Cc: James Morris <jmorris@namei.org> > >> Cc: "Serge E. Hallyn" <serge@hallyn.com> > >> Cc: "Horia Geantă" <horia.geanta@nxp.com> > >> Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > >> Cc: Herbert Xu <herbert@gondor.apana.org.au> > >> Cc: "David S. Miller" <davem@davemloft.net> > >> Cc: Udit Agarwal <udit.agarwal@nxp.com> > >> Cc: Eric Biggers <ebiggers@kernel.org> > >> Cc: Jan Luebbe <j.luebbe@pengutronix.de> > >> Cc: David Gstir <david@sigma-star.at> > >> Cc: Richard Weinberger <richard@nod.at> > >> Cc: Franck LENORMAND <franck.lenormand@nxp.com> > >> Cc: Sumit Garg <sumit.garg@linaro.org> > >> Cc: keyrings@vger.kernel.org > >> Cc: linux-crypto@vger.kernel.org > >> Cc: linux-integrity@vger.kernel.org > >> Cc: linux-kernel@vger.kernel.org > >> Cc: linux-security-module@vger.kernel.org > >> --- > >> include/keys/trusted-type.h | 2 +- > >> security/keys/trusted-keys/trusted_core.c | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h > >> index d89fa2579ac0..4eb64548a74f 100644 > >> --- a/include/keys/trusted-type.h > >> +++ b/include/keys/trusted-type.h > >> @@ -64,7 +64,7 @@ struct trusted_key_ops { > >> /* Unseal a key. */ > >> int (*unseal)(struct trusted_key_payload *p, char *datablob); > >> > >> - /* Get a randomized key. */ > >> + /* Optional: Get a randomized key. */ > >> int (*get_random)(unsigned char *key, size_t key_len); > >> > >> /* Exit key interface. */ > >> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c > >> index 569af9af8df0..d2b7626cde8b 100644 > >> --- a/security/keys/trusted-keys/trusted_core.c > >> +++ b/security/keys/trusted-keys/trusted_core.c > >> @@ -334,7 +334,7 @@ static int __init init_trusted(void) > >> continue; > >> > >> get_random = trusted_key_sources[i].ops->get_random; > >> - if (trusted_kernel_rng) > >> + if (trusted_kernel_rng || !get_random) > >> get_random = kernel_get_random; > >> > > > > For ease of understanding, I would prefer to write it as: > > > > get_random = trusted_key_sources[i].ops->get_random ?: > > kernel_get_random; > > if (trusted_kernel_rng) > > get_random = kernel_get_random; > > > > With that: > > > > Acked-by: Sumit Garg <sumit.garg@linaro.org> > > I don't think it improves readability to split up the conditional. > At least I need to take a second pass over the code to understand > the second conditional. Ternary operators are pain to read, unless a super trivial case. I'd stick to what you did. /Jarkko
On Mon, 9 Aug 2021 at 15:26, Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Mon, Aug 09, 2021 at 09:52:20AM +0200, Ahmad Fatoum wrote: > > Hello Sumit, > > > > On 22.07.21 08:31, Sumit Garg wrote: > > > On Wed, 21 Jul 2021 at 22:19, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > >> > > >> The two existing trusted key sources don't make use of the kernel RNG, > > >> but instead let the hardware that does the sealing/unsealing also > > >> generate the random key material. While a previous change offers users > > >> the choice to use the kernel RNG instead for both, new trust sources > > >> may want to unconditionally use the kernel RNG for generating key > > >> material, like it's done elsewhere in the kernel. > > >> > > >> This is especially prudent for hardware that has proven-in-production > > >> HWRNG drivers implemented, as otherwise code would have to be duplicated > > >> only to arrive at a possibly worse result. > > >> > > >> Make this possible by turning struct trusted_key_ops::get_random > > >> into an optional member. If a driver leaves it NULL, kernel RNG > > >> will be used instead. > > >> > > >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > > >> --- > > >> To: James Bottomley <jejb@linux.ibm.com> > > >> To: Jarkko Sakkinen <jarkko@kernel.org> > > >> To: Mimi Zohar <zohar@linux.ibm.com> > > >> To: David Howells <dhowells@redhat.com> > > >> Cc: James Morris <jmorris@namei.org> > > >> Cc: "Serge E. Hallyn" <serge@hallyn.com> > > >> Cc: "Horia Geantă" <horia.geanta@nxp.com> > > >> Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > > >> Cc: Herbert Xu <herbert@gondor.apana.org.au> > > >> Cc: "David S. Miller" <davem@davemloft.net> > > >> Cc: Udit Agarwal <udit.agarwal@nxp.com> > > >> Cc: Eric Biggers <ebiggers@kernel.org> > > >> Cc: Jan Luebbe <j.luebbe@pengutronix.de> > > >> Cc: David Gstir <david@sigma-star.at> > > >> Cc: Richard Weinberger <richard@nod.at> > > >> Cc: Franck LENORMAND <franck.lenormand@nxp.com> > > >> Cc: Sumit Garg <sumit.garg@linaro.org> > > >> Cc: keyrings@vger.kernel.org > > >> Cc: linux-crypto@vger.kernel.org > > >> Cc: linux-integrity@vger.kernel.org > > >> Cc: linux-kernel@vger.kernel.org > > >> Cc: linux-security-module@vger.kernel.org > > >> --- > > >> include/keys/trusted-type.h | 2 +- > > >> security/keys/trusted-keys/trusted_core.c | 2 +- > > >> 2 files changed, 2 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h > > >> index d89fa2579ac0..4eb64548a74f 100644 > > >> --- a/include/keys/trusted-type.h > > >> +++ b/include/keys/trusted-type.h > > >> @@ -64,7 +64,7 @@ struct trusted_key_ops { > > >> /* Unseal a key. */ > > >> int (*unseal)(struct trusted_key_payload *p, char *datablob); > > >> > > >> - /* Get a randomized key. */ > > >> + /* Optional: Get a randomized key. */ > > >> int (*get_random)(unsigned char *key, size_t key_len); > > >> > > >> /* Exit key interface. */ > > >> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c > > >> index 569af9af8df0..d2b7626cde8b 100644 > > >> --- a/security/keys/trusted-keys/trusted_core.c > > >> +++ b/security/keys/trusted-keys/trusted_core.c > > >> @@ -334,7 +334,7 @@ static int __init init_trusted(void) > > >> continue; > > >> > > >> get_random = trusted_key_sources[i].ops->get_random; > > >> - if (trusted_kernel_rng) > > >> + if (trusted_kernel_rng || !get_random) > > >> get_random = kernel_get_random; > > >> > > > > > > For ease of understanding, I would prefer to write it as: > > > > > > get_random = trusted_key_sources[i].ops->get_random ?: > > > kernel_get_random; > > > if (trusted_kernel_rng) > > > get_random = kernel_get_random; > > > > > > With that: > > > > > > Acked-by: Sumit Garg <sumit.garg@linaro.org> > > > > I don't think it improves readability to split up the conditional. > > At least I need to take a second pass over the code to understand > > the second conditional. > > Ternary operators are pain to read, unless a super trivial case. > > I'd stick to what you did. Fair enough, I am fine with the current patch. -Sumit > > /Jarkko
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h index d89fa2579ac0..4eb64548a74f 100644 --- a/include/keys/trusted-type.h +++ b/include/keys/trusted-type.h @@ -64,7 +64,7 @@ struct trusted_key_ops { /* Unseal a key. */ int (*unseal)(struct trusted_key_payload *p, char *datablob); - /* Get a randomized key. */ + /* Optional: Get a randomized key. */ int (*get_random)(unsigned char *key, size_t key_len); /* Exit key interface. */ diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c index 569af9af8df0..d2b7626cde8b 100644 --- a/security/keys/trusted-keys/trusted_core.c +++ b/security/keys/trusted-keys/trusted_core.c @@ -334,7 +334,7 @@ static int __init init_trusted(void) continue; get_random = trusted_key_sources[i].ops->get_random; - if (trusted_kernel_rng) + if (trusted_kernel_rng || !get_random) get_random = kernel_get_random; static_call_update(trusted_key_init,
The two existing trusted key sources don't make use of the kernel RNG, but instead let the hardware that does the sealing/unsealing also generate the random key material. While a previous change offers users the choice to use the kernel RNG instead for both, new trust sources may want to unconditionally use the kernel RNG for generating key material, like it's done elsewhere in the kernel. This is especially prudent for hardware that has proven-in-production HWRNG drivers implemented, as otherwise code would have to be duplicated only to arrive at a possibly worse result. Make this possible by turning struct trusted_key_ops::get_random into an optional member. If a driver leaves it NULL, kernel RNG will be used instead. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- To: James Bottomley <jejb@linux.ibm.com> To: Jarkko Sakkinen <jarkko@kernel.org> To: Mimi Zohar <zohar@linux.ibm.com> To: David Howells <dhowells@redhat.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: "Horia Geantă" <horia.geanta@nxp.com> Cc: Aymen Sghaier <aymen.sghaier@nxp.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: Udit Agarwal <udit.agarwal@nxp.com> Cc: Eric Biggers <ebiggers@kernel.org> Cc: Jan Luebbe <j.luebbe@pengutronix.de> Cc: David Gstir <david@sigma-star.at> Cc: Richard Weinberger <richard@nod.at> Cc: Franck LENORMAND <franck.lenormand@nxp.com> Cc: Sumit Garg <sumit.garg@linaro.org> Cc: keyrings@vger.kernel.org Cc: linux-crypto@vger.kernel.org Cc: linux-integrity@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-security-module@vger.kernel.org --- include/keys/trusted-type.h | 2 +- security/keys/trusted-keys/trusted_core.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)