google / go-sev-guest Goto Github PK

It would be good to update this to include subsequent hardware models.

As noted in Issue#108, separating the Product collection from the quote collection makes the interface pointier than it needs to be. Keeping this issue open to reference in TODO comments to remove client and verifier uses of Product during the migration to a custom entry in the certificate table.

Hey, there is this warning that is a bit annoying when using the embedded AMD root cert:

ERROR: Logging before logger.Init.
WARN : 2024/01/08 11:42:54.739159 verify.go:501: Using embedded AMD certificates for SEV-SNP attestation root of trust

Actually, I think it is perfectly fine to use the certificate that is embedded in this library, and I don't expect the lib to throw a warning (and error) about something that is perfectly fine.

If you think the internal certificate shouldn't be used, I'd suggest to remove it completely and require callers to pass the certificate where needed.

For some reason, github.com/google/go-tpm-tools depends on this package and causes build to fail on 32bits architectures:

../../go/pkg/mod/github.com/google/[email protected]/abi/amdsp.go:105:53: cannot use 0x100000000 (untyped int constant 4294967296) as SevFirmwareStatus value in constant declaration (overflows)

This is blocking the whole build and is a bit problematic.

A way to fix this is to modify https://github.com/google/go-sev-guest/blob/main/abi/amdsp.go#L21C1-L21C27

type SevFirmwareStatus int

To

type SevFirmwareStatus int64

I tested with GOARCH=386 go build ./... and confirmed there are no other issues preventing building this repository.

Im getting the below error when trying to obtain the report for the device using GetExtendedReport. What could be causing this?

error querying certificate length: invalid argument

So far Ive done client.OpenDevice() under the /dev/sev path to get the device. Then tried to call client.GetExtendedReport(device, reportData) which returns the above error.

Currently, GetAttestationFromReport doesn't allow specifying a certificate chain, and fillInAttestation is private. Does this have any particular reason? Otherwise, one is not allowed to specify a partially present certificate chain (e.g. only ARK) and have the rest filled in. While changing adding an additional parameter to GetAttestationFromReport would be a breaking change, an additional method could be added (e.g. GetAttestationFromReportAndCertChain) to take both a report and a cert chain and fill in only the missing certificates, or the fillInAttestation method could be made public.

When the guest kernel patches are fixed to remove the uninitialized memory from the response, we can remove our check for EIO to avoid doing anything to the firmware error value. Added in PR#3

bee858a#diff-11eac54c87d256754b194b9fc9f84436b29b78da33c6104ba962067db5199d0eL174-L176

I think it is better not to arbitrarily change function signatures, especially within a minor version change, such as this breaking change occurring from v0.7.0 to v0.7.1. It is better to annotate the change with a comment like Deprecated: use xxx instead, so that other developers can understand how to modify their code after upgrading.

Thanks

This issue is to track the mismatch between cpuid(1).eax & 0xf and the VCEK certificate extension 1.3.6.1.4.1.3704.1.2 (productName). AMD EPYC 7B13 has stepping: 1, but the certificate claims Milan-B0, which corresponds to stepping: 0.

Until KDS fixes its CHIP_ID -> stepping/productName mapping, we need to skip the cross-check of stepping values from CPUID and the certificate extension.

Dear Sir/Madam, Currently the repository is empty please add some files or README.md file to get more knowledge on this one. So that we can contribute to the project 👍🏻

Here's an article to get more info on readme file : README File – Everything you Need to Know

We should be able to use Github actions to build the attest/check CLI tools and include them in their own release tarball. While it's not a signed package in a Google-endorsed package repository, it's something to reduce the cost of adoption.

Using go-tpm-tools/client introduces go-sev-guest/abi as dependency. This in turn depends on glog which leaks as shown below. Sadly, glog is also unmaintained, so there's no way for us to provide a fix. Is it possible to replace glog with klog? The interface seems to be similar.

