2024-02-21 12:33:20

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

There are few uses of CoCo that don't rely on working cryptography and
hence a working RNG. Unfortunately, the CoCo threat model means that the
VM host cannot be trusted and may actively work against guests to
extract secrets or manipulate computation. Since a malicious host can
modify or observe nearly all inputs to guests, the only remaining source
of entropy for CoCo guests is RDRAND.

If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
is meant to gracefully continue on gathering entropy from other sources,
but since there aren't other sources on CoCo, this is catastrophic.
This is mostly a concern at boot time when initially seeding the RNG, as
after that the consequences of a broken RDRAND are much more
theoretical.

So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
fails, panic(). This will also trigger if the system is booted without
RDRAND, as RDRAND is essential for a safe CoCo boot.

This patch is deliberately written to be "just a CoCo x86 driver
feature" and not part of the RNG itself. Many device drivers and
platforms have some desire to contribute something to the RNG, and
add_device_randomness() is specifically meant for this purpose. Any
driver can call this with seed data of any quality, or even garbage
quality, and it can only possibly make the quality of the RNG better or
have no effect, but can never make it worse. Rather than trying to
build something into the core of the RNG, this patch interprets the
particular CoCo issue as just a CoCo issue, and therefore separates this
all out into driver (well, arch/platform) code.

Cc: Borislav Petkov <[email protected]>
Cc: Daniel P. Berrangé <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Elena Reshetova <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v2->v3:
- Remove patch that handled generic RDRAND failures, because that
doesn't really have any implication for the RNG, since it's supposed
to run fine on systems without RDRAND anyway, and CoCo is a weird
special case. If people still want an extra generic RDRAND failure
handler, that's standalone anyway, so we can do that disconnected from
this patch. No need to make it a series.
- Update comments and commit message to reflect this.

Changes v1->v2:
- panic() instead of BUG_ON(), as suggested by Andi Kleen.
- Update comments, now that we have info from AMD and Intel.

arch/x86/coco/core.c | 36 ++++++++++++++++++++++++++++++++++++
arch/x86/include/asm/coco.h | 2 ++
arch/x86/kernel/setup.c | 2 ++
3 files changed, 40 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index eeec9986570e..0a5d59966d6d 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -3,13 +3,16 @@
* Confidential Computing Platform Capability checks
*
* Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
*
* Author: Tom Lendacky <[email protected]>
*/

#include <linux/export.h>
#include <linux/cc_platform.h>
+#include <linux/random.h>

+#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>

@@ -153,3 +156,36 @@ __init void cc_set_mask(u64 mask)
{
cc_mask = mask;
}
+
+__init void cc_random_init(void)
+{
+ unsigned long rng_seed[32 / sizeof(long)];
+ size_t i, longs;
+
+ if (cc_vendor == CC_VENDOR_NONE)
+ return;
+
+ /*
+ * Since the CoCo threat model includes the host, the only reliable
+ * source of entropy that can be neither observed nor manipulated is
+ * RDRAND. Usually, RDRAND failure is considered tolerable, but since
+ * CoCo guests have no other unobservable source of entropy, it's
+ * important to at least ensure the RNG gets some initial random seeds.
+ */
+ for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
+ longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
+
+ /*
+ * A zero return value means that the guest doesn't have RDRAND
+ * or the CPU is physically broken, and in both cases that
+ * means most crypto inside of the CoCo instance will be
+ * broken, defeating the purpose of CoCo in the first place. So
+ * just panic here because it's absolutely unsafe to continue
+ * executing.
+ */
+ if (longs == 0)
+ panic("RDRAND is defective.");
+ }
+ add_device_randomness(rng_seed, sizeof(rng_seed));
+ memzero_explicit(rng_seed, sizeof(rng_seed));
+}
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index 76c310b19b11..e9d059449885 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -15,6 +15,7 @@ extern enum cc_vendor cc_vendor;
void cc_set_mask(u64 mask);
u64 cc_mkenc(u64 val);
u64 cc_mkdec(u64 val);
+void cc_random_init(void);
#else
#define cc_vendor (CC_VENDOR_NONE)

@@ -27,6 +28,7 @@ static inline u64 cc_mkdec(u64 val)
{
return val;
}
+static inline void cc_random_init(void) { }
#endif

#endif /* _ASM_X86_COCO_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 84201071dfac..30a653cfc7d2 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -36,6 +36,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
#include <asm/gart.h>
@@ -994,6 +995,7 @@ void __init setup_arch(char **cmdline_p)
* memory size.
*/
mem_encrypt_setup_arch();
+ cc_random_init();

efi_fake_memmap();
efi_find_mirror();
--
2.43.0



2024-02-21 14:35:20

by Elena Reshetova

[permalink] [raw]
Subject: RE: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems


> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrangé <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Elena Reshetova <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

Reviewed-by: Elena Reshetova <[email protected]>

