[PAL/Linux-SGX] Add AEX-Notify support #1530

lzha101 · 2023-09-01T06:26:52Z

According to PR #1488, will construct three PRs to support AEX-Notify in Gramine. This PR includes the first Part.

AEX-Notify is a security feature used to mitigate the SGX-Step type attacks on SGX. The SGX-Step type attacks rely on frequent enclave preemptions (e.g. interrupts) to execute an SGX enclave in small increments and strategically extract secret data from the enclave through one or more side channels. The AEX Notify feature allows a mitigation handler to run after ERESUME from each Async Exit, aiming to mitigate the side-channel information exposed by the attacks.

This commit includes below changes for aex-notify support:

add a new manifest option sgx.enable_aex_notify
update related structures for aex-notify feature
add initialization/finalization code for aex-notify
update related tools and document

Description of the changes

How to test this PR?

This change is

dimakuv

Reviewed 14 of 14 files at r1, all commit messages.
Reviewable status: all files reviewed, 47 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ), period found at the end of a one-liner (waiting on @lzha101)

-- commits line 2 at r1:
I would say simply [PAL/Linux-SGX] Add AEX-Notify enabling and documentation

-- commits line 8 at r1:
AEX-Notify (with a dash)

-- commits line 9 at r1:
after each asynchronous exit (AEX)

-- commits line 10 at r1:
I would remove this phrase (aiming to ...), as it is obvious from the rest of this para.

-- commits line 12 at r1:
This commit is first in a series that adds the complete AEX-Notify feature. It includes the following enablement changes:

-- commits line 14 at r1:
AEX-Notify

-- commits line 15 at r1:
AEX-Notify

-- commits line 16 at r1:
update documentation

Documentation/manifest-syntax.rst line 1144 at r1 (raw file):

::

    sgx.enable_aex_notify = [true|false]

Actually, can we rename to sgx.experimental_enable_aex_notify? I don't feel very confident in this feature; I think it will take several months of testing (after it is merged) before we can say that the implementation is robust and correct...

Documentation/manifest-syntax.rst line 1147 at r1 (raw file):

    (Default: false)

When enabled, this option instructs Gramine to use the new AEX Notify hardware

AEX-Notify

Documentation/manifest-syntax.rst line 1147 at r1 (raw file):

    (Default: false)

When enabled, this option instructs Gramine to use the new AEX Notify hardware

Please remove new.

Documentation/manifest-syntax.rst line 1148 at r1 (raw file):

When enabled, this option instructs Gramine to use the new AEX Notify hardware
feature, which is a security feature used to mitigate the SGX-Step type attacks

Please add a link to the SGX-Step paper.

Documentation/manifest-syntax.rst line 1152 at r1 (raw file):

to execute an SGX enclave in small increments and strategically extract secret data
from the enclave through one or more side channels. The AEX Notify feature allows
a mitigation handler to run after ERESUME from each Async Exit, aiming to mitigate the

See my comments in the commit message, and apply the same here.

Documentation/manifest-syntax.rst line 1153 at r1 (raw file):

from the enclave through one or more side channels. The AEX Notify feature allows
a mitigation handler to run after ERESUME from each Async Exit, aiming to mitigate the
side-channel information exposed by the attacks.

Please add a newline symbol at the end.

pal/src/host/linux-sgx/generated_offsets.c line 59 at r1 (raw file):

    OFFSET_T(SGX_GPR_EXITINFO, sgx_pal_gpr_t, exitinfo),
    OFFSET_T(SGX_GPR_AEXNOTIFY, sgx_pal_gpr_t, aexnotify),
    OFFSET_T(SGX_GPR_FSBASE, sgx_pal_gpr_t, fsbase),

This change feels completely irrelevant in this PR. Please remove.

pal/src/host/linux-sgx/generated_offsets.c line 175 at r1 (raw file):

    /* pal.h */
    DEFINE(PAL_EVENT_NO_EVENT, PAL_EVENT_NO_EVENT),
    DEFINE(PAL_EVENT_INTERRUPTED, PAL_EVENT_INTERRUPTED),

This change feels completely irrelevant in this PR. Please remove.

pal/src/host/linux-sgx/host_framework.c line 165 at r1 (raw file):

Please remove one empty line

pal/src/host/linux-sgx/host_framework.c line 168 at r1 (raw file):

