-
Notifications
You must be signed in to change notification settings - Fork 0
[applications] Add Escrow-Xput Function In Bare-Metal SNP #57
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
Conversation
|
/gemini review |
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.
Code Review
This pull request introduces significant new functionality for running and attesting applications in a bare-metal AMD SEV-SNP environment. This includes a new escrow-xput application, extensive CLI tooling in accli for managing confidential VMs (cVMs) and a new attestation service, and the core logic for SNP report verification in the attestation service itself. The changes are extensive and well-structured, particularly the cVM orchestration in accli and the caching mechanisms in the attestation service.
My review focuses on a critical security gap in the SNP attestation flow, several maintainability issues in the new application code, and opportunities to improve the robustness of the new tooling. The most critical issue is the missing validation of the SNP report's measurement, which must be addressed to ensure only trusted code is attested. Other points include removing a large amount of commented-out code and improving configuration handling in the new benchmark application.
7ab1ea7 to
56da6ea
Compare
56da6ea to
7e915c5
Compare
88d6ebf to
5f35f1b
Compare
d33ed5c to
739bbcc
Compare
739bbcc to
9100a3e
Compare
Closes #25