-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AA: Add API to extend measurement register at runtime #392
Conversation
5fc3fea
to
b88a8f9
Compare
7d01fcd
to
ff9d2bd
Compare
_register_index: Option<u64>, | ||
) -> Result<()> { | ||
for event in events { | ||
match tdx_attest_rs::tdx_att_extend(&event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The underlying tdx_att_extend
function is defined in https://github.com/intel/SGXDataCenterAttestationPrimitives/blob/dcap_1.16_reproducible/QuoteGeneration/quote_wrapper/tdx_attest/tdx_attest.c#L477-L479
Do we need to specify a structured input of this function. In other words, will a random byte string work as an input here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jialez0 I think the commit message should be updated to reflect that this api is implemented only for TDX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xynnn007 good point, yes this is the expected struct:
https://github.com/intel/SGXDataCenterAttestationPrimitives/blob/5b320f93e16d0bebf700f2ce4bb5c63c4195bf58/QuoteGeneration/quote_wrapper/tdx_attest/tdx_attest.h#L89-L96
Now the event data itself should also follow a format so that we can verify it.
ACON uses an RTMR event log. @binxing can say more on that.
I don't think we need that event format in place for this PR, but this is a good reminder that we should start specifying it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think a structured input is definitely necessary, otherwise the caller wouldn't have any idea what tdx_att_extend
expects.
Maybe a bit off the topic. Have we ever considered keeping measurement logs for RTMRs? With logs, we can support more usages such as measuring the order in which the containers are started, or upgrading containers at runtime.
What is the use-case for this API in CoCo? |
We need support runtime attestation for sidecar containers and FaaS scenarios, they both need measure the workloads like container image. |
Won't sidecar images be measured with signatures or dm-verity hashes like any other image? I'm a little unclear on the use case here as well, although I think we might as well add support in the AA. What component do you think will call this API? Kata agent? image-rs? |
Current CoCo supported use cases requires container images are signed by the CoCo user, we also need support scenarios that sidecar/serving containers are from third party or untrusted CSP, CoCo user may only provide the workload or like AI model, then CoCo user will requires add these container image into the measurement evidence. We can call this API in |
AFAIUI, the policy allows users to define what goes into the pod. |
Yes, CoCo user may allow some sidecar containers owned by third party to run in the pod, current image integrity is protected by singing and these images are not signed by CoCo user, in these scenarios, we need also measure these images. |
The CoCo policy allows users to define the checksums of each layer belonging to the "container group", including the non-signed sidecars. I guess @danmihai1 can say more on what's possible and what is not. |
I am still a bit skeptical about this use case. In theory sidecar images can be validated via public keys. If anything it might make sense to measure the public keys at runtime (at least if we're using signatures). Either way I think it is fine to add underlying support in the AA. One more question, though. When we update the runtime measurement, what should we do regarding existing tokens/connections? Should existing connections be invalidated? |
Ok, it seems like this might also help us with measuring InitData/Policy for peer pods. We should try to get this merged and then add on some vTPM based backends. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jialez0 LGTM!
For the use case described above (allowing side-car containers), this is typical for what should be covered by a policy, no? As far as I understand, this will only work for TDX's RTMR registers (and possibly TPM based TEEs). If that's the case, does it make sense to introduce a generalized API? What would be the workarounds for e.g. SNP or would we prescribe different methods for asserting legal side-car containers in TDX (RTMR) and SNP (policy hash)? For I'm not sure this (or measuring sidecar containers) qualifies as "runtime measurement" from the CoCo perspective, though. Logically it's still part of a static measurement of a CoCo TCB, the measurement just occurs in userland. |
ff9d2bd
to
2eaecc0
Compare
Signed-off-by: Jiale Zhang <[email protected]>
2eaecc0
to
cd9f2bc
Compare
cc @arronwy