All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] prandom_u32: make output less predictable
@ 2020-09-01  6:43 Willy Tarreau
  2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01  6:43 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Sedat Dilek, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

This is the cleanup of the latest series of prandom_u32 experimentations
consisting in using SipHash instead of Tausworthe to produce the randoms
used by the network stack. The changes to the files were kept minimal,
and the controversial commit that used to take noise from the fast_pool
(f227e3ec3b5c) was reverted. Instead, a dedicated "net_rand_noise" per_cpu
variable is fed from various sources of activities (networking, scheduling)
to perturb the SipHash state using fast, non-trivially predictable data,
instead of keeping it fully deterministic. The goal is essentially to make
any occasional memory leakage or brute-force attempt useless.

The resulting code was verified to be very slightly faster on x86_64 than
what is was with the controversial commit above, though this remains barely
above measurement noise. It was only build-tested on arm & arm64.

George Spelvin (1):
  random32: make prandom_u32() output unpredictable

Willy Tarreau (1):
  random32: add noise from network and scheduling activity

 drivers/char/random.c   |   1 -
 include/linux/prandom.h |  55 ++++-
 kernel/time/timer.c     |   9 +-
 lib/random32.c          | 438 ++++++++++++++++++++++++----------------
 net/core/dev.c          |   4 +
 5 files changed, 326 insertions(+), 181 deletions(-)

Cc: George Spelvin <lkml@sdf.org>
Cc: Amit Klein <aksecurity@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: tytso@mit.edu
Cc: Florian Westphal <fw@strlen.de>
Cc: Marc Plumb <lkml.mplumb@gmail.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>

