[PATCH v3 0/5] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[PATCH v3 0/5] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Andy Lutomirski]

This introduces and uses a very simple synchronous mechanism to get
/dev/urandom-style bits appropriate for initial KVM PV guest RNG
seeding.

virtio-rng is not suitable for this purpose.  It's too difficult to
enumerate for use in early boot (e.g. KASLR, which runs before we
even have an IDT).  It also provides /dev/random-style bits, which
means that making guest boot wait for virtio-rng is unacceptably
slow, and doing it asynchronously means that /dev/urandom might
still be predictable when userspace starts.

I sent the corresponding kvm-unit-tests and qemu changes separately.

There's room for bikeshedding on the same arch_get_slow_rng_u64.  I
considered arch_get_rng_seed_u64, but that could be confused with
arch_get_random_seed_long, which is not interchangeable.

Changes from v2:
 - Bisection fix (patch 2 had a misplaced brace).  The final states is
   identical to that of v2.
 - Improve the 0/5 description a little bit.

Changes from v1:
 - Split patches 2 and 3
 - Log all arch sources in init_std_data
 - Fix the 32-bit kaslr build

Andy Lutomirski (5):
  x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit
  random,x86: Add arch_get_slow_rng_u64
  random: Seed pools from arch_get_slow_rng_u64 at startup
  random: Log how many bits we managed to seed with in init_std_data
  x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

 Documentation/virtual/kvm/cpuid.txt  |  3 +++
 arch/x86/Kconfig                     |  4 ++++
 arch/x86/boot/compressed/aslr.c      | 27 +++++++++++++++++++++++++++
 arch/x86/include/asm/archslowrng.h   | 30 ++++++++++++++++++++++++++++++
 arch/x86/include/asm/processor.h     | 21 ++++++++++++++++++---
 arch/x86/include/uapi/asm/kvm_para.h |  2 ++
 arch/x86/kernel/kvm.c                | 22 ++++++++++++++++++++++
 arch/x86/kvm/cpuid.c                 |  3 ++-
 arch/x86/kvm/x86.c                   |  4 ++++
 drivers/char/random.c                | 20 ++++++++++++++++++--
 include/linux/random.h               |  9 +++++++++
 11 files changed, 139 insertions(+), 6 deletions(-)
 create mode 100644 arch/x86/include/asm/archslowrng.h

-- 
1.9.3

[PATCH v3 1/5] x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit

[Andy Lutomirski]

This adds a simple interface to allow a guest to request 64 bits of
host nonblocking entropy.  This is independent of virtio-rng for a
couple of reasons:

 - It's intended to be usable during early boot, when a trivial
   synchronous interface is needed.

 - virtio-rng gives blocking entropy, and making guest boot wait for
   the host's /dev/random will cause problems.

MSR_KVM_GET_RNG_SEED is intended to provide 64 bits of best-effort
cryptographically secure data for use as a seed.  It provides no
guarantee that the result contains any actual entropy.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 Documentation/virtual/kvm/cpuid.txt  | 3 +++
 arch/x86/include/uapi/asm/kvm_para.h | 2 ++
 arch/x86/kvm/cpuid.c                 | 3 ++-
 arch/x86/kvm/x86.c                   | 4 ++++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 3c65feb..0ab043b 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
                                    ||       || before enabling paravirtualized
                                    ||       || spinlock support.
 ------------------------------------------------------------------------------
+KVM_FEATURE_GET_RNG_SEED           ||     8 || host provides rng seed data via
+                                   ||       || MSR_KVM_GET_RNG_SEED.
+------------------------------------------------------------------------------
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
                                    ||       || per-cpu warps are expected in
                                    ||       || kvmclock.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 94dc8ca..e2eaf93 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -24,6 +24,7 @@
 #define KVM_FEATURE_STEAL_TIME		5
 #define KVM_FEATURE_PV_EOI		6
 #define KVM_FEATURE_PV_UNHALT		7