goleak: Errors on successful test run: found unexpected goroutines:
[Goroutine 19 in state chan receive, with github.com/golang/glog.(*loggingT).flushDaemon on top of the stack:
goroutine 19 [chan receive]:
github.com/golang/glog.(*loggingT).flushDaemon(0x0?)
	/home/euler/go/pkg/mod/github.com/golang/[email protected]/glog.go:882 +0x6a
created by github.com/golang/glog.init.0
	/home/euler/go/pkg/mod/github.com/golang/[email protected]/glog.go:410 +0x1bf

Getting the extended report requires the hypervisor to call SNP_SET_EXT_CONFIG with the cert chain, before the corresponding ioctl can succeed.

If the hypervisor never does so (which is optional) the new GetRawQuote API is unusable, even if a report without cert chain is needed, since the request to get the extended report will fail.

In particular, the following error is returned when the certificate cannot be provided: error querying certificate length: unexpected firmware status (see SEV API spec): 6.
Maybe go-sev-guest can detect this error (INVALID_CERTIFICATE) and fall back to a code path without certificate chain?

Thank you for providing this library!
The verifier needs to know the product name to retrieve certs from KDS. This means it is best to exchange the Attestation proto that already contains this info. Otherwise, we need to hardcode the product name on the verifier side or send report and product name in a custom format.
We have an environment where the extended report is not available.
This means we can either issue reports using QuoteProvider.GetRawQuote and call QuoteProvider.Product and assemble the Attestation proto from that, or we call client.GetQuoteProto and see a large number of warnings:

│ ERROR: Logging before logger.Init.                                                                                                                                                                                                                                                            │
│ WARN : 2024/02/09 10:03:08.276541 abi.go:861: Warning: Neither VCEK nor VLEK certificate found in data pages                                                                                                                                                                                  │
│ ERROR: Logging before logger.Init.                                                                                                                                                                                                                                                            │
│ WARN : 2024/02/09 10:03:08.276791 abi.go:865: ASK certificate not found in data pages                                                                                                                                                                                                         │
│ ERROR: Logging before logger.Init.                                                                                                                                                                                                                                                            │
│ WARN : 2024/02/09 10:03:08.276809 abi.go:868: ARK certificate not found in data pages  

I'd be happy to contribute an API that works with only the report (not extended) and contains all the necessary info so it can be used on the verifier side without hardcoding a product name.
Please also note that we have seen some rate-limiting when querying the KDS from the issuer side, so that might not be a great fix for us.

I'm currently trying out SEV-SNP attestation on a Genoa CPU, AMD EPYC 9124 16-Core to be precise.
I'm running into issues when verifying the attestation.

Using the following sample code, calling verify.RawSnpReport throws the error could not recreate attestation from report: unknown product name (new stepping published?): "Genoa"

package main

import (
	_ "embed"
	"fmt"

	"github.com/google/go-sev-guest/client"
	"github.com/google/go-sev-guest/proto/sevsnp"
	"github.com/google/go-sev-guest/verify"
	"github.com/google/go-sev-guest/verify/trust"
	"google.golang.org/protobuf/types/known/wrapperspb"
)

var (
	// ask_ark_genoa.cert from https://download.amd.com/developer/eula/sev/ask_ark_genoa.cert
	//go:embed ask_ark_genoa.cert
	askArkGenoaVcekBytes []byte
	// ask and ark pem certificates from https://kdsintf.amd.com/vcek/v1/Genoa/cert_chain
	//go:embed ask_genoa.pem
	askGenoaPEM []byte
	//go:embed ark_genoa.pem
	arkGenoaPEM []byte
)

func main() {
	// Get report
	reportData := [64]byte{}
	quoteProvider, err := client.GetQuoteProvider()
	if err != nil {
		panic(err)
	}
	reportRaw, err := quoteProvider.GetRawQuote(reportData)
	if err != nil {
		panic(err)
	}

	// Set up verification
	genoaRootCert := new(trust.AMDRootCerts)
	genoaRootCert.Product = "Genoa"
	if err := genoaRootCert.Unmarshal(askArkGenoaVcekBytes); err != nil {
		panic(err)
	}
	if err := genoaRootCert.Decode(askGenoaPEM, arkGenoaPEM); err != nil {
		panic(err)
	}
	verifyOpts := &verify.Options{
		Product: &sevsnp.SevProduct{
			Name:            sevsnp.SevProduct_SEV_PRODUCT_GENOA,
			MachineStepping: &wrapperspb.UInt32Value{Value: 2},
		},
		CheckRevocations: true,
		Getter:           trust.DefaultHTTPSGetter(),

		TrustedRoots: map[string][]*trust.AMDRootCerts{
			"Genoa": {genoaRootCert},
		},
	}

	// Verify report
	err = verify.RawSnpReport(reportRaw, verifyOpts)
	fmt.Println(err)
}

Looking at the code, a map is used to determine the stepping of the CPU for which the VCEK was issued:

The VCEK I'm receiving from the AMD API simply contains Genoa for its productName certificate extension, so it never matches any of the values in this map.

when using the structure from https://github.com/AMDESE/linux-svsm

i'm setting the certs using https://github.com/AMDESE/sev-guest

as hinted in

AMDESE/linux-svsm#14

which works fine with sev-guest-get-report but not with this lib.

pressumably svsm doesnt implement the right call?
can we just assume a default size if it fails?

In abi/abi.go & verify/verify.go, the pborman/uuid library is used, but hasn't been updated in 4 years.

It adds little functionality, compared to the google/uuid library. Would it be possible to update the code accordingly ?

Hey,

currently the Report type does not parse the nested types it includes. For example the TCB versions included are parsed as uint64 and require another function to parse the included data from the field.

How do you feel about accepting a contribution that would change the existing Report type to a nested type that includes subtypes like TCBParts? It seems to me like this would change the layout of the lib in a few places, hence the question.

Best,
Otto

Hi there,

In addition to SEV-SNP, the legacy SEV also produces a guest report, even though it's coming from the hypervisor. The report from the legacy SEV can be obtained from ATTESTATION ioctl from the hypervisor, and it contains a signature from PEK. Is there a plan to support the validate and verify for the legacy SEV report in this project?

Hey there!

If abi.SevProduct() is called in a guest with a malfunctioning SEV-SNP device, this results in the cpuid() call to error with a nil-pointer exception as follows (line numbers based on v0.8.0 this library):

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x411ded9]
goroutine 170 [running]:
github.com/google/go-sev-guest/abi.SevProduct()
external/com_github_google_go_sev_guest/abi/abi.go:810 +0x19
github.com/google/go-sev-guest/client.(*LinuxDevice).Product(0xc00068c580?)
external/com_github_google_go_sev_guest/client/client_linux.go:124 +0xf
github.com/google/go-sev-guest/client.GetExtendedReportAtVmpl({0x15728d0, 0xc000c59a10}, {0xca, 0xcb, 0x75, 0x2, 0xfd, 0x96, 0x8, 0x32, ...}, ...)
external/com_github_google_go_sev_guest/client/client.go:180 +0xf9
github.com/google/go-sev-guest/client.GetExtendedReport(...)
external/com_github_google_go_sev_guest/client/client.go:186