> ---
> Changes v2->v3:
> - Remove patch that handled generic RDRAND failures, because that
> doesn't really have any implication for the RNG, since it's supposed
> to run fine on systems without RDRAND anyway, and CoCo is a weird
> special case. If people still want an extra generic RDRAND failure
> handler, that's standalone anyway, so we can do that disconnected from
> this patch. No need to make it a series.
> - Update comments and commit message to reflect this.
>
> Changes v1->v2:
> - panic() instead of BUG_ON(), as suggested by Andi Kleen.
> - Update comments, now that we have info from AMD and Intel.
>
> arch/x86/coco/core.c | 36 ++++++++++++++++++++++++++++++++++++
> arch/x86/include/asm/coco.h | 2 ++
> arch/x86/kernel/setup.c | 2 ++
> 3 files changed, 40 insertions(+)
>
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index eeec9986570e..0a5d59966d6d 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -3,13 +3,16 @@
> * Confidential Computing Platform Capability checks
> *
> * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights
> Reserved.
> *
> * Author: Tom Lendacky <[email protected]>
> */
>
> #include <linux/export.h>
> #include <linux/cc_platform.h>
> +#include <linux/random.h>
>
> +#include <asm/archrandom.h>
> #include <asm/coco.h>
> #include <asm/processor.h>
>
> @@ -153,3 +156,36 @@ __init void cc_set_mask(u64 mask)
> {
> cc_mask = mask;
> }
> +
> +__init void cc_random_init(void)
> +{
> + unsigned long rng_seed[32 / sizeof(long)];
> + size_t i, longs;
> +
> + if (cc_vendor == CC_VENDOR_NONE)
> + return;
> +
> + /*
> + * Since the CoCo threat model includes the host, the only reliable
> + * source of entropy that can be neither observed nor manipulated is
> + * RDRAND. Usually, RDRAND failure is considered tolerable, but since
> + * CoCo guests have no other unobservable source of entropy, it's
> + * important to at least ensure the RNG gets some initial random seeds.
> + */
> + for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
> + longs = arch_get_random_longs(&rng_seed[i],
> ARRAY_SIZE(rng_seed) - i);
> +
> + /*
> + * A zero return value means that the guest doesn't have RDRAND
> + * or the CPU is physically broken, and in both cases that
> + * means most crypto inside of the CoCo instance will be
> + * broken, defeating the purpose of CoCo in the first place. So
> + * just panic here because it's absolutely unsafe to continue
> + * executing.
> + */
> + if (longs == 0)
> + panic("RDRAND is defective.");
> + }
> + add_device_randomness(rng_seed, sizeof(rng_seed));
> + memzero_explicit(rng_seed, sizeof(rng_seed));
> +}
> diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
> index 76c310b19b11..e9d059449885 100644
> --- a/arch/x86/include/asm/coco.h
> +++ b/arch/x86/include/asm/coco.h
> @@ -15,6 +15,7 @@ extern enum cc_vendor cc_vendor;
> void cc_set_mask(u64 mask);
> u64 cc_mkenc(u64 val);
> u64 cc_mkdec(u64 val);
> +void cc_random_init(void);
> #else
> #define cc_vendor (CC_VENDOR_NONE)
>
> @@ -27,6 +28,7 @@ static inline u64 cc_mkdec(u64 val)
> {
> return val;
> }
> +static inline void cc_random_init(void) { }
> #endif
>
> #endif /* _ASM_X86_COCO_H */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 84201071dfac..30a653cfc7d2 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -36,6 +36,7 @@
> #include <asm/bios_ebda.h>
> #include <asm/bugs.h>
> #include <asm/cacheinfo.h>
> +#include <asm/coco.h>
> #include <asm/cpu.h>
> #include <asm/efi.h>
> #include <asm/gart.h>
> @@ -994,6 +995,7 @@ void __init setup_arch(char **cmdline_p)
> * memory size.
> */
> mem_encrypt_setup_arch();
> + cc_random_init();
>
> efi_fake_memmap();
> efi_find_mirror();
> --
> 2.43.0
>

2024-02-21 15:36:28

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Wed, Feb 21, 2024 at 1:33 PM Jason A. Donenfeld <[email protected]> wrote:
>
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrangé <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Elena Reshetova <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

Also,

Cc: [email protected]

At least, I think that's probably what we want, though I don't know
what version range is relevant for CoCo.

2024-02-21 15:59:15

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Wed, Feb 21, 2024 at 01:32:40PM +0100, Jason A. Donenfeld wrote:
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrang? <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Elena Reshetova <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

Reviewed-by: Theodore Ts'o <[email protected]>

- Ted

