Add Azure TDX (preview) support #170
Conversation
Marking get_evidence as async allows running async functions from an attester. There are two motivators for this: - TDX report->quote conversion can require an HTTP request, and it's a good idea to run that async. - as we move to support the RATS passport model better the attester itself might need to talk to MAA or Amber to fetch an attestation token. If get_evidence is not async, then it becomes tricky to use reqwest from get_evidence. reqwest::blocking::Client panics because it internally uses tokio and get_evidence is called from a tokio runtime. Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
| Path::new("/dev/tdx-attest"), | ||
| Path::new("/dev/tdx-guest"), | ||
| Path::new("/dev/tdx_guest"), |
There was a problem hiding this comment.
Getting back to the cpuid topic one more time ;-) Would it be sufficient to check TD guest cpuid bit here or we want to explicitly check the device nodes are present?
There was a problem hiding this comment.
In this spot we could check the TD guest bit instead, i don't mind doing it that way
There was a problem hiding this comment.
Do you have a pointer to the cpuid TD guest bit? All I can find cpuid leaf 0x21 which returns the "IntelTDX" string after reassembling registers.
There was a problem hiding this comment.
i'm now using the cpuid leaf 0x21
There was a problem hiding this comment.
Do you have a pointer to the cpuid TD guest bit? All I can find cpuid leaf 0x21 which returns the "IntelTDX" string after reassembling registers.
This is what I meant. Sorry for the confusion about that "bit" which was a misleading word.
| let report = base64::encode_config(tdx_req.tdreport, base64::URL_SAFE_NO_PAD); | ||
| let tdquotereq = AzTdQuoteRequest { report }; | ||
| let req = reqwest::Client::new() | ||
| .post("http://169.254.169.254/acc/tdquote") |
There was a problem hiding this comment.
Seems that the conversion from report to quote doesn't follow the standard way, s.t. call to host. Why?
There was a problem hiding this comment.
Unfortunately it doesn't look like there is a single standard way for TDX, see this https://lore.kernel.org/lkml/cover.1684048511.git.sathyanarayanan.kuppuswamy@linux.intel.com/:
Although attestation software can use communication methods like TCP/IP or vsock to send the TDREPORT to QE, not all platforms support these communication models. So TDX GHCI specification [1] defines a method for Quote generation via hypercalls.
It looks like TCP/IP using the IMDS service as a proxy to QE was the best way to implement the communication for preview. It might look different for GA.
There was a problem hiding this comment.
Is the QE on the same platform/host? I mean the conversion from report to quote rely on the verification of the MAC of the report, and the key can only be generated on the same platform via EGETKEY.
There was a problem hiding this comment.
I think yes, the QE VM is on the same node as the TD VM.
This is equally good as a check if the TDX attester should be used. An additional consideration is that while we can check for "known" device node names, there is no way to specify which ones the Intel SGX libraries use (hardcoded in C source) and the ioctl API also differs for the different names, including for the /dev/tdx_guest device that was upstreamed. Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
| [features] | ||
| default = ["all-attesters"] | ||
| all-attesters = ["tdx-attester", "occlum-attester", "az-snp-vtpm-attester", "snp-attester"] | ||
|
|
||
| tdx-attester = ["tdx-attest-rs"] | ||
| tdx-attester = ["tdx-attest-rs", "dep:reqwest", "dep:ioctl-sys", "dep:raw-cpuid"] |
There was a problem hiding this comment.
I'm thinking about this: if more CSPs are going to add their attester code under tdx, like tdx/az, tdx/ali, tdx/another-company. They all shares a same feature here, but must introduce some dependencies they will not use, e.g. CoCo native does not need reqwest, ioctl-sys and raw-cpuid.
The advantage is the community version binary of attestation-agent can be easy to build (only one!)
The disadvantage is big footprint, which will influence the performance and cost.
I do not have a clear solution here, and maybe we should bring a more robust verified build pipeline (CD) of the components of CoCo Community to solve the trustiness here.
There was a problem hiding this comment.
I think for community builds we should support as many different attesters as possible, so that it is easy to try out coco.
But we will need to have ways to disable/enable things that pull in a lot of dependencies so that companies don't need to build in code that will never be used in their environment. We'll add them over time.
Of these dependencies reqwest is the largest, but it is also already used by kbs_protocol.
I do not have a clear solution here, and maybe we should bring a more robust verified build pipeline (CD) of the components of CoCo Community to solve the trustiness here.
👍
To get TDX attestation to work on Azure there are several changes needed: - the device node is called /dev/tdx_guest (upstream kernel name) - quote generation uses the IMDS (instance metadata service) instead of tdvmcall or vsock. This also means we can't use tdx_att_get_quote which combines quote and report fetching - no CCEL Implement the evidence gathering in a sub-module attester, but keep it within the TDX module because the evidence is fully compatible with the existing TDX verifier. It would be possible to use tdx_att_get_report, but calling an ioctl is easy enough that it doesn't make sense to add the dependency on the native library. The Intel library also seems to have an outdated definition of the ioctl. Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
|
Let's not merge this - it was a nice experiment but the TDX interfaces will change from those exposed during the preview. Will revisit in the coming months. |
These changes were needed to get the TDX attestation to work on Azure.
Based on intel/trustauthority-client-for-go@c590bde.
Closes: confidential-containers/attestation-agent#221 (@mythi started reviewing)