Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <03c6c9cecd281d64d0efd48cb40135092dc2d0df.camel@intel.com>
Subject: Re: [PATCH v9 2/6] selftests: tdx: Test GetReport TDX attestation
 feature
From:   Kai Huang <kai.huang@intel.com>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org
Cc:     "H . Peter Anvin" <hpa@zytor.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Wander Lairson Costa <wander@redhat.com>,
        Isaku Yamahata <isaku.yamahata@gmail.com>,
        marcelo.cerri@canonical.com, tim.gardner@canonical.com,
        khalid.elmously@canonical.com, philip.cox@canonical.com,
        linux-kernel@vger.kernel.org
Date:   Thu, 28 Jul 2022 22:32:32 +1200
In-Reply-To: <20220728034420.648314-3-sathyanarayanan.kuppuswamy@linux.intel.com>
References: <20220728034420.648314-1-sathyanarayanan.kuppuswamy@linux.intel.com>
         <20220728034420.648314-3-sathyanarayanan.kuppuswamy@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

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.
>=20
> 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.
>=20
> Intel's TDX guest driver exposes TDX_CMD_GET_REPORT IOCTL interface to
> get the TDREPORT from the user space.
>=20
> Add a kernel selftest module to test this ABI and verify the validity
> of generated TDREPORT.
>=20
> 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@lin=
ux.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
>=20
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/M=
akefile
> index de11992dc577..807a839d69c4 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -69,6 +69,7 @@ TARGETS +=3D sync
>  TARGETS +=3D syscall_user_dispatch
>  TARGETS +=3D sysctl
>  TARGETS +=3D tc-testing
> +TARGETS +=3D tdx
>  TARGETS +=3D timens
>  ifneq (1, $(quicktest))
>  TARGETS +=3D timers
> diff --git a/tools/testing/selftests/tdx/Makefile b/tools/testing/selftes=
ts/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 +=3D -O3 -Wl,-no-as-needed -Wall -static
> +
> +TEST_GEN_PROGS :=3D tdx_attest_test
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/tdx/tdx_attest_test.c b/tools/testin=
g/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 configurati=
on 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=E2=80=99s TEE's e.g., SGX a=
nd TDX.
> + * It is MAC-protected and contains hashes of the remainder of the repor=
t
> + * structure which includes the TEE=E2=80=99s measurements, and where ap=
plicable,
> + * the measurements of additional TCB elements not reflected in CPUSVN =
=E2=80=93
> + * e.g., a SEAM=E2=80=99s 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 b=
e
more confusing.

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)?  Pe=
rhaps
a macro to define REPORTDATA offset in TDREPORT is good enough?  Or maybe I=
 am
missing something.

> +
> +#ifdef DEBUG
> +static void print_array_hex(const char *title, const char *prefix_str,
> +		const void *buf, int len)
> +{
> +	const __u8 *ptr =3D buf;
> +	int i, rowsize =3D HEX_DUMP_SIZE;
> +
> +	if (!len || !buf)
> +		return;
> +
> +	printf("\t\t%s", title);
> +
> +	for (i =3D 0; i < len; i++) {
> +		if (!(i % rowsize))
> +			printf("\n%s%.8x:", prefix_str, i);
> +		printf(" %.2x", ptr[i]);
> +	}
> +
> +	printf("\n");
> +}
> +#endif
> +
> +TEST(verify_report)
> +{
> +	__u8 reportdata[TDX_REPORTDATA_LEN];
> +	struct tdreport_data *tdr_data;
> +	__u8 tdreport[TDX_REPORT_LEN];
> +	struct tdx_report_req req;
> +	int devfd, i;
> +
> +	devfd =3D open(devname, O_RDWR | O_SYNC);
> +
> +	ASSERT_LT(0, devfd);
> +
> +	/* Generate sample report data */
> +	for (i =3D 0; i < TDX_REPORTDATA_LEN; i++)
> +		reportdata[i] =3D i;
> +
> +	/* Initialize IOCTL request */
> +	req.subtype     =3D 0;
> +	req.reportdata  =3D (__u64)reportdata;
> +	req.rpd_len     =3D TDX_REPORTDATA_LEN;
> +	req.tdreport    =3D (__u64)tdreport;
> +	req.tdr_len     =3D TDX_REPORT_LEN;
> +
> +	/* Get TDREPORT */
> +	ASSERT_EQ(0, ioctl(devfd, TDX_CMD_GET_REPORT, &req));
> +
> +	tdr_data =3D (struct tdreport_data *)tdreport;
> +
> +#ifdef DEBUG
> +	print_array_hex("\n\t\tTDX report data\n", "", reportdata,
> +			sizeof(reportdata));
> +
> +	print_array_hex("\n\t\tTDX tdreport data\n", "", &tdreport,
> +			sizeof(tdreport));
> +#endif
> +
> +	/* Make sure TDREPORT data includes the REPORTDATA passed */
> +	ASSERT_EQ(0, memcmp(&tdr_data->_reportmac.reportdata[0], reportdata,
> +				sizeof(reportdata)));
> +
> +	ASSERT_EQ(0, close(devfd));
> +}
> +
> +TEST_HARNESS_MAIN