-
Notifications
You must be signed in to change notification settings - Fork 93
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
ear: add TDX sample policy checks #667
Conversation
Populate the default EAR policy with an initial TDX policy configuration. Co-developed-by: Jorge Almansa <[email protected]> Signed-off-by: Mikko Ylinen <[email protected]>
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.
Awesome. I don't know enough about TDX to really confirm this but it looks like a big step forward.
thanks. This should be enough to get started. I need to make a few adjustments once I get |
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.
LGTM. Looks like one important thing is to ensure the boot chain for tdx guest. If we use td-shim, we will have tdx.ccel.kernel
in parsed claims. I am not sure what ovmf from ubuntu upstream will generate, needs a try.
# Check TDX Module version and its hash. Also check OVMF code hash. | ||
input.tdx.quote.body.mr_seam in data.reference.mr_seam | ||
input.tdx.quote.body.tcb_svn in data.reference.tcb_svn | ||
input.tdx.quote.body.mr_td in data.reference.mr_td |
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.
Following https://github.com/confidential-containers/trustee/blob/main/attestation-service/docs/parsed_claims.md#intel-tdx, seems that we can also add
tdx.quote.body.mrsigner_seam
also?
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.
I left that out on purpose for now since that's all zeros for Intel's TDX Module. Do you think it would be good to add a check for zero 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.
As a sample policy, it is ok imo.
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.
OK as it is now?
it does not work and that is one thing on my list two fix |
Something I drafted together with @jorgealmansa.
Populate the default EAR policy with an initial TDX policy configuration.