While I still need to figure out what in cpuid exactly causes this failure (haven't got to read into the spec of CPUID yet), I think an easy approach would be to recover from the panic and return an error in that case that could be passed on to the caller, which would lead to an early failure and thus be my personal favorite. Otherwise, falling back to the DefaultSevProduct might be an option too if the function's signature should be kept as is.

Feel free to let me know your thoughts on this and I will be happy to implement it!

The VLEK key type was added in the API revision of November 2022 (primarily an AWS-requested feature), but we don't have proper support for the key type in our KDS cert validation or in the change to the 32-bits that include AUTHOR_KEY_EN. This issue should be closed when we have fake SEV device VLEK support, unit tests, cert chain proto representation of the cert type, and ensure that VLEK/VCEK keys are only acceptable for report types that embed that key type in the report.

While we could pun the VCEK field with the VLEK, it's not the cleanest solution. I think I'd prefer separate fields in the certificate chain representation.

In the trust package, there is a certificate cache in the global state:

This causes tests with multiple test cases to be flaky, as potential errors are not reached if a successful request was cached before and ClearProductCertCache() was not called. The certificate cache should rather be scoped to an object, implementing GetProductChain() as a method. If you are fine with this, I will implement it.

do we have a tool to generate an id block?

unfortunately i can't really figure out the structure from the sev doc.

as far as i understand this

is the result of secure processor already doing its thing with the idblock and not the input format

Hey 👋

I was trying to use 0.7.0 in order to verify an SNP attestation report on AWS. I know now that VLEK is used instead of VCEK and found #67. I will comment there as well once I played around with it :)

While using 0.7.0 I got a somewhat confusing error. The error I got was timeout while downloading the VCEK. So I wasted quite a bit of time debugging (non existing network issues). What was actually happening is that the library was trying to fetch https://kdsintf.amd.com/vcek/v1/Milan/00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000?blSPL=3&teeSPL=0&snpSPL=10&ucodeSPL=207 and AMD correctly responded with 404. Currently everything >300 is handled the same. I think we could narrow down the status code a bit and / or inform users about the underlying error while retrying.

AMD seems to honor the status codes documented in Chapter 4.1. So we could test for HTTP 429 and error early otherwise?

Let me know what you think. I am open to put in a PR as well.

Hi,there.

d.Product() returns product name SevProduct_SEV_PRODUCT_UNKNOWN while I'm using model Milan for sure. Do i need to run a command like snphost import to set CPU id value for my guests?

Last release announced upcoming removal of a set of functions from this module. However, those functions are not marked as deprecated in a way that common Go tooling can detect it.

from https://go.dev/blog/godoc:

To signal that an identifier should not be used, add a paragraph to its doc comment that begins with "Deprecated:" followed by some information about the deprecation.

Hi @deeglaze, I'm currently trying to embed this repo to my snp attestation workflow. When using GetExtendedReport(), it seems that i should first set ark/ask/vcek on host platform. So I have the following questions:

1. How to get vcek on host? Since chip_id and tcb_version are required to derive vcek from amd sev kds. As far as I know, they are presented in attestation report, but the report cannot be directly accessed by host.
2. How to set ark/ask/vcek on host? Is there any tool available now?(I found a rust repo snphost)

How verify function needs to be run by having the binary report and which files should be provided, can you provide the example command?

I reported the issue that #44 works around to AMD. They said they'll change KDS's behavior, but it may take a few months given everything else going on. Once the back-dating is in, we shouldn't need the skew behavior in PR44. We should keep the API signature changes though.

The library currently uses the CurrentTCB values of a report when querying the AMD KDS for the VCEK certificate, as opposed to the AMD SEV-SNP ABI spec, which says:

The firmware maintains a TCB_VERSION called the ReportedTcb. ReportedTcb is used to derive
the VCEK that signs the attestation report.

When ReportedTCB != CurrentTCB, this results in a signature verification error when checking the report's signature with the VCEK certificate's public key here:

When there are more SEV-SNP products than just "Milan", we'll need a way to determine what kind of machine produced an attestation report. This can either be deduced from the guest context at collection time through CPUID, an a priori option at verification time, or some unknown third option where we can determine the product from attestation report contents itself. This will be useful for collecting certificates from the KDS using just an attestation report.

	min, err := kds.ComposeTCBParts(options.MinimumTCB)
	if err != nil {
		return fmt.Errorf("option MinimumTCB error: %v", err)
	}
	if kds.TCBVersion(report.GetCurrentTcb()) < min {
		return fmt.Errorf("firmware's current TCB %x is less than required %x",
			report.GetCurrentTcb(), min)
	}

This treats the TCB_VERSION as a single number instead of an array of lower bounds for each kind of SPL. This means the accidental position of one SPL can mask a version mismatch in a lower order version. For example, [2, 1] is greater than [1, 3], but you don't want to allow a lower SPL for the number on the right because the number on the left is higher.

We should treat the minimum TCB_VERSION as a piecewise minimum.

There are some confusing field names with respect to what is meant by product, product name, product string, and the new "product line" term. This is an issue to track cleanup TODO comments after a sufficient deprecation period.

Liunx kernel 6.1 incorporates @pgonda's IV reuse crypto fix, which deletes the VMPCK when the host VMM returns any error. The x86/urgent follow-up to retry commands when throttled to avoid deleting the VMPCK is not in 6.1 yet.

While not a panacea, we can self-throttle in this library as a workaround until that throttling awareness fix is in.