Extend v3 support to KDS and change proto representation #145
Conversation
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
Adds consideration for Turin and the model value in Cpuid, not just extended model.
deeglaze
force-pushed
the
kdsworkaround
branch
from
853d0a1
to
6583b01
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR #141 added report version 3 support to the abi and protocol buffer.
This PR changes the representation to prefer the CPUID_1_EAX representation of family, model, stepping values.
We have quite a lot of fussiness about trying to determine which product a report version should be evaluated against, which V3 should make simpler.
Unfortunately we need to support both V3 and V2, so the implementation is just more complex. The test.GetProduct function shouldn't be used for determining the product to use for the fake signer if the report we're evaluating includes its product information.
We fully ignore checking the productName extension of VCEK for V3 since the attestation report is the more trustworthy source of that information, and we should not expect the two to match given Issue #115.
With added support for family, model, stepping, I've taken the opportunity to additionally represent the first Zen5 model that supports SEV-SNP: Turin. The extended model is not enough to describe Turin. I've chosen to combine extended model and model to interpret the CPUID more precisely. This changes the mask and how model values are computed and compared.
We may learn from AMD that extended model is still the only relevant value for determining productName, and the model 2 for Turin is a red herring. We can change that later.
Tested successfully on hardware.