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;
MIME-Version: 1.0
References: <20230413034108.1902712-1-sathyanarayanan.kuppuswamy@linux.intel.com>
 <20230413034108.1902712-4-sathyanarayanan.kuppuswamy@linux.intel.com>
In-Reply-To: <20230413034108.1902712-4-sathyanarayanan.kuppuswamy@linux.intel.com>
From:   Dionna Amalie Glaze <dionnaglaze@google.com>
Date:   Wed, 26 Apr 2023 08:47:39 -0700
Message-ID: <CAAH4kHb_PAhR5jLFi2KsGku9ALNtjtxb-JO-k1ULK-Mj5mi=pw@mail.gmail.com>
Subject: Re: [PATCH v2 3/3] selftests/tdx: Test GetQuote TDX attestation feature
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc:     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,
        Shuah Khan <shuah@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        "H . Peter Anvin" <hpa@zytor.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Wander Lairson Costa <wander@redhat.com>,
        Erdem Aktas <erdemaktas@google.com>,
        Chong Cai <chongc@google.com>, Qinkun Bao <qinkun@apache.org>,
        Guorui Yu <GuoRui.Yu@linux.alibaba.com>,
        Du Fan <fan.du@intel.com>, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Apr 12, 2023 at 8:42=E2=80=AFPM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> In TDX guest, the second stage of the attestation process is Quote
> generation. This process is required to convert the locally generated
> TDREPORT into a remotely verifiable Quote. It involves sending the
> TDREPORT data to a Quoting Enclave (QE) which will verify the
> integerity of the TDREPORT and sign it with an attestation key.

nit: integrity

>
> Intel's TDX attestation driver exposes TDX_CMD_GET_QUOTE IOCTL to
> allow user agent get the TD Quote.

nit: to get
>
> Add a kernel selftest module to verify the Quote generation feature.
>
> TD Quote generation involves following steps:
>
> * Get the TDREPORT data using TDX_CMD_GET_REPORT IOCTL.
> * Embed the TDREPORT data in quote buffer and request for quote
>   generation via TDX_CMD_GET_QUOTE IOCTL request.
> * Upon completion of the GetQuote request, check for non zero value
>   in the status field of Quote header to make sure the generated
>   quote is valid.
>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> Reviewed-by: Andi Kleen <ak@linux.intel.com>
> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
> Reviewed-by: Mika Westerberg <mika.westerberg@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/tdx/tdx_guest_test.c | 68 ++++++++++++++++++--
>  1 file changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/tdx/tdx_guest_test.c b/tools/testing=
/selftests/tdx/tdx_guest_test.c
> index 81d8cb88ea1a..2eccde54185b 100644
> --- a/tools/testing/selftests/tdx/tdx_guest_test.c
> +++ b/tools/testing/selftests/tdx/tdx_guest_test.c
> @@ -18,6 +18,7 @@
>  #define TDX_GUEST_DEVNAME "/dev/tdx_guest"
>  #define HEX_DUMP_SIZE 8
>  #define DEBUG 0
> +#define QUOTE_SIZE 8192
>
>  /**
>   * struct tdreport_type - Type header of TDREPORT_STRUCT.
> @@ -128,21 +129,29 @@ static void print_array_hex(const char *title, cons=
t char *prefix_str,
>         printf("\n");
>  }
>
> +/* Helper function to get TDREPORT */
> +long get_tdreport0(int devfd, struct tdx_report_req *req)
> +{
> +       int i;
> +
> +       /* Generate sample report data */
> +       for (i =3D 0; i < TDX_REPORTDATA_LEN; i++)
> +               req->reportdata[i] =3D i;
> +

Shouldn't req be zeroed before populating reportdata? We wouldn't want
uninitialized memory to leave the guest. I know this is just a test,
but it's best to model good practices for anyone that might
copy/paste.

> +       return ioctl(devfd, TDX_CMD_GET_REPORT0, req);
> +}
> +
>  TEST(verify_report)
>  {
>         struct tdx_report_req req;
>         struct tdreport *tdreport;
> -       int devfd, i;
> +       int devfd;
>
>         devfd =3D open(TDX_GUEST_DEVNAME, O_RDWR | O_SYNC);
>         ASSERT_LT(0, devfd);
>
> -       /* Generate sample report data */
> -       for (i =3D 0; i < TDX_REPORTDATA_LEN; i++)
> -               req.reportdata[i] =3D i;
> -
>         /* Get TDREPORT */
> -       ASSERT_EQ(0, ioctl(devfd, TDX_CMD_GET_REPORT0, &req));
> +       ASSERT_EQ(0, get_tdreport0(devfd, &req));
>
>         if (DEBUG) {
>                 print_array_hex("\n\t\tTDX report data\n", "",
> @@ -160,4 +169,51 @@ TEST(verify_report)
>         ASSERT_EQ(0, close(devfd));
>  }
>
> +TEST(verify_quote)
> +{
> +       struct tdx_quote_hdr *quote_hdr;
> +       struct tdx_report_req rep_req;
> +       struct tdx_quote_req req;
> +       __u64 quote_buf_size;
> +       __u8 *quote_buf;
> +       int devfd;
> +
> +       /* Open attestation device */
> +       devfd =3D open(TDX_GUEST_DEVNAME, O_RDWR | O_SYNC);
> +
> +       ASSERT_LT(0, devfd);
> +
> +       /* Add size for quote header */
> +       quote_buf_size =3D sizeof(*quote_hdr) + QUOTE_SIZE;
> +
> +       /* Allocate quote buffer */
> +       quote_buf =3D malloc(quote_buf_size);
> +       ASSERT_NE(NULL, quote_buf);
> +
> +       quote_hdr =3D (struct tdx_quote_hdr *)quote_buf;
> +
> +       /* Initialize GetQuote header */
> +       quote_hdr->version =3D 1;
> +       quote_hdr->status  =3D GET_QUOTE_SUCCESS;
> +       quote_hdr->in_len  =3D TDX_REPORT_LEN;
> +       quote_hdr->out_len =3D 0;
> +
> +       /* Get TDREPORT data */
> +       ASSERT_EQ(0, get_tdreport0(devfd, &rep_req));
> +
> +       /* Fill GetQuote request */
> +       memcpy(quote_hdr->data, rep_req.tdreport, TDX_REPORT_LEN);
> +       req.buf   =3D (__u64)quote_buf;
> +       req.len   =3D quote_buf_size;
> +
> +       ASSERT_EQ(0, ioctl(devfd, TDX_CMD_GET_QUOTE, &req));
> +
> +       /* Check whether GetQuote request is successful */
> +       EXPECT_EQ(0, quote_hdr->status);
> +
> +       free(quote_buf);
> +
> +       ASSERT_EQ(0, close(devfd));
> +}
> +
>  TEST_HARNESS_MAIN
> --
> 2.34.1
>


--=20
-Dionna Glaze, PhD (she/her)