-- 
2.28.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  6:43 [PATCH 0/2] prandom_u32: make output less predictable Willy Tarreau
@ 2020-09-01  6:43 ` Willy Tarreau
  2020-09-01  8:33   ` Yann Ylavic
  2020-09-01 13:10   ` David Laight
  2020-09-01  6:43 ` [PATCH 2/2] random32: add noise from network and scheduling activity Willy Tarreau
  2020-09-01 14:41 ` [PATCH 0/2] prandom_u32: make output less predictable Sedat Dilek
  2 siblings, 2 replies; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01  6:43 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Sedat Dilek, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

From: George Spelvin <lkml@sdf.org>

Non-cryptographic PRNGs may have great statistical proprties, but
are usually trivially predictable to someone who knows the algorithm,
given a small sample of their output.  An LFSR like prandom_u32() is
particularly simple, even if the sample is widely scattered bits.

It turns out the network stack uses prandom_u32() for some things like
random port numbers which it would prefer are *not* trivially predictable.
Predictability led to a practical DNS spoofing attack.  Oops.

This patch replaces the LFSR with a homebrew cryptographic PRNG based
on the SipHash round function, which is in turn seeded with 128 bits
of strong random key.  (The authors of SipHash have *not* been consulted
about this abuse of their algorithm.)  Speed is prioritized over security;
attacks are rare, while performance is always wanted.

Replacing all callers of prandom_u32() is the quick fix.
Whether to reinstate a weaker PRNG for uses which can tolerate it
is an open question.

Commit f227e3ec3b5c ("random32: update the net random state on interrupt
and activity") was an earlier attempt at a solution.  This patch replaces
it.

Reported-by: Amit Klein <aksecurity@gmail.com>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: tytso@mit.edu
Cc: Florian Westphal <fw@strlen.de>
Cc: Marc Plumb <lkml.mplumb@gmail.com>
Fixes: f227e3ec3b5c ("random32: update the net random state on interrupt and activity")
Signed-off-by: George Spelvin <lkml@sdf.org>
Link: https://lore.kernel.org/netdev/20200808152628.GA27941@SDF.ORG/
[ willy: partial reversal of f227e3ec3b5c; moved SIPROUND definitions
  to prandom.h for later use; merged George's prandom_seed() proposal;
  inlined siprand_u32(); replaced the net_rand_state[] array with 4
  members to fix a build issue; cosmetic cleanups to make checkpatch
  happy ]
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 drivers/char/random.c   |   1 -
 include/linux/prandom.h |  36 +++-
 kernel/time/timer.c     |   7 -
 lib/random32.c          | 433 ++++++++++++++++++++++++----------------
 4 files changed, 296 insertions(+), 181 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d20ba1b104ca..2a41b21623ae 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1277,7 +1277,6 @@ void add_interrupt_randomness(int irq, int irq_flags)
 
 	fast_mix(fast_pool);
 	add_interrupt_bench(cycles);
-	this_cpu_add(net_rand_state.s1, fast_pool->pool[cycles & 3]);
 
 	if (unlikely(crng_init == 0)) {
 		if ((fast_pool->count >= 64) &&
diff --git a/include/linux/prandom.h b/include/linux/prandom.h
index aa16e6468f91..cc1e71334e53 100644
--- a/include/linux/prandom.h
+++ b/include/linux/prandom.h
@@ -16,12 +16,44 @@ void prandom_bytes(void *buf, size_t nbytes);
 void prandom_seed(u32 seed);
 void prandom_reseed_late(void);
 
+#if BITS_PER_LONG == 64
+/*
+ * The core SipHash round function.  Each line can be executed in
+ * parallel given enough CPU resources.
+ */
+#define PRND_SIPROUND(v0, v1, v2, v3) ( \
+	v0 += v1, v1 = rol64(v1, 13),  v2 += v3, v3 = rol64(v3, 16), \
+	v1 ^= v0, v0 = rol64(v0, 32),  v3 ^= v2,                     \
+	v0 += v3, v3 = rol64(v3, 21),  v2 += v1, v1 = rol64(v1, 17), \
+	v3 ^= v0,                      v1 ^= v2, v2 = rol64(v2, 32)  \
+)
+
+#define PRND_K0 (0x736f6d6570736575 ^ 0x6c7967656e657261)
+#define PRND_K1 (0x646f72616e646f6d ^ 0x7465646279746573)
+
+#elif BITS_PER_LONG == 32
+/*
+ * On 32-bit machines, we use HSipHash, a reduced-width version of SipHash.
+ * This is weaker, but 32-bit machines are not used for high-traffic
+ * applications, so there is less output for an attacker to analyze.
+ */
+#define PRND_SIPROUND(v0, v1, v2, v3) ( \
+	v0 += v1, v1 = rol32(v1,  5),  v2 += v3, v3 = rol32(v3,  8), \
+	v1 ^= v0, v0 = rol32(v0, 16),  v3 ^= v2,                     \
+	v0 += v3, v3 = rol32(v3,  7),  v2 += v1, v1 = rol32(v1, 13), \
+	v3 ^= v0,                      v1 ^= v2, v2 = rol32(v2, 16)  \
+)
+#define PRND_K0 0x6c796765
+#define PRND_K1 0x74656462
+
+#else
+#error Unsupported BITS_PER_LONG
+#endif
+
 struct rnd_state {
 	__u32 s1, s2, s3, s4;
 };
 
-DECLARE_PER_CPU(struct rnd_state, net_rand_state);
-
 u32 prandom_u32_state(struct rnd_state *state);
 void prandom_bytes_state(struct rnd_state *state, void *buf, size_t nbytes);
 void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state);
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index a50364df1054..401fcb9d7388 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1715,13 +1715,6 @@ void update_process_times(int user_tick)
 	scheduler_tick();
 	if (IS_ENABLED(CONFIG_POSIX_TIMERS))
 		run_posix_cpu_timers();
-
-	/* The current CPU might make use of net randoms without receiving IRQs
-	 * to renew them often enough. Let's update the net_rand_state from a
-	 * non-constant value that's not affine to the number of calls to make
-	 * sure it's updated when there's some activity (we don't care in idle).
-	 */
-	this_cpu_add(net_rand_state.s1, rol32(jiffies, 24) + user_tick);
 }
 
 /**
diff --git a/lib/random32.c b/lib/random32.c
index 932345323af0..00fa925a4487 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -49,8 +49,6 @@ static inline void prandom_state_selftest(void)
 }
 #endif
 
-DEFINE_PER_CPU(struct rnd_state, net_rand_state);
-
 /**
  *	prandom_u32_state - seeded pseudo-random number generator.
  *	@state: pointer to state structure holding seeded state.
@@ -70,26 +68,6 @@ u32 prandom_u32_state(struct rnd_state *state)
 }
 EXPORT_SYMBOL(prandom_u32_state);
 
-/**
- *	prandom_u32 - pseudo random number generator
- *
- *	A 32 bit pseudo-random number is generated using a fast
- *	algorithm suitable for simulation. This algorithm is NOT
- *	considered safe for cryptographic use.
- */
-u32 prandom_u32(void)
-{
-	struct rnd_state *state = &get_cpu_var(net_rand_state);
-	u32 res;
-
-	res = prandom_u32_state(state);
-	trace_prandom_u32(res);
-	put_cpu_var(net_rand_state);
-
-	return res;
-}
-EXPORT_SYMBOL(prandom_u32);
-
 /**
  *	prandom_bytes_state - get the requested number of pseudo-random bytes
  *
@@ -121,20 +99,6 @@ void prandom_bytes_state(struct rnd_state *state, void *buf, size_t bytes)
 }
 EXPORT_SYMBOL(prandom_bytes_state);
 
-/**
- *	prandom_bytes - get the requested number of pseudo-random bytes
- *	@buf: where to copy the pseudo-random bytes to
- *	@bytes: the requested number of bytes
- */
-void prandom_bytes(void *buf, size_t bytes)
-{
-	struct rnd_state *state = &get_cpu_var(net_rand_state);
-
-	prandom_bytes_state(state, buf, bytes);
-	put_cpu_var(net_rand_state);
-}
-EXPORT_SYMBOL(prandom_bytes);
-
 static void prandom_warmup(struct rnd_state *state)
 {
 	/* Calling RNG ten times to satisfy recurrence condition */
@@ -150,96 +114,6 @@ static void prandom_warmup(struct rnd_state *state)
 	prandom_u32_state(state);
 }
 
-static u32 __extract_hwseed(void)
-{
-	unsigned int val = 0;
-
-	(void)(arch_get_random_seed_int(&val) ||
-	       arch_get_random_int(&val));
-
-	return val;
-}
-
-static void prandom_seed_early(struct rnd_state *state, u32 seed,
-			       bool mix_with_hwseed)
-{
-#define LCG(x)	 ((x) * 69069U)	/* super-duper LCG */
-#define HWSEED() (mix_with_hwseed ? __extract_hwseed() : 0)
-	state->s1 = __seed(HWSEED() ^ LCG(seed),        2U);
-	state->s2 = __seed(HWSEED() ^ LCG(state->s1),   8U);
-	state->s3 = __seed(HWSEED() ^ LCG(state->s2),  16U);
-	state->s4 = __seed(HWSEED() ^ LCG(state->s3), 128U);
-}
-
-/**
- *	prandom_seed - add entropy to pseudo random number generator
- *	@entropy: entropy value
- *
- *	Add some additional entropy to the prandom pool.
- */
-void prandom_seed(u32 entropy)
-{
-	int i;
-	/*
-	 * No locking on the CPUs, but then somewhat random results are, well,
-	 * expected.
-	 */
-	for_each_possible_cpu(i) {
-		struct rnd_state *state = &per_cpu(net_rand_state, i);
-
-		state->s1 = __seed(state->s1 ^ entropy, 2U);
-		prandom_warmup(state);
-	}
-}
-EXPORT_SYMBOL(prandom_seed);
-
-/*
- *	Generate some initially weak seeding values to allow
- *	to start the prandom_u32() engine.
- */
-static int __init prandom_init(void)
-{
-	int i;
-
-	prandom_state_selftest();
-
-	for_each_possible_cpu(i) {
-		struct rnd_state *state = &per_cpu(net_rand_state, i);
-		u32 weak_seed = (i + jiffies) ^ random_get_entropy();
-
-		prandom_seed_early(state, weak_seed, true);
-		prandom_warmup(state);
-	}
-
-	return 0;
-}
-core_initcall(prandom_init);
-
-static void __prandom_timer(struct timer_list *unused);
-
-static DEFINE_TIMER(seed_timer, __prandom_timer);
-
-static void __prandom_timer(struct timer_list *unused)
-{
-	u32 entropy;
-	unsigned long expires;
-
-	get_random_bytes(&entropy, sizeof(entropy));
-	prandom_seed(entropy);
-
-	/* reseed every ~60 seconds, in [40 .. 80) interval with slack */
-	expires = 40 + prandom_u32_max(40);
-	seed_timer.expires = jiffies + msecs_to_jiffies(expires * MSEC_PER_SEC);
-
-	add_timer(&seed_timer);
-}
-
-static void __init __prandom_start_seed_timer(void)
-{
-	seed_timer.expires = jiffies + msecs_to_jiffies(40 * MSEC_PER_SEC);
-	add_timer(&seed_timer);
-}
-
 void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
 {
 	int i;
@@ -259,51 +133,6 @@ void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
 }
 EXPORT_SYMBOL(prandom_seed_full_state);
 
-/*
- *	Generate better values after random number generator
- *	is fully initialized.
- */
-static void __prandom_reseed(bool late)
-{
-	unsigned long flags;
-	static bool latch = false;
-	static DEFINE_SPINLOCK(lock);
-
-	/* Asking for random bytes might result in bytes getting
-	 * moved into the nonblocking pool and thus marking it
-	 * as initialized. In this case we would double back into
-	 * this function and attempt to do a late reseed.
-	 * Ignore the pointless attempt to reseed again if we're
-	 * already waiting for bytes when the nonblocking pool
-	 * got initialized.
-	 */
-
-	/* only allow initial seeding (late == false) once */
-	if (!spin_trylock_irqsave(&lock, flags))
-		return;
-
-	if (latch && !late)
-		goto out;
-
-	latch = true;
-	prandom_seed_full_state(&net_rand_state);
-out:
-	spin_unlock_irqrestore(&lock, flags);
-}
-
-void prandom_reseed_late(void)
-{
-	__prandom_reseed(true);
-}
-
-static int __init prandom_reseed(void)
-{
-	__prandom_reseed(false);
-	__prandom_start_seed_timer();
-	return 0;
-}
-late_initcall(prandom_reseed);
-
 #ifdef CONFIG_RANDOM32_SELFTEST
 static struct prandom_test1 {
 	u32 seed;
@@ -465,3 +294,265 @@ static void __init prandom_state_selftest(void)
 		pr_info("prandom: %d self tests passed\n", runs);
 }
 #endif
+
+
+
+/*
+ * The prandom_u32() implementation is now completely separate from the
+ * prandom_state() functions, which are retained (for now) for compatibility.
+ *
+ * Because of (ab)use in the networking code for choosing random TCP/UDP port
+ * numbers, which open DoS possibilities if guessable, we want something
+ * stronger than a standard PRNG.  But the performance requirements of
+ * the network code do not allow robust crypto for this application.
+ *
+ * So this is a homebrew Junior Spaceman implementation, based on the
+ * lowest-latency trustworthy crypto primitive available, SipHash.
+ * (The authors of SipHash have not been consulted about this abuse of
+ * their work.)
+ *
+ * Standard SipHash-2-4 uses 2n+4 rounds to hash n words of input to
+ * one word of output.  This abbreviated version uses 2 rounds per word
+ * of output.
+ */
+
+struct siprand_state {
+	unsigned long v0;
+	unsigned long v1;
+	unsigned long v2;
+	unsigned long v3;
+};
+
+static DEFINE_PER_CPU(struct siprand_state, net_rand_state) __latent_entropy;
+
+/*
+ * This is the core CPRNG function.  As "pseudorandom", this is not used
+ * for truly valuable things, just intended to be a PITA to guess.
+ * For maximum speed, we do just two SipHash rounds per word.  This is
+ * the same rate as 4 rounds per 64 bits that SipHash normally uses,
+ * so hopefully it's reasonably secure.
+ *
+ * There are two changes from the official SipHash finalization:
+ * - We omit some constants XORed with v2 in the SipHash spec as irrelevant;
+ *   they are there only to make the output rounds distinct from the input
+ *   rounds, and this application has no input rounds.
+ * - Rather than returning v0^v1^v2^v3, return v1+v3.
+ *   If you look at the SipHash round, the last operation on v3 is
+ *   "v3 ^= v0", so "v0 ^ v3" just undoes that, a waste of time.
+ *   Likewise "v1 ^= v2".  (The rotate of v2 makes a difference, but
+ *   it still cancels out half of the bits in v2 for no benefit.)
+ *   Second, since the last combining operation was xor, continue the
+ *   pattern of alternating xor/add for a tiny bit of extra non-linearity.
+ */
+static inline u32 siprand_u32(struct siprand_state *s)
+{
+	unsigned long v0 = s->v0, v1 = s->v1, v2 = s->v2, v3 = s->v3;
+
+	PRND_SIPROUND(v0, v1, v2, v3);
+	PRND_SIPROUND(v0, v1, v2, v3);
+	s->v0 = v0;  s->v1 = v1;  s->v2 = v2;  s->v3 = v3;
+	return v1 + v3;
+}
+
+
+/**
+ *	prandom_u32 - pseudo random number generator
+ *
+ *	A 32 bit pseudo-random number is generated using a fast
+ *	algorithm suitable for simulation. This algorithm is NOT
+ *	considered safe for cryptographic use.
+ */
+u32 prandom_u32(void)
+{
+	struct siprand_state *state = get_cpu_ptr(&net_rand_state);
+	u32 res = siprand_u32(state);
+
+	trace_prandom_u32(res);
+	put_cpu_ptr(&net_rand_state);
+	return res;
+}
+EXPORT_SYMBOL(prandom_u32);
+
+/**
+ *	prandom_bytes - get the requested number of pseudo-random bytes
+ *	@buf: where to copy the pseudo-random bytes to
+ *	@bytes: the requested number of bytes
+ */
+void prandom_bytes(void *buf, size_t bytes)
+{
+	struct siprand_state *state = get_cpu_ptr(&net_rand_state);
+	u8 *ptr = buf;
+
+	while (bytes >= sizeof(u32)) {
+		put_unaligned(siprand_u32(state), (u32 *)ptr);
+		ptr += sizeof(u32);
+		bytes -= sizeof(u32);
+	}
+
+	if (bytes > 0) {
+		u32 rem = siprand_u32(state);
+
+		do {
+			*ptr++ = (u8)rem;
+			rem >>= BITS_PER_BYTE;
+		} while (--bytes > 0);
+	}
+	put_cpu_ptr(&net_rand_state);
+}
+EXPORT_SYMBOL(prandom_bytes);
+
+/**
+ *	prandom_seed - add entropy to pseudo random number generator
+ *	@entropy: entropy value
+ *
+ *	Add some additional seed material to the prandom pool.
+ *	The "entropy" is actually our IP address (the only caller is
+ *	the network code), not for unpredictability, but to ensure that
+ *	different machines are initialized differently.
+ */
+void prandom_seed(u32 entropy)
+{
+	int i;
+
+	add_device_randomness(&entropy, sizeof(entropy));
+
+	for_each_possible_cpu(i) {
+		struct siprand_state *state = per_cpu_ptr(&net_rand_state, i);
+		unsigned long v0 = state->v0, v1 = state->v1;
+		unsigned long v2 = state->v2, v3 = state->v3;
+
+		do {
+			v3 ^= entropy;
+			PRND_SIPROUND(v0, v1, v2, v3);
+			PRND_SIPROUND(v0, v1, v2, v3);
+			v0 ^= entropy;
+		} while (unlikely(!v0 || !v1 || !v2 || !v3));
+
+		WRITE_ONCE(state->v0, v0);
+		WRITE_ONCE(state->v1, v1);
+		WRITE_ONCE(state->v2, v2);
+		WRITE_ONCE(state->v3, v3);
+	}
+}
+EXPORT_SYMBOL(prandom_seed);
+
+/*
+ *	Generate some initially weak seeding values to allow
+ *	the prandom_u32() engine to be started.
+ */
+static int __init prandom_init_early(void)
+{
+	int i;
+	unsigned long v0, v1, v2, v3;
+
+	if (!arch_get_random_long(&v0))
+		v0 = jiffies;
+	if (!arch_get_random_long(&v1))
+		v0 = random_get_entropy();
+	v2 = v0 ^ PRND_K0;
+	v3 = v1 ^ PRND_K1;
+
+	for_each_possible_cpu(i) {
+		struct siprand_state *state;
+
+		v3 ^= i;
+		PRND_SIPROUND(v0, v1, v2, v3);
+		PRND_SIPROUND(v0, v1, v2, v3);
+		v0 ^= i;
+
+		state = per_cpu_ptr(&net_rand_state, i);
+		state->v0 = v0;  state->v1 = v1;
+		state->v2 = v2;  state->v3 = v3;
+	}
+
+	return 0;
+}
+core_initcall(prandom_init_early);
+
+
+/* Stronger reseeding when available, and periodically thereafter. */
+static void prandom_reseed(struct timer_list *unused);
+
+static DEFINE_TIMER(seed_timer, prandom_reseed);
+
+static void prandom_reseed(struct timer_list *unused)
+{
+	unsigned long expires;
+	int i;
+
+	/*
+	 * Reinitialize each CPU's PRNG with 128 bits of key.
+	 * No locking on the CPUs, but then somewhat random results are,
+	 * well, expected.
+	 */
+	for_each_possible_cpu(i) {
+		struct siprand_state *state;
+		unsigned long v0 = get_random_long(), v2 = v0 ^ PRND_K0;
+		unsigned long v1 = get_random_long(), v3 = v1 ^ PRND_K1;
+#if BITS_PER_LONG == 32
+		int j;
+
+		/*
+		 * On 32-bit machines, hash in two extra words to
+		 * approximate 128-bit key length.  Not that the hash
+		 * has that much security, but this prevents a trivial
+		 * 64-bit brute force.
+		 */
+		for (j = 0; j < 2; j++) {
+			unsigned long m = get_random_long();
+
+			v3 ^= m;
+			PRND_SIPROUND(v0, v1, v2, v3);
+			PRND_SIPROUND(v0, v1, v2, v3);
+			v0 ^= m;
+		}
+#endif
+		/*
+		 * Probably impossible in practice, but there is a
+		 * theoretical risk that a race between this reseeding
+		 * and the target CPU writing its state back could
+		 * create the all-zero SipHash fixed point.
+		 *
+		 * To ensure that never happens, ensure the state
+		 * we write contains no zero words.
+		 */
+		state = per_cpu_ptr(&net_rand_state, i);
+		WRITE_ONCE(state->v0, v0 ? v0 : -1ul);
+		WRITE_ONCE(state->v1, v1 ? v1 : -1ul);
+		WRITE_ONCE(state->v2, v2 ? v2 : -1ul);
+		WRITE_ONCE(state->v3, v3 ? v3 : -1ul);
+	}
+
+	/* reseed every ~60 seconds, in [40 .. 80) interval with slack */
+	expires = round_jiffies(jiffies + 40 * HZ + prandom_u32_max(40 * HZ));
+	mod_timer(&seed_timer, expires);
+}
+
+/*
+ * The random ready callback can be called from almost any interrupt.
+ * To avoid worrying about whether it's safe to delay that interrupt
+ * long enough to seed all CPUs, just schedule an immediate timer event.
+ */
+static void prandom_timer_start(struct random_ready_callback *unused)
+{
+	mod_timer(&seed_timer, jiffies);
+}
+
+/*
+ * Start periodic full reseeding as soon as strong
+ * random numbers are available.
+ */
+static int __init prandom_init_late(void)
+{
+	static struct random_ready_callback random_ready = {
+		.func = prandom_timer_start
+	};
+	int ret = add_random_ready_callback(&random_ready);
+
+	if (ret == -EALREADY) {
+		prandom_timer_start(&random_ready);
+		ret = 0;
+	}
+	return ret;
+}
+late_initcall(prandom_init_late);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] random32: add noise from network and scheduling activity
  2020-09-01  6:43 [PATCH 0/2] prandom_u32: make output less predictable Willy Tarreau
  2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
@ 2020-09-01  6:43 ` Willy Tarreau
  2020-09-01 10:24   ` Eric Dumazet
  2020-09-01 14:41 ` [PATCH 0/2] prandom_u32: make output less predictable Sedat Dilek
  2 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01  6:43 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Sedat Dilek, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

