Conversation
|
Thank you, @deeglaze and @larrydewey for reviewing this PR. :) I've addressed your comments. |
Turin and later families have FMC SPL in their TCBs. Additionally, the HwID in the TCB is only 8 bytes long for Turin and later, compared to 64 bytes or Genoa and earlier. Please see section 3.1 in the spec below: https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/57230.pdf Signed-off-by: Jagannathan Raman <jraman567@gmail.com>
jraman567
force-pushed
the
turin-tcb-v2
branch
2 times, most recently
from
0160907 to
68e5829
Compare
|
@deeglaze Thanks for reviewing! I've addressed your comment to make the version fields private. |
| // TCBStructVersion0 is the version of the TCB structure for Milan & Genoa | ||
| TCBStructVersion0 = uint8(0) | ||
| // TCBStructVersion1 is the version of the TCB structure for Turin | ||
| TCBStructVersion1 = uint8(1) |
There was a problem hiding this comment.
Struct version constants shouldn't need to be exported given the version fields aren't exported.
|
@deeglaze I've addressed your recent comments |
| switch c.String(productLine) { | ||
| case "Turin": | ||
| return &TCBVersionStruct{version: tcbStructVersion1, TCB: tcb}, nil | ||
| default: |
There was a problem hiding this comment.
This should fail closed. We don't want "Venice" to accidentally end up misinterpreted here.
There was a problem hiding this comment.
@deeglaze When I do that, some of the validate tests are failing. The root cause is that the Reports generated by the QuoteProvider don't have the FMS set. Do you know if we could configure the QuoteProvider to fill the FMS in the report?
There was a problem hiding this comment.
You'll need to change snpReportVersion in validate_test.go to 3 before you do that, but I'd also like to know what the product is that it think's it's on.
There was a problem hiding this comment.
The FMS is 0. So, if we decoded the productLine from FMS, it is "Unknown".
There was a problem hiding this comment.
The issue is the report generated in the following code doesn't have FMS in it:
{
name: "just reportData",
attestation: func() *spb.Attestation {
q, err := sg.GetQuoteProto(qp0, nonce0s1)
if err != nil {
t.Fatal(err)
}
report := q.Report
//report.Cpuid1EaxFms = 0xa00f11
return &spb.Attestation{
Report: report,
CertificateChain: &spb.CertificateChain{
AskCert: sign0.Ask.Raw,
ArkCert: sign0.Ark.Raw,
VcekCert: sign0.Vcek.Raw,
},
}
}(),
opts: &Options{ReportData: nonce0s1[:], GuestPolicy: abi.SnpPolicy{Debug: true}},
},There was a problem hiding this comment.
Yes, we can use test data that depends on version 3 and populates the FMS.
There was a problem hiding this comment.
Thanks, @deeglaze ! The tests in the "tools/check" directory are failing too. The reports that they generate are v2, which don't have FMS. So, we're unable to decode the product family from them.
There was a problem hiding this comment.
I could add file and track an issue to use v3 reports in tools/check and validate_test, if you're OK with defaulting to structVersion0 here.
deeglaze
left a comment
deeglaze
left a comment
There was a problem hiding this comment.
LGTM with final comments.
- fix lint errors reported by Ubuntu CI - make error messages in tests reader friendly - address Larry's feedback - address backwards compatibility issues raised in the review Signed-off-by: Jagannathan Raman <jraman567@gmail.com>
4 participants
Turin and later families have FMC SPL in their TCBs. Additionally, the HwID in the TCB is only 8 bytes long for Turin and later, compared to 64 bytes or Genoa and earlier.
Please see section 3.1 in the spec below:
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/57230.pdf