On Wed Jul 19, 2023 at 5:24 PM EEST, Jo Van Bulck wrote:
> The System V x86-64 ABI used by the C compiler defines certain low-level
> CPU configuration registers to be set to expected values upon function
> entry. However, SGX enclaves cannot expect the untrusted caller to respect
> these ABI conventions. Therefore, adhere to SGX runtime best practices by
> sanitizing RFLAGS.DF=0 before transitioning to C code. Additionally
> sanitize RFLAGS.AC=0 to protect against known #AC-fault side channels for
> unaligned memory accesses.
>
> Note that the test enclave does currently not use any floating-point
> instructions (-mno-sse). Hence, keep the code simple by _not_ using XRSTOR
> to cleanse extended x87/SSE state.
>
> Signed-off-by: Jo Van Bulck <jo.vanbulck@xxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/sgx/Makefile | 2 +-
> tools/testing/selftests/sgx/main.c | 21 +++++++++++++++++++
> .../selftests/sgx/test_encl_bootstrap.S | 12 +++++++++++
> 3 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 50aab6b57..c2a13bc6e 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -14,7 +14,7 @@ endif
> INCLUDES := -I$(top_srcdir)/tools/include
> HOST_CFLAGS := -Wall -Werror -g $(INCLUDES) -fPIC -z noexecstack
> ENCL_CFLAGS := -Wall -Werror -static -nostdlib -nostartfiles -fPIC \
> - -fno-stack-protector -mrdrnd $(INCLUDES)
> + -fno-stack-protector -mrdrnd -mno-sse $(INCLUDES)
>
> TEST_CUSTOM_PROGS := $(OUTPUT)/test_sgx
> TEST_FILES := $(OUTPUT)/test_encl.elf
> diff --git a/tools/testing/selftests/sgx/main.c b/tools/testing/selftests/sgx/main.c
> index 9820b3809..d3c7a39f4 100644
> --- a/tools/testing/selftests/sgx/main.c
> +++ b/tools/testing/selftests/sgx/main.c
> @@ -307,6 +307,27 @@ TEST_F(enclave, unclobbered_vdso)
> EXPECT_EQ(self->run.user_data, 0);
> }
>
Since the amount of tests is increasing over time, I'd put here:
/*
* Explanation what the test does and why it exists.
*/
> +TEST_F(enclave, poison_args)
> +{
> + struct encl_op_header nop_op;
> + uint64_t flags = -1;
> +
> + ASSERT_TRUE(setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata));
> +
> + memset(&self->run, 0, sizeof(self->run));
> + self->run.tcs = self->encl.encl_base;
> +
> + /* attempt ABI register poisoning */
> + nop_op.type = ENCL_OP_NOP;
> + asm("std\n\t");
> + EXPECT_EQ(ENCL_CALL(&nop_op, &self->run, false), 0);
> + asm("pushfq\n\t" \
> + "popq %0\n\t" \
> + : "=m"(flags) : : );
> + EXPECT_EEXIT(&self->run);
> + EXPECT_EQ(flags & 0x40400, 0);
> +}
> +
> /*
> * A section metric is concatenated in a way that @low bits 12-31 define the
> * bits 12-31 of the metric and @high bits 0-19 define the bits 32-51 of the
> diff --git a/tools/testing/selftests/sgx/test_encl_bootstrap.S b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> index 03ae0f57e..3b69fea61 100644
> --- a/tools/testing/selftests/sgx/test_encl_bootstrap.S
> +++ b/tools/testing/selftests/sgx/test_encl_bootstrap.S
> @@ -57,6 +57,18 @@ encl_entry_core:
> push %rcx # push the address after EENTER
> push %rbx # push the enclave base address
>
> + # Sanitize CPU state: x86-64 ABI requires RFLAGS.DF=0 on function
> + # entry, and we additionally clear RFLAGS.AC to prevent #AC-fault side
> + # channels.
> + # NOTE: Real-world enclave runtimes should also cleanse extended CPU
> + # state (i.e., x87 FPU and SSE/AVX/...) configuration registers,
> + # preferably using XRSTOR. This is _not_ done below to simplify the
> + # test enclave, which does not use any floating-point instructions.
> + cld
> + pushfq
> + andq $~0x40000, (%rsp)
> + popfq
> +
> call encl_body
>
> pop %rbx # pop the enclave base address
> --
> 2.34.1
BR, Jarkko
