| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 23 Aug 2022 08:49:18 -0700
From: Evan Green <evgreen@...omium.org>
To: "Chang S. Bae" <chang.seok.bae@...el.com>
Cc: linux-crypto@...r.kernel.org, dm-devel@...hat.com,
herbert@...dor.apana.org.au, Eric Biggers <ebiggers@...nel.org>,
Ard Biesheuvel <ardb@...nel.org>, x86@...nel.org,
luto@...nel.org, Thomas Gleixner <tglx@...utronix.de>, bp@...e.de,
dave.hansen@...ux.intel.com, mingo@...nel.org,
LKML <linux-kernel@...r.kernel.org>,
Dan Williams <dan.j.williams@...el.com>,
charishma1.gairuboyina@...el.com, kumar.n.dwarakanath@...el.com,
ravi.v.shankar@...el.com
Subject: Re: [PATCH v5 07/12] x86/cpu/keylocker: Load an internal wrapping key
at boot-time
On Wed, Jan 12, 2022 at 1:21 PM Chang S. Bae <chang.seok.bae@...el.com> wrote:
>
> The Internal Wrapping Key (IWKey) is an entity of Key Locker to encode a
> clear text key into a key handle. This key is a pivot in protecting user
> keys. So the value has to be randomized before being loaded in the
> software-invisible CPU state.
>
> IWKey needs to be established before the first user. Given that the only
> proposed Linux use case for Key Locker is dm-crypt, the feature could be
> lazily enabled when the first dm-crypt user arrives, but there is no
> precedent for late enabling of CPU features and it adds maintenance burden
> without demonstrative benefit outside of minimizing the visibility of
> Key Locker to userspace.
>
> The kernel generates random bytes and load them at boot time. These bytes
> are flushed out immediately.
>
> Setting the CR4.KL bit does not always enable the feature so ensure the
> dynamic CPU bit (CPUID.AESKLE) is set before loading the key.
>
> Given that the Linux Key Locker support is only intended for bare metal
> dm-crypt consumption, and that switching IWKey per VM is untenable,
> explicitly skip Key Locker setup in the X86_FEATURE_HYPERVISOR case.
>
> Signed-off-by: Chang S. Bae <chang.seok.bae@...el.com>
> Reviewed-by: Dan Williams <dan.j.williams@...el.com>
> Cc: x86@...nel.org
> Cc: linux-kernel@...r.kernel.org
> ---
> Changes from RFC v2:
> * Make bare metal only.
> * Clean up the code (e.g. dynamically allocate the key cache).
> (Dan Williams)
> * Massage the changelog.
> * Move out the LOADIWKEY wrapper and the Key Locker CPUID defines.
>
> Note, Dan wonders that given that the only proposed Linux use case for
> Key Locker is dm-crypt, the feature could be lazily enabled when the
> first dm-crypt user arrives, but as Dave notes there is no precedent
> for late enabling of CPU features and it adds maintenance burden
> without demonstrative benefit outside of minimizing the visibility of
> Key Locker to userspace.
> ---
> arch/x86/include/asm/keylocker.h | 9 ++++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/cpu/common.c | 5 +-
> arch/x86/kernel/keylocker.c | 79 ++++++++++++++++++++++++++++++++
> arch/x86/kernel/smpboot.c | 2 +
> 5 files changed, 95 insertions(+), 1 deletion(-)
> create mode 100644 arch/x86/kernel/keylocker.c
>
> diff --git a/arch/x86/include/asm/keylocker.h b/arch/x86/include/asm/keylocker.h
> index e85dfb6c1524..820ac29c06d9 100644
> --- a/arch/x86/include/asm/keylocker.h
> +++ b/arch/x86/include/asm/keylocker.h
> @@ -5,6 +5,7 @@
>
> #ifndef __ASSEMBLY__
>
> +#include <asm/processor.h>
> #include <linux/bits.h>
> #include <asm/fpu/types.h>
>
> @@ -28,5 +29,13 @@ struct iwkey {
> #define KEYLOCKER_CPUID_EBX_WIDE BIT(2)
> #define KEYLOCKER_CPUID_EBX_BACKUP BIT(4)
>
> +#ifdef CONFIG_X86_KEYLOCKER
> +void setup_keylocker(struct cpuinfo_x86 *c);
> +void destroy_keylocker_data(void);
> +#else
> +#define setup_keylocker(c) do { } while (0)
> +#define destroy_keylocker_data() do { } while (0)
> +#endif
> +
> #endif /*__ASSEMBLY__ */
> #endif /* _ASM_KEYLOCKER_H */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 2ff3e600f426..e15efa238497 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -144,6 +144,7 @@ obj-$(CONFIG_PERF_EVENTS) += perf_regs.o
> obj-$(CONFIG_TRACING) += tracepoint.o
> obj-$(CONFIG_SCHED_MC_PRIO) += itmt.o
> obj-$(CONFIG_X86_UMIP) += umip.o
> +obj-$(CONFIG_X86_KEYLOCKER) += keylocker.o
>
> obj-$(CONFIG_UNWINDER_ORC) += unwind_orc.o
> obj-$(CONFIG_UNWINDER_FRAME_POINTER) += unwind_frame.o
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 0083464de5e3..23b4aa437c1e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -57,6 +57,8 @@
> #include <asm/microcode_intel.h>
> #include <asm/intel-family.h>
> #include <asm/cpu_device_id.h>
> +#include <asm/keylocker.h>
> +
> #include <asm/uv/uv.h>
> #include <asm/sigframe.h>
>
> @@ -1595,10 +1597,11 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> /* Disable the PN if appropriate */
> squash_the_stupid_serial_number(c);
>
> - /* Set up SMEP/SMAP/UMIP */
> + /* Setup various Intel-specific CPU security features */
> setup_smep(c);
> setup_smap(c);
> setup_umip(c);
> + setup_keylocker(c);
>
> /* Enable FSGSBASE instructions if available. */
> if (cpu_has(c, X86_FEATURE_FSGSBASE)) {
> diff --git a/arch/x86/kernel/keylocker.c b/arch/x86/kernel/keylocker.c
> new file mode 100644
> index 000000000000..87d775a65716
> --- /dev/null
> +++ b/arch/x86/kernel/keylocker.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Setup Key Locker feature and support internal wrapping key
> + * management.
> + */
> +
> +#include <linux/random.h>
> +#include <linux/poison.h>
> +
> +#include <asm/fpu/api.h>
> +#include <asm/keylocker.h>
> +#include <asm/tlbflush.h>
> +
> +static __initdata struct keylocker_setup_data {
> + struct iwkey key;
> +} kl_setup;
> +
> +static void __init generate_keylocker_data(void)
> +{
> + get_random_bytes(&kl_setup.key.integrity_key, sizeof(kl_setup.key.integrity_key));
> + get_random_bytes(&kl_setup.key.encryption_key, sizeof(kl_setup.key.encryption_key));
> +}
> +
> +void __init destroy_keylocker_data(void)
> +{
> + memset(&kl_setup.key, KEY_DESTROY, sizeof(kl_setup.key));
> +}
> +
> +static void __init load_keylocker(void)
I am late to this party by 6 months, but:
load_keylocker() cannot be __init, as it gets called during SMP core onlining.
Powered by blists - more mailing lists