Hi Ted, Herbert,
I sent a question to the ATH9K RNG some time ago to the developers.
See https://www.mail-archive.com/[email protected]/msg19115.html
I have not yet received a word and I think this issue should be resolved.
Thanks
Stephan
---8<---
The ATH9K driver implements an RNG which is completely bypassing the
standard Linux HW generator logic.
The RNG may or may not deliver entropy. Considering the conservative
approach in treating entropy with respect to non-auditable sources, this
patch changes the delivered entropy value to zero. The RNG still feeds
data into the input_pool but it is assumed to have no entropy.
When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
the entropy estimation considering that a user can change that value at
boot and runtime.
Signed-off-by: Stephan Mueller <[email protected]>
---
drivers/net/wireless/ath/ath9k/rng.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index d38e50f..d63dc48 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
fail_stats = 0;
/* sleep until entropy bits under write_wakeup_threshold */
- add_hwgenerator_randomness((void *)rng_buf, bytes_read,
- ATH9K_RNG_ENTROPY(bytes_read));
+ add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
}
kfree(rng_buf);
--
2.7.4
Hi Stephan,
On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
> Hi Ted, Herbert,
>
> I sent a question to the ATH9K RNG some time ago to the developers.
> See https://www.mail-archive.com/[email protected]/msg19115.html
>
> I have not yet received a word and I think this issue should be resolved.
>
> Thanks
> Stephan
>
> ---8<---
If the above text is placed below the three dashes, "---", below ...
> The ATH9K driver implements an RNG which is completely bypassing the
> standard Linux HW generator logic.
>
> The RNG may or may not deliver entropy. Considering the conservative
> approach in treating entropy with respect to non-auditable sources, this
> patch changes the delivered entropy value to zero. The RNG still feeds
> data into the input_pool but it is assumed to have no entropy.
>
> When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> the entropy estimation considering that a user can change that value at
> boot and runtime.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
here, then the mail can be applied directly without editing.
> drivers/net/wireless/ath/ath9k/rng.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
> index d38e50f..d63dc48 100644
> --- a/drivers/net/wireless/ath/ath9k/rng.c
> +++ b/drivers/net/wireless/ath/ath9k/rng.c
> @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
> fail_stats = 0;
>
> /* sleep until entropy bits under write_wakeup_threshold */
> - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> - ATH9K_RNG_ENTROPY(bytes_read));
This is the only use of this macro. I'd remove the #define on line 25
as well.
> + add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
> }
>
> kfree(rng_buf);
Other than that,
Reviewed-by: Jason Cooper <[email protected]>
thx,
Jason.
Am Samstag, 6. August 2016, 19:45:51 CEST schrieb Jason Cooper:
Hi Jason,
> Hi Stephan,
>
> On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
> > Hi Ted, Herbert,
> >
> > I sent a question to the ATH9K RNG some time ago to the developers.
> > See
> > https://www.mail-archive.com/[email protected]/msg19115.html
> >
> > I have not yet received a word and I think this issue should be resolved.
> >
> > Thanks
> > Stephan
> >
> > ---8<---
>
> If the above text is placed below the three dashes, "---", below ...
>
> > The ATH9K driver implements an RNG which is completely bypassing the
> > standard Linux HW generator logic.
> >
> > The RNG may or may not deliver entropy. Considering the conservative
> > approach in treating entropy with respect to non-auditable sources, this
> > patch changes the delivered entropy value to zero. The RNG still feeds
> > data into the input_pool but it is assumed to have no entropy.
> >
> > When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> > the entropy estimation considering that a user can change that value at
> > boot and runtime.
> >
> > Signed-off-by: Stephan Mueller <[email protected]>
> > ---
>
> here, then the mail can be applied directly without editing.
Thank you for the hint. I will resend the patch that can be applied.
>
> > drivers/net/wireless/ath/ath9k/rng.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/rng.c
> > b/drivers/net/wireless/ath/ath9k/rng.c index d38e50f..d63dc48 100644
> > --- a/drivers/net/wireless/ath/ath9k/rng.c
> > +++ b/drivers/net/wireless/ath/ath9k/rng.c
> > @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
> >
> > fail_stats = 0;
> >
> > /* sleep until entropy bits under write_wakeup_threshold */
> >
> > - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> > - ATH9K_RNG_ENTROPY(bytes_read));
>
> This is the only use of this macro. I'd remove the #define on line 25
> as well.
My idea for leaving it was that folks who would bring the RNG into the
hwrandom framework could reuse the ideas from the original authors.
What about commenting it out with #if 0 ?
>
> > + add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
> >
> > }
> >
> > kfree(rng_buf);
>
> Other than that,
>
> Reviewed-by: Jason Cooper <[email protected]>
Thank you.
>
> thx,
>
> Jason.
--
Ciao
Stephan
Hi Stephan,
On Sat, Aug 06, 2016 at 10:03:58PM +0200, Stephan Mueller wrote:
> Am Samstag, 6. August 2016, 19:45:51 CEST schrieb Jason Cooper:
> > On Fri, Aug 05, 2016 at 05:08:14PM +0200, Stephan Mueller wrote:
...
> > > diff --git a/drivers/net/wireless/ath/ath9k/rng.c
> > > b/drivers/net/wireless/ath/ath9k/rng.c index d38e50f..d63dc48 100644
> > > --- a/drivers/net/wireless/ath/ath9k/rng.c
> > > +++ b/drivers/net/wireless/ath/ath9k/rng.c
> > > @@ -92,8 +92,7 @@ static int ath9k_rng_kthread(void *data)
> > >
> > > fail_stats = 0;
> > >
> > > /* sleep until entropy bits under write_wakeup_threshold */
> > >
> > > - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> > > - ATH9K_RNG_ENTROPY(bytes_read));
> >
> > This is the only use of this macro. I'd remove the #define on line 25
> > as well.
>
> My idea for leaving it was that folks who would bring the RNG into the
> hwrandom framework could reuse the ideas from the original authors.
>
> What about commenting it out with #if 0 ?
#if 0 is frowned upon. If that calculation is documented somewhere,
then it can be redone from the spec. If it isn't, then I'd be curious
to know where it came from.
Perhaps one of the ath9k devs can point to a document containing the
formula? We could put the reference in a comment.
thx,
Jason.
The ATH9K driver implements an RNG which is completely bypassing the
standard Linux HW generator logic.
The RNG may or may not deliver entropy. Considering the conservative
approach in treating entropy with respect to non-auditable sources, this
patch changes the delivered entropy value to zero. The RNG still feeds
data into the input_pool but it is assumed to have no entropy.
When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
the entropy estimation considering that a user can change that value at
boot and runtime.
Reviewed-by: Jason Cooper <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
drivers/net/wireless/ath/ath9k/rng.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index d38e50f..1ed8338 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -22,7 +22,6 @@
#include "ar9003_phy.h"
#define ATH9K_RNG_BUF_SIZE 320
-#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 320) >> 10) /* quality: 320/1024 */
static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size)
{
@@ -92,8 +91,7 @@ static int ath9k_rng_kthread(void *data)
fail_stats = 0;
/* sleep until entropy bits under write_wakeup_threshold */
- add_hwgenerator_randomness((void *)rng_buf, bytes_read,
- ATH9K_RNG_ENTROPY(bytes_read));
+ add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
}
kfree(rng_buf);
--
2.7.4
The entropy was evaluated by crypto expert, the analysis report show the ADC with at least 10bits and up to 22 bits of min-entropy for a 32 bits value, we conservatively assume the min-entropy is 10 bits out of 32 bits, so that's why set entropy quality to 320/1024 = 10/32. Also we have explained in the commit message why can't use the HW RNG framework.
Otherwise, your patch will cause high CPU load, as continuously read ADC data if entropy bits under write_wakeup_threshold.
--
Miaoqing
-----Original Message-----
From: Stephan Mueller [mailto:[email protected]]
Sent: Sunday, August 07, 2016 5:36 PM
To: Ted Tso <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; ath9k-devel <[email protected]>; [email protected]; [email protected]; Kalle Valo <[email protected]>; Jason Cooper <[email protected]>
Subject: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
The ATH9K driver implements an RNG which is completely bypassing the standard Linux HW generator logic.
The RNG may or may not deliver entropy. Considering the conservative approach in treating entropy with respect to non-auditable sources, this patch changes the delivered entropy value to zero. The RNG still feeds data into the input_pool but it is assumed to have no entropy.
When the ATH9K RNG changes to use the HW RNG framework, it may re-enable the entropy estimation considering that a user can change that value at boot and runtime.
Reviewed-by: Jason Cooper <[email protected]>
Signed-off-by: Stephan Mueller <[email protected]>
---
drivers/net/wireless/ath/ath9k/rng.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rng.c b/drivers/net/wireless/ath/ath9k/rng.c
index d38e50f..1ed8338 100644
--- a/drivers/net/wireless/ath/ath9k/rng.c
+++ b/drivers/net/wireless/ath/ath9k/rng.c
@@ -22,7 +22,6 @@
#include "ar9003_phy.h"
#define ATH9K_RNG_BUF_SIZE 320
-#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 320) >> 10) /* quality: 320/1024 */
static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32 buf_size) { @@ -92,8 +91,7 @@ static int ath9k_rng_kthread(void *data)
fail_stats = 0;
/* sleep until entropy bits under write_wakeup_threshold */
- add_hwgenerator_randomness((void *)rng_buf, bytes_read,
- ATH9K_RNG_ENTROPY(bytes_read));
+ add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
}
kfree(rng_buf);
--
2.7.4
Am Montag, 8. August 2016, 02:03:36 CEST schrieb Pan, Miaoqing:
Hi Miaoqing,
> The entropy was evaluated by crypto expert, the analysis report show the
> ADC with at least 10bits and up to 22 bits of min-entropy for a 32 bits
> value, we conservatively assume the min-entropy is 10 bits out of 32 bits,
> so that's why set entropy quality to 320/1024 = 10/32. Also we have
> explained in the commit message why can't use the HW RNG framework.
Where is the description of the RNG, where is the test implementation?
>
> Otherwise, your patch will cause high CPU load, as continuously read ADC
> data if entropy bits under write_wakeup_threshold.
The issue is that although you may have analyzed it, others are unable to
measure the quality of the RNG and assess the design as well as the
implementation of the RNG. This RNG is the only implementation of a hardware
RNG that per default and without being able to change it at runtime injects
data into the input_pool where the noise source cannot be audited. Note, even
other respected RNG noise sources like the Intel RDRAND will not feed into /
dev/random per default in a way that dominates all other noise sources.
I would like to be able to deactivate that noise source to the extent that it
does not cause /dev/random to unblock. The reason is that your noise source
starts to dominate all other noise sources.
If you think that this patch is a challenge because your driver starts to
spin, please help and offer another solution.
>
> --
> Miaoqing
>
> -----Original Message-----
> From: Stephan Mueller [mailto:[email protected]]
> Sent: Sunday, August 07, 2016 5:36 PM
> To: Ted Tso <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; ath9k-devel <[email protected]>;
> [email protected]; [email protected]; Kalle Valo
> <[email protected]>; Jason Cooper <[email protected]> Subject: [PATCH
> v2] RANDOM: ATH9K RNG delivers zero bits of entropy
>
> The ATH9K driver implements an RNG which is completely bypassing the
> standard Linux HW generator logic.
>
> The RNG may or may not deliver entropy. Considering the conservative
> approach in treating entropy with respect to non-auditable sources, this
> patch changes the delivered entropy value to zero. The RNG still feeds data
> into the input_pool but it is assumed to have no entropy.
>
> When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
the
> entropy estimation considering that a user can change that value at boot
> and runtime.
>
> Reviewed-by: Jason Cooper <[email protected]>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/rng.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/rng.c
> b/drivers/net/wireless/ath/ath9k/rng.c index d38e50f..1ed8338 100644
> --- a/drivers/net/wireless/ath/ath9k/rng.c
> +++ b/drivers/net/wireless/ath/ath9k/rng.c
> @@ -22,7 +22,6 @@
> #include "ar9003_phy.h"
>
> #define ATH9K_RNG_BUF_SIZE 320
> -#define ATH9K_RNG_ENTROPY(x) (((x) * 8 * 320) >> 10) /* quality: 320/1024
> */
>
> static int ath9k_rng_data_read(struct ath_softc *sc, u32 *buf, u32
> buf_size) { @@ -92,8 +91,7 @@ static int ath9k_rng_kthread(void *data)
> fail_stats = 0;
>
> /* sleep until entropy bits under write_wakeup_threshold */
> - add_hwgenerator_randomness((void *)rng_buf, bytes_read,
> - ATH9K_RNG_ENTROPY(bytes_read));
> + add_hwgenerator_randomness((void *)rng_buf, bytes_read, 0);
> }
>
> kfree(rng_buf);
> --
> 2.7.4
Ciao
Stephan
Hi Stephan, Miaoqing Pan,
On Mon, Aug 08, 2016 at 08:41:36AM +0200, Stephan Mueller wrote:
> Am Montag, 8. August 2016, 02:03:36 CEST schrieb Pan, Miaoqing:
> > The entropy was evaluated by crypto expert, the analysis report show the
> > ADC with at least 10bits and up to 22 bits of min-entropy for a 32 bits
> > value, we conservatively assume the min-entropy is 10 bits out of 32 bits,
> > so that's why set entropy quality to 320/1024 = 10/32.
Ok, so the relevant commit is:
ed14dc0af7cce ath9k: feeding entropy in kernel from ADC capture
Which refers to a previous commit:
6301566e0b2d ath9k: export HW random number generator
> > Also we have explained in the commit message why can't use the HW
> > RNG framework.
>From ed14dc0af7cce:
"""
Since ADC was not designed to be a dedicated HW RNG, we do not want to
bind it to /dev/hwrng framework directly.
"""
> Where is the description of the RNG, where is the test implementation?
> >
> > Otherwise, your patch will cause high CPU load, as continuously read ADC
> > data if entropy bits under write_wakeup_threshold.
>
> The issue is that although you may have analyzed it, others are unable to
> measure the quality of the RNG and assess the design as well as the
> implementation of the RNG. This RNG is the only implementation of a hardware
> RNG that per default and without being able to change it at runtime injects
> data into the input_pool where the noise source cannot be audited. Note, even
> other respected RNG noise sources like the Intel RDRAND will not feed into /
> dev/random per default in a way that dominates all other noise sources.
>
> I would like to be able to deactivate that noise source to the extent that it
> does not cause /dev/random to unblock. The reason is that your noise source
> starts to dominate all other noise sources.
I think the short-term problem here is the config logic:
config ATH9K_HWRNG
bool "Random number generator support"
depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
default y
If you have *any* hwrngs you want to use and you have an ath9k card
(HW_RANDOM = y and ATH9K != n), you get the behavior Stephan is pointing
out.
Short term, we should just default no here.
> If you think that this patch is a challenge because your driver starts to
> spin, please help and offer another solution.
Well, I don't buy the reasoning listed above for not using the hwrng
framework. Interrupt timings were never designed to be a source of entropy
either. We need to grab it where ever we can find it, especially on
embedded systems. Documentation/hw_random.txt even says:
"""
This data is NOT CHECKED by any fitness tests, and could potentially be
bogus (if the hardware is faulty or has been tampered with).
"""
I really don't think there's a problem with adding these sorts of
sources under char/hw_random/. I think the only thing we would be
concerned about, other than the already addressed entropy estimation,
would be constraining the data rate.
Is ath9k the only wireless card that exposes ADC registers? What about
sound cards?
thx,
Jason.
Hi Stephan,
On Mon, Aug 08, 2016 at 05:29:30PM +0000, Jason Cooper wrote:
> On Mon, Aug 08, 2016 at 08:41:36AM +0200, Stephan Mueller wrote:
...
> > If you think that this patch is a challenge because your driver starts to
> > spin, please help and offer another solution.
>
> Well, I don't buy the reasoning listed above for not using the hwrng
> framework. Interrupt timings were never designed to be a source of entropy
> either. We need to grab it where ever we can find it, especially on
> embedded systems. Documentation/hw_random.txt even says:
>
> """
> This data is NOT CHECKED by any fitness tests, and could potentially be
> bogus (if the hardware is faulty or has been tampered with).
> """
>
> I really don't think there's a problem with adding these sorts of
> sources under char/hw_random/. I think the only thing we would be
> concerned about, other than the already addressed entropy estimation,
> would be constraining the data rate.
Further research yields char/hw_random/timeriomem-rng.c
It could use an update to ->read() vice data_{present,read}(), but it's
functionally exactly what the ath9k rng is doing. :)
thx,
Jason.
Hi Jason, Stephan,
Agree with Jason's point, also understand Stephan's concern. The date rate can be roughly estimated by 'cat /dev/random |rngtest -c 1000', the average speed is 1111.294Kibits/s. I will sent the patch to disable ath9k RNG by default.
Thanks,
Miaoqing
-----Original Message-----
From: Jason Cooper [mailto:[email protected]]
Sent: Tuesday, August 09, 2016 1:30 AM
To: Stephan Mueller <[email protected]>
Cc: Pan, Miaoqing <[email protected]>; Ted Tso <[email protected]>; Sepehrdad, Pouyan <[email protected]>; [email protected]; [email protected]; [email protected]; ath9k-devel <[email protected]>; [email protected]; [email protected]; Kalle Valo <[email protected]>
Subject: Re: [PATCH v2] RANDOM: ATH9K RNG delivers zero bits of entropy
Hi Stephan, Miaoqing Pan,
On Mon, Aug 08, 2016 at 08:41:36AM +0200, Stephan Mueller wrote:
> Am Montag, 8. August 2016, 02:03:36 CEST schrieb Pan, Miaoqing:
> > The entropy was evaluated by crypto expert, the analysis report
> > show the ADC with at least 10bits and up to 22 bits of min-entropy
> > for a 32 bits value, we conservatively assume the min-entropy is 10
> > bits out of 32 bits, so that's why set entropy quality to 320/1024 = 10/32.
Ok, so the relevant commit is:
ed14dc0af7cce ath9k: feeding entropy in kernel from ADC capture
Which refers to a previous commit:
6301566e0b2d ath9k: export HW random number generator
> > Also we have explained in the commit message why can't use the HW
> > RNG framework.
>From ed14dc0af7cce:
"""
Since ADC was not designed to be a dedicated HW RNG, we do not want to bind it to /dev/hwrng framework directly.
"""
> Where is the description of the RNG, where is the test implementation?
> >
> > Otherwise, your patch will cause high CPU load, as continuously
> > read ADC data if entropy bits under write_wakeup_threshold.
>
> The issue is that although you may have analyzed it, others are unable
> to measure the quality of the RNG and assess the design as well as the
> implementation of the RNG. This RNG is the only implementation of a
> hardware RNG that per default and without being able to change it at
> runtime injects data into the input_pool where the noise source cannot
> be audited. Note, even other respected RNG noise sources like the
> Intel RDRAND will not feed into / dev/random per default in a way that dominates all other noise sources.
>
> I would like to be able to deactivate that noise source to the extent
> that it does not cause /dev/random to unblock. The reason is that your
> noise source starts to dominate all other noise sources.
I think the short-term problem here is the config logic:
config ATH9K_HWRNG
bool "Random number generator support"
depends on ATH9K && (HW_RANDOM = y || HW_RANDOM = ATH9K)
default y
If you have *any* hwrngs you want to use and you have an ath9k card (HW_RANDOM = y and ATH9K != n), you get the behavior Stephan is pointing out.
Short term, we should just default no here.
> If you think that this patch is a challenge because your driver starts
> to spin, please help and offer another solution.
Well, I don't buy the reasoning listed above for not using the hwrng framework. Interrupt timings were never designed to be a source of entropy either. We need to grab it where ever we can find it, especially on embedded systems. Documentation/hw_random.txt even says:
"""
This data is NOT CHECKED by any fitness tests, and could potentially be bogus (if the hardware is faulty or has been tampered with).
"""
I really don't think there's a problem with adding these sorts of sources under char/hw_random/. I think the only thing we would be concerned about, other than the already addressed entropy estimation, would be constraining the data rate.
Is ath9k the only wireless card that exposes ADC registers? What about sound cards?
thx,
Jason.
On Tue, Aug 09, 2016 at 06:30:03AM +0000, Pan, Miaoqing wrote:
> Agree with Jason's point, also understand Stephan's concern. The
> date rate can be roughly estimated by 'cat /dev/random |rngtest -c
> 1000', the average speed is 1111.294Kibits/s. I will sent the patch
> to disable ath9k RNG by default.
If the ATH9K is generating some random amount of data, but you don't
know how random, and you're gathering it opportunistically --- for
example, suppose a wireless driver gets the radio's signal strength
through the normal course of its operation, and while there might not
be that much randomness for someone who can observe the exact details
of how the phone is positioned in the room --- but for which the
analyst sitting in Fort Meade won't know whether or not the phone is
on the desk, or in a knapsack under the table, the right interface to
use is:
void add_device_randomness(const void *buf, unsigned int size);
This won't increase the entropy count, but if you have the bit of
potential randomness "for free", you might as well contribute it to
the entropy pool. If it turns out that the chip was manufactured in
China, and the MSS has backdoored it out the wazoo, it won't do any
harm --- where as using the hwrng framework would be disastrous.
Cheerfs,
- Ted
Hi Ted,
On Tue, Aug 09, 2016 at 07:56:22AM -0400, Theodore Ts'o wrote:
> On Tue, Aug 09, 2016 at 06:30:03AM +0000, Pan, Miaoqing wrote:
> > Agree with Jason's point, also understand Stephan's concern. The
> > date rate can be roughly estimated by 'cat /dev/random |rngtest -c
> > 1000', the average speed is 1111.294Kibits/s. I will sent the patch
> > to disable ath9k RNG by default.
>
> If the ATH9K is generating some random amount of data, but you don't
> know how random, and you're gathering it opportunistically --- for
> example, suppose a wireless driver gets the radio's signal strength
> through the normal course of its operation, and while there might not
> be that much randomness for someone who can observe the exact details
> of how the phone is positioned in the room --- but for which the
> analyst sitting in Fort Meade won't know whether or not the phone is
> on the desk, or in a knapsack under the table, the right interface to
> use is:
>
> void add_device_randomness(const void *buf, unsigned int size);
>
> This won't increase the entropy count, but if you have the bit of
> potential randomness "for free", you might as well contribute it to
> the entropy pool. If it turns out that the chip was manufactured in
> China, and the MSS has backdoored it out the wazoo, it won't do any
> harm --- where as using the hwrng framework would be disastrous.
Ok, I get that. However, we have Documentation/hw_random.txt saying:
This data is NOT CHECKED by any fitness tests, and could potentially be
bogus (if the hardware is faulty or has been tampered with). Data is
only output if the hardware "has-data" flag is set, but nevertheless a
security-conscious person would run fitness tests on the data before
assuming it is truly random.
Which I would read as "Don't assume 1 bit read from /dev/hwrng equals 1
bit of entropy." Which runs counter to Stephan's reading of the rngd
code.
And then we have drivers like timeriomem-rng.c that appear to be
spitting out the raw 32bit register value of $SOC's timer.
Thankfully, most hw_random drivers don't set the quality. So unless the
user sets the default_quality param, it's zero.
iiuc, Ted, you're saying using the hw_random framework would be
disasterous because despite most drivers having a default quality of 0,
rngd assumes 1 bit of entropy for every bit read?
thx,
Jason.
On Tue, Aug 09, 2016 at 02:04:44PM +0000, Jason Cooper wrote:
>
> iiuc, Ted, you're saying using the hw_random framework would be
> disasterous because despite most drivers having a default quality of 0,
> rngd assumes 1 bit of entropy for every bit read?
Sorry, what I was trying to say (but failed) was that bypassing the
hwrng framework and injecting entropy directly the entropy pool was
disatrous.
> Thankfully, most hw_random drivers don't set the quality. So unless the
> user sets the default_quality param, it's zero.
The fact that this is "most" and not "all" does scare me a little.
As far as I'm concerned *all* hw_random drivers should set quality to
zero, since it should be up to the system administrator. Perhaps the
one exception is virtio_rng, since if you don't trust the hypvervisor,
the security of the VM is hopeless. That being said, I have seen
configurations of KVM which use:
-object rng-random,filename=/dev/urandom,id=rng0 \
-device virtio-rng-pci,rng=rng0
Which is somewhat non-ideal. (Try running od -x /dev/random on such a
guest system....)
- Ted
Hey Ted,
On Wed, Aug 10, 2016 at 07:44:25PM -0400, Theodore Ts'o wrote:
> On Tue, Aug 09, 2016 at 02:04:44PM +0000, Jason Cooper wrote:
> > iiuc, Ted, you're saying using the hw_random framework would be
> > disasterous because despite most drivers having a default quality of 0,
> > rngd assumes 1 bit of entropy for every bit read?
>
> Sorry, what I was trying to say (but failed) was that bypassing the
> hwrng framework and injecting entropy directly the entropy pool was
> disatrous.
Ok, whew. :)
> > Thankfully, most hw_random drivers don't set the quality. So unless the
> > user sets the default_quality param, it's zero.
>
> The fact that this is "most" and not "all" does scare me a little.
My recent grep showed that only virtio-rng set it to a non-zero value.
> As far as I'm concerned *all* hw_random drivers should set quality to
> zero, since it should be up to the system administrator.
Agreed.
Gathering conversation about this from a few related threads, I have one
concern. Apparently there is some confusion in userspace consumers of
/dev/hwrng data as to the quality of it. Specifically, rngd (spotted by
Stephan Mueller) appears to assume 1bit of entropy per 1 bit read. :-/
So, while moving ath9k-rng to the hwrng framework makes complete sense
internally, it's not so good for existing userspace assumptions. I'd
think that timeriomem-rng falls in this same category.
In light of this, do you think it's worth the effort (I'm volunteering)
to create a subcategory of hwrng drivers that are 'environemntal' rngs?
They can contribute to the kernel entropy pools, but not to /dev/hwrng.
thx,
Jason.
"Pan, Miaoqing" <[email protected]> writes:
> The entropy was evaluated by crypto expert, the analysis report show
> the ADC with at least 10bits and up to 22 bits of min-entropy for a 32
> bits value, we conservatively assume the min-entropy is 10 bits out of
> 32 bits, so that's why set entropy quality to 320/1024 = 10/32. Also
> we have explained in the commit message why can't use the HW RNG
> framework.
>
> Otherwise, your patch will cause high CPU load, as continuously read
> ADC data if entropy bits under write_wakeup_threshold.
Please don't top post, it breaks patchwork which is extremely annoying
for me:
https://patchwork.kernel.org/patch/9266265/
https://patchwork.kernel.org/patch/9266617/
--
Kalle Valo