+#define KVM_FEATURE_GET_RNG_SEED	8
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -40,6 +41,7 @@
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
 #define MSR_KVM_PV_EOI_EN      0x4b564d04
+#define MSR_KVM_GET_RNG_SEED 0x4b564d05
 
 struct kvm_steal_time {
 	__u64 steal;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 38a0afe..40d6763 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -479,7 +479,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_ASYNC_PF) |
 			     (1 << KVM_FEATURE_PV_EOI) |
 			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT) |
-			     (1 << KVM_FEATURE_PV_UNHALT);
+			     (1 << KVM_FEATURE_PV_UNHALT) |
+			     (1 << KVM_FEATURE_GET_RNG_SEED);
 
 		if (sched_info_on())
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f644933..4e81853 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -48,6 +48,7 @@
 #include <linux/pci.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/pvclock_gtod.h>
+#include <linux/random.h>
 #include <trace/events/kvm.h>
 
 #define CREATE_TRACE_POINTS
@@ -2480,6 +2481,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_KVM_PV_EOI_EN:
 		data = vcpu->arch.pv_eoi.msr_val;
 		break;
+	case MSR_KVM_GET_RNG_SEED:
+		get_random_bytes(&data, sizeof(data));
+		break;
 	case MSR_IA32_P5_MC_ADDR:
 	case MSR_IA32_P5_MC_TYPE:
 	case MSR_IA32_MCG_CAP:
-- 
1.9.3

Re: [PATCH v3 1/5] x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit

[Andrew Honig]

> +       case MSR_KVM_GET_RNG_SEED:
> +               get_random_bytes(&data, sizeof(data));
> +               break;

Should this be rate limited in the interest of conserving randomness?
If there ever is an attack on the prng, this would create very
favorable conditions for an attacker to exploit it.

Re: [PATCH v3 1/5] x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit

[Andy Lutomirski]

On Thu, Jul 17, 2014 at 10:43 AM, Andrew Honig <ahonig@google.com> wrote:
>> +       case MSR_KVM_GET_RNG_SEED:
>> +               get_random_bytes(&data, sizeof(data));
>> +               break;
>
> Should this be rate limited in the interest of conserving randomness?
> If there ever is an attack on the prng, this would create very
> favorable conditions for an attacker to exploit it.

IMO if the nonblocking pool has a weakness that requires us to
conserve its output, then this is the least of our worries.

--Andy

[PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

arch_get_slow_rng_u64 tries to get 64 bits of RNG seed data.  Unlike
arch_get_random_{bytes,seed}, etc., it makes no claims about entropy
content.  It's also likely to be much slower and should not be used
frequently.  That being said, it should be fast enough to call
several times during boot without any noticeable slowdown.

This initial implementation backs it with MSR_KVM_GET_RNG_SEED if
available.  The intent is for other hypervisor guest implementations
to implement this interface.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig                   |  4 ++++
 arch/x86/include/asm/archslowrng.h | 30 ++++++++++++++++++++++++++++++
 arch/x86/kernel/kvm.c              | 22 ++++++++++++++++++++++
 include/linux/random.h             |  9 +++++++++
 4 files changed, 65 insertions(+)
 create mode 100644 arch/x86/include/asm/archslowrng.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a8f749e..4dfb539 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -593,6 +593,7 @@ config KVM_GUEST
 	bool "KVM Guest support (including kvmclock)"
 	depends on PARAVIRT
 	select PARAVIRT_CLOCK
+	select ARCH_SLOW_RNG
 	default y
 	---help---
 	  This option enables various optimizations for running under the KVM
@@ -627,6 +628,9 @@ config PARAVIRT_TIME_ACCOUNTING
 config PARAVIRT_CLOCK
 	bool
 
+config ARCH_SLOW_RNG
+       bool
+
 endif #HYPERVISOR_GUEST
 
 config NO_BOOTMEM
diff --git a/arch/x86/include/asm/archslowrng.h b/arch/x86/include/asm/archslowrng.h
new file mode 100644
index 0000000..c8e8d0d
--- /dev/null
+++ b/arch/x86/include/asm/archslowrng.h
@@ -0,0 +1,30 @@
+/*
+ * This file is part of the Linux kernel.
+ *
+ * Copyright (c) 2014 Andy Lutomirski
+ * Authors: Andy Lutomirski <luto@amacapital.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef ASM_X86_ARCHSLOWRANDOM_H
+#define ASM_X86_ARCHSLOWRANDOM_H
+
+#ifndef CONFIG_ARCH_SLOW_RNG
+# error archslowrng.h should not be included if !CONFIG_ARCH_SLOW_RNG
+#endif
+
+/*
+ * Performance is irrelevant here, so there's no point in using the
+ * paravirt ops mechanism.  Instead just use a function pointer.
+ */
+extern int (*arch_get_slow_rng_u64)(u64 *v);
+
+#endif /* ASM_X86_ARCHSLOWRANDOM_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 3dd8e2c..8d64d28 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -416,6 +416,25 @@ void kvm_disable_steal_time(void)
 	wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
 }
 