With the removal of the interrupt perturbations in previous random32
change (random32: make prandom_u32() output unpredictable), the PRNG
has become 100% deterministic again. While SipHash is expected to be
way more robust against brute force than the previous Tausworthe LFSR,
there's still the risk that whoever has even one temporary access to
the PRNG's internal state is able to predict all subsequent draws till
the next reseed (roughly every minute). This may happen through a side
channel attack or any data leak.

This patch restores the spirit of commit f227e3ec3b5c ("random32: update
the net random state on interrupt and activity") in that it will perturb
the internal PRNG's statee using externally collected noise, except that
it will not pick that noise from the random pool's bits nor upon
interrupt, but will rather combine a few elements along the Tx path
that are collectively hard to predict, such as dev, skb and txq
pointers, packet length and jiffies values. These ones are combined
using a single round of SipHash into a single long variable that is
mixed with the net_rand_state upon each invocation.

The operation was inlined because it produces very small and efficient
code, typically 3 xor, 2 add and 2 rol. The performance was measured
to be the same (even very slightly better) than before the switch to
SipHash; on a 6-core 12-thread Core i7-8700k equipped with a 40G NIC
(i40e), the connection rate dropped from 556k/s to 555k/s while the
SYN cookie rate grew from 5.38 Mpps to 5.45 Mpps.

Link: https://lore.kernel.org/netdev/20200808152628.GA27941@SDF.ORG/
Cc: George Spelvin <lkml@sdf.org>
Cc: Amit Klein <aksecurity@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: tytso@mit.edu
Cc: Florian Westphal <fw@strlen.de>
Cc: Marc Plumb <lkml.mplumb@gmail.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 include/linux/prandom.h | 19 +++++++++++++++++++
 kernel/time/timer.c     |  2 ++
 lib/random32.c          |  5 +++++
 net/core/dev.c          |  4 ++++
 4 files changed, 30 insertions(+)

diff --git a/include/linux/prandom.h b/include/linux/prandom.h
index cc1e71334e53..aa7de3432e0f 100644
--- a/include/linux/prandom.h
+++ b/include/linux/prandom.h
@@ -16,6 +16,12 @@ void prandom_bytes(void *buf, size_t nbytes);
 void prandom_seed(u32 seed);
 void prandom_reseed_late(void);
 
+DECLARE_PER_CPU(unsigned long, net_rand_noise);
+
+#define PRANDOM_ADD_NOISE(a, b, c, d) \
+	prandom_u32_add_noise((unsigned long)(a), (unsigned long)(b), \
+			      (unsigned long)(c), (unsigned long)(d))
+
 #if BITS_PER_LONG == 64
 /*
  * The core SipHash round function.  Each line can be executed in
@@ -50,6 +56,18 @@ void prandom_reseed_late(void);
 #error Unsupported BITS_PER_LONG
 #endif
 
+static inline void prandom_u32_add_noise(unsigned long a, unsigned long b,
+					 unsigned long c, unsigned long d)
+{
+	/*
+	 * This is not used cryptographically; it's just
+	 * a convenient 4-word hash function. (3 xor, 2 add, 2 rol)
+	 */
+	a ^= __this_cpu_read(net_rand_noise);
+	PRND_SIPROUND(a, b, c, d);
+	__this_cpu_write(net_rand_noise, d);
+}
+
 struct rnd_state {
 	__u32 s1, s2, s3, s4;
 };