2024-02-21 16:55:52

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On 2/21/24 04:32, Jason A. Donenfeld wrote:
> +__init void cc_random_init(void)
> +{
> + unsigned long rng_seed[32 / sizeof(long)];

My only nit with this is the magic "32".

Why not 16? Or 64?

2024-02-21 17:20:37

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Wed, Feb 21, 2024 at 5:55 PM Dave Hansen <[email protected]> wrote:
>
> On 2/21/24 04:32, Jason A. Donenfeld wrote:
> > +__init void cc_random_init(void)
> > +{
> > + unsigned long rng_seed[32 / sizeof(long)];
>
> My only nit with this is the magic "32".
>
> Why not 16? Or 64?

32 bytes = 256-bits = what we're targeting. Very normal thing to see
places in the RNG, used all over random.c and lots of platform
drivers. Pretty obvious and straightforward to anyone familiar with
this kind of code. Not the kind of thing you'd want to replace with
some abstracted constant that makes you search.

2024-02-21 19:45:22

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Wed, Feb 21, 2024 at 01:32:40PM +0100, Jason A. Donenfeld wrote:
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrang? <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Elena Reshetova <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Kirill A. Shutemov <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

Reviewed-by: Kirill A. Shutemov <[email protected]>

--
Kiryl Shutsemau / Kirill A. Shutemov

2024-02-21 22:49:08

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On 2/21/24 09:19, Jason A. Donenfeld wrote:
> On Wed, Feb 21, 2024 at 5:55 PM Dave Hansen <[email protected]> wrote:
>> On 2/21/24 04:32, Jason A. Donenfeld wrote:
>>> +__init void cc_random_init(void)
>>> +{
>>> + unsigned long rng_seed[32 / sizeof(long)];
>> My only nit with this is the magic "32".
>>
>> Why not 16? Or 64?
> 32 bytes = 256-bits = what we're targeting. Very normal thing to see
> places in the RNG, used all over random.c and lots of platform
> drivers. Pretty obvious and straightforward to anyone familiar with
> this kind of code. Not the kind of thing you'd want to replace with
> some abstracted constant that makes you search.

OK, so we're trying to get 256 bits of seed data from RDRAND?

There's an entire section of the Intel whitepaper[1]: "Generating Seeds
from RDRAND". It describes one "method of turning 512 128-bit samples
from the DRNG into a 128-bit seed value". I was naively thinking that
if the kernel wants 256 bits of seed data from RDRAND, it might take
2*(512 128-bit samples).

I'm not suggesting that we use the exact construction from that
whitepaper, but I'm reasonably sure I could actually explain to someone
where a magic 1024 came from.

I also went through a smattering of add_device_randomness() users. I
didn't see much of a pattern there that seemed to line up with a
256-bits convention. If anything they seemed to just use what they had
laying around. I saw byte counts of 16, 21, 12, 8, 1, strlen(), 56.
But no pattern I could discern. Did you mean something different by
"platform drivers"?

If we're going to have arch/x86-specific crud, it would be great to make
it obvious and straightforward to those of us simple folk that are
familiar with arch/x86 code.

1.
https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html



2024-02-21 23:09:53

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Wed, Feb 21, 2024 at 11:47 PM Dave Hansen <[email protected]> wrote:
> OK, so we're trying to get 256 bits of seed data from RDRAND?

Yes. This fills a 32-byte buffer of longs up with output from
arch_get_random_longs().

> But no pattern I could discern. Did you mean something different by
> "platform drivers"?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/um/kernel/um_arch.c#n413
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c#n655
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/gcc-plugins/latent_entropy_plugin.c#n90


> If we're going to have arch/x86-specific crud, it would be great to make
> it obvious and straightforward to those of us simple folk that are
> familiar with arch/x86 code.

If you insist, I'll stick a local `enum { RANDOM_BYTES_COUNT = 32 }`
in that function or something. Seems unnecessary to me but if that's
what you need no problem. Would that suffice? Or a different variable
name? Or a comment on "what is 32 about?"

Jason

2024-02-21 23:35:36

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On 2/21/24 15:09, Jason A. Donenfeld wrote:
> On Wed, Feb 21, 2024 at 11:47 PM Dave Hansen <[email protected]> wrote:
>> OK, so we're trying to get 256 bits of seed data from RDRAND?
>
> Yes. This fills a 32-byte buffer of longs up with output from
> arch_get_random_longs().

That's what it's doing mechanically, but what's the end goal?

>> But no pattern I could discern. Did you mean something different by
>> "platform drivers"?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/um/kernel/um_arch.c#n413
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c#n655

Gotcha.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/gcc-plugins/latent_entropy_plugin.c#n90

Isn't that one 32 HOST_WIDE_INTs and not 32 bytes? :)

>> If we're going to have arch/x86-specific crud, it would be great to make
>> it obvious and straightforward to those of us simple folk that are
>> familiar with arch/x86 code.
>
> If you insist, I'll stick a local `enum { RANDOM_BYTES_COUNT = 32 }`
> in that function or something. Seems unnecessary to me but if that's
> what you need no problem. Would that suffice? Or a different variable
> name? Or a comment on "what is 32 about?"

"what is 32 about?" would be great. What's the goal, and why is 32
enough to reach that goal?

2024-02-22 02:07:10

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v4] x86/coco: Require seeding RNG with RDRAND on CoCo systems

There are few uses of CoCo that don't rely on working cryptography and
hence a working RNG. Unfortunately, the CoCo threat model means that the
VM host cannot be trusted and may actively work against guests to
extract secrets or manipulate computation. Since a malicious host can
modify or observe nearly all inputs to guests, the only remaining source
of entropy for CoCo guests is RDRAND.

If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
is meant to gracefully continue on gathering entropy from other sources,
but since there aren't other sources on CoCo, this is catastrophic.
This is mostly a concern at boot time when initially seeding the RNG, as
after that the consequences of a broken RDRAND are much more
theoretical.

So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
fails, panic(). This will also trigger if the system is booted without
RDRAND, as RDRAND is essential for a safe CoCo boot.

This patch is deliberately written to be "just a CoCo x86 driver
feature" and not part of the RNG itself. Many device drivers and
platforms have some desire to contribute something to the RNG, and
add_device_randomness() is specifically meant for this purpose. Any
driver can call this with seed data of any quality, or even garbage
quality, and it can only possibly make the quality of the RNG better or
have no effect, but can never make it worse. Rather than trying to
build something into the core of the RNG, this patch interprets the
particular CoCo issue as just a CoCo issue, and therefore separates this
all out into driver (well, arch/platform) code.

Cc: Borislav Petkov <[email protected]>
Cc: Daniel P. Berrangé <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Reviewed-by: Elena Reshetova <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Theodore Ts'o <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v3->v4:
- Add stable@ tag and reviewed-by lines.
- Add comment for Dave explaining where the "32" comes from.

arch/x86/coco/core.c | 40 +++++++++++++++++++++++++++++++++++++
arch/x86/include/asm/coco.h | 2 ++
arch/x86/kernel/setup.c | 2 ++
3 files changed, 44 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index eeec9986570e..0e988bff4aec 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -3,13 +3,16 @@
* Confidential Computing Platform Capability checks
*
* Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
*
* Author: Tom Lendacky <[email protected]>
*/

#include <linux/export.h>
#include <linux/cc_platform.h>
+#include <linux/random.h>

+#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>

@@ -153,3 +156,40 @@ __init void cc_set_mask(u64 mask)
{
cc_mask = mask;
}
+
+__init void cc_random_init(void)
+{
+ /*
+ * The seed is 32 bytes (in units of longs), which is 256 bits, which
+ * is the security level that the RNG is targeting.
+ */
+ unsigned long rng_seed[32 / sizeof(long)];
+ size_t i, longs;
+
+ if (cc_vendor == CC_VENDOR_NONE)
+ return;
+
+ /*
+ * Since the CoCo threat model includes the host, the only reliable
+ * source of entropy that can be neither observed nor manipulated is
+ * RDRAND. Usually, RDRAND failure is considered tolerable, but since
+ * CoCo guests have no other unobservable source of entropy, it's
+ * important to at least ensure the RNG gets some initial random seeds.
+ */
+ for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
+ longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
+
+ /*
+ * A zero return value means that the guest doesn't have RDRAND
+ * or the CPU is physically broken, and in both cases that
+ * means most crypto inside of the CoCo instance will be
+ * broken, defeating the purpose of CoCo in the first place. So
+ * just panic here because it's absolutely unsafe to continue
+ * executing.
+ */
+ if (longs == 0)
+ panic("RDRAND is defective.");
+ }
+ add_device_randomness(rng_seed, sizeof(rng_seed));
+ memzero_explicit(rng_seed, sizeof(rng_seed));
+}
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index 76c310b19b11..e9d059449885 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -15,6 +15,7 @@ extern enum cc_vendor cc_vendor;
void cc_set_mask(u64 mask);
u64 cc_mkenc(u64 val);
u64 cc_mkdec(u64 val);
+void cc_random_init(void);
#else
#define cc_vendor (CC_VENDOR_NONE)

@@ -27,6 +28,7 @@ static inline u64 cc_mkdec(u64 val)
{
return val;
}
+static inline void cc_random_init(void) { }
#endif

#endif /* _ASM_X86_COCO_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 84201071dfac..30a653cfc7d2 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -36,6 +36,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
#include <asm/gart.h>
@@ -994,6 +995,7 @@ void __init setup_arch(char **cmdline_p)
* memory size.
*/
mem_encrypt_setup_arch();
+ cc_random_init();

efi_fake_memmap();
efi_find_mirror();
--
2.43.2


2024-02-22 07:35:43

by Elena Reshetova

[permalink] [raw]
Subject: RE: [PATCH v3] x86/coco: Require seeding RNG with RDRAND on CoCo systems


> On 2/21/24 09:19, Jason A. Donenfeld wrote:
> > On Wed, Feb 21, 2024 at 5:55 PM Dave Hansen <[email protected]>
> wrote:
> >> On 2/21/24 04:32, Jason A. Donenfeld wrote:
> >>> +__init void cc_random_init(void)
> >>> +{
> >>> + unsigned long rng_seed[32 / sizeof(long)];
> >> My only nit with this is the magic "32".
> >>
> >> Why not 16? Or 64?
> > 32 bytes = 256-bits = what we're targeting. Very normal thing to see
> > places in the RNG, used all over random.c and lots of platform
> > drivers. Pretty obvious and straightforward to anyone familiar with
> > this kind of code. Not the kind of thing you'd want to replace with
> > some abstracted constant that makes you search.
>
> OK, so we're trying to get 256 bits of seed data from RDRAND?
>
> There's an entire section of the Intel whitepaper[1]: "Generating Seeds
> from RDRAND". It describes one "method of turning 512 128-bit samples
> from the DRNG into a 128-bit seed value". I was naively thinking that
> if the kernel wants 256 bits of seed data from RDRAND, it might take
> 2*(512 128-bit samples).

Yes, this would be a proper way of doing it in the absence of 100% entropic
seed from RDSEED, but this guidance doesn’t quite
apply in Linux RNG case since we are not really able to seed ChaCha PRNG
directly anyhow with the input from RDRAND: we are only allowed to
contribute to the entropy pool. If you want to properly follow
NIST standards on DRNGs (SP800-90A/B/C), we have many non-compliancy
issues in Linux RNG even without the CoCo case. Solving this while taking
into account people's fears about RDRAND/RDSEED trustworthiness is not
easy.

Best Regards,
Elena.

>
> I'm not suggesting that we use the exact construction from that
> whitepaper, but I'm reasonably sure I could actually explain to someone
> where a magic 1024 came from.
>
> I also went through a smattering of add_device_randomness() users. I
> didn't see much of a pattern there that seemed to line up with a
> 256-bits convention. If anything they seemed to just use what they had
> laying around. I saw byte counts of 16, 21, 12, 8, 1, strlen(), 56.
> But no pattern I could discern. Did you mean something different by
> "platform drivers"?
>
> If we're going to have arch/x86-specific crud, it would be great to make
> it obvious and straightforward to those of us simple folk that are
> familiar with arch/x86 code.
>
> 1.
> https://www.intel.com/content/www/us/en/developer/articles/guide/intel-
> digital-random-number-generator-drng-software-implementation-guide.html
>

2024-02-23 22:05:44

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v4] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On 2/21/24 20:05, Jason A. Donenfeld wrote:
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrangé <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> Reviewed-by: Elena Reshetova <[email protected]>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
> Reviewed-by: Theodore Ts'o <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>
> ---
> Changes v3->v4:
> - Add stable@ tag and reviewed-by lines.
> - Add comment for Dave explaining where the "32" comes from.
>
> arch/x86/coco/core.c | 40 +++++++++++++++++++++++++++++++++++++
> arch/x86/include/asm/coco.h | 2 ++
> arch/x86/kernel/setup.c | 2 ++
> 3 files changed, 44 insertions(+)
>
> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
> index eeec9986570e..0e988bff4aec 100644
> --- a/arch/x86/coco/core.c
> +++ b/arch/x86/coco/core.c
> @@ -3,13 +3,16 @@
> * Confidential Computing Platform Capability checks
> *
> * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
> *
> * Author: Tom Lendacky <[email protected]>
> */
>
> #include <linux/export.h>
> #include <linux/cc_platform.h>
> +#include <linux/random.h>
>
> +#include <asm/archrandom.h>
> #include <asm/coco.h>
> #include <asm/processor.h>
>
> @@ -153,3 +156,40 @@ __init void cc_set_mask(u64 mask)
> {
> cc_mask = mask;
> }
> +
> +__init void cc_random_init(void)
> +{
> + /*
> + * The seed is 32 bytes (in units of longs), which is 256 bits, which
> + * is the security level that the RNG is targeting.
> + */
> + unsigned long rng_seed[32 / sizeof(long)];
> + size_t i, longs;
> +
> + if (cc_vendor == CC_VENDOR_NONE)

I responded to an earlier version of this patch, adding that response here:

You probably want to use:

if (!cc_platform_has(CC_GUEST_MEM_ENCRYPT))
return;

Otherwise, you can hit the bare-metal case where AMD SME is active and
then cc_vendor will not be CC_VENDOR_NONE.

Thanks,
Tom

> + return;
> +
> + /*
> + * Since the CoCo threat model includes the host, the only reliable
> + * source of entropy that can be neither observed nor manipulated is
> + * RDRAND. Usually, RDRAND failure is considered tolerable, but since
> + * CoCo guests have no other unobservable source of entropy, it's
> + * important to at least ensure the RNG gets some initial random seeds.
> + */
> + for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
> + longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
> +
> + /*
> + * A zero return value means that the guest doesn't have RDRAND
> + * or the CPU is physically broken, and in both cases that
> + * means most crypto inside of the CoCo instance will be
> + * broken, defeating the purpose of CoCo in the first place. So
> + * just panic here because it's absolutely unsafe to continue
> + * executing.
> + */
> + if (longs == 0)
> + panic("RDRAND is defective.");
> + }
> + add_device_randomness(rng_seed, sizeof(rng_seed));
> + memzero_explicit(rng_seed, sizeof(rng_seed));
> +}
> diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
> index 76c310b19b11..e9d059449885 100644
> --- a/arch/x86/include/asm/coco.h
> +++ b/arch/x86/include/asm/coco.h
> @@ -15,6 +15,7 @@ extern enum cc_vendor cc_vendor;
> void cc_set_mask(u64 mask);
> u64 cc_mkenc(u64 val);
> u64 cc_mkdec(u64 val);
> +void cc_random_init(void);
> #else
> #define cc_vendor (CC_VENDOR_NONE)
>
> @@ -27,6 +28,7 @@ static inline u64 cc_mkdec(u64 val)
> {
> return val;
> }
> +static inline void cc_random_init(void) { }
> #endif
>
> #endif /* _ASM_X86_COCO_H */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 84201071dfac..30a653cfc7d2 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -36,6 +36,7 @@
> #include <asm/bios_ebda.h>
> #include <asm/bugs.h>
> #include <asm/cacheinfo.h>
> +#include <asm/coco.h>
> #include <asm/cpu.h>
> #include <asm/efi.h>
> #include <asm/gart.h>
> @@ -994,6 +995,7 @@ void __init setup_arch(char **cmdline_p)
> * memory size.
> */
> mem_encrypt_setup_arch();
> + cc_random_init();
>
> efi_fake_memmap();
> efi_find_mirror();

2024-02-24 01:16:01

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v4] x86/coco: Require seeding RNG with RDRAND on CoCo systems

Hi Tom,

On Fri, Feb 23, 2024 at 11:05 PM Tom Lendacky <[email protected]> wrote:
>
> On 2/21/24 20:05, Jason A. Donenfeld wrote:
> > +
> > + if (cc_vendor == CC_VENDOR_NONE)
>
> I responded to an earlier version of this patch, adding that response here:
>
> You probably want to use:
>
> if (!cc_platform_has(CC_GUEST_MEM_ENCRYPT))
> return;
>
> Otherwise, you can hit the bare-metal case where AMD SME is active and
> then cc_vendor will not be CC_VENDOR_NONE.

Nice catch, thanks. I'll do that for v+1.


Jason

2024-02-24 01:27:43

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v5] x86/coco: Require seeding RNG with RDRAND on CoCo systems

There are few uses of CoCo that don't rely on working cryptography and
hence a working RNG. Unfortunately, the CoCo threat model means that the
VM host cannot be trusted and may actively work against guests to
extract secrets or manipulate computation. Since a malicious host can
modify or observe nearly all inputs to guests, the only remaining source
of entropy for CoCo guests is RDRAND.

If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
is meant to gracefully continue on gathering entropy from other sources,
but since there aren't other sources on CoCo, this is catastrophic.
This is mostly a concern at boot time when initially seeding the RNG, as
after that the consequences of a broken RDRAND are much more
theoretical.

So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
fails, panic(). This will also trigger if the system is booted without
RDRAND, as RDRAND is essential for a safe CoCo boot.

This patch is deliberately written to be "just a CoCo x86 driver
feature" and not part of the RNG itself. Many device drivers and
platforms have some desire to contribute something to the RNG, and
add_device_randomness() is specifically meant for this purpose. Any
driver can call this with seed data of any quality, or even garbage
quality, and it can only possibly make the quality of the RNG better or
have no effect, but can never make it worse. Rather than trying to
build something into the core of the RNG, this patch interprets the
particular CoCo issue as just a CoCo issue, and therefore separates this
all out into driver (well, arch/platform) code.

Cc: Borislav Petkov <[email protected]>
Cc: Daniel P. Berrangé <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Reviewed-by: Elena Reshetova <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Theodore Ts'o <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v4->v5:
- Use `cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)` instead of checking
cc_vendor, per Tom's recommendation.

arch/x86/coco/core.c | 40 +++++++++++++++++++++++++++++++++++++
arch/x86/include/asm/coco.h | 2 ++
arch/x86/kernel/setup.c | 2 ++
3 files changed, 44 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index eeec9986570e..b419a29ef0e5 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -3,13 +3,16 @@
* Confidential Computing Platform Capability checks
*
* Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
*
* Author: Tom Lendacky <[email protected]>
*/

#include <linux/export.h>
#include <linux/cc_platform.h>
+#include <linux/random.h>

+#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>

