Re: [PATCH] [v2] x86/sgx: Allow enclaves to use Asynchrounous Exit Notification

From: Jarkko Sakkinen
Date: Thu Jul 28 2022 - 03:58:58 EST


On Wed, Jul 20, 2022 at 12:13:47PM -0700, Dave Hansen wrote:
> Changes from v1:
> * Make sure SGX_ATTR_ASYNC_EXIT_NOTIFY is in the masks that are
> used at bare-metal enclave initialization and that enumerates
> available attributes to KVM guests.
>
> --
>
> Short Version:
>
> Allow enclaves to use the new Asynchronous EXit (AEX)
> notification mechanism. This mechanism lets enclaves run a
> handler after an AEX event. These handlers can run mitigations
> for things like SGX-Step[1].
>
> AEX Notify will be made available both on upcoming processors and
> on some older processors through microcode updates.
>
> Long Version:
>
> == SGX Attribute Background ==
>
> The SGX architecture includes a list of SGX "attributes". These
> attributes ensure consistency and transparency around specific
> enclave features.
>
> As a simple example, the "DEBUG" attribute allows an enclave to
> be debugged, but also destroys virtually all of SGX security.
> Using attributes, enclaves can know that they are being debugged.
> Attributes also affect enclave attestation so an enclave can, for
> instance, be denied access to secrets while it is being debugged.
>
> The kernel keeps a list of known attributes and will only
> initialize enclaves that use a known set of attributes. This
> kernel policy eliminates the chance that a new SGX attribute
> could cause undesired effects.
>
> For example, imagine a new attribute was added called
> "PROVISIONKEY2" that provided similar functionality to
> "PROVISIIONKEY". A kernel policy that allowed indiscriminate use
> of unknown attributes and thus PROVISIONKEY2 would undermine the
> existing kernel policy which limits use of PROVISIONKEY enclaves.
>
> == AEX Notify Background ==
>
> "Intel Architecture Instruction Set Extensions and Future
> Features - Version 45" is out[2]. There is a new chapter:
>
> Asynchronous Enclave Exit Notify and the EDECCSSA User Leaf Function.
>
> Enclaves exit can be either synchronous and consensual (EEXIT for
> instance) or asynchronous (on an interrupt or fault). The
> asynchronous ones can evidently be exploited to single step
> enclaves[1], on top of which other naughty things can be built.
>
> AEX Notify will be made available both on upcoming processors and
> on some older processors through microcode updates.
>
> == The Problem ==
>
> These attacks are currently entirely opaque to the enclave since
> the hardware does the save/restore under the covers. The
> Asynchronous Enclave Exit Notify (AEX Notify) mechanism provides
> enclaves an ability to detect and mitigate potential exposure to
> these kinds of attacks.
>
> == The Solution ==
>
> Define the new attribute value for AEX Notification. Ensure the
> attribute is cleared from the list reserved attributes. Instead
> of adding to the open-coded lists of individual attributes,
> add named lists of privileged (disallowed by default) and
> unprivileged (allowed by default) attributes. Add the AEX notify
> attribute as an unprivileged attribute, which will keep the kernel
> from rejecting enclaves with it set.
>
> I just built this and ran it to make sure there were no obvious
> regressions since I do not have the hardware (and new microcde)
> to test it.
>
> Testing on bare-metal and in VMs accompanied by Tested-by's
> would be much appreciated. (This means you, Intel folks who
> actually have systems with the microcode that can do this.)
>
> 1. https://github.com/jovanbulck/sgx-step
> 2. https://cdrdv2.intel.com/v1/dl/getContent/671368?explicitVersion=true
>
> Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Kai Huang <kai.huang@xxxxxxxxx>
> Cc: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: linux-sgx@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
> arch/x86/include/asm/sgx.h | 33 ++++++++++++++++++++++++++-------
> arch/x86/kernel/cpu/sgx/ioctl.c | 2 +-
> arch/x86/kvm/cpuid.c | 4 +---
> 3 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h
> index 3f9334ef67cd..3004dfe76498 100644
> --- a/arch/x86/include/asm/sgx.h
> +++ b/arch/x86/include/asm/sgx.h
> @@ -110,17 +110,36 @@ enum sgx_miscselect {
> * %SGX_ATTR_EINITTOKENKEY: Allow to use token signing key that is used to
> * sign cryptographic tokens that can be passed to
> * EINIT as an authorization to run an enclave.
> + * %SGX_ATTR_ASYNC_EXIT_NOTIFY: Allow enclaves to be notified after an
> + * asynchronous exit has occurred.
> */
> enum sgx_attribute {
> - SGX_ATTR_INIT = BIT(0),
> - SGX_ATTR_DEBUG = BIT(1),
> - SGX_ATTR_MODE64BIT = BIT(2),
> - SGX_ATTR_PROVISIONKEY = BIT(4),
> - SGX_ATTR_EINITTOKENKEY = BIT(5),
> - SGX_ATTR_KSS = BIT(7),
> + SGX_ATTR_INIT = BIT(0),
> + SGX_ATTR_DEBUG = BIT(1),
> + SGX_ATTR_MODE64BIT = BIT(2),
> + /* BIT(3) is reserved */
> + SGX_ATTR_PROVISIONKEY = BIT(4),
> + SGX_ATTR_EINITTOKENKEY = BIT(5),
> + /* BIT(6) is for CET */
> + SGX_ATTR_KSS = BIT(7),
> + /* BIT(8) is reserved */
> + /* BIT(9) is reserved */
> + SGX_ATTR_ASYNC_EXIT_NOTIFY = BIT(10),
> };
>
> -#define SGX_ATTR_RESERVED_MASK (BIT_ULL(3) | BIT_ULL(6) | GENMASK_ULL(63, 8))
> +#define SGX_ATTR_RESERVED_MASK (BIT_ULL(3) | \
> + BIT_ULL(6) | \
> + BIT_ULL(8) | \
> + BIT_ULL(9) | \
> + GENMASK_ULL(63, 11))
> +
> +#define SGX_ATTR_UNPRIV_MASK (SGX_ATTR_DEBUG | \
> + SGX_ATTR_MODE64BIT | \
> + SGX_ATTR_KSS | \
> + SGX_ATTR_ASYNC_EXIT_NOTIFY)
> +
> +#define SGX_ATTR_PRIV_MASK (SGX_ATTR_PROVISIONKEY | \
> + SGX_ATTR_EINITTOKENKEY)
>
> /**
> * struct sgx_secs - SGX Enclave Control Structure (SECS)
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 83df20e3e633..37d523895244 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -110,7 +110,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
> encl->base = secs->base;
> encl->size = secs->size;
> encl->attributes = secs->attributes;
> - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS;
> + encl->attributes_mask = SGX_ATTR_UNPRIV_MASK;
>
> /* Set only after completion, as encl->lock has not been taken. */
> set_bit(SGX_ENCL_CREATED, &encl->flags);
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0c1ba6aa0765..96a73b5b4369 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1022,9 +1022,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
> * userspace. ATTRIBUTES.XFRM is not adjusted as userspace is
> * expected to derive it from supported XCR0.
> */
> - entry->eax &= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT |
> - SGX_ATTR_PROVISIONKEY | SGX_ATTR_EINITTOKENKEY |
> - SGX_ATTR_KSS;
> + entry->eax &= SGX_ATTR_PRIV_MASK | SGX_ATTR_UNPRIV_MASK;
> entry->ebx &= 0;
> break;
> /* Intel PT */
> --
> 2.34.1
>

Acked-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>

BR, Jarkkko