From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A66BC3F6B0 for ; Mon, 1 Aug 2022 17:49:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233592AbiHARtr (ORCPT ); Mon, 1 Aug 2022 13:49:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56582 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232650AbiHARtq (ORCPT ); Mon, 1 Aug 2022 13:49:46 -0400 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DC1FB2DAAB for ; Mon, 1 Aug 2022 10:49:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1659376184; x=1690912184; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=+d6jxagz6nPCDxYu6Ow7W2Ev5jlGEPwhsUzneKE5yEI=; b=Jrl5tAKJuSLYmveY+t6vH+Rsu8mBIhddldCNyFvfFt6sjc7zgZgcH4uN /xY/NKReNH731aKpREEX6FOsTZHwHAbMrdybyujTV4Pldaq/xzPj8cv7X C2Q5UpeoedfX4mawjvMEdtxhmtw9knuBhDuQ5YcOn/BlXP9ztWQwu8DJp V1a5m5aqZYq8quLhzlIjNMIw0P9CKX5/2aOdhmwEY0HOomKxi4d7eqkCG JY7UU5HF+4IsRr5rmm5ZnfvSuoJWpfVanzMpLWSmMAeoHe8TCO6OUDF8/ 6idP7s7ahMrdujeJmCBMnvvASAkPNJY8VI8qGJs1nsiKCOp58//gHLqgZ Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10426"; a="269589486" X-IronPort-AV: E=Sophos;i="5.93,208,1654585200"; d="scan'208";a="269589486" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Aug 2022 10:49:44 -0700 X-IronPort-AV: E=Sophos;i="5.93,208,1654585200"; d="scan'208";a="705057376" Received: from jrluquin-mobl.amr.corp.intel.com (HELO [10.212.184.211]) ([10.212.184.211]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Aug 2022 10:49:44 -0700 Message-ID: <2d5e6b7f-c499-aaa1-a308-cb17b5500c84@linux.intel.com> Date: Mon, 1 Aug 2022 10:49:43 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0 Thunderbird/91.11.0 Subject: Re: [PATCH v9 2/6] selftests: tdx: Test GetReport TDX attestation feature Content-Language: en-US To: Kai Huang , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org Cc: "H . Peter Anvin" , "Kirill A . Shutemov" , Tony Luck , Andi Kleen , Wander Lairson Costa , Isaku Yamahata , marcelo.cerri@canonical.com, tim.gardner@canonical.com, khalid.elmously@canonical.com, philip.cox@canonical.com, linux-kernel@vger.kernel.org References: <20220728034420.648314-1-sathyanarayanan.kuppuswamy@linux.intel.com> <20220728034420.648314-3-sathyanarayanan.kuppuswamy@linux.intel.com> <03c6c9cecd281d64d0efd48cb40135092dc2d0df.camel@intel.com> From: Sathyanarayanan Kuppuswamy In-Reply-To: <03c6c9cecd281d64d0efd48cb40135092dc2d0df.camel@intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> Reviewed-by: Andi Kleen >> Acked-by: Kirill A. Shutemov >> Signed-off-by: Kuppuswamy Sathyanarayanan >> --- >> 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 >> + */ >> + >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#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