Messages in this thread | | | Date | Mon, 1 Aug 2022 10:49:43 -0700 | Subject | Re: [PATCH v9 2/6] selftests: tdx: Test GetReport TDX attestation feature | From | Sathyanarayanan Kuppuswamy <> |
| |
On 7/28/22 3:32 AM, Kai Huang wrote: > On Wed, 2022-07-27 at 20:44 -0700, Kuppuswamy Sathyanarayanan wrote: >> In TDX guest, attestation is used to verify the trustworthiness of a >> TD. During the TD bring-up, Intel TDX module measures and records the >> initial contents and configuration of TD, and at runtime, TD software >> uses runtime measurement registers (RMTRs) to measure and record >> details related to kernel image, command line params, ACPI tables, >> initrd, etc. At TD runtime, Intel SGX attestation infrastructure is >> re-used to attest to these measurement data. >> >> First step in the TDX attestation process is to get the TDREPORT data. >> It is fixed size data structure generated by the TDX module which >> includes the above mentioned measurements data, a MAC to protect the >> integerity of the TDREPORT, and a 64-Byte of user specified data passed >> during TDREPORT request which can uniquely identify the TDREPORT. >> >> Intel's TDX guest driver exposes TDX_CMD_GET_REPORT IOCTL interface to >> get the TDREPORT from the user space. >> >> Add a kernel selftest module to test this ABI and verify the validity >> of generated TDREPORT. >> >> Reviewed-by: Tony Luck <tony.luck@intel.com> >> Reviewed-by: Andi Kleen <ak@linux.intel.com> >> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> >> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> --- >> tools/testing/selftests/Makefile | 1 + >> tools/testing/selftests/tdx/Makefile | 7 + >> tools/testing/selftests/tdx/tdx_attest_test.c | 160 ++++++++++++++++++ >> 3 files changed, 168 insertions(+) >> create mode 100644 tools/testing/selftests/tdx/Makefile >> create mode 100644 tools/testing/selftests/tdx/tdx_attest_test.c >> >> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile >> index de11992dc577..807a839d69c4 100644 >> --- a/tools/testing/selftests/Makefile >> +++ b/tools/testing/selftests/Makefile >> @@ -69,6 +69,7 @@ TARGETS += sync >> TARGETS += syscall_user_dispatch >> TARGETS += sysctl >> TARGETS += tc-testing >> +TARGETS += tdx >> TARGETS += timens >> ifneq (1, $(quicktest)) >> TARGETS += timers >> diff --git a/tools/testing/selftests/tdx/Makefile b/tools/testing/selftests/tdx/Makefile >> new file mode 100644 >> index 000000000000..281db209f9d6 >> --- /dev/null >> +++ b/tools/testing/selftests/tdx/Makefile >> @@ -0,0 +1,7 @@ >> +# SPDX-License-Identifier: GPL-2.0 >> + >> +CFLAGS += -O3 -Wl,-no-as-needed -Wall -static >> + >> +TEST_GEN_PROGS := tdx_attest_test >> + >> +include ../lib.mk >> diff --git a/tools/testing/selftests/tdx/tdx_attest_test.c b/tools/testing/selftests/tdx/tdx_attest_test.c >> new file mode 100644 >> index 000000000000..7155cc751eaa >> --- /dev/null >> +++ b/tools/testing/selftests/tdx/tdx_attest_test.c >> @@ -0,0 +1,160 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Test TDX attestation feature >> + * >> + * Copyright (C) 2022 Intel Corporation. All rights reserved. >> + * >> + * Author: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> >> + */ >> + >> + >> +#include <errno.h> >> +#include <fcntl.h> >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <sys/ioctl.h> >> +#include <sys/time.h> >> +#include <sys/types.h> >> +#include <time.h> >> +#include <unistd.h> >> + >> +#include "../kselftest_harness.h" >> +#include "../../../../arch/x86/include/uapi/asm/tdx.h" >> + >> +#define devname "/dev/tdx-guest" >> +#define HEX_DUMP_SIZE 8 >> + >> +/* >> + * struct td_info - It contains the measurements and initial configuration of >> + * the TD that was locked at initialization and a set of measurement >> + * registers that are run-time extendable. These values are copied from the >> + * TDCS by the TDG.MR.REPORT function. >> + */ >> +struct td_info { >> + /* TD attributes (like debug, spet_disable, etc) */ >> + __u8 attr[8]; >> + __u64 xfam; >> + /* Measurement registers */ >> + __u64 mrtd[6]; >> + __u64 mrconfigid[6]; >> + __u64 mrowner[6]; >> + __u64 mrownerconfig[6]; >> + /* Runtime measurement registers */ >> + __u64 rtmr[24]; >> + __u64 reserved[14]; >> +}; >> + >> +/* >> + * Trusted Execution Environment (TEE) report (TDREPORT_STRUCT) type, >> + * sub type and version.. >> + */ >> +struct tdreport_type { >> + /* 0 - SGX, 81 -TDX, rest are reserved */ >> + __u8 type; >> + /* Default value is 0 */ >> + __u8 sub_type; >> + /* Default value is 0 */ >> + __u8 version; >> + __u8 reserved; >> +}; >> + >> +/* >> + * struct reportmac - First field in the TEE report structure >> + * (TRDREPORT_STRUCT). It is common to Intel’s TEE's e.g., SGX and TDX. >> + * It is MAC-protected and contains hashes of the remainder of the report >> + * structure which includes the TEE’s measurements, and where applicable, >> + * the measurements of additional TCB elements not reflected in CPUSVN – >> + * e.g., a SEAM’s measurements. >> + */ >> +struct reportmac { >> + struct tdreport_type type; >> + __u8 reserved1[12]; >> + /* CPU security version */ >> + __u8 cpu_svn[16]; >> + /* SHA384 hash of TEE TCB INFO */ >> + __u8 tee_tcb_info_hash[48]; >> + /* SHA384 hash of TDINFO_STRUCT */ >> + __u8 tee_td_info_hash[48]; >> + /* User defined unique data passed in TDG.MR.REPORT request */ >> + __u8 reportdata[64]; >> + __u8 reserved2[32]; >> + __u8 mac[32]; >> +}; >> + >> +struct tee_tcb_info { >> + __u8 data[239]; >> +}; >> + >> +struct tdreport_data { >> + struct reportmac _reportmac; >> + struct tee_tcb_info _tcb_info; >> + __u8 reserved[17]; >> + struct td_info _tdinfo; >> +}; > > I think 'struct tdreport' is enough. The _data postfix only causes it to be > more confusing.
Ok.
> > Btw, as it appears you only verified reportdata below, is it worth to have all > those data structures (and they are used by hardware but not __packed)? Perhaps > a macro to define REPORTDATA offset in TDREPORT is good enough? Or maybe I am > missing something.
I have added these data structs to make it easier for readers to understand the contents of the TDREPORT. I thought a simple offset based check would look like a magic number. If the maintainers are fine with offset based comparison, I am ok with it. -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
| |