@@ -99,6 +117,7 @@ static inline void prandom_seed_state(struct rnd_state *state, u64 seed)
 	state->s2 = __seed(i,   8U);
 	state->s3 = __seed(i,  16U);
 	state->s4 = __seed(i, 128U);
+	PRANDOM_ADD_NOISE(state, i, 0, 0);
 }
 
 /* Pseudo random number generator from numerical recipes. */
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 401fcb9d7388..bebcf2fc1226 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1704,6 +1704,8 @@ void update_process_times(int user_tick)
 {
 	struct task_struct *p = current;
 
+	PRANDOM_ADD_NOISE(jiffies, user_tick, p, 0);
+
 	/* Note: this timer irq context must be accounted for as well. */
 	account_process_tick(p, user_tick);
 	run_local_timers();
diff --git a/lib/random32.c b/lib/random32.c
index 00fa925a4487..38db382a8cf5 100644
--- a/lib/random32.c
+++ b/lib/random32.c
@@ -324,6 +324,8 @@ struct siprand_state {
 };
 
 static DEFINE_PER_CPU(struct siprand_state, net_rand_state) __latent_entropy;
+DEFINE_PER_CPU(unsigned long, net_rand_noise);
+EXPORT_PER_CPU_SYMBOL(net_rand_noise);
 
 /*
  * This is the core CPRNG function.  As "pseudorandom", this is not used
@@ -347,9 +349,12 @@ static DEFINE_PER_CPU(struct siprand_state, net_rand_state) __latent_entropy;
 static inline u32 siprand_u32(struct siprand_state *s)
 {
 	unsigned long v0 = s->v0, v1 = s->v1, v2 = s->v2, v3 = s->v3;
+	unsigned long n = __this_cpu_read(net_rand_noise);
 
+	v3 ^= n;
 	PRND_SIPROUND(v0, v1, v2, v3);
 	PRND_SIPROUND(v0, v1, v2, v3);
+	v0 ^= n;
 	s->v0 = v0;  s->v1 = v1;  s->v2 = v2;  s->v3 = v3;
 	return v1 + v3;
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index b9c6f31ae96e..e075f7e0785a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -144,6 +144,7 @@
 #include <linux/indirect_call_wrapper.h>
 #include <net/devlink.h>
 #include <linux/pm_runtime.h>
+#include <linux/prandom.h>
 
 #include "net-sysfs.h"
 
@@ -3557,6 +3558,7 @@ static int xmit_one(struct sk_buff *skb, struct net_device *dev,
 		dev_queue_xmit_nit(skb, dev);
 
 	len = skb->len;
+	PRANDOM_ADD_NOISE(skb, dev, txq, len + jiffies);
 	trace_net_dev_start_xmit(skb, dev);
 	rc = netdev_start_xmit(skb, dev, txq, more);
 	trace_net_dev_xmit(skb, rc, dev, len);
@@ -4129,6 +4131,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 			if (!skb)
 				goto out;
 
+			PRANDOM_ADD_NOISE(skb, dev, txq, jiffies);
 			HARD_TX_LOCK(dev, txq, cpu);
 
 			if (!netif_xmit_stopped(txq)) {
@@ -4194,6 +4197,7 @@ int dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
 
 	skb_set_queue_mapping(skb, queue_id);
 	txq = skb_get_tx_queue(dev, skb);
+	PRANDOM_ADD_NOISE(skb, dev, txq, jiffies);
 
 	local_bh_disable();
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
@ 2020-09-01  8:33   ` Yann Ylavic
  2020-09-01  8:39     ` Willy Tarreau
  2020-09-01 13:10   ` David Laight
  1 sibling, 1 reply; 18+ messages in thread
From: Yann Ylavic @ 2020-09-01  8:33 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: linux-kernel, Linux Kernel Network Developers, Sedat Dilek,
	George Spelvin, Amit Klein, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Tue, Sep 1, 2020 at 8:45 AM Willy Tarreau <w@1wt.eu> wrote:
>
> +/*
> + *     Generate some initially weak seeding values to allow
> + *     the prandom_u32() engine to be started.
> + */
> +static int __init prandom_init_early(void)
> +{
> +       int i;
> +       unsigned long v0, v1, v2, v3;
> +
> +       if (!arch_get_random_long(&v0))
> +               v0 = jiffies;
> +       if (!arch_get_random_long(&v1))
> +               v0 = random_get_entropy();

Shouldn't the above be:
                  v1 = random_get_entropy();
?

> +       v2 = v0 ^ PRND_K0;
> +       v3 = v1 ^ PRND_K1;
> +
> +       for_each_possible_cpu(i) {
> +               struct siprand_state *state;
> +
> +               v3 ^= i;
> +               PRND_SIPROUND(v0, v1, v2, v3);
> +               PRND_SIPROUND(v0, v1, v2, v3);
> +               v0 ^= i;
> +
> +               state = per_cpu_ptr(&net_rand_state, i);
> +               state->v0 = v0;  state->v1 = v1;
> +               state->v2 = v2;  state->v3 = v3;
> +       }
> +
> +       return 0;
> +}
> +core_initcall(prandom_init_early);


Regards;
Yann.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  8:33   ` Yann Ylavic
@ 2020-09-01  8:39     ` Willy Tarreau
  2020-09-01  8:46       ` Sedat Dilek
  0 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01  8:39 UTC (permalink / raw)
  To: Yann Ylavic
  Cc: linux-kernel, Linux Kernel Network Developers, Sedat Dilek,
	George Spelvin, Amit Klein, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Tue, Sep 01, 2020 at 10:33:40AM +0200, Yann Ylavic wrote:
> On Tue, Sep 1, 2020 at 8:45 AM Willy Tarreau <w@1wt.eu> wrote:
> >
> > +/*
> > + *     Generate some initially weak seeding values to allow
> > + *     the prandom_u32() engine to be started.
> > + */
> > +static int __init prandom_init_early(void)
> > +{
> > +       int i;
> > +       unsigned long v0, v1, v2, v3;
> > +
> > +       if (!arch_get_random_long(&v0))
> > +               v0 = jiffies;
> > +       if (!arch_get_random_long(&v1))
> > +               v0 = random_get_entropy();
> 
> Shouldn't the above be:
>                   v1 = random_get_entropy();
> ?

Very good catch, many thanks Yann! Now fixed in my local tree.

Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  8:39     ` Willy Tarreau
@ 2020-09-01  8:46       ` Sedat Dilek
  2020-09-01  8:56         ` Willy Tarreau
  0 siblings, 1 reply; 18+ messages in thread
From: Sedat Dilek @ 2020-09-01  8:46 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Yann Ylavic, linux-kernel, Linux Kernel Network Developers,
	George Spelvin, Amit Klein, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Tue, Sep 1, 2020 at 10:39 AM Willy Tarreau <w@1wt.eu> wrote:
>
> On Tue, Sep 01, 2020 at 10:33:40AM +0200, Yann Ylavic wrote:
> > On Tue, Sep 1, 2020 at 8:45 AM Willy Tarreau <w@1wt.eu> wrote:
> > >
> > > +/*
> > > + *     Generate some initially weak seeding values to allow
> > > + *     the prandom_u32() engine to be started.
> > > + */
> > > +static int __init prandom_init_early(void)
> > > +{
> > > +       int i;
> > > +       unsigned long v0, v1, v2, v3;
> > > +
> > > +       if (!arch_get_random_long(&v0))
> > > +               v0 = jiffies;
> > > +       if (!arch_get_random_long(&v1))
> > > +               v0 = random_get_entropy();
> >
> > Shouldn't the above be:
> >                   v1 = random_get_entropy();
> > ?
>
> Very good catch, many thanks Yann! Now fixed in my local tree.
>

Thanks for offering a new patchset, Willy.

Will you push the updated patchset to your prandom Git - for easy fetching?

Thanks.

- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  8:46       ` Sedat Dilek
@ 2020-09-01  8:56         ` Willy Tarreau
  2020-09-01  9:26           ` Sedat Dilek
  0 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01  8:56 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Yann Ylavic, linux-kernel, Linux Kernel Network Developers,
	George Spelvin, Amit Klein, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Tue, Sep 01, 2020 at 10:46:16AM +0200, Sedat Dilek wrote:
> Will you push the updated patchset to your prandom Git - for easy fetching?

Yeah if you want, feel free to use this one :
   https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise

Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  8:56         ` Willy Tarreau
@ 2020-09-01  9:26           ` Sedat Dilek
  0 siblings, 0 replies; 18+ messages in thread
From: Sedat Dilek @ 2020-09-01  9:26 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Yann Ylavic, linux-kernel, Linux Kernel Network Developers,
	George Spelvin, Amit Klein, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Tue, Sep 1, 2020 at 10:57 AM Willy Tarreau <w@1wt.eu> wrote:
>
> On Tue, Sep 01, 2020 at 10:46:16AM +0200, Sedat Dilek wrote:
> > Will you push the updated patchset to your prandom Git - for easy fetching?
>
> Yeah if you want, feel free to use this one :
>    https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise
>

Thanks,
- sed@ -

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] random32: add noise from network and scheduling activity
  2020-09-01  6:43 ` [PATCH 2/2] random32: add noise from network and scheduling activity Willy Tarreau
@ 2020-09-01 10:24   ` Eric Dumazet
  2020-09-01 11:57     ` Willy Tarreau
  0 siblings, 1 reply; 18+ messages in thread
From: Eric Dumazet @ 2020-09-01 10:24 UTC (permalink / raw)
  To: Willy Tarreau, linux-kernel, netdev
  Cc: Sedat Dilek, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb



On 8/31/20 11:43 PM, Willy Tarreau wrote:
> With the removal of the interrupt perturbations in previous random32
> change (random32: make prandom_u32() output unpredictable), the PRNG
> has become 100% deterministic again. While SipHash is expected to be
> way more robust against brute force than the previous Tausworthe LFSR,
> there's still the risk that whoever has even one temporary access to
> the PRNG's internal state is able to predict all subsequent draws till
> the next reseed (roughly every minute). This may happen through a side
> channel attack or any data leak.
> 
> This patch restores the spirit of commit f227e3ec3b5c ("random32: update
> the net random state on interrupt and activity") in that it will perturb
> the internal PRNG's statee using externally collected noise, except that
> it will not pick that noise from the random pool's bits nor upon
> interrupt, but will rather combine a few elements along the Tx path
> that are collectively hard to predict, such as dev, skb and txq
> pointers, packet length and jiffies values. These ones are combined
> using a single round of SipHash into a single long variable that is
> mixed with the net_rand_state upon each invocation.
> 
> The operation was inlined because it produces very small and efficient
> code, typically 3 xor, 2 add and 2 rol. The performance was measured
> to be the same (even very slightly better) than before the switch to
> SipHash; on a 6-core 12-thread Core i7-8700k equipped with a 40G NIC
> (i40e), the connection rate dropped from 556k/s to 555k/s while the
> SYN cookie rate grew from 5.38 Mpps to 5.45 Mpps.
> 

> diff --git a/net/core/dev.c b/net/core/dev.c
> index b9c6f31ae96e..e075f7e0785a 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -144,6 +144,7 @@
>  #include <linux/indirect_call_wrapper.h>
>  #include <net/devlink.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/prandom.h>
>  
>  #include "net-sysfs.h"
>  
> @@ -3557,6 +3558,7 @@ static int xmit_one(struct sk_buff *skb, struct net_device *dev,
>  		dev_queue_xmit_nit(skb, dev);
>  
>  	len = skb->len;
> +	PRANDOM_ADD_NOISE(skb, dev, txq, len + jiffies);
>  	trace_net_dev_start_xmit(skb, dev);
>  	rc = netdev_start_xmit(skb, dev, txq, more);
>  	trace_net_dev_xmit(skb, rc, dev, len);
> @@ -4129,6 +4131,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>  			if (!skb)
>  				goto out;
>  
> +			PRANDOM_ADD_NOISE(skb, dev, txq, jiffies);
>  			HARD_TX_LOCK(dev, txq, cpu);
>  
>  			if (!netif_xmit_stopped(txq)) {
> @@ -4194,6 +4197,7 @@ int dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
>  
>  	skb_set_queue_mapping(skb, queue_id);
>  	txq = skb_get_tx_queue(dev, skb);
> +	PRANDOM_ADD_NOISE(skb, dev, txq, jiffies);
>  
>  	local_bh_disable();
>  
> 

Hi Willy

There is not much entropy here really :

1) dev & txq are mostly constant on a typical host (at least the kind of hosts that is targeted by 
Amit Klein and others in their attacks.

2) len is also known by the attacker, attacking an idle host.

3) skb are also allocations from slab cache, which tend to recycle always the same pointers (on idle hosts)


4) jiffies might be incremented every 4 ms (if HZ=250)

Maybe we could feed percpu prandom noise with samples of ns resolution timestamps,
lazily cached from ktime_get() or similar functions.

This would use one instruction on x86 to update the cache, with maybe more generic noise.

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4c47f388a83f17860fdafa3229bba0cc605ec25a..a3e026cbbb6e8c5499ed780e57de5fa09bc010b6 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -751,7 +751,7 @@ ktime_t ktime_get(void)
 {
        struct timekeeper *tk = &tk_core.timekeeper;
        unsigned int seq;
-       ktime_t base;
+       ktime_t res, base;
        u64 nsecs;
 
        WARN_ON(timekeeping_suspended);
@@ -763,7 +763,9 @@ ktime_t ktime_get(void)
 
        } while (read_seqcount_retry(&tk_core.seq, seq));
 
-       return ktime_add_ns(base, nsecs);
+       res = ktime_add_ns(base, nsecs);
+       __this_cpu_add(prandom_noise, (unsigned long)ktime_to_ns(res));
+       return res;
 }
 EXPORT_SYMBOL_GPL(ktime_get);


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] random32: add noise from network and scheduling activity
  2020-09-01 10:24   ` Eric Dumazet
@ 2020-09-01 11:57     ` Willy Tarreau
  0 siblings, 0 replies; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01 11:57 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: linux-kernel, netdev, Sedat Dilek, George Spelvin, Amit Klein,
	Eric Dumazet, Jason A. Donenfeld, Andy Lutomirski, Kees Cook,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, tytso,
	Florian Westphal, Marc Plumb

Hi Eric,

On Tue, Sep 01, 2020 at 12:24:38PM +0200, Eric Dumazet wrote:
> There is not much entropy here really :
> 
> 1) dev & txq are mostly constant on a typical host (at least the kind of hosts that is targeted by 
> Amit Klein and others in their attacks.
> 
> 2) len is also known by the attacker, attacking an idle host.
> 
> 3) skb are also allocations from slab cache, which tend to recycle always the same pointers (on idle hosts)
> 
> 
> 4) jiffies might be incremented every 4 ms (if HZ=250)

I know. The point is essentially that someone "remote" or with rare access
to the host's memory (i.e. in a VM on the same CPU sharing L1 with some
CPU vulnerabilities) cannot synchronize with the PRNG and easily stay
synchronized forever. Otherwise I totally agree that these are pretty
weak. But in my opinion they are sufficient to turn a 100% success into
way less. I try not to forget that we're just trying to make a ~15-bit
port require ~2^14 attempts on average. Oh and by the way the number of
calls also counts here.

> Maybe we could feed percpu prandom noise with samples of ns resolution timestamps,
> lazily cached from ktime_get() or similar functions.
>
> This would use one instruction on x86 to update the cache, with maybe more generic noise.

Sure! I think the principle here allows to easily extend it to various
places, and the more the better. Maybe actually we'll figure that there
are plenty of sources of randomness that were not considered secure enough
to feed /dev/random while they're perfectly fine for such use cases.

> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 4c47f388a83f17860fdafa3229bba0cc605ec25a..a3e026cbbb6e8c5499ed780e57de5fa09bc010b6 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -751,7 +751,7 @@ ktime_t ktime_get(void)
>  {
>         struct timekeeper *tk = &tk_core.timekeeper;
>         unsigned int seq;
> -       ktime_t base;
> +       ktime_t res, base;
>         u64 nsecs;
>  
>         WARN_ON(timekeeping_suspended);
> @@ -763,7 +763,9 @@ ktime_t ktime_get(void)
>  
>         } while (read_seqcount_retry(&tk_core.seq, seq));
>  
> -       return ktime_add_ns(base, nsecs);
> +       res = ktime_add_ns(base, nsecs);
> +       __this_cpu_add(prandom_noise, (unsigned long)ktime_to_ns(res));
> +       return res;
>  }
>  EXPORT_SYMBOL_GPL(ktime_get);

Actually it could even be nice to combine it with __builtin_return_address(0)
given the large number of callers this one has! But I generally agree with
your proposal.

Thanks,
Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
  2020-09-01  8:33   ` Yann Ylavic
@ 2020-09-01 13:10   ` David Laight
  2020-09-01 13:16     ` Willy Tarreau
  1 sibling, 1 reply; 18+ messages in thread
From: David Laight @ 2020-09-01 13:10 UTC (permalink / raw)
  To: 'Willy Tarreau', linux-kernel, netdev
  Cc: Sedat Dilek, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

From: Willy Tarreau
> Sent: 01 September 2020 07:43
...
> +/*
> + *	Generate some initially weak seeding values to allow
> + *	the prandom_u32() engine to be started.
> + */
> +static int __init prandom_init_early(void)
> +{
> +	int i;
> +	unsigned long v0, v1, v2, v3;
> +
> +	if (!arch_get_random_long(&v0))
> +		v0 = jiffies;

Isn't jiffies likely to be zero here?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-01 13:10   ` David Laight
@ 2020-09-01 13:16     ` Willy Tarreau
       [not found]       ` <CANEQ_+Kuw6cxWRBE6NyXkr=8p3W-1f=o1q91ZESeueEnna9fvw@mail.gmail.com>
  0 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01 13:16 UTC (permalink / raw)
  To: David Laight
  Cc: linux-kernel, netdev, Sedat Dilek, George Spelvin, Amit Klein,
	Eric Dumazet, Jason A. Donenfeld, Andy Lutomirski, Kees Cook,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, tytso,
	Florian Westphal, Marc Plumb

