From 8da1394ad3583e8116f7a1bfcb2a2d0d453a4e71 Mon Sep 17 00:00:00 2001 From: Dionna Glaze Date: Sat, 21 Oct 2023 19:20:19 +0000 Subject: [PATCH 1/2] Add product utility unit tests Improves test coverage, but also expands GetAttestationFromReport to fill in the Product field of the Attestation message from the fetched VCEK/VLEK certificate's extension. Signed-off-by: Dionna Glaze --- abi/abi_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++ kds/kds.go | 6 ++++++ kds/kds_test.go | 30 ++++++++++++++++++++++++++ verify/verify.go | 29 +++++++++++++++++++------ verify/verify_test.go | 33 ++++++++++++++++++++++------ 5 files changed, 134 insertions(+), 14 deletions(-) diff --git a/abi/abi_test.go b/abi/abi_test.go index 7afc49d..299296d 100644 --- a/abi/abi_test.go +++ b/abi/abi_test.go @@ -21,9 +21,12 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" spb "github.com/google/go-sev-guest/proto/sevsnp" "github.com/pborman/uuid" "google.golang.org/protobuf/encoding/prototext" + "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/types/known/wrapperspb" ) var emptyReport = ` @@ -267,3 +270,50 @@ func TestCertTableProto(t *testing.T) { t.Fatalf("Extras[%q] = %v, want %v", extraGUID, gotExtra, extraraw) } } + +func TestSevProduct(t *testing.T) { + oldCpuid := cpuid + defer func() { cpuid = oldCpuid }() + tcs := []struct { + eax uint32 + want *spb.SevProduct + }{ + { + eax: 0x00a00f10, + want: &spb.SevProduct{ + Name: spb.SevProduct_SEV_PRODUCT_MILAN, + MachineStepping: &wrapperspb.UInt32Value{Value: 0}}, + }, + { + eax: 0x00a00f11, + want: &spb.SevProduct{ + Name: spb.SevProduct_SEV_PRODUCT_MILAN, + MachineStepping: &wrapperspb.UInt32Value{Value: 1}}, + }, + { + eax: 0x00a10f10, + want: &spb.SevProduct{ + Name: spb.SevProduct_SEV_PRODUCT_GENOA, + MachineStepping: &wrapperspb.UInt32Value{Value: 0}}, + }, + { + eax: 0x00a10f12, + want: &spb.SevProduct{ + Name: spb.SevProduct_SEV_PRODUCT_GENOA, + MachineStepping: &wrapperspb.UInt32Value{Value: 2}}, + }, + { + eax: 0x0b010f0, + want: &spb.SevProduct{ + Name: spb.SevProduct_SEV_PRODUCT_UNKNOWN, + MachineStepping: &wrapperspb.UInt32Value{Value: 0}}, + }, + } + for _, tc := range tcs { + cpuid = func(op uint32) (uint32, uint32, uint32, uint32) { return tc.eax, 0, 0, 0 } + got := SevProduct() + if diff := cmp.Diff(got, tc.want, protocmp.Transform()); diff != "" { + t.Errorf("SevProduct() = %+v, want %+v. Diff: %s", got, tc.want, diff) + } + } +} diff --git a/kds/kds.go b/kds/kds.go index 3a9206d..a31c21a 100644 --- a/kds/kds.go +++ b/kds/kds.go @@ -406,6 +406,9 @@ func preEndorsementKeyCertificateExtensions(cert *x509.Certificate) (*Extensions // VcekCertificateExtensions returns the x509v3 extensions from the KDS specification of a VCEK // certificate interpreted into a struct type. func VcekCertificateExtensions(cert *x509.Certificate) (*Extensions, error) { + if cert == nil { + return nil, fmt.Errorf("cert cannot be nil") + } exts, err := preEndorsementKeyCertificateExtensions(cert) if err != nil { return nil, err @@ -422,6 +425,9 @@ func VcekCertificateExtensions(cert *x509.Certificate) (*Extensions, error) { // VlekCertificateExtensions returns the x509v3 extensions from the KDS specification of a VLEK // certificate interpreted into a struct type. func VlekCertificateExtensions(cert *x509.Certificate) (*Extensions, error) { + if cert == nil { + return nil, fmt.Errorf("cert cannot be nil") + } exts, err := preEndorsementKeyCertificateExtensions(cert) if err != nil { return nil, err diff --git a/kds/kds_test.go b/kds/kds_test.go index ddd4068..637cae7 100644 --- a/kds/kds_test.go +++ b/kds/kds_test.go @@ -218,6 +218,30 @@ func TestProductName(t *testing.T) { }, want: "badstepping", }, + { + name: "unknown milan stepping", + input: &pb.SevProduct{ + Name: pb.SevProduct_SEV_PRODUCT_MILAN, + MachineStepping: &wrapperspb.UInt32Value{Value: 15}, + }, + want: "unmappedMilanStepping", + }, + { + name: "unknown genoa stepping", + input: &pb.SevProduct{ + Name: pb.SevProduct_SEV_PRODUCT_GENOA, + MachineStepping: &wrapperspb.UInt32Value{Value: 15}, + }, + want: "unmappedGenoaStepping", + }, + { + name: "unknown", + input: &pb.SevProduct{ + Name: pb.SevProduct_SEV_PRODUCT_UNKNOWN, + MachineStepping: &wrapperspb.UInt32Value{Value: 15}, + }, + want: "Unknown", + }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { @@ -266,6 +290,12 @@ func TestParseProductName(t *testing.T) { Name: pb.SevProduct_SEV_PRODUCT_GENOA, }, }, + { + name: "Unhandled report signer", + input: "ignored", + key: abi.NoneReportSigner, + wantErr: "internal: unhandled reportSigner", + }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { diff --git a/verify/verify.go b/verify/verify.go index f195932..aaa68ec 100644 --- a/verify/verify.go +++ b/verify/verify.go @@ -456,7 +456,7 @@ func checkProductName(got, want *spb.SevProduct, key abi.ReportSigner) error { return fmt.Errorf("stepping value in VCEK certificate should not be nil") } if got.MachineStepping.Value != want.MachineStepping.Value { - return fmt.Errorf("%v cert product stepping number %02X is not %02X", + return fmt.Errorf("%v cert product stepping number 0x%X is not 0x%X", key, got.MachineStepping.Value, want.MachineStepping.Value) } } @@ -659,11 +659,6 @@ func SnpAttestation(attestation *spb.Attestation, options *Options) error { if err := fillInAttestation(attestation, options); err != nil { return err } - // Pass along the expected product information for VcekDER. fillInAttestation will ensure - // that this is a noop if options.Product began as non-nil. - if err := updateProductExpectation(&options.Product, attestation.Product); err != nil { - return err - } report := attestation.GetReport() info, err := abi.ParseSignerInfo(report.GetSignerInfo()) @@ -758,7 +753,10 @@ func fillInAttestation(attestation *spb.Attestation, options *Options) error { return ErrMissingVlek } } - return nil + + // Pass along the expected product information for VcekDER. fillInAttestation will ensure + // that this is a noop if options.Product began as non-nil. + return updateProductExpectation(&options.Product, attestation.Product) } // GetAttestationFromReport uses AMD's Key Distribution Service (KDS) to download the certificate @@ -772,6 +770,23 @@ func GetAttestationFromReport(report *spb.Report, options *Options) (*spb.Attest if err := fillInAttestation(result, options); err != nil { return nil, err } + // Attempt to fill in the product field of the attestation. Don't error at this + // point since this is not validation. + info, _ := abi.ParseSignerInfo(report.SignerInfo) + var exts *kds.Extensions + parse := func(der []byte) *x509.Certificate { + out, _ := x509.ParseCertificate(der) + return out + } + switch info.SigningKey { + case abi.VcekReportSigner: + exts, _ = kds.VcekCertificateExtensions(parse(result.CertificateChain.VcekCert)) + case abi.VlekReportSigner: + exts, _ = kds.VlekCertificateExtensions(parse(result.CertificateChain.VlekCert)) + } + if exts != nil { + result.Product, _ = kds.ParseProductName(exts.ProductName, info.SigningKey) + } return result, nil } diff --git a/verify/verify_test.go b/verify/verify_test.go index bfa13f3..99d90c1 100644 --- a/verify/verify_test.go +++ b/verify/verify_test.go @@ -557,13 +557,32 @@ func TestRealAttestationVerification(t *testing.T) { "https://kdsintf.amd.com/vcek/v1/Milan/3ac3fe21e13fb0990eb28a802e3fb6a29483a6b0753590c951bdd3b8e53786184ca39e359669a2b76a1936776b564ea464cdce40c05f63c9b610c5068b006b5d?blSPL=2&teeSPL=0&snpSPL=5&ucodeSPL=68": testdata.VcekBytes, }, ) - if err := RawSnpReport(testdata.AttestationBytes, &Options{ - Getter: getter, - Product: &pb.SevProduct{ - Name: pb.SevProduct_SEV_PRODUCT_MILAN, - MachineStepping: &wrapperspb.UInt32Value{Value: 0}, - }}); err != nil { - t.Error(err) + tcs := []struct { + name string + product *pb.SevProduct + wantErr string + }{ + { + name: "happy path", + product: &pb.SevProduct{ + Name: pb.SevProduct_SEV_PRODUCT_MILAN, + MachineStepping: &wrapperspb.UInt32Value{Value: 0}, + }, + }, + { + name: "bad vcek stepping", + product: &pb.SevProduct{ + Name: pb.SevProduct_SEV_PRODUCT_MILAN, + MachineStepping: &wrapperspb.UInt32Value{Value: 12}, + }, + wantErr: "expected product stepping 12, got 0", + }, + } + for _, tc := range tcs { + opts := &Options{Getter: getter, Product: tc.product} + if err := RawSnpReport(testdata.AttestationBytes, opts); !test.Match(err, tc.wantErr) { + t.Errorf("RawSnpReport(_, %+v) = %v errored unexpectedly. Want %q", opts, err, tc.wantErr) + } } } From 6f2d2fe4bc56a46a396ef3527b90eb07334d80cb Mon Sep 17 00:00:00 2001 From: Dionna Glaze Date: Thu, 26 Oct 2023 06:23:30 +0000 Subject: [PATCH 2/2] Fix SevProduct defaults for downstring testclient go-tpm-tools can't cleanly upgrade to v0.9.2 since the mocks.go Product returns Milan-B0 but the abi default is Milan-B1. This breaks existing tests. --- testing/mocks.go | 6 +----- testing/test_cases.go | 17 +++++++++++++---- verify/verify.go | 20 +++++++++++++------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/testing/mocks.go b/testing/mocks.go index 8b87ba7..cb40121 100644 --- a/testing/mocks.go +++ b/testing/mocks.go @@ -25,7 +25,6 @@ import ( spb "github.com/google/go-sev-guest/proto/sevsnp" "github.com/pkg/errors" "golang.org/x/sys/unix" - "google.golang.org/protobuf/types/known/wrapperspb" ) // GetReportResponse represents a mocked response to a command request. @@ -144,10 +143,7 @@ func (d *Device) Ioctl(command uintptr, req any) (uintptr, error) { // Product returns the mocked product info or the default. func (d *Device) Product() *spb.SevProduct { if d.SevProduct == nil { - return &spb.SevProduct{ - Name: spb.SevProduct_SEV_PRODUCT_MILAN, - MachineStepping: &wrapperspb.UInt32Value{Value: 0}, - } + return abi.DefaultSevProduct() } return d.SevProduct } diff --git a/testing/test_cases.go b/testing/test_cases.go index 56345eb..c883f2a 100644 --- a/testing/test_cases.go +++ b/testing/test_cases.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-sev-guest/abi" labi "github.com/google/go-sev-guest/client/linuxabi" "github.com/google/go-sev-guest/kds" + spb "github.com/google/go-sev-guest/proto/sevsnp" ) // userZeros defines a ReportData example that is all zeros @@ -151,15 +152,22 @@ func CreateRawReport(opts *TestReportOptions) [labi.SnpReportRespReportSize]byte // DeviceOptions specifies customizations for a fake sev-guest device. type DeviceOptions struct { - Keys map[string][]byte - Now time.Time - Signer *AmdSigner + Keys map[string][]byte + Now time.Time + Signer *AmdSigner + Product *spb.SevProduct } func makeTestCerts(opts *DeviceOptions) ([]byte, *AmdSigner, error) { signer := opts.Signer + var productString string + if opts.Product != nil { + productString = kds.ProductString(opts.Product) + } else { + productString = kds.DefaultProductString() + } if signer == nil { - s, err := DefaultTestOnlyCertChain(kds.DefaultProductString(), opts.Now) + s, err := DefaultTestOnlyCertChain(productString, opts.Now) if err != nil { return nil, nil, err } @@ -250,5 +258,6 @@ func TcDevice(tcs []TestCase, opts *DeviceOptions) (*Device, error) { Certs: certs, Signer: signer, Keys: opts.Keys, + SevProduct: opts.Product, }, nil } diff --git a/verify/verify.go b/verify/verify.go index f195932..a810ad3 100644 --- a/verify/verify.go +++ b/verify/verify.go @@ -687,17 +687,18 @@ func SnpAttestation(attestation *spb.Attestation, options *Options) error { // certificate chain. func fillInAttestation(attestation *spb.Attestation, options *Options) error { var productOverridden bool - if options.Product != nil { - attestation.Product = options.Product - productOverridden = true - } else if attestation.Product == nil { - attestation.Product = abi.DefaultSevProduct() + if attestation.Product == nil { + if options.Product != nil { + attestation.Product = options.Product + } else { + attestation.Product = abi.DefaultSevProduct() + } productOverridden = true } if options.DisableCertFetching { return nil } - product := kds.ProductString(options.Product) + product := kds.ProductString(attestation.Product) getter := options.Getter if getter == nil { getter = trust.DefaultHTTPSGetter() @@ -736,6 +737,8 @@ func fillInAttestation(attestation *spb.Attestation, options *Options) error { } } chain.VcekCert = vcek + // An attempt was made with defaults or the option's product, so now use + // the VCEK cert to determine the real product info. if productOverridden { cert, err := x509.ParseCertificate(vcek) if err != nil { @@ -758,7 +761,10 @@ func fillInAttestation(attestation *spb.Attestation, options *Options) error { return ErrMissingVlek } } - return nil + + // Pass along the expected product information for VcekDER. fillInAttestation will ensure + // that this is a noop if options.Product began as non-nil. + return updateProductExpectation(&options.Product, attestation.Product) } // GetAttestationFromReport uses AMD's Key Distribution Service (KDS) to download the certificate