On 4/3/2025 4:10 PM, Daniel P. Berrangé wrote:
On Thu, Apr 03, 2025 at 03:28:43PM +0800, Xiaoyao Li wrote:
On 4/2/2025 11:49 PM, Daniel P. Berrangé wrote:
On Wed, Apr 02, 2025 at 11:26:11PM +0800, Xiaoyao Li wrote:
I guess the raw mode was introduced due to the design was changed to let
guest kernel to forward to TD report to host QGS via TDVMCALL instead of
guest application communicates with host QGS via vsock, and Linux TD guest
driver doesn't integrate any QGS protocol but just forward the raw TD report
data to KVM.
IMHO, QEMU should be made to pack & unpack the TDX report from
the guest into the GET_QUOTE_REQ / GET_QUOTE_RESP messages, and
this "raw" mode should be removed to QGS as it is inherantly
dangerous to have this magic protocol overloading.
There is no enforcement that the input data of TDVMCALL.GetQuote is the raw
data of TD report. It is just the current Linux tdx-guest driver of tsm
implementation send the raw data. For other TDX OS, or third-party driver,
they might encapsulate the raw TD report data with QGS message header. For
such cases, if QEMU adds another layer of package, it leads to the wrong
result.
If I look at the GHCI spec
https://cdrdv2-public.intel.com/726790/TDX%20Guest-Hypervisor%20Communication%20Interface_1.0_344426_006%20-%2020230311.pdf
In "3.3 TDG.VP.VMCALL<GetQuote>", it indicates the parameter is a
"TDREPORT_STRUCT". IOW, it doesn't look valid to allow the guest to
send arbitrary other data as QGS protocol messages.
In table 3-7, the description of R12 is
Shared GPA as input - the memory contains a TDREPORT_STRUCT.
The same buffer is used as output - the memory contains a TD Quote.
table 3-10, describes the detailed format of the shared GPA:
starting from offset 24 bytes, it is the "Data"
On input, the data filled by TD with input length. The data should
include TDREPORT_STRUCT. TD should zeroize the remaining buffer to
avoid information leak if size of shared GPA (R13) > Input Length.
It uses the word "contains" and "include", but without "only". So it is not
clear to me.
I will work with internal attestation folks to make it clearer that who (TD
guest or host VMM) is responsible to encapsulate the raw TDERPORT_STRCUT
with QGS MSG protocol, and update the spec accordingly.
To be clear, my strong preference is that the spec be updated to only
permit the raw TDREPORT_STRUCT.
IMHO allowing arbitrary QGS MSGs would be a significant host security
weakness, as it exposes a huge amount of the QGS codebase to direct
attack from the guest.
If I remember correctly, the QGS instance keeps the vsock interface so
that TD guest can communicate with QGS directly without going through
host VMM. (I'm not sure if latest QGS implementation still keeps it.)
So QGS should know how to protect itself.
Regarding QEMU avoids from being exploited to forward arbitrary data
from malicious TDX guest to QGS, QEMU can check the beginning of the
data to be a valid QGS MSG header before handing it over to QGS socket.
QEMU needs to be able to block that attack
vector. Without that, the benefit/value of shuffling of TDREPORTs via
the GetQuote hypercall is largely eliminated, and might as well have
just exposed QGS over VSOCK.
If I remember correctly, one of the reason we changed from TD guest
communicates with QGS directly with vsock to current TDVMCALL(GETQUOTE),
is some of the TD guest environment might not have network stack for
vsock to work.
Anyway, I don't have preference. Either is OK to me. Let's see what
decision the attestation folks will make.
With regards,
Daniel