On Tue, Sep 01, 2020 at 01:10:18PM +0000, David Laight wrote:
> From: Willy Tarreau
> > Sent: 01 September 2020 07:43
> ...
> > +/*
> > + *	Generate some initially weak seeding values to allow
> > + *	the prandom_u32() engine to be started.
> > + */
> > +static int __init prandom_init_early(void)
> > +{
> > +	int i;
> > +	unsigned long v0, v1, v2, v3;
> > +
> > +	if (!arch_get_random_long(&v0))
> > +		v0 = jiffies;
> 
> Isn't jiffies likely to be zero here?

I don't know. But do we really care ? I'd personally have been fine
with not even assigning it in this case and leaving whatever was in
the stack in this case, though it could make some static code analyzer
unhappy.

Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] prandom_u32: make output less predictable
  2020-09-01  6:43 [PATCH 0/2] prandom_u32: make output less predictable Willy Tarreau
  2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
  2020-09-01  6:43 ` [PATCH 2/2] random32: add noise from network and scheduling activity Willy Tarreau
@ 2020-09-01 14:41 ` Sedat Dilek
  2020-09-01 14:55   ` Willy Tarreau
  2 siblings, 1 reply; 18+ messages in thread
From: Sedat Dilek @ 2020-09-01 14:41 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: linux-kernel, netdev, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

On Tue, Sep 1, 2020 at 8:43 AM Willy Tarreau <w@1wt.eu> wrote:
>
> This is the cleanup of the latest series of prandom_u32 experimentations
> consisting in using SipHash instead of Tausworthe to produce the randoms
> used by the network stack. The changes to the files were kept minimal,
> and the controversial commit that used to take noise from the fast_pool
> (f227e3ec3b5c) was reverted. Instead, a dedicated "net_rand_noise" per_cpu
> variable is fed from various sources of activities (networking, scheduling)
> to perturb the SipHash state using fast, non-trivially predictable data,
> instead of keeping it fully deterministic. The goal is essentially to make
> any occasional memory leakage or brute-force attempt useless.
>
> The resulting code was verified to be very slightly faster on x86_64 than
> what is was with the controversial commit above, though this remains barely
> above measurement noise. It was only build-tested on arm & arm64.
>
> George Spelvin (1):
>   random32: make prandom_u32() output unpredictable
>
> Willy Tarreau (1):
>   random32: add noise from network and scheduling activity
>
>  drivers/char/random.c   |   1 -
>  include/linux/prandom.h |  55 ++++-
>  kernel/time/timer.c     |   9 +-
>  lib/random32.c          | 438 ++++++++++++++++++++++++----------------
>  net/core/dev.c          |   4 +
>  5 files changed, 326 insertions(+), 181 deletions(-)
>
> Cc: George Spelvin <lkml@sdf.org>
> Cc: Amit Klein <aksecurity@gmail.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: tytso@mit.edu
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Marc Plumb <lkml.mplumb@gmail.com>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
>

I have tested with the patchset from [1].
( Later I saw, you dropped "WIP: tcp: reuse incoming skb hash in
tcp_conn_request()". )

- Sedat -

https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise


> --
> 2.28.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] prandom_u32: make output less predictable
  2020-09-01 14:41 ` [PATCH 0/2] prandom_u32: make output less predictable Sedat Dilek
@ 2020-09-01 14:55   ` Willy Tarreau
  2020-09-01 15:19     ` Eric Dumazet
  0 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-01 14:55 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: linux-kernel, netdev, George Spelvin, Amit Klein, Eric Dumazet,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, tytso, Florian Westphal,
	Marc Plumb

On Tue, Sep 01, 2020 at 04:41:13PM +0200, Sedat Dilek wrote:
> I have tested with the patchset from [1].
> ( Later I saw, you dropped "WIP: tcp: reuse incoming skb hash in
> tcp_conn_request()". )

Yes because it's a bit out of the cope of this series and makes sense
even without these patches, thus I assume Eric will take care of it
separately.

Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] prandom_u32: make output less predictable
  2020-09-01 14:55   ` Willy Tarreau
@ 2020-09-01 15:19     ` Eric Dumazet
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Dumazet @ 2020-09-01 15:19 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Sedat Dilek, LKML, netdev, George Spelvin, Amit Klein,
	Jason A. Donenfeld, Andy Lutomirski, Kees Cook, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds, Theodore Ts'o,
	Florian Westphal, Marc Plumb