bool is_aexnotify_supported(void) {
    uint32_t cpuinfo[4];
    memset(cpuinfo, 0, sizeof(cpuinfo));

There is no reason for this memset. Remove.

pal/src/host/linux-sgx/host_framework.c line 170 at r1 (raw file):

    memset(cpuinfo, 0, sizeof(cpuinfo));

    /* Check the platform support AEX Notify or not. */

Useless comment, remove

pal/src/host/linux-sgx/host_framework.c line 171 at r1 (raw file):

    /* Check the platform support AEX Notify or not. */
    cpuid(0x12, 1, cpuinfo);

We have INTEL_SGX_LEAF macro for this

pal/src/host/linux-sgx/host_framework.c line 173 at r1 (raw file):

    cpuid(0x12, 1, cpuinfo);

    if (!((cpuinfo[0] >> 10) & 0x1)) {

0 -> CPUID_WORD_EAX

pal/src/host/linux-sgx/host_framework.c line 175 at r1 (raw file):

    if (!((cpuinfo[0] >> 10) & 0x1)) {
        log_debug(
            "AEX Notify is not supported.");

Please move to previous line

pal/src/host/linux-sgx/host_framework.c line 175 at r1 (raw file):

    if (!((cpuinfo[0] >> 10) & 0x1)) {
        log_debug(
            "AEX Notify is not supported.");

AEX-Notify hardware feature

pal/src/host/linux-sgx/host_framework.c line 179 at r1 (raw file):

    }

    memset(cpuinfo, 0, sizeof(cpuinfo));

No need for this memset

pal/src/host/linux-sgx/host_framework.c line 180 at r1 (raw file):

    memset(cpuinfo, 0, sizeof(cpuinfo));
    /* Check the platform support ENCLU[EDECCSSA] leaf instruction or not. */

Useless comment, remove

pal/src/host/linux-sgx/host_framework.c line 181 at r1 (raw file):

    memset(cpuinfo, 0, sizeof(cpuinfo));
    /* Check the platform support ENCLU[EDECCSSA] leaf instruction or not. */
    cpuid(0x12, 0, cpuinfo);

We have INTEL_SGX_LEAF macro for this

pal/src/host/linux-sgx/host_framework.c line 185 at r1 (raw file):

    if (!((cpuinfo[0] >> 11) & 0x1)) {
        log_debug(
            "ENCLU[EDECCSSA] leaf instruction is not supported.");

Please move to previous line

pal/src/host/linux-sgx/host_internal.h line 62 at r1 (raw file):

int open_sgx_driver(void);
bool is_wrfsbase_supported(void);
bool is_aexnotify_supported(void);

Please keep an empty line after this func.

pal/src/host/linux-sgx/host_main.c line 527 at r1 (raw file):

                gs->heap_max = (void*)pal_area->addr;
                gs->thread = NULL;
                /* below fields are used by AEX Notify (counter of AEXs, temp sp for aex checkpoint) */

temp sp for aex checkpoint -- remove it, this will be in a follow up PR

Actually, remove everything in brackets -- even the counter of AEXs is not used in this PR

pal/src/host/linux-sgx/pal_exception.c line 29 at r1 (raw file):

    SET_ENCLAVE_TCB(aex_counter, 0UL);
    SET_ENCLAVE_TCB(ready_for_aex_notify, 1UL);
    MB();

Could you remind if there's a specific reason we put a memory barrier here?

pal/src/host/linux-sgx/pal_exception.c line 32 at r1 (raw file):

    uint8_t* ssa0_aex_notify_byte = &GET_ENCLAVE_TCB(gpr)->aexnotify;
    *ssa0_aex_notify_byte |= 1;

Why such a weird way? Why not simply GET_ENCLAVE_TCB(gpr)->aexnotify = 1?

pal/src/host/linux-sgx/pal_exception.c line 40 at r1 (raw file):

    SET_ENCLAVE_TCB(ready_for_aex_notify, 0UL);
    MB();

ditto

pal/src/host/linux-sgx/pal_exception.c line 43 at r1 (raw file):

    uint8_t* ssa0_aex_notify_byte = &GET_ENCLAVE_TCB(gpr)->aexnotify;
    *ssa0_aex_notify_byte &= 0xfe;

Why such a weird way? Why not simply GET_ENCLAVE_TCB(gpr)->aexnotify = 0?

pal/src/host/linux-sgx/pal_exception.c line 46 at r1 (raw file):

    log_debug("AEX counter of thread = %lu",
              GET_ENCLAVE_TCB(aex_counter));

Not used, please remove.

pal/src/host/linux-sgx/pal_main.c line 754 at r1 (raw file):

    ret = toml_bool_in(g_pal_public_state.manifest_root, "sgx.enable_aex_notify",
                    /*defaultval=*/false, &g_aex_notify_enabled);

Please align correctly

pal/src/host/linux-sgx/pal_tcb.h line 22 at r1 (raw file):

    int32_t count;
};
typedef struct aex_notify_entropy aex_notify_entropy_t;

What's this? Looks like a helper struct for some mitigation. Since you won't introduce this mitigation until some later commit, please remove it from this PR.

pal/src/host/linux-sgx/pal_tcb.h line 57 at r1 (raw file):

    struct untrusted_area untrusted_area_cache;

    /* below fields are used by AEX Notify */

AEX-Notify, here and everywhere else. Let's use the official name (as is used in file:///C:/Users/dkuvaisk/Downloads/aex-notify-white-paper-public.pdf) everywhere in these PRs.

pal/src/host/linux-sgx/pal_tcb.h line 58 at r1 (raw file):

    /* below fields are used by AEX Notify */
    uint64_t  ready_for_aex_notify; /* Set when it is ready to enable aex-notify*/

Here and below: add a space before closing */

pal/src/host/linux-sgx/pal_tcb.h line 58 at r1 (raw file):

    /* below fields are used by AEX Notify */
    uint64_t  ready_for_aex_notify; /* Set when it is ready to enable aex-notify*/

Here and below: start the sentence with small letter (set when ...)

pal/src/host/linux-sgx/pal_tcb.h line 59 at r1 (raw file):

    /* below fields are used by AEX Notify */
    uint64_t  ready_for_aex_notify; /* Set when it is ready to enable aex-notify*/
    uint64_t  aex_notify_flag;      /* Used to indicate whether we need to handle aex-notify across enclave boundary*/

Simpler: /* whether to run the AEX-Notify flow */

Do I understand the need for this flag correctly? This seems to corresponds to SSA[0].GPRSGX.AEXNOTIFY?

pal/src/host/linux-sgx/pal_tcb.h line 60 at r1 (raw file):

    uint64_t  ready_for_aex_notify; /* Set when it is ready to enable aex-notify*/
    uint64_t  aex_notify_flag;      /* Used to indicate whether we need to handle aex-notify across enclave boundary*/
    uint64_t  aex_counter;          /* Counter for AEXs*/

This seems to be unused completely? Let's remove from this PR.

pal/src/host/linux-sgx/pal_tcb.h line 61 at r1 (raw file):

    uint64_t  aex_notify_flag;      /* Used to indicate whether we need to handle aex-notify across enclave boundary*/
    uint64_t  aex_counter;          /* Counter for AEXs*/
    aex_notify_entropy_t entropy;   /* An entropy used in aex-notify mitigation*/

ditto

pal/src/host/linux-sgx/sgx_arch.h line 197 at r1 (raw file):

    uint64_t gsbase;
} sgx_pal_gpr_t;

Can you also add a static assert on the offset of aexnotify to equal 167?

python/graminelibos/manifest.py line 110 at r1 (raw file):

            sgx.setdefault('enclave_size', DEFAULT_ENCLAVE_SIZE_NO_EDMM)

        sgx.setdefault('enable_aex_notify', False)

Please move it right-after edmm_enable

tools/sgx/is-sgx-available/is_sgx_available.cpp line 168 at r1 (raw file):

    // Key separation and sharing (KSS) support (CONFIGID, CONFIGSVN, ISVEXTPRODID, ISVFAMILYID report fields)
    bool kss_supported() const { return kss_supported_; }
    // AEX Notify support

AEX-Notify

tools/sgx/is-sgx-available/is_sgx_available.cpp line 220 at r1 (raw file):

    printf("Key separation and sharing (KSS) support (CONFIGID, CONFIGSVN, ISVEXTPRODID, "
           "ISVFAMILYID report fields): %s\n", bool2str(cpu_checker.kss_supported()));
    printf("AEX Notify support: %s\n", bool2str(cpu_checker.aex_notify_supported()));

AEX-Notify

tools/sgx/is-sgx-available/is_sgx_available.cpp line 221 at r1 (raw file):

           "ISVFAMILYID report fields): %s\n", bool2str(cpu_checker.kss_supported()));
    printf("AEX Notify support: %s\n", bool2str(cpu_checker.aex_notify_supported()));
    printf("AEX Notify (EDECCSSA) support: %s\n", bool2str(cpu_checker.edeccssa_supported()));

AEX-Notify

lzha101

Reviewable status: 2 of 14 files reviewed, 47 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ), period found at the end of a one-liner (waiting on @dimakuv)

-- commits line 2 at r1: