linux-kernel - Re: [PATCH v1] virt: tdx-guest: Handle GetQuote request error code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240110125451.whabimhece7dw5jn@box>
Date: Wed, 10 Jan 2024 15:54:51 +0300
From: "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc: x86@...nel.org, Dave Hansen <dave.hansen@...ux.intel.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Xiaoyao Li <xiaoyao.li@...el.com>, linux-kernel@...r.kernel.org,
	linux-coco@...ts.linux.dev
Subject: Re: [PATCH v1] virt: tdx-guest: Handle GetQuote request error code

On Tue, Jan 09, 2024 at 07:56:56PM -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> 
> On 1/9/2024 5:17 AM, Kirill A . Shutemov wrote:
> > On Tue, Jan 09, 2024 at 05:46:04AM +0000, Kuppuswamy Sathyanarayanan wrote:
> >> Currently when a user requests for the Quote generation, the Quote
> >> generation handler (tdx_report_new()) only checks whether the VMM
> >> successfully processes the Quote generation request (status !=
> >> GET_QUOTE_IN_FLIGHT) and returns the output to the user without
> >> validating the status of the output data. Since VMM can return error
> >> even after processing the Quote request, returning success just after
> >> successful processing will create confusion to the user. Although for
> >> the failed request, output buffer length will be zero and can also be
> >> used by the user to identify the failure case, it will be more clear to
> >> return error for all failed cases. So validate the Quote output status
> >> and return error code for all failed cases.
> > 
> > Could you split commit message into several paragraphs? It would be easier
> > to get along.
> > 
> > It can be helpful to follow structure like:
> > 
> > <Background>
> > 
> > <Problem>
> > 
> > <Solution>
> > 
> 
> How about the following version?
> 
> During the TDX guest attestation process, TSM ConfigFS ABI is used by
> the user attestation agent to get the signed VM measurement data (a.k.a
> Quote), which can be used by a remote verifier to validate the
> trustworthiness of the guest. When a user requests for the Quote data
> via the ConfigFS ABI, the TDX Quote generation handler
> (tdx_report_new()) forwards the request to VMM (or QE) via a hypercall,
> and then shares the output with the user.
> 
> Currently, when handling the Quote generation request, tdx_report_new()
> handler only checks whether the VMM successfully processed the request
> and if it is true it returns success and shares the output to the user
> without actually validating the output data. Since the VMM can return
> error even after processing the Quote request, always returning success
> for the processed requests is incorrect and will create confusion to
> the user. Although for the failed request, output buffer length will
> be zero and can also be used by the user to identify the failure case,
> it will be more clear to return error for all failed cases.
> 
> So when handling the Quote generation request, validate the Quote data
> output status and return error code for all failed cases.

I would drop the start of the sentence upto ',' here, but otherwise looks
good to me.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov