Date:   Wed, 26 Apr 2023 08:47:39 -0700
From:   Dionna Amalie Glaze <dionnaglaze@...gle.com>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Shuah Khan <shuah@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Wander Lairson Costa <wander@...hat.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Chong Cai <chongc@...gle.com>, Qinkun Bao <qinkun@...che.org>,
        Guorui Yu <GuoRui.Yu@...ux.alibaba.com>,
        Du Fan <fan.du@...el.com>, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 3/3] selftests/tdx: Test GetQuote TDX attestation feature

On Wed, Apr 12, 2023 at 8:42 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@...ux.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@...el.com>
> Reviewed-by: Andi Kleen <ak@...ux.intel.com>
> Reviewed-by: Shuah Khan <skhan@...uxfoundation.org>
> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...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, const 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 = 0; i < TDX_REPORTDATA_LEN; i++)
> +               req->reportdata[i] = 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 = open(TDX_GUEST_DEVNAME, O_RDWR | O_SYNC);
>         ASSERT_LT(0, devfd);
>
> -       /* Generate sample report data */
> -       for (i = 0; i < TDX_REPORTDATA_LEN; i++)
> -               req.reportdata[i] = 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 = open(TDX_GUEST_DEVNAME, O_RDWR | O_SYNC);
> +
> +       ASSERT_LT(0, devfd);
> +
> +       /* Add size for quote header */
> +       quote_buf_size = sizeof(*quote_hdr) + QUOTE_SIZE;
> +
> +       /* Allocate quote buffer */
> +       quote_buf = malloc(quote_buf_size);
> +       ASSERT_NE(NULL, quote_buf);
> +
> +       quote_hdr = (struct tdx_quote_hdr *)quote_buf;
> +
> +       /* Initialize GetQuote header */
> +       quote_hdr->version = 1;
> +       quote_hdr->status  = GET_QUOTE_SUCCESS;
> +       quote_hdr->in_len  = TDX_REPORT_LEN;
> +       quote_hdr->out_len = 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   = (__u64)quote_buf;
> +       req.len   = 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
>


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

Powered by blists - more mailing lists