@@ -153,3 +156,40 @@ __init void cc_set_mask(u64 mask)
{
cc_mask = mask;
}
+
+__init void cc_random_init(void)
+{
+ /*
+ * The seed is 32 bytes (in units of longs), which is 256 bits, which
+ * is the security level that the RNG is targeting.
+ */
+ unsigned long rng_seed[32 / sizeof(long)];
+ size_t i, longs;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+ return;
+
+ /*
+ * Since the CoCo threat model includes the host, the only reliable
+ * source of entropy that can be neither observed nor manipulated is
+ * RDRAND. Usually, RDRAND failure is considered tolerable, but since
+ * CoCo guests have no other unobservable source of entropy, it's
+ * important to at least ensure the RNG gets some initial random seeds.
+ */
+ for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
+ longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
+
+ /*
+ * A zero return value means that the guest doesn't have RDRAND
+ * or the CPU is physically broken, and in both cases that
+ * means most crypto inside of the CoCo instance will be
+ * broken, defeating the purpose of CoCo in the first place. So
+ * just panic here because it's absolutely unsafe to continue
+ * executing.
+ */
+ if (longs == 0)
+ panic("RDRAND is defective.");
+ }
+ add_device_randomness(rng_seed, sizeof(rng_seed));
+ memzero_explicit(rng_seed, sizeof(rng_seed));
+}
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index 76c310b19b11..e9d059449885 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -15,6 +15,7 @@ extern enum cc_vendor cc_vendor;
void cc_set_mask(u64 mask);
u64 cc_mkenc(u64 val);
u64 cc_mkdec(u64 val);
+void cc_random_init(void);
#else
#define cc_vendor (CC_VENDOR_NONE)

@@ -27,6 +28,7 @@ static inline u64 cc_mkdec(u64 val)
{
return val;
}
+static inline void cc_random_init(void) { }
#endif

#endif /* _ASM_X86_COCO_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 84201071dfac..30a653cfc7d2 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -36,6 +36,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
#include <asm/gart.h>
@@ -994,6 +995,7 @@ void __init setup_arch(char **cmdline_p)
* memory size.
*/
mem_encrypt_setup_arch();
+ cc_random_init();

efi_fake_memmap();
efi_find_mirror();
--
2.43.2


2024-03-13 23:34:16

by Jason A. Donenfeld

[permalink] [raw]
Subject: Re: [PATCH v5] x86/coco: Require seeding RNG with RDRAND on CoCo systems

Hi Borislav,

On Sat, Feb 24, 2024 at 02:18:56AM +0100, Jason A. Donenfeld wrote:
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrangé <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Reviewed-by: Elena Reshetova <[email protected]>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
> Reviewed-by: Theodore Ts'o <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

This has been sitting on the list for a while with a few positive
reviews and no outstanding objections, afaict. Can you merge this to
tip?

Thanks,
Jason

2024-03-14 11:14:49

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v5] x86/coco: Require seeding RNG with RDRAND on CoCo systems

Hi Jason,

On Thu, Mar 14, 2024 at 12:34:03AM +0100, Jason A. Donenfeld wrote:
> This has been sitting on the list for a while with a few positive
> reviews and no outstanding objections, afaict. Can you merge this to
> tip?

the tip tree is open for new code after the merge window is over.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2024-03-26 11:22:13

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v5] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On Sat, Feb 24, 2024 at 02:18:56AM +0100, Jason A. Donenfeld wrote:
> +__init void cc_random_init(void)
> +{
> + /*
> + * The seed is 32 bytes (in units of longs), which is 256 bits, which
> + * is the security level that the RNG is targeting.
> + */
> + unsigned long rng_seed[32 / sizeof(long)];
> + size_t i, longs;
> +
> + if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> + return;
> +
> + /*
> + * Since the CoCo threat model includes the host, the only reliable
> + * source of entropy that can be neither observed nor manipulated is
> + * RDRAND. Usually, RDRAND failure is considered tolerable, but since
> + * CoCo guests have no other unobservable source of entropy, it's
> + * important to at least ensure the RNG gets some initial random seeds.
> + */
> + for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
> + longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
> +
> + /*
> + * A zero return value means that the guest doesn't have RDRAND
> + * or the CPU is physically broken, and in both cases that
> + * means most crypto inside of the CoCo instance will be
> + * broken, defeating the purpose of CoCo in the first place. So
> + * just panic here because it's absolutely unsafe to continue
> + * executing.
> + */
> + if (longs == 0)
> + panic("RDRAND is defective.");
> + }
> + add_device_randomness(rng_seed, sizeof(rng_seed));
> + memzero_explicit(rng_seed, sizeof(rng_seed));

Please redo your patch ontop of latest tip/master:

arch/x86/coco/core.c: In function ‘cc_random_init’:
arch/x86/coco/core.c:189:9: error: implicit declaration of function ‘memzero_explicit’ [-Werror=implicit-function-declaration]
189 | memzero_explicit(rng_seed, sizeof(rng_seed));
| ^~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:244: arch/x86/coco/core.o] Error 1
make[3]: *** [scripts/Makefile.build:485: arch/x86/coco] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/mnt/kernel/kernel/2nd/linux/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2024-03-26 16:09:03

by Jason A. Donenfeld

[permalink] [raw]
Subject: [PATCH v6] x86/coco: Require seeding RNG with RDRAND on CoCo systems

There are few uses of CoCo that don't rely on working cryptography and
hence a working RNG. Unfortunately, the CoCo threat model means that the
VM host cannot be trusted and may actively work against guests to
extract secrets or manipulate computation. Since a malicious host can
modify or observe nearly all inputs to guests, the only remaining source
of entropy for CoCo guests is RDRAND.

If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
is meant to gracefully continue on gathering entropy from other sources,
but since there aren't other sources on CoCo, this is catastrophic.
This is mostly a concern at boot time when initially seeding the RNG, as
after that the consequences of a broken RDRAND are much more
theoretical.

