Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1660920 > unrolled thread

Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 13/13] random: warn when kernel uses unseeded  randomness Theodore Ts'o <tytso@mit.edu> - 2017-06-08 10:20 +0200
    Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-08 14:10 +0200
    Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness Michael Ellerman <mpe@ellerman.id.au> - 2017-06-15 13:10 +0200
      Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness Stephan Müller <smueller@chronox.de> - 2017-06-15 14:00 +0200
        Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel  uses unseeded randomness Theodore Ts'o <tytso@mit.edu> - 2017-06-18 17:50 +0200
          Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness Stephan Müller <smueller@chronox.de> - 2017-06-18 20:00 +0200
            Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel  uses unseeded randomness "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-18 21:20 +0200
          Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel  uses unseeded randomness "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-18 21:20 +0200

#1660920 — Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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.
> 
> However, we don't leave it _completely_ by default. An earlier version
> of this patch simply had `default y`. I'd really love that, but it turns
> out, this problem with unseeded randomness being used is really quite
> present and is going to take a long time to fix. Thus, as a compromise
> between log-messages-for-all and nobody-knows, this is `default y`,
> except it is also `depends on DEBUG_KERNEL`. This will ensure that the
> curious see the messages while others don't have to.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

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
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
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
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
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
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
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
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: neigh_hash_alloc+0x77/0x8f get_random_u32 called with crng_init = 0
random: neigh_hash_alloc+0x77/0x8f get_random_u32 called with crng_init = 0
random: neigh_hash_alloc+0x77/0x8f get_random_u32 called with crng_init = 0
random: neigh_hash_alloc+0x77/0x8f get_random_u32 called with crng_init = 0
random: rt_genid_init+0x24/0x2f 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).

						- Ted

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>

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);

[toc] | [next] | [standalone]

#1661092 — Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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.

[toc] | [prev] | [next] | [standalone]

#1666657 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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

[toc] | [prev] | [next] | [standalone]

#1666679 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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

[toc] | [prev] | [next] | [standalone]

#1668629 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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

[toc] | [prev] | [next] | [standalone]

#1668638 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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

[toc] | [prev] | [next] | [standalone]

#1668649 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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.

[toc] | [prev] | [next] | [standalone]

#1668648 — Re: [kernel-hardening] Re: [PATCH v4 13/13] random: warn when kernel uses unseeded randomness

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web