Update attestation_report 'flags' bitmask

[derpsteb]

I was fetching a report with the lib and was faced with this error message: mbz bits at offset 0x48 not zero: 0x00000004.

Looking at page 44 of the ABI spec here this seems like a valid value. After updating the bitmask with this patch the report was generated.

I am happy to remove the gofumpt formatting if you prefer that. It looked like you also use it, which is why I left it in.

Cheers.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[derpsteb]

I just realized that this probably has changed since you built the library since there is now the possibility to have the attestation report signed with a VLEK, instead of a VCEK.

This fix is not correct. Will update later.

[deeglaze]

Thanks for bringing this to my attention, I wasn't aware of the VLEK before this PR. We'll still make sure this generic library stays up to date with the spec if you decide not to pursue a full support PR, just maybe not immediately. Thanks!

Edit: removed part of response with an incorrect understanding of the effect on CSP in TCB.

[deeglaze]

I think for VLEK to be added to go-sev-guest, we need to have a better sense of the whole story so we can have e2e tests with hardware. I know this library doesn't show how to install certificates into /dev/sev, so assuming a loaded VLEK could be okay, but I don't know what the difference will be for VLEK certificate shapes, since our kds library validates expected data about certificates.

I think for now we can extend the ABI representation, but not yet validate VLEK-based attestation reports.

[derpsteb]

Hey! Thanks for the quick response.

re KDS support: the only VLEK documentation I found so far is the ABI spec. It mentioned that the KDS would offer an endpoint to query for the CSP-specific VLEK. However, I couldn't find any information how that API looks like. Playing around with the API I was able to pull a valid certchain from https://kdsintf.amd.com/vlek/v1/Milan/cert_chain. But some official docs would be nice to build out the KDS lib.

re this PR and VLEK support: I am not sure if I am familiar with the code yet to implement full VLEK support. I updated the commit msg to reflect the change more accurately. I am not sure if merging this would introduce incorrect behavior in some places, but it allows me to do what I need. And I don't think it would break anything.

My PoC based on this commit:

• read a SNP report signed by a VLEK using sevclient.GetRawExtendedReportAtVmpl
• parse the cert table using abi.CertTable.Unmarshal and abi.CertTable.GetByGUIDString
• fetch and validate cert chain without go-sev-guest
• verify report signature with verify.SnpReportSignature. Giving it the parsed VLEK works without problems.

Based on this, would you be willing to accept the PR?

[deeglaze]

I think the protocol buffers need to be updated as well. The certificatechain needs the vlek_cert.

I think also the author_key_en field of the attestation report message should be renamed and give new handling in the validation code akin to how the guest policy can be decomposed into its component parts. Decomposition can fail if there are set mbz ranges–I think that interpretation failure would be better than the difficult-to-understand bitmask in use both at tip and in this PR.

Validation options should change to accommodate the new richness of this uint32 field, but I think I can take this responsibility.

When you say you're able to verify a report with a VLEK cert, you're working from a certificate that you've already negotiated a CSP shared secret with AMD using an unpublished KDS API update?

[derpsteb]

The VLEK certificate is part of the extended report. go-sev-guest can retrieve that correctly. From what I understand in the docs, I can treat that like a VCEK and don't have to do anything special.

[deeglaze]

I haven't found the time to fully implement VLEK support with unit tests and fake support, but I'll merge this to unblock. I've opened Issue#53 to track everything that I hope to get to, but alas I can't prioritize that work just yet.