So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
fails, panic(). This will also trigger if the system is booted without
RDRAND, as RDRAND is essential for a safe CoCo boot.

This patch is deliberately written to be "just a CoCo x86 driver
feature" and not part of the RNG itself. Many device drivers and
platforms have some desire to contribute something to the RNG, and
add_device_randomness() is specifically meant for this purpose. Any
driver can call this with seed data of any quality, or even garbage
quality, and it can only possibly make the quality of the RNG better or
have no effect, but can never make it worse. Rather than trying to
build something into the core of the RNG, this patch interprets the
particular CoCo issue as just a CoCo issue, and therefore separates this
all out into driver (well, arch/platform) code.

Cc: Borislav Petkov <[email protected]>
Cc: Daniel P. Berrangé <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: [email protected]
Reviewed-by: Elena Reshetova <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Theodore Ts'o <[email protected]>
Signed-off-by: Jason A. Donenfeld <[email protected]>
---
Changes v5->v6:
- Rebase on tip/master.
- Add string.h include.

arch/x86/coco/core.c | 41 +++++++++++++++++++++++++++++++++++++
arch/x86/include/asm/coco.h | 2 ++
arch/x86/kernel/setup.c | 2 ++
3 files changed, 45 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index d07be9d05cd0..ddd4efdc79d6 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -3,13 +3,17 @@
* Confidential Computing Platform Capability checks
*
* Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
*
* Author: Tom Lendacky <[email protected]>
*/

#include <linux/export.h>
#include <linux/cc_platform.h>
+#include <linux/string.h>
+#include <linux/random.h>

+#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>

@@ -148,3 +152,40 @@ u64 cc_mkdec(u64 val)
}
}
EXPORT_SYMBOL_GPL(cc_mkdec);
+
+__init void cc_random_init(void)
+{
+ /*
+ * The seed is 32 bytes (in units of longs), which is 256 bits, which
+ * is the security level that the RNG is targeting.
+ */
+ unsigned long rng_seed[32 / sizeof(long)];
+ size_t i, longs;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+ return;
+
+ /*
+ * Since the CoCo threat model includes the host, the only reliable
+ * source of entropy that can be neither observed nor manipulated is
+ * RDRAND. Usually, RDRAND failure is considered tolerable, but since
+ * CoCo guests have no other unobservable source of entropy, it's
+ * important to at least ensure the RNG gets some initial random seeds.
+ */
+ for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
+ longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
+
+ /*
+ * A zero return value means that the guest doesn't have RDRAND
+ * or the CPU is physically broken, and in both cases that
+ * means most crypto inside of the CoCo instance will be
+ * broken, defeating the purpose of CoCo in the first place. So
+ * just panic here because it's absolutely unsafe to continue
+ * executing.
+ */
+ if (longs == 0)
+ panic("RDRAND is defective.");
+ }
+ add_device_randomness(rng_seed, sizeof(rng_seed));
+ memzero_explicit(rng_seed, sizeof(rng_seed));
+}
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index fb7388bbc212..c086699b0d0c 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -22,6 +22,7 @@ static inline void cc_set_mask(u64 mask)

u64 cc_mkenc(u64 val);
u64 cc_mkdec(u64 val);
+void cc_random_init(void);
#else
#define cc_vendor (CC_VENDOR_NONE)

@@ -34,6 +35,7 @@ static inline u64 cc_mkdec(u64 val)
{
return val;
}
+static inline void cc_random_init(void) { }
#endif

#endif /* _ASM_X86_COCO_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f04cef846e51..e3f01caf104b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -37,6 +37,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
#include <asm/gart.h>
@@ -993,6 +994,7 @@ void __init setup_arch(char **cmdline_p)
* memory size.
*/
mem_encrypt_setup_arch();
+ cc_random_init();

efi_fake_memmap();
efi_find_mirror();
--
2.44.0


2024-03-28 15:13:39

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v6] x86/coco: Require seeding RNG with RDRAND on CoCo systems

On 3/26/24 11:07, Jason A. Donenfeld wrote:
> There are few uses of CoCo that don't rely on working cryptography and
> hence a working RNG. Unfortunately, the CoCo threat model means that the
> VM host cannot be trusted and may actively work against guests to
> extract secrets or manipulate computation. Since a malicious host can
> modify or observe nearly all inputs to guests, the only remaining source
> of entropy for CoCo guests is RDRAND.
>
> If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
> is meant to gracefully continue on gathering entropy from other sources,
> but since there aren't other sources on CoCo, this is catastrophic.
> This is mostly a concern at boot time when initially seeding the RNG, as
> after that the consequences of a broken RDRAND are much more
> theoretical.
>
> So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
> fails, panic(). This will also trigger if the system is booted without
> RDRAND, as RDRAND is essential for a safe CoCo boot.
>
> This patch is deliberately written to be "just a CoCo x86 driver
> feature" and not part of the RNG itself. Many device drivers and
> platforms have some desire to contribute something to the RNG, and
> add_device_randomness() is specifically meant for this purpose. Any
> driver can call this with seed data of any quality, or even garbage
> quality, and it can only possibly make the quality of the RNG better or
> have no effect, but can never make it worse. Rather than trying to
> build something into the core of the RNG, this patch interprets the
> particular CoCo issue as just a CoCo issue, and therefore separates this
> all out into driver (well, arch/platform) code.
>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel P. Berrangé <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Cc: [email protected]
> Reviewed-by: Elena Reshetova <[email protected]>
> Reviewed-by: Kirill A. Shutemov <[email protected]>
> Reviewed-by: Theodore Ts'o <[email protected]>
> Signed-off-by: Jason A. Donenfeld <[email protected]>

Tested-by: Tom Lendacky <[email protected]>

