-
Notifications
You must be signed in to change notification settings - Fork 55
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
Make PCRs part of payload in Security Overview #3480
Open
flxflx
wants to merge
1
commit into
main
Choose a base branch
from
flxflx/aux-overview
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 don't think this neither accurately describes the current nor the best case attestation flow. Though, I'm not familiar with the attestation flow COCONUT has.
@daniel-weisse likely is the expert on (most/all) current implementations in Constellation. But as far as I know, the CC attestation is used to create trust in the vTPM via the Endorsement Key/Attestation Key.
For instance see: https://github.com/edgelesssys/constellation/blob/euler/ref/pin-crane-and-npm/internal/attestation/vtpm/vtpm.go#L38
https://github.com/edgelesssys/constellation/blob/euler/ref/pin-crane-and-npm/internal/attestation/gcp/snp/snp.go#L23
https://github.com/edgelesssys/constellation/blob/euler/ref/pin-crane-and-npm/internal/attestation/azure/snp/snp.go#L10
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.
Spoke with @daniel-weisse, he says the PCR values are included directly in the remote-attestation statement.
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 "Remote attestation" section already starts of by saying
payload
is the TLS public key of the node, and establishesauxiliary data
andlaunch digest
as part ofR
.Further establishing
payload
to be public key + TPM measurements, in case we can't rely on the firmware, makes the whole set up more confusing, when really the TPM measurements replacelaunch digest
andauxiliary data
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.
So, as the only non-expert here, I’ll take the liberty of asking some "stupid" questions! From what I understand, every piece of code that gets loaded is tracked in the PCRs, including the initial memory content of the CVM. Since the CVM is closed-source and provided by the cloud provider, the only observable information is that it changes. This is referred to as the "launch digest."
Additionally, the code you load or boot for the nodes is also tracked in the PCRs. This is called "auxiliary data." Or is it only your code that gets tracked in the PCRs? Can the two be separated, or are they combined into a single digest? For example, is it structured like:
Launch digest = Hash(third-party code)
Auxiliary data = Hash(your code)
Or could it look more like:
Launch digest = Hash(third-party code)
Auxiliary data = Hash(your code, Hash(third-party code))?
From my understanding, these are separate digests, and the CPU signs them together with the public key. Is that correct?
Also, a second question: Are field names like "payload," "auxiliary data," and "launch digest" predefined by the format of the attestation statement? Or do you have flexibility in deciding what gets written into each field? For instance, is "launch digest" always fixed to represent the initial firmware measurement, or can this be defined differently?
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 PCRs track both code from the cloud provider as well as our own code, giving a total of 16 digests we can work with.
The digests can mostly be separated by our code and third-party code, though some PCRs contain a digest mixing both.
We give an overview about what is tracked by each PCR in this table of our docs.
This is true for
launch digest
, but not for the auxiliary data, which is signed with a key manged by the TPM.Additionally, the TPM key signs a digest of the TLS public key.
launch digest
is a term defined by the AMD SEV-SNP attestation report format, other technologies, e.g. Intel TDX, use different names for similar things. On AMD platforms it is fixed to measure the initial memory contents of the VM, which contain the firmware measurements.Afaik the terms "auxilary data" and "payload" is something we decided to use for our docs and there is no preset definition. Another term for payload is "report data".
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.
Okay, so it seems this is more about presentation. Terms like "auxiliary data" and "payload" aren’t actual fields in the attestation statement. For AMD, the only formal field is the "launch digest." The 16 PCR contents are signed by a CPU key, while the "launch digest" and the TLS public key are signed by a different key, specifically a TPM key.
The use of CPU-sig(...) seems to abstract this distinction, probably to simplify the explanation and make it easier to follow.
That brings me to a question: Why do we have a separate "launch digest" if the firmware is already tracked in the PCRs?
As a suggestion, perhaps we could rename "payload" to "TLS public key" if that’s the only thing it represents. Similarly, "auxiliary data" could be renamed to "node digest" for clarity, though it’s worth noting that the PCRs include more than just the node digest. Of course if they are common terms it is different.
This way, the statement would look something like this:
R = CPU-sig(launch digest, node digest, TLS public key).
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 think I have a better understanding now. The launch digest is generated and tracked by the Confidential Computing (CC) hardware during the setup of a CVM. After the initial CVM setup (e.g., within the virtual UEFI environment), the vTPM takes over and starts tracking the boot process inside the virtual environment, recording measurements in the virtual PCRs.
One of the issues is that the vTPM logic is provided by the cloud provider and is closed-source and not verifiable. So the cloud provider could theoretically forge measurements. If this logic is implemented as part of the hypervisor, then the hypervisor becomes part of the Trusted Computing Base (TCB). This contradicts the principles of Confidential Computing, where the goal is to explicitly exclude the hypervisor from the TCB. I'm getting there...
Anyhow, I think we should consider renaming auxiliary data and payload for clarity, as suggested earlier, provided they are not reserved terms.