+static int nop_get_slow_rng_u64(u64 *v)
+{
+	return 0;
+}
+
+static int kvm_get_slow_rng_u64(u64 *v)
+{
+	/*
+	 * Allow migration from a hypervisor with the GET_RNG_SEED
+	 * feature to a hypervisor without it.
+	 */
+	if (rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0)
+		return 1;
+	else
+		return 0;
+}
+
+int (*arch_get_slow_rng_u64)(u64 *v) = nop_get_slow_rng_u64;
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
@@ -493,6 +512,9 @@ void __init kvm_guest_init(void)
 	if (kvmclock_vsyscall)
 		kvm_setup_vsyscall_timeinfo();
 
+	if (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED))
+		arch_get_slow_rng_u64 = kvm_get_slow_rng_u64;
+
 #ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
 	register_cpu_notifier(&kvm_cpu_notifier);
diff --git a/include/linux/random.h b/include/linux/random.h
index 57fbbff..ceafbcf 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -106,6 +106,15 @@ static inline int arch_has_random_seed(void)
 }
 #endif
 
+#ifdef CONFIG_ARCH_SLOW_RNG
+# include <asm/archslowrng.h>
+#else
+static inline int arch_get_slow_rng_u64(u64 *v)
+{
+	return 0;
+}
+#endif
+
 /* Pseudo random number generator from numerical recipes. */
 static inline u32 next_pseudo_random32(u32 seed)
 {
-- 
1.9.3

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[H. Peter Anvin]

On 07/16/2014 02:45 PM, Andy Lutomirski wrote:
> diff --git a/arch/x86/include/asm/archslowrng.h b/arch/x86/include/asm/archslowrng.h
> new file mode 100644
> index 0000000..c8e8d0d
> --- /dev/null
> +++ b/arch/x86/include/asm/archslowrng.h
> @@ -0,0 +1,30 @@
> +/*
> + * This file is part of the Linux kernel.
> + *
> + * Copyright (c) 2014 Andy Lutomirski
> + * Authors: Andy Lutomirski <luto@amacapital.net>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef ASM_X86_ARCHSLOWRANDOM_H
> +#define ASM_X86_ARCHSLOWRANDOM_H
> +
> +#ifndef CONFIG_ARCH_SLOW_RNG
> +# error archslowrng.h should not be included if !CONFIG_ARCH_SLOW_RNG
> +#endif
> +

I'm *seriously* questioning the wisdom of this.  A much saner thing
would be to do:

#ifndef CONFIG_ARCH_SLOW_RNG

/* Not supported */
static inline int arch_get_slow_rng_u64(u64 *v)
{
	(void)v;
	return 0;
}

#endif

... which is basically what we do for the archrandom stuff.

I'm also wondering if it makes sense to have a function which prefers
arch_get_random*() over this one as a preferred interface.  Something like:

int get_random_arch_u64_slow_ok(u64 *v)
{
	int i;
	u64 x = 0;
	unsigned long l;

	for (i = 0; i < 64/BITS_PER_LONG; i++) {
		if (!arch_get_random_long(&l))
			return arch_get_slow_rng_u64(v);

		x |=  l << (i*BITS_PER_LONG);
	}
	*v = l;
	return 0;
}

This still doesn't address the issue e.g. on x86 where RDRAND is
available but we haven't set up alternatives yet.  So it might be that
what we really want is to encapsulate this fallback in arch code and do
a more direct enumeration.

> +
> +static int kvm_get_slow_rng_u64(u64 *v)
> +{
> +	/*
> +	 * Allow migration from a hypervisor with the GET_RNG_SEED
> +	 * feature to a hypervisor without it.
> +	 */
> +	if (rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0)
> +		return 1;
> +	else
> +		return 0;
> +}

How about:

return rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0;

The naming also feels really inconsistent...
	
	-hpa

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Wed, Jul 16, 2014 at 2:59 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/16/2014 02:45 PM, Andy Lutomirski wrote:
>> diff --git a/arch/x86/include/asm/archslowrng.h b/arch/x86/include/asm/archslowrng.h
>> new file mode 100644
>> index 0000000..c8e8d0d
>> --- /dev/null
>> +++ b/arch/x86/include/asm/archslowrng.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * This file is part of the Linux kernel.
>> + *
>> + * Copyright (c) 2014 Andy Lutomirski
>> + * Authors: Andy Lutomirski <luto@amacapital.net>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#ifndef ASM_X86_ARCHSLOWRANDOM_H
>> +#define ASM_X86_ARCHSLOWRANDOM_H
>> +
>> +#ifndef CONFIG_ARCH_SLOW_RNG
>> +# error archslowrng.h should not be included if !CONFIG_ARCH_SLOW_RNG
>> +#endif
>> +
>
> I'm *seriously* questioning the wisdom of this.  A much saner thing
> would be to do:
>
> #ifndef CONFIG_ARCH_SLOW_RNG
>
> /* Not supported */
> static inline int arch_get_slow_rng_u64(u64 *v)
> {
>         (void)v;
>         return 0;
> }
>
> #endif
>
> ... which is basically what we do for the archrandom stuff.

The archrandom stuff defines the "not supported" variant in the
generic header, which is what I'm doing here.  I could wrap all of
asm/archslowrng.h in #ifdef CONFIG_ARCH_SLOW_RNG instead of putting
the #error in there, but I have no strong preference.

>
> I'm also wondering if it makes sense to have a function which prefers
> arch_get_random*() over this one as a preferred interface.  Something like:
>
> int get_random_arch_u64_slow_ok(u64 *v)
> {
>         int i;
>         u64 x = 0;
>         unsigned long l;
>
>         for (i = 0; i < 64/BITS_PER_LONG; i++) {
>                 if (!arch_get_random_long(&l))
>                         return arch_get_slow_rng_u64(v);
>
>                 x |=  l << (i*BITS_PER_LONG);
>         }
>         *v = l;
>         return 0;
> }

I played with something like this earlier, but I dropped it when it
ended up having exactly one user.  I suspect that the highly paranoid
will actually prefer seeding with both sources in init_std_data even
if RDRAND is available -- it costs very little and it provides a bit
of extra assurance.

>
> This still doesn't address the issue e.g. on x86 where RDRAND is
> available but we haven't set up alternatives yet.  So it might be that
> what we really want is to encapsulate this fallback in arch code and do
> a more direct enumeration.

My personal preference is to defer this until some user shows up.  I
think that even this would be too complicated for KASLR, which is the
only extremely early-boot user that I found.

Hmm.  Does the prandom stuff want to use this?

>
>> +
>> +static int kvm_get_slow_rng_u64(u64 *v)
>> +{
>> +     /*
>> +      * Allow migration from a hypervisor with the GET_RNG_SEED
>> +      * feature to a hypervisor without it.
>> +      */
>> +     if (rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0)
>> +             return 1;
>> +     else
>> +             return 0;
>> +}
>
> How about:
>
> return rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0;
>
> The naming also feels really inconsistent...

Better ideas welcome.  I could call the generic function
arch_get_pv_random_seed, but maybe someone will come up with a
non-paravirt implementation.

--Andy

>
>         -hpa
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Wed, Jul 16, 2014 at 3:13 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> My personal preference is to defer this until some user shows up.  I
> think that even this would be too complicated for KASLR, which is the
> only extremely early-boot user that I found.
>
> Hmm.  Does the prandom stuff want to use this?

prandom isn't even using rdrand.  I'd suggest fixing this separately,
or even just waiting until someone goes and deletes prandom.

--Andy

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[H. Peter Anvin]

On 07/16/2014 03:40 PM, Andy Lutomirski wrote:
> On Wed, Jul 16, 2014 at 3:13 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> My personal preference is to defer this until some user shows up.  I
>> think that even this would be too complicated for KASLR, which is the
>> only extremely early-boot user that I found.
>>
>> Hmm.  Does the prandom stuff want to use this?
> 
> prandom isn't even using rdrand.  I'd suggest fixing this separately,
> or even just waiting until someone goes and deletes prandom.
> 

prandom is exactly the opposite; it is designed for when we need
possibly low quality random numbers very quickly.  RDRAND is actually
too slow.

	-hpa

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Jul 16, 2014 4:00 PM, "H. Peter Anvin" <hpa@zytor.com> wrote:
>
> On 07/16/2014 03:40 PM, Andy Lutomirski wrote:
> > On Wed, Jul 16, 2014 at 3:13 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >> My personal preference is to defer this until some user shows up.  I
> >> think that even this would be too complicated for KASLR, which is the
> >> only extremely early-boot user that I found.
> >>
> >> Hmm.  Does the prandom stuff want to use this?
> >
> > prandom isn't even using rdrand.  I'd suggest fixing this separately,
> > or even just waiting until someone goes and deletes prandom.
> >
>
> prandom is exactly the opposite; it is designed for when we need
> possibly low quality random numbers very quickly.  RDRAND is actually
> too slow.

I meant that prandom isn't using rdrand for early seeding.

--Andy

>
>         -hpa
>
>

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[H. Peter Anvin]

On 07/16/2014 05:03 PM, Andy Lutomirski wrote:
>>
>> prandom is exactly the opposite; it is designed for when we need
>> possibly low quality random numbers very quickly.  RDRAND is actually
>> too slow.
> 
> I meant that prandom isn't using rdrand for early seeding.
> 

We should probably fix that.

	-hpa

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Theodore Ts'o]

On Wed, Jul 16, 2014 at 09:55:15PM -0700, H. Peter Anvin wrote:
> On 07/16/2014 05:03 PM, Andy Lutomirski wrote:
> >>
> > I meant that prandom isn't using rdrand for early seeding.
> > 
> 
> We should probably fix that.

It wouldn't hurt to explicitly use arch_get_random_long() in prandom,
but it does use get_random_bytes() in early seed, and for CPU's with
RDRAND present, we do use it in init_std_data() in
drivers/char/random.c, so prandom is already getting initialized via
an RNG (which is effectively a DRBG even if it doesn't pass all of
NIST's rules) which is derived from RDRAND.

Cheers,

					- Ted

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[H. Peter Anvin]

On 07/17/2014 03:33 AM, Theodore Ts'o wrote:
> On Wed, Jul 16, 2014 at 09:55:15PM -0700, H. Peter Anvin wrote:
>> On 07/16/2014 05:03 PM, Andy Lutomirski wrote:
>>>>
>>> I meant that prandom isn't using rdrand for early seeding.
>>>
>>
>> We should probably fix that.
> 
> It wouldn't hurt to explicitly use arch_get_random_long() in prandom,
> but it does use get_random_bytes() in early seed, and for CPU's with
> RDRAND present, we do use it in init_std_data() in
> drivers/char/random.c, so prandom is already getting initialized via
> an RNG (which is effectively a DRBG even if it doesn't pass all of
> NIST's rules) which is derived from RDRAND.
> 

I assumed he was referring to before alternatives.  Not sure if we use
prandom before that point, though.

	-hpa

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Thu, Jul 17, 2014 at 9:39 AM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/17/2014 03:33 AM, Theodore Ts'o wrote:
>> On Wed, Jul 16, 2014 at 09:55:15PM -0700, H. Peter Anvin wrote:
>>> On 07/16/2014 05:03 PM, Andy Lutomirski wrote:
>>>>>
>>>> I meant that prandom isn't using rdrand for early seeding.
>>>>
>>>
>>> We should probably fix that.
>>
>> It wouldn't hurt to explicitly use arch_get_random_long() in prandom,
>> but it does use get_random_bytes() in early seed, and for CPU's with
>> RDRAND present, we do use it in init_std_data() in
>> drivers/char/random.c, so prandom is already getting initialized via
>> an RNG (which is effectively a DRBG even if it doesn't pass all of
>> NIST's rules) which is derived from RDRAND.
>>
>
> I assumed he was referring to before alternatives.  Not sure if we use
> prandom before that point, though.

Unless I'm reading the code wrong, the prandom_reseed_late call can
happen after userspace is running.

Anyway, I'm working on a near-complete rewrite of the guest part of all of this.

--Andy

>
>         -hpa
>
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Theodore Ts'o]

On Thu, Jul 17, 2014 at 10:12:27AM -0700, Andy Lutomirski wrote:
> 
> Unless I'm reading the code wrong, the prandom_reseed_late call can
> happen after userspace is running.

But there is also the prandom_reseed() call, which happens early.

    	     	      		       	     	   	   - Ted

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Thu, Jul 17, 2014 at 10:32 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Thu, Jul 17, 2014 at 10:12:27AM -0700, Andy Lutomirski wrote:
>>
>> Unless I'm reading the code wrong, the prandom_reseed_late call can
>> happen after userspace is running.
>
> But there is also the prandom_reseed() call, which happens early.
>

Right -- I missed that.

>                                                            - Ted



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Hannes Frederic Sowa]

On Thu, Jul 17, 2014, at 19:34, Andy Lutomirski wrote:
> On Thu, Jul 17, 2014 at 10:32 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> > On Thu, Jul 17, 2014 at 10:12:27AM -0700, Andy Lutomirski wrote:
> >>
> >> Unless I'm reading the code wrong, the prandom_reseed_late call can
> >> happen after userspace is running.
> >
> > But there is also the prandom_reseed() call, which happens early.
> >
> 
> Right -- I missed that.

prandom_init is a core_initcall, prandom_reseed is a late_initcall.
During initialization of the network stack we have calls to prandom_u32
before the late_initcall happens. That said, I think it is not that
important to seed prandom with rdseed/rdrand as security relevant
entropy extraction should always use get_random_bytes(), but we should
do it nonetheless.

Bye,
Hannes

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

On Thu, Jul 17, 2014 at 11:42 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>
>
> On Thu, Jul 17, 2014, at 19:34, Andy Lutomirski wrote:
>> On Thu, Jul 17, 2014 at 10:32 AM, Theodore Ts'o <tytso@mit.edu> wrote:
>> > On Thu, Jul 17, 2014 at 10:12:27AM -0700, Andy Lutomirski wrote:
>> >>
>> >> Unless I'm reading the code wrong, the prandom_reseed_late call can
>> >> happen after userspace is running.
>> >
>> > But there is also the prandom_reseed() call, which happens early.
>> >
>>
>> Right -- I missed that.
>
> prandom_init is a core_initcall, prandom_reseed is a late_initcall.
> During initialization of the network stack we have calls to prandom_u32
> before the late_initcall happens. That said, I think it is not that
> important to seed prandom with rdseed/rdrand as security relevant
> entropy extraction should always use get_random_bytes(), but we should
> do it nonetheless.
>

Regardless, I don't want to do this as part of this patch series.  One
thing at a time...

--Andy

Re: [PATCH v3 2/5] random,x86: Add arch_get_slow_rng_u64

[Daniel Borkmann]

On 07/17/2014 12:59 AM, H. Peter Anvin wrote:
> On 07/16/2014 03:40 PM, Andy Lutomirski wrote:
>> On Wed, Jul 16, 2014 at 3:13 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> My personal preference is to defer this until some user shows up.  I
>>> think that even this would be too complicated for KASLR, which is the
>>> only extremely early-boot user that I found.
>>>
>>> Hmm.  Does the prandom stuff want to use this?
>>
>> prandom isn't even using rdrand.  I'd suggest fixing this separately,
>> or even just waiting until someone goes and deletes prandom.
>
> prandom is exactly the opposite; it is designed for when we need
> possibly low quality random numbers very quickly.  RDRAND is actually
> too slow.

Yep, prandom() is quite heavily used in the network stack where it's
traded for speed.

[PATCH v3 3/5] random: Seed pools from arch_get_slow_rng_u64 at startup

[Andy Lutomirski]

This should help solve the problem of guests starting out with
predictable RNG state.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..17ad33d 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1261,6 +1261,13 @@ static void init_std_data(struct entropy_store *r)
 		mix_pool_bytes(r, &rv, sizeof(rv), NULL);
 	}
 	mix_pool_bytes(r, utsname(), sizeof(*(utsname())), NULL);
+
+	for (i = 0; i < 4; i++) {
+		u64 rv64;
+
+		if (arch_get_slow_rng_u64(&rv64))
+			mix_pool_bytes(r, &rv64, sizeof(rv64), NULL);
+	}
 }
 
 /*
-- 
1.9.3

[PATCH v3 4/5] random: Log how many bits we managed to seed with in init_std_data

[Andy Lutomirski]

This is useful for making sure that init_std_data is working
correctly and for allaying fear when this happens:

    random: xyz urandom read with SMALL_NUMBER bits of entropy available

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 17ad33d..10e9642 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1251,12 +1251,16 @@ static void init_std_data(struct entropy_store *r)
 	int i;
 	ktime_t now = ktime_get_real();
 	unsigned long rv;
+	int arch_seed_bits = 0, arch_random_bits = 0, slow_rng_bits = 0;
 
 	r->last_pulled = jiffies;
 	mix_pool_bytes(r, &now, sizeof(now), NULL);
 	for (i = r->poolinfo->poolbytes; i > 0; i -= sizeof(rv)) {
-		if (!arch_get_random_seed_long(&rv) &&
-		    !arch_get_random_long(&rv))
+		if (arch_get_random_seed_long(&rv))
+			arch_seed_bits += 8 * sizeof(rv);
+		else if (arch_get_random_long(&rv))
+			arch_random_bits += 8 * sizeof(rv);
+		else
 			rv = random_get_entropy();
 		mix_pool_bytes(r, &rv, sizeof(rv), NULL);
 	}
@@ -1265,9 +1269,14 @@ static void init_std_data(struct entropy_store *r)
 	for (i = 0; i < 4; i++) {
 		u64 rv64;
 
-		if (arch_get_slow_rng_u64(&rv64))
+		if (arch_get_slow_rng_u64(&rv64)) {
 			mix_pool_bytes(r, &rv64, sizeof(rv64), NULL);
+			slow_rng_bits += 8 * sizeof(rv64);
+		}
 	}
+
+	pr_info("random: seeded %s pool with %d bits of arch random seed, %d bits of arch random, and %d bits of arch slow rng\n",
+		r->name, arch_seed_bits, arch_random_bits, slow_rng_bits);
 }
 
 /*
-- 
1.9.3

[PATCH v3 5/5] x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

[Andy Lutomirski]

It's considerably better than any of the alternatives on KVM.

Rather than reinventing all of the cpu feature query code, this fixes
native_cpuid to work in PIC objects.

I haven't combined it with boot/cpuflags.c's cpuid implementation:
including asm/processor.h from boot/cpuflags.c results in a flood of
unrelated errors, and fixing it might be messy.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/boot/compressed/aslr.c  | 27 +++++++++++++++++++++++++++
 arch/x86/include/asm/processor.h | 21 ++++++++++++++++++---
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index fc6091a..8583f0e 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -5,6 +5,8 @@
 #include <asm/archrandom.h>
 #include <asm/e820.h>
 
+#include <uapi/asm/kvm_para.h>
+
 #include <generated/compile.h>
 #include <linux/module.h>
 #include <linux/uts.h>
@@ -15,6 +17,22 @@
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
 
+static bool kvm_para_has_feature(unsigned int feature)
+{
+	u32 kvm_base;
+	u32 features;
+
+	if (!has_cpuflag(X86_FEATURE_HYPERVISOR))
+		return false;
+
+	kvm_base = hypervisor_cpuid_base("KVMKVMKVM\0\0\0", KVM_CPUID_FEATURES);
+	if (!kvm_base)
+		return false;
+
+	features = cpuid_eax(kvm_base | KVM_CPUID_FEATURES);
+	return features & (1UL << feature);
+}
+
 #define I8254_PORT_CONTROL	0x43
 #define I8254_PORT_COUNTER0	0x40
 #define I8254_CMD_READBACK	0xC0
@@ -81,6 +99,15 @@ static unsigned long get_random_long(void)
 		}
 	}
 
+	if (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED)) {
+		u64 seed;
+
+		debug_putstr(" MSR_KVM_GET_RNG_SEED");
+		rdmsrl(MSR_KVM_GET_RNG_SEED, seed);
+		random ^= (unsigned long)seed;
+		use_i8254 = false;
+	}
+
 	if (has_cpuflag(X86_FEATURE_TSC)) {
 		debug_putstr(" RDTSC");
 		rdtscll(raw);
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a4ea023..6096f3c 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -189,10 +189,25 @@ static inline int have_cpuid_p(void)
 static inline void native_cpuid(unsigned int *eax, unsigned int *ebx,
 				unsigned int *ecx, unsigned int *edx)
 {
-	/* ecx is often an input as well as an output. */
-	asm volatile("cpuid"
+	/*
+	 * This function can be used from the boot code, so it needs
+	 * to avoid using EBX in constraints in PIC mode.
+	 *
+	 * ecx is often an input as well as an output.
+	 */
+	asm volatile(".ifnc %%ebx,%1 ; .ifnc %%rbx,%1           \n\t"
+		     "movl  %%ebx,%1                            \n\t"
+		     ".endif ; .endif                           \n\t"
+		     "cpuid					\n\t"
+		     ".ifnc %%ebx,%1 ; .ifnc %%rbx,%1           \n\t"
+		     "xchgl %%ebx,%1                            \n\t"
+		     ".endif ; .endif"
 	    : "=a" (*eax),
-	      "=b" (*ebx),
+#if defined(__i386__) && defined(__PIC__)
+	      "=r" (*ebx),	/* gcc won't let us use ebx */
+#else
+	      "=b" (*ebx),	/* ebx is okay */
+#endif
 	      "=c" (*ecx),
 	      "=d" (*edx)
 	    : "0" (*eax), "2" (*ecx)
-- 
1.9.3