| Message ID | 20170608081919.zbtwdjl32vbvd7jt@thunk.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
On Thu, Jun 8, 2017 at 10:19 AM, Theodore Ts'o <tytso@mit.edu> wrote: > At the very least we probably should do a logical "uniq" on the output > (e.g., if we have complained about the previous callsite, don't whinge > about it again). That seems okay to me.
Theodore Ts'o <tytso@mit.edu> writes: > On Tue, Jun 06, 2017 at 07:48:04PM +0200, Jason A. Donenfeld wrote: >> This enables an important dmesg notification about when drivers have >> used the crng without it being seeded first. Prior, these errors would >> occur silently, and so there hasn't been a great way of diagnosing these >> types of bugs for obscure setups. By adding this as a config option, we >> can leave it on by default, so that we learn where these issues happen, >> in the field, will still allowing some people to turn it off, if they >> really know what they're doing and do not want the log entries. ... > > This patch is pretty spammy. On my KVM test kernel: > > random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = 0 > random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = 0 > random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = 0 > random: bucket_table_alloc+0x15f/0x190 get_random_u32 called with crng_init = 0 ... > > At the very least we probably should do a logical "uniq" on the output > (e.g., if we have complained about the previous callsite, don't whinge > about it again). > > commit 9d9035bc6d7871a73d7f9aada4e63cb190874a68 > Author: Theodore Ts'o <tytso@mit.edu> > Date: Thu Jun 8 04:16:59 2017 -0400 > > random: suppress duplicate crng_init=0 warnings > > Suppress duplicate CONFIG_WARN_UNSEEDED_RANDOM warnings to avoid > spamming dmesg. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> Even with this patch, it's still pretty spammy (today's linux-next): random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 Initializing random number generator... random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 random: arch_mmap_rnd+0x78/0xb0 get_random_u64 called with crng_init=0 random: load_elf_binary+0x57c/0x1550 get_random_u64 called with crng_init=0 random: arch_randomize_brk+0xa4/0xd0 get_random_u64 called with crng_init=0 Do I need to be doing anything to fix these? (this is on powerpc) cheers
Am Donnerstag, 15. Juni 2017, 13:03:48 CEST schrieb Michael Ellerman: Hi Michael, > > Even with this patch, it's still pretty spammy (today's linux-next): > I would think that the issue regarding the logging is relevant for cryptographic use cases or use cases requiring strong random numbers only. Only those use cases should be fixed eventually to wait for a fully seeded DRNG. The logged messages you present here indicate use cases where no strong security is required. It looks like that the logs show ASLR related use of random numbers. Those do not require a fully seeded ChaCha20 DRNG. IMHO, users using the get_random_u64 or get_random_u32 are use cases that do not require a fully seeded DRNG thus do not need a cryptographically strong random number. Hence, I would think that the logging should be removed from get_random_u32/u64. Yet, logging should remain for get_random_bytes which should be denominated as the interface for use cases where cryptographically strong random numbers are required. Ciao Stephan
On Thu, Jun 15, 2017 at 01:59:43PM +0200, Stephan Müller wrote: > I would think that the issue regarding the logging is relevant for > cryptographic use cases or use cases requiring strong random numbers only. > Only those use cases should be fixed eventually to wait for a fully seeded > DRNG. > > The logged messages you present here indicate use cases where no strong > security is required. It looks like that the logs show ASLR related use of > random numbers. Those do not require a fully seeded ChaCha20 DRNG. I suspect there is a range of opinions aobut whether or not ASLR requires strongly secure random numbers or not. It seems pretty clear that if we proposed using prandom_u32 for ASLR, people would object very strongly indeed, since that would make it trivially easy for attackers to circumvent ASLR protections. > IMHO, users using the get_random_u64 or get_random_u32 are use cases that do > not require a fully seeded DRNG thus do not need a cryptographically strong > random number. Hence, I would think that the logging should be removed from > get_random_u32/u64. You are effectively proposing that there ought to be a middle range of security between prandom_32, get_random_u32/get_random_u64 and get_random_bytes(). I think that's going to lead to all sorts of complexity and bugs from people not understanding when they should use get_random_u32 vs get_random_bytes versus prandom_u32. And then we'll end up needing to audit all of the callsites for get_random_u32() so they don't violate this new usage rule that you are proposing. - Ted
Am Sonntag, 18. Juni 2017, 17:46:25 CEST schrieb Theodore Ts'o: Hi Theodore, > > IMHO, users using the get_random_u64 or get_random_u32 are use cases that > > do not require a fully seeded DRNG thus do not need a cryptographically > > strong random number. Hence, I would think that the logging should be > > removed from get_random_u32/u64. > > You are effectively proposing that there ought to be a middle range of > security between prandom_32, get_random_u32/get_random_u64 and > get_random_bytes(). I think that's going to lead to all sorts of > complexity and bugs from people not understanding when they should use > get_random_u32 vs get_random_bytes versus prandom_u32. And then we'll > end up needing to audit all of the callsites for get_random_u32() so > they don't violate this new usage rule that you are proposing. I only proposed to get rid of the log messages indicating a non-seeded DRNG. But you bring up an interesting point: if it is true you say that it is hard for people to use differnent types of APIs regarding entropy and random numbers right (which I would concur with), and considering that you imply that get_random_bytes, get_random_u32 and get_random_u64 have the same security strength, why do we have these three APIs to begin with? The get_random_bytes API would then be more than enough. Ciao Stephan
On Sun, Jun 18, 2017 at 5:46 PM, Theodore Ts'o <tytso@mit.edu> wrote: > You are effectively proposing that there ought to be a middle range of > security between prandom_32, get_random_u32/get_random_u64 and > get_random_bytes(). I think that's going to lead to all sorts of > complexity and bugs from people not understanding when they should use > get_random_u32 vs get_random_bytes versus prandom_u32. And then we'll > end up needing to audit all of the callsites for get_random_u32() so > they don't violate this new usage rule that you are proposing. I agree with you wholeheartedly. get_random_* provides the secure random numbers. prandom_* provides the insecure random numbers. Introducing some kind of middle ground will result in needless complexity and inevitable bugs.
On Sun, Jun 18, 2017 at 7:55 PM, Stephan Müller <smueller@chronox.de> wrote: > But you bring up an interesting point: if it is true you say that it is hard > for people to use differnent types of APIs regarding entropy and random > numbers right (which I would concur with), and considering that you imply that > get_random_bytes, get_random_u32 and get_random_u64 have the same security > strength, why do we have these three APIs to begin with? The get_random_bytes > API would then be more than enough. Because there are efficiences we can benefit from for getting integer sized outputs. Use get_random_{u32,u64} when you want a secure random number. Use get_random_bytes when you want a longer secure random bytestring.
diff --git a/drivers/char/random.c b/drivers/char/random.c index 798f353f0d3c..3bdeef13afda 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -1481,9 +1481,14 @@ void get_random_bytes(void *buf, int nbytes) __u8 tmp[CHACHA20_BLOCK_SIZE]; #ifdef CONFIG_WARN_UNSEEDED_RANDOM - if (!crng_ready()) + static void *previous = NULL; + void *caller = (void *) _RET_IP_; + + if (!crng_ready() && (READ_ONCE(previous) != caller)) { printk(KERN_NOTICE "random: %pF get_random_bytes called " - "with crng_init = %d\n", (void *) _RET_IP_, crng_init); + "with crng_init=%d\n", caller, crng_init); + WRITE_ONCE(previous, caller); + } #endif trace_get_random_bytes(nbytes, _RET_IP_); @@ -2064,6 +2069,10 @@ u64 get_random_u64(void) bool use_lock = crng_init < 2; unsigned long flags; struct batched_entropy *batch; +#ifdef CONFIG_WARN_UNSEEDED_RANDOM + static void *previous = NULL; + void *caller = (void *) _RET_IP_; +#endif #if BITS_PER_LONG == 64 if (arch_get_random_long((unsigned long *)&ret)) @@ -2075,9 +2084,11 @@ u64 get_random_u64(void) #endif #ifdef CONFIG_WARN_UNSEEDED_RANDOM - if (!crng_ready()) + if (!crng_ready() && (READ_ONCE(previous) != caller)) { printk(KERN_NOTICE "random: %pF get_random_u64 called " - "with crng_init = %d\n", (void *) _RET_IP_, crng_init); + "with crng_init=%d\n", caller, crng_init); + WRITE_ONCE(previous, caller); + } #endif batch = &get_cpu_var(batched_entropy_u64); @@ -2102,14 +2113,20 @@ u32 get_random_u32(void) bool use_lock = crng_init < 2; unsigned long flags; struct batched_entropy *batch; +#ifdef CONFIG_WARN_UNSEEDED_RANDOM + static void *previous = NULL; + void *caller = (void *) _RET_IP_; +#endif if (arch_get_random_int(&ret)) return ret; #ifdef CONFIG_WARN_UNSEEDED_RANDOM - if (!crng_ready()) + if (!crng_ready() && READ_ONCE(previous) != caller) { printk(KERN_NOTICE "random: %pF get_random_u32 called " - "with crng_init = %d\n", (void *) _RET_IP_, crng_init); + "with crng_init=%d\n", caller, crng_init); + WRITE_ONCE(previous, caller); + } #endif batch = &get_cpu_var(batched_entropy_u32);