> ---
> Changes v5->v6:
> - Rebase on tip/master.
> - Add string.h include.
>
> arch/x86/coco/core.c | 41 +++++++++++++++++++++++++++++++++++++
> arch/x86/include/asm/coco.h | 2 ++
> arch/x86/kernel/setup.c | 2 ++
> 3 files changed, 45 insertions(+)
>

Subject: [tip: x86/urgent] x86/coco: Require seeding RNG with RDRAND on CoCo systems

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 99485c4c026f024e7cb82da84c7951dbe3deb584
Gitweb: https://git.kernel.org/tip/99485c4c026f024e7cb82da84c7951dbe3deb584
Author: Jason A. Donenfeld <[email protected]>
AuthorDate: Tue, 26 Mar 2024 17:07:35 +01:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Thu, 04 Apr 2024 10:40:19 +02:00

x86/coco: Require seeding RNG with RDRAND on CoCo systems

There are few uses of CoCo that don't rely on working cryptography and
hence a working RNG. Unfortunately, the CoCo threat model means that the
VM host cannot be trusted and may actively work against guests to
extract secrets or manipulate computation. Since a malicious host can
modify or observe nearly all inputs to guests, the only remaining source
of entropy for CoCo guests is RDRAND.

If RDRAND is broken -- due to CPU hardware fault -- the RNG as a whole
is meant to gracefully continue on gathering entropy from other sources,
but since there aren't other sources on CoCo, this is catastrophic.
This is mostly a concern at boot time when initially seeding the RNG, as
after that the consequences of a broken RDRAND are much more
theoretical.

So, try at boot to seed the RNG using 256 bits of RDRAND output. If this
fails, panic(). This will also trigger if the system is booted without
RDRAND, as RDRAND is essential for a safe CoCo boot.

Add this deliberately to be "just a CoCo x86 driver feature" and not
part of the RNG itself. Many device drivers and platforms have some
desire to contribute something to the RNG, and add_device_randomness()
is specifically meant for this purpose.

Any driver can call it with seed data of any quality, or even garbage
quality, and it can only possibly make the quality of the RNG better or
have no effect, but can never make it worse.

Rather than trying to build something into the core of the RNG, consider
the particular CoCo issue just a CoCo issue, and therefore separate it
all out into driver (well, arch/platform) code.

[ bp: Massage commit message. ]

Signed-off-by: Jason A. Donenfeld <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Reviewed-by: Elena Reshetova <[email protected]>
Reviewed-by: Kirill A. Shutemov <[email protected]>
Reviewed-by: Theodore Ts'o <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/coco/core.c | 41 ++++++++++++++++++++++++++++++++++++-
arch/x86/include/asm/coco.h | 2 ++-
arch/x86/kernel/setup.c | 2 ++-
3 files changed, 45 insertions(+)

diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
index d07be9d..ddd4efd 100644
--- a/arch/x86/coco/core.c
+++ b/arch/x86/coco/core.c
@@ -3,13 +3,17 @@
* Confidential Computing Platform Capability checks
*
* Copyright (C) 2021 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024 Jason A. Donenfeld <[email protected]>. All Rights Reserved.
*
* Author: Tom Lendacky <[email protected]>
*/

#include <linux/export.h>
#include <linux/cc_platform.h>
+#include <linux/string.h>
+#include <linux/random.h>

+#include <asm/archrandom.h>
#include <asm/coco.h>
#include <asm/processor.h>

@@ -148,3 +152,40 @@ u64 cc_mkdec(u64 val)
}
}
EXPORT_SYMBOL_GPL(cc_mkdec);
+
+__init void cc_random_init(void)
+{
+ /*
+ * The seed is 32 bytes (in units of longs), which is 256 bits, which
+ * is the security level that the RNG is targeting.
+ */
+ unsigned long rng_seed[32 / sizeof(long)];
+ size_t i, longs;
+
+ if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+ return;
+
+ /*
+ * Since the CoCo threat model includes the host, the only reliable
+ * source of entropy that can be neither observed nor manipulated is
+ * RDRAND. Usually, RDRAND failure is considered tolerable, but since
+ * CoCo guests have no other unobservable source of entropy, it's
+ * important to at least ensure the RNG gets some initial random seeds.
+ */
+ for (i = 0; i < ARRAY_SIZE(rng_seed); i += longs) {
+ longs = arch_get_random_longs(&rng_seed[i], ARRAY_SIZE(rng_seed) - i);
+
+ /*
+ * A zero return value means that the guest doesn't have RDRAND
+ * or the CPU is physically broken, and in both cases that
+ * means most crypto inside of the CoCo instance will be
+ * broken, defeating the purpose of CoCo in the first place. So
+ * just panic here because it's absolutely unsafe to continue
+ * executing.
+ */
+ if (longs == 0)
+ panic("RDRAND is defective.");
+ }
+ add_device_randomness(rng_seed, sizeof(rng_seed));
+ memzero_explicit(rng_seed, sizeof(rng_seed));
+}
diff --git a/arch/x86/include/asm/coco.h b/arch/x86/include/asm/coco.h
index fb7388b..c086699 100644
--- a/arch/x86/include/asm/coco.h
+++ b/arch/x86/include/asm/coco.h
@@ -22,6 +22,7 @@ static inline void cc_set_mask(u64 mask)

u64 cc_mkenc(u64 val);
u64 cc_mkdec(u64 val);
+void cc_random_init(void);
#else
#define cc_vendor (CC_VENDOR_NONE)

@@ -34,6 +35,7 @@ static inline u64 cc_mkdec(u64 val)
{
return val;
}
+static inline void cc_random_init(void) { }
#endif

#endif /* _ASM_X86_COCO_H */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 0109e6c..e125e05 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -35,6 +35,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
#include <asm/gart.h>
@@ -991,6 +992,7 @@ void __init setup_arch(char **cmdline_p)
* memory size.
*/
mem_encrypt_setup_arch();
+ cc_random_init();

efi_fake_memmap();
efi_find_mirror();