| Message ID | 20170719074458.9247-1-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On 7/19/2017 10:45 AM, Oleksij Rempel wrote: > According documentation, it is NIST certified TRNG. > So, set high quality to let the HWRNG framework automatically use it. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > drivers/crypto/caam/caamrng.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c > index 41398da3edf4..684c0bc88dfd 100644 > --- a/drivers/crypto/caam/caamrng.c > +++ b/drivers/crypto/caam/caamrng.c > @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) > return 0; > } > > +/* > + * hwrng register struct > + * The trng is suppost to have 100% entropy, and thus > + * we register with a very high quality value. > + */ > static struct hwrng caam_rng = { > .name = "rng-caam", > .cleanup = caam_cleanup, > .read = caam_read, > + .quality = 999, Why not 1024, i.e. where is 999 coming from? Thanks, Horia
On Wed, Jul 19, 2017 at 12:49:47PM +0000, Horia Geantă wrote: > On 7/19/2017 10:45 AM, Oleksij Rempel wrote: > > According documentation, it is NIST certified TRNG. > > So, set high quality to let the HWRNG framework automatically use it. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > drivers/crypto/caam/caamrng.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c > > index 41398da3edf4..684c0bc88dfd 100644 > > --- a/drivers/crypto/caam/caamrng.c > > +++ b/drivers/crypto/caam/caamrng.c > > @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) > > return 0; > > } > > > > +/* > > + * hwrng register struct > > + * The trng is suppost to have 100% entropy, and thus > > + * we register with a very high quality value. > > + */ > > static struct hwrng caam_rng = { > > .name = "rng-caam", > > .cleanup = caam_cleanup, > > .read = caam_read, > > + .quality = 999, > > Why not 1024, i.e. where is 999 coming from? It comes from s390-trng.c driver. Should I use 1024 instead?
On 7/19/2017 7:32 PM, Oleksij Rempel wrote: > On Wed, Jul 19, 2017 at 12:49:47PM +0000, Horia Geantă wrote: >> On 7/19/2017 10:45 AM, Oleksij Rempel wrote: >>> According documentation, it is NIST certified TRNG. >>> So, set high quality to let the HWRNG framework automatically use it. >>> >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>> --- >>> drivers/crypto/caam/caamrng.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c >>> index 41398da3edf4..684c0bc88dfd 100644 >>> --- a/drivers/crypto/caam/caamrng.c >>> +++ b/drivers/crypto/caam/caamrng.c >>> @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) >>> return 0; >>> } >>> >>> +/* >>> + * hwrng register struct >>> + * The trng is suppost to have 100% entropy, and thus >>> + * we register with a very high quality value. >>> + */ >>> static struct hwrng caam_rng = { >>> .name = "rng-caam", >>> .cleanup = caam_cleanup, >>> .read = caam_read, >>> + .quality = 999, >> >> Why not 1024, i.e. where is 999 coming from? > > It comes from s390-trng.c driver. > Should I use 1024 instead? > AFAICT the range for quality is from 0 to 1024 (no entropy -> perfect entropy). 1024 should be used since I'd expect a HW TRNG to provide perfect entropy unless otherwise stated. [Cc-ing Harald and Martin for awareness wrt. s390/trng.] Thanks, Horia
On Wed, Jul 19, 2017 at 04:53:21PM +0000, Horia Geantă wrote: > On 7/19/2017 7:32 PM, Oleksij Rempel wrote: > > On Wed, Jul 19, 2017 at 12:49:47PM +0000, Horia Geantă wrote: > >> On 7/19/2017 10:45 AM, Oleksij Rempel wrote: > >>> According documentation, it is NIST certified TRNG. > >>> So, set high quality to let the HWRNG framework automatically use it. > >>> > >>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >>> --- > >>> drivers/crypto/caam/caamrng.c | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c > >>> index 41398da3edf4..684c0bc88dfd 100644 > >>> --- a/drivers/crypto/caam/caamrng.c > >>> +++ b/drivers/crypto/caam/caamrng.c > >>> @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) > >>> return 0; > >>> } > >>> > >>> +/* > >>> + * hwrng register struct > >>> + * The trng is suppost to have 100% entropy, and thus > >>> + * we register with a very high quality value. > >>> + */ > >>> static struct hwrng caam_rng = { > >>> .name = "rng-caam", > >>> .cleanup = caam_cleanup, > >>> .read = caam_read, > >>> + .quality = 999, > >> > >> Why not 1024, i.e. where is 999 coming from? > > > > It comes from s390-trng.c driver. > > Should I use 1024 instead? > > > AFAICT the range for quality is from 0 to 1024 (no entropy -> perfect > entropy). > > 1024 should be used since I'd expect a HW TRNG to provide perfect > entropy unless otherwise stated. I assume 1024 can be given only on verified HW with accessible verilog files and compared with resulting chip :) May be this would be a good example https://www.sifive.com/
On 07/19/2017 08:13 PM, Oleksij Rempel wrote: > On Wed, Jul 19, 2017 at 04:53:21PM +0000, Horia Geantă wrote: >> On 7/19/2017 7:32 PM, Oleksij Rempel wrote: >>> On Wed, Jul 19, 2017 at 12:49:47PM +0000, Horia Geantă wrote: >>>> On 7/19/2017 10:45 AM, Oleksij Rempel wrote: >>>>> According documentation, it is NIST certified TRNG. >>>>> So, set high quality to let the HWRNG framework automatically use it. >>>>> >>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>> --- >>>>> drivers/crypto/caam/caamrng.c | 6 ++++++ >>>>> 1 file changed, 6 insertions(+) >>>>> >>>>> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c >>>>> index 41398da3edf4..684c0bc88dfd 100644 >>>>> --- a/drivers/crypto/caam/caamrng.c >>>>> +++ b/drivers/crypto/caam/caamrng.c >>>>> @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) >>>>> return 0; >>>>> } >>>>> >>>>> +/* >>>>> + * hwrng register struct >>>>> + * The trng is suppost to have 100% entropy, and thus >>>>> + * we register with a very high quality value. >>>>> + */ >>>>> static struct hwrng caam_rng = { >>>>> .name = "rng-caam", >>>>> .cleanup = caam_cleanup, >>>>> .read = caam_read, >>>>> + .quality = 999, >>>> Why not 1024, i.e. where is 999 coming from? >>> It comes from s390-trng.c driver. >>> Should I use 1024 instead? >>> >> AFAICT the range for quality is from 0 to 1024 (no entropy -> perfect >> entropy). >> >> 1024 should be used since I'd expect a HW TRNG to provide perfect >> entropy unless otherwise stated. > I assume 1024 can be given only on verified HW with accessible verilog > files and compared with resulting chip :) > May be this would be a good example https://www.sifive.com/ > Well, the header file says: ... /** * struct hwrng - Hardware Random Number Generator driver * @name: Unique RNG name. * @init: Initialization callback (can be NULL). * @cleanup: Cleanup callback (can be NULL). * @data_present: Callback to determine if data is available * on the RNG. If NULL, it is assumed that * there is always data available. *OBSOLETE* * @data_read: Read data from the RNG device. * Returns the number of lower random bytes in "data". * Must not be NULL. *OBSOLETE* * @read: New API. drivers can fill up to max bytes of data * into the buffer. The buffer is aligned for any type * and max is a multiple of 4 and >= 32 bytes. * @priv: Private data, for use by the RNG driver. * @quality: Estimation of true entropy in RNG's bitstream * (per mill). */ ... quality = estimation of true entropy per mill. I understand this as quality=1000 meaning 100% entropy. However, the core code currently does not really check this value. When more than one hwrng sources do register, simple the one with the higher quality value wins :-) The value is not even checked to be in a given range. I searched through some device drivers which do register at the hwrng and it looks like most of the drivers do not even set this value. My feeling is, you should use 999 when your hardware provides 'perfect' random. So there is a chance for an even 'more perfect' hardware coming up later to overrule your 'perfect' hardware. regards Harald Freudenberger
On 7/20/2017 4:08 PM, Harald Freudenberger wrote: > On 07/19/2017 08:13 PM, Oleksij Rempel wrote: >> On Wed, Jul 19, 2017 at 04:53:21PM +0000, Horia Geantă wrote: >>> On 7/19/2017 7:32 PM, Oleksij Rempel wrote: >>>> On Wed, Jul 19, 2017 at 12:49:47PM +0000, Horia Geantă wrote: >>>>> On 7/19/2017 10:45 AM, Oleksij Rempel wrote: >>>>>> According documentation, it is NIST certified TRNG. >>>>>> So, set high quality to let the HWRNG framework automatically use it. >>>>>> >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>> --- >>>>>> drivers/crypto/caam/caamrng.c | 6 ++++++ >>>>>> 1 file changed, 6 insertions(+) >>>>>> >>>>>> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c >>>>>> index 41398da3edf4..684c0bc88dfd 100644 >>>>>> --- a/drivers/crypto/caam/caamrng.c >>>>>> +++ b/drivers/crypto/caam/caamrng.c >>>>>> @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +/* >>>>>> + * hwrng register struct >>>>>> + * The trng is suppost to have 100% entropy, and thus >>>>>> + * we register with a very high quality value. >>>>>> + */ >>>>>> static struct hwrng caam_rng = { >>>>>> .name = "rng-caam", >>>>>> .cleanup = caam_cleanup, >>>>>> .read = caam_read, >>>>>> + .quality = 999, >>>>> Why not 1024, i.e. where is 999 coming from? >>>> It comes from s390-trng.c driver. >>>> Should I use 1024 instead? >>>> >>> AFAICT the range for quality is from 0 to 1024 (no entropy -> perfect >>> entropy). >>> >>> 1024 should be used since I'd expect a HW TRNG to provide perfect >>> entropy unless otherwise stated. >> I assume 1024 can be given only on verified HW with accessible verilog >> files and compared with resulting chip :) >> May be this would be a good example https://www.sifive.com/ >> > Well, the header file says: > ... > /** > * struct hwrng - Hardware Random Number Generator driver > * @name: Unique RNG name. > * @init: Initialization callback (can be NULL). > * @cleanup: Cleanup callback (can be NULL). > * @data_present: Callback to determine if data is available > * on the RNG. If NULL, it is assumed that > * there is always data available. *OBSOLETE* > * @data_read: Read data from the RNG device. > * Returns the number of lower random bytes in "data". > * Must not be NULL. *OBSOLETE* > * @read: New API. drivers can fill up to max bytes of data > * into the buffer. The buffer is aligned for any type > * and max is a multiple of 4 and >= 32 bytes. > * @priv: Private data, for use by the RNG driver. > * @quality: Estimation of true entropy in RNG's bitstream > * (per mill). > */ > ... > quality = estimation of true entropy per mill. "per mill as in https://en.wikipedia.org/wiki/Mill_(currency) ? I consider it rather unfortunate, as already noticed here: https://lkml.org/lkml/2014/3/27/210 And isn't this inaccurate, since the (de)rating factor is quality/1024, not quality/1000? > I understand this as quality=1000 meaning 100% entropy. > However, the core code currently does not really check this value. > When more than one hwrng sources do register, simple the one with > the higher quality value wins :-) The value is not even checked > to be in a given range. > > I searched through some device drivers which do register at > the hwrng and it looks like most of the drivers do not even > set this value. My feeling is, you should use 999 when your Maybe this is because it's not clear how to determine quality's value? Take CAAM's engine HWRNG: it can work both as a TRNG and as a TRNG-seeded DRBG (that's how it's currently configured). IIUC, both setups are fit as source for the entropy pool. Do I have to set quality value comparing the two cases? (It's a bit like comparing the quality of entropy offered by RDSEED vs. RDRAND.) Meaning: give full credit - maximum quality - for the TRNG setup, and provide a lower value for quality in the case of TRNG-seeded DRBG? > hardware provides 'perfect' random. So there is a chance for > an even 'more perfect' hardware coming up later to overrule > your 'perfect' hardware. I am not sure why the hwrng with the best quality wins, instead of using all available resources, as suggested here: https://lkml.org/lkml/2014/3/27/210 Thanks, Horia
On Wed, Aug 02, 2017 at 02:03:14PM +0000, Horia Geantă wrote: > > Take CAAM's engine HWRNG: it can work both as a TRNG and as a > TRNG-seeded DRBG (that's how it's currently configured). > IIUC, both setups are fit as source for the entropy pool. So which is it? If it's a DRBG then it should not be exposed through the hwrng interface. Only TRNG should go through hwrng. DRBGs can use the crypto rng API. Cheers,
On 8/3/2017 6:17 AM, Herbert Xu wrote: > On Wed, Aug 02, 2017 at 02:03:14PM +0000, Horia Geantă wrote: >> >> Take CAAM's engine HWRNG: it can work both as a TRNG and as a >> TRNG-seeded DRBG (that's how it's currently configured). >> IIUC, both setups are fit as source for the entropy pool. > > So which is it? If it's a DRBG then it should not be exposed through > the hwrng interface. Only TRNG should go through hwrng. DRBGs > can use the crypto rng API. Right now it's configured as a DRBG. If I read correctly, it doesn't matter it's using the internal TRNG for (automated) seeding, it still shouldn't use hwrng. This means it's broken since the very beginning: e24f7c9e87d4 crypto: caam - hwrng support Thanks, Horia
On 03.08.2017 09:48, Horia Geantă wrote: > On 8/3/2017 6:17 AM, Herbert Xu wrote: >> On Wed, Aug 02, 2017 at 02:03:14PM +0000, Horia Geantă wrote: >>> >>> Take CAAM's engine HWRNG: it can work both as a TRNG and as a >>> TRNG-seeded DRBG (that's how it's currently configured). >>> IIUC, both setups are fit as source for the entropy pool. >> >> So which is it? If it's a DRBG then it should not be exposed through >> the hwrng interface. Only TRNG should go through hwrng. DRBGs >> can use the crypto rng API. > > Right now it's configured as a DRBG. > If I read correctly, it doesn't matter it's using the internal TRNG for > (automated) seeding, it still shouldn't use hwrng. > This means it's broken since the very beginning: > e24f7c9e87d4 crypto: caam - hwrng support Hmmm.. - what is the security risk of this issue? For example system which use /dev/hwrng directly? - And who will fix it?
On Thu, Aug 03, 2017 at 07:48:51AM +0000, Horia Geantă wrote: > > Right now it's configured as a DRBG. > If I read correctly, it doesn't matter it's using the internal TRNG for > (automated) seeding, it still shouldn't use hwrng. > This means it's broken since the very beginning: > e24f7c9e87d4 crypto: caam - hwrng support Right. If it can be switched over to a TRNG then we should do that. Otherwise it should be converted over to use the crypto rng API. Thanks,
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c index 41398da3edf4..684c0bc88dfd 100644 --- a/drivers/crypto/caam/caamrng.c +++ b/drivers/crypto/caam/caamrng.c @@ -292,10 +292,16 @@ static int caam_init_rng(struct caam_rng_ctx *ctx, struct device *jrdev) return 0; } +/* + * hwrng register struct + * The trng is suppost to have 100% entropy, and thus + * we register with a very high quality value. + */ static struct hwrng caam_rng = { .name = "rng-caam", .cleanup = caam_cleanup, .read = caam_read, + .quality = 999, }; static void __exit caam_rng_exit(void)
According documentation, it is NIST certified TRNG. So, set high quality to let the HWRNG framework automatically use it. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/crypto/caam/caamrng.c | 6 ++++++ 1 file changed, 6 insertions(+)