On Tue, Sep 1, 2020 at 4:55 PM Willy Tarreau <w@1wt.eu> wrote:
>
> On Tue, Sep 01, 2020 at 04:41:13PM +0200, Sedat Dilek wrote:
> > I have tested with the patchset from [1].
> > ( Later I saw, you dropped "WIP: tcp: reuse incoming skb hash in
> > tcp_conn_request()". )
>
> Yes because it's a bit out of the cope of this series and makes sense
> even without these patches, thus I assume Eric will take care of it
> separately.

Yes, I am still pondering on this one and really there is no hurry.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
       [not found]       ` <CANEQ_+Kuw6cxWRBE6NyXkr=8p3W-1f=o1q91ZESeueEnna9fvw@mail.gmail.com>
@ 2020-09-14 16:16         ` Sedat Dilek
  2020-09-14 16:29           ` Willy Tarreau
  0 siblings, 1 reply; 18+ messages in thread
From: Sedat Dilek @ 2020-09-14 16:16 UTC (permalink / raw)
  To: Amit Klein
  Cc: Willy Tarreau, David Laight, linux-kernel, netdev,
	George Spelvin, Eric Dumazet, Jason A. Donenfeld,
	Andy Lutomirski, Kees Cook, Thomas Gleixner, Peter Zijlstra,
	Linus Torvalds, tytso, Florian Westphal, Marc Plumb

On Mon, Sep 14, 2020 at 4:53 PM Amit Klein <aksecurity@gmail.com> wrote:
>
> Hi
>
> Is this patch being pushed to any branch? I don't see it deployed anywhere (unless I'm missing something...).
>

It's here:

[1] https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise

> Best,
> -Amit
>
>
>
> On Tue, Sep 1, 2020 at 4:16 PM Willy Tarreau <w@1wt.eu> wrote:
>>
>> On Tue, Sep 01, 2020 at 01:10:18PM +0000, David Laight wrote:
>> > From: Willy Tarreau
>> > > Sent: 01 September 2020 07:43
>> > ...
>> > > +/*
>> > > + * Generate some initially weak seeding values to allow
>> > > + * the prandom_u32() engine to be started.
>> > > + */
>> > > +static int __init prandom_init_early(void)
>> > > +{
>> > > +   int i;
>> > > +   unsigned long v0, v1, v2, v3;
>> > > +
>> > > +   if (!arch_get_random_long(&v0))
>> > > +           v0 = jiffies;
>> >
>> > Isn't jiffies likely to be zero here?
>>
>> I don't know. But do we really care ? I'd personally have been fine
>> with not even assigning it in this case and leaving whatever was in
>> the stack in this case, though it could make some static code analyzer
>> unhappy.
>>
>> Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-14 16:16         ` Sedat Dilek
@ 2020-09-14 16:29           ` Willy Tarreau
  2020-09-14 16:48             ` Sedat Dilek
  0 siblings, 1 reply; 18+ messages in thread
From: Willy Tarreau @ 2020-09-14 16:29 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Amit Klein, David Laight, linux-kernel, netdev, George Spelvin,
	Eric Dumazet, Jason A. Donenfeld, Andy Lutomirski, Kees Cook,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, tytso,
	Florian Westphal, Marc Plumb

On Mon, Sep 14, 2020 at 06:16:40PM +0200, Sedat Dilek wrote:
> On Mon, Sep 14, 2020 at 4:53 PM Amit Klein <aksecurity@gmail.com> wrote:
> >
> > Hi
> >
> > Is this patch being pushed to any branch? I don't see it deployed anywhere (unless I'm missing something...).
> >
> 
> It's here:
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise

By the way I didn't get any feedback from those who initially disagreed
with the one that was mergd, so for now I'm not doing anything on it
anymore. I can propose it again for 5.10-rc1 but will not push anymore
if there's no interest behind it.

Cheers,
Willy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] random32: make prandom_u32() output unpredictable
  2020-09-14 16:29           ` Willy Tarreau
@ 2020-09-14 16:48             ` Sedat Dilek
  0 siblings, 0 replies; 18+ messages in thread
From: Sedat Dilek @ 2020-09-14 16:48 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Amit Klein, David Laight, linux-kernel, netdev, George Spelvin,
	Eric Dumazet, Jason A. Donenfeld, Andy Lutomirski, Kees Cook,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, tytso,
	Florian Westphal, Marc Plumb

On Mon, Sep 14, 2020 at 6:29 PM Willy Tarreau <w@1wt.eu> wrote:
>
> On Mon, Sep 14, 2020 at 06:16:40PM +0200, Sedat Dilek wrote:
> > On Mon, Sep 14, 2020 at 4:53 PM Amit Klein <aksecurity@gmail.com> wrote:
> > >
> > > Hi
> > >
> > > Is this patch being pushed to any branch? I don't see it deployed anywhere (unless I'm missing something...).
> > >
> >
> > It's here:
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/prandom.git/log/?h=20200901-siphash-noise
>
> By the way I didn't get any feedback from those who initially disagreed
> with the one that was mergd, so for now I'm not doing anything on it
> anymore. I can propose it again for 5.10-rc1 but will not push anymore
> if there's no interest behind it.
>

As a feedback:

Just some minutes ago...
I have booted into Linux v5.9-rc5 with your (above mentioned) patchset
plus some individual mostly Clang related patchset.

While dealing with that topic, there was a "fast random" patchset from
[1] offered in this context.
I am not subscribed to any linux-random mailing-list, but I have this
one included, too.
Unsure, if there was any feedback on this.
With WARN_ALL_UNSEEDED_RANDOM=y it reduces here the number of warnings.

As a use-case I ran this PERF-SESSION...

Link: https://github.com/ClangBuiltLinux/linux/issues/1086#issuecomment-675783804

/home/dileks/bin/perf list | grep prandom_u32 | column -t
random:prandom_u32  [Tracepoint  event]

cd /opt/ltp

echo 0 | tee /proc/sys/kernel/kptr_restrict /proc/sys/kernel/perf_event_paranoid

/home/dileks/bin/perf record -a -g -e random:prandom_u32 ./runltp -f
net.features -s tcp_fastopen
/home/dileks/bin/perf report --no-children --stdio > ./perf-report.txt
/home/dileks/bin/perf script > ./perf-script.txt

echo 1 | tee /proc/sys/kernel/kptr_restrict /proc/sys/kernel/perf_event_paranoid

I was curious (mostly) to see what the impact of tcp_conn_request()
<-> prandom_u32() was and the improvements by the patch from Eric.
I can send the perf-report.txt if desired.

- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=random/fast

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-09-14 16:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01  6:43 [PATCH 0/2] prandom_u32: make output less predictable Willy Tarreau
2020-09-01  6:43 ` [PATCH 1/2] random32: make prandom_u32() output unpredictable Willy Tarreau
2020-09-01  8:33   ` Yann Ylavic
2020-09-01  8:39     ` Willy Tarreau
2020-09-01  8:46       ` Sedat Dilek
2020-09-01  8:56         ` Willy Tarreau
2020-09-01  9:26           ` Sedat Dilek
2020-09-01 13:10   ` David Laight
2020-09-01 13:16     ` Willy Tarreau
     [not found]       ` <CANEQ_+Kuw6cxWRBE6NyXkr=8p3W-1f=o1q91ZESeueEnna9fvw@mail.gmail.com>
2020-09-14 16:16         ` Sedat Dilek
2020-09-14 16:29           ` Willy Tarreau
2020-09-14 16:48             ` Sedat Dilek
2020-09-01  6:43 ` [PATCH 2/2] random32: add noise from network and scheduling activity Willy Tarreau
2020-09-01 10:24   ` Eric Dumazet
2020-09-01 11:57     ` Willy Tarreau
2020-09-01 14:41 ` [PATCH 0/2] prandom_u32: make output less predictable Sedat Dilek
2020-09-01 14:55   ` Willy Tarreau
2020-09-01 15:19     ` Eric Dumazet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.