Update handling of productName and add a temporary workaround #116
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
There is inconsistent wording for product string arguments that leads to some confusing input/expectation mismatches. This change not only fixes some wording, but also allows tests that expect to validate a report's product to only care about the product line, and not the product name, which can include the stepping version. The test condition relaxation allows a Milan fleet of mixed staging values to pass verify_test.
The default value for this flag is false, since we hope to only advise a small number of users to include this flag temporarily while the CPUID does not match the VCEK cert productName.
deeglaze
force-pushed
the
productstring
branch
from
a978ed6
to
2b3721a
Compare
kongoshuu
approved these changes
Feb 29, 2024
| @@ -47,7 +48,9 @@ const ( | |||
| var ( | |||
| // ErrMissingVlek is returned when attempting to verify a VLEK-signed report that doesn't also | |||
| // have its VLEK certificate attached. | |||
| ErrMissingVlek = errors.New("report signed with VLEK, but VLEK certificate is missing") | |||
| ErrMissingVlek = errors.New("report signed with VLEK, but VLEK certificate is missing") | |||
| workaroundStepping = flag.Bool("workaround_kds_productname", false, "If true, don't compare "+ | |||
There was a problem hiding this comment.
If this is default to false, users will still run into issues unless they explicitly set this flag. Shall this be default to true until the issue is addressed?
|
No, secure defaults. I’d be willing to update the readme or wiki with info
…On Wed, Feb 28, 2024 at 18:10 Chong Cai ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In verify/verify.go
<#116 (comment)>:
> @@ -47,7 +48,9 @@ const (
var (
// ErrMissingVlek is returned when attempting to verify a VLEK-signed report that doesn't also
// have its VLEK certificate attached.
- ErrMissingVlek = errors.New("report signed with VLEK, but VLEK certificate is missing")
+ ErrMissingVlek = errors.New("report signed with VLEK, but VLEK certificate is missing")
+ workaroundStepping = flag.Bool("workaround_kds_productname", false, "If true, don't compare "+
If this is default to false, users will still run into issues unless they
explicitly set this flag. Shall this be default to true until the issue is
addressed?
—
Reply to this email directly, view it on GitHub
<#116 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFL4HA42CJDS73VA5SIP3DYV7PRTAVCNFSM6AAAAABD64OM3OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBXHAYDINBSGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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.
Given the CPUID mismatch between machine and certificate, any verification failures that see "0x0 is not 0x1" should add
--workaround_kds_productname=trueto skip Stepping comparisons.There is some confusion in the KDS specification between
product_namethat's used in the URL, andproductNamethat's used in the VCEK certificate extensions. I've changed the disparate uses ofproduct,productString, andproductNameto instead always have the following meaning:productshould only mean*spb.SevProductproductStringis deprecated. UseproductLinefor the name of the family and model without stepping, e.g., Milan, Genoa.productNameshould only mean the decoded IA5String value of certificate extension 1.3.6.1.4.1.3704.1.2.