Add Azure TDX (preview) support#221
Add Azure TDX (preview) support#221jepio wants to merge 2 commits intoconfidential-containers:mainfrom
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>
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>
| } | ||
|
|
||
| pub(super) fn detect_platform() -> bool { | ||
| // check cpuid if we are in a Hyper-V guest |
There was a problem hiding this comment.
| // check cpuid if we are in a Hyper-V guest | |
| // check cpuid if we are in a TD guest |
There was a problem hiding this comment.
this is actually checking the hypervisor id so the comment is correct
There was a problem hiding this comment.
TDX module spec defines "TD guest" cpuid bit as well (I've tested it works in Azure). Would that be more robust check here?
There was a problem hiding this comment.
I think that would defeat the purpose of the check. This check happens after we already checked for the presence of the tdx_guest device, which internally checks the cpuid bit (https://elixir.bootlin.com/linux/latest/source/drivers/virt/coco/tdx-guest/tdx-guest.c#L87).
The purpose of the check is to find out whether we're running in Azure (on Hyper-V) and if so - to use the tcp endpoint for report->quote conversion
| pub fn make_attester() -> Box<dyn Attester + Sync + Send> { | ||
| if az::detect_platform() { | ||
| Box::<az::TdxAttester>::default() | ||
| } else { | ||
| Box::<TdxAttester>::default() | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like a nice idea for me to follow in the SGX attester, thanks
| // Upstream kernel exposes a /dev/tdx_guest device but Intel's lib expects | ||
| // /dev/tdx-guest | ||
| let paths = [ | ||
| Path::new("/dev/tdx-attest"), | ||
| Path::new("/dev/tdx-guest"), | ||
| Path::new("/dev/tdx_guest"), | ||
| ]; |
There was a problem hiding this comment.
I personally don't like this approach after feeling the pain of fixing/using code using the non-standard paths in the SGX world. What would be the changes to force only /dev/tdx_guest usage here and get other parts fixed as early as possible (or have udev rules with symlinks)?
There was a problem hiding this comment.
SGXDataCenterAttestationPrimitives would need to be fixed, because it uses a different ioctl/API to talk to /dev/tdx-guest than what was merged upstream. This also means symlinks won't simply work.
I think we're going to need to transition and phase out the older paths.
|
btw, should this move to |
Yes, good idea - i missed that the merge already happened. |
|
Moved the PR here: confidential-containers/guest-components#170 |
2 participants
These changes were needed to get the TDX attestation to work on Azure.
Based on intel/trustauthority-client-for-go@c590bde.