Fix check tool product behavior on hardware

The product and stepping values that are default may not be the same across all testing environments, so add product_name as a flag to testing library. The stepping value does not necessarily need to be checked, so deprecate the stepping field of SevProduct and replace with a UInt32Value that can be nil. The checkProductName drops checking Stepping in favor of checking the nullable MachineStepping field. Signed-off-by: Dionna Glaze <[email protected]>
google · Oct 19, 2023 · 5307b18 · 5307b18
1 parent 1265664
commit 5307b18
Show file tree

Hide file tree

Showing 13 changed files with 330 additions and 175 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -55,6 +55,8 @@ jobs:
         run: go build -v ./...
       - name: Test all packages
         run: go test -v ./...
+      - name: Run Go Vet
+        run: go vet ./...
 
   lint:
     strategy:
@@ -70,7 +72,7 @@ jobs:
         with:
           go-version: ${{ matrix.go-version }}
       - name: Run golangci-lint
-        uses: golangci/golangci-lint-action@v3.3.0
+        uses: golangci/golangci-lint-action@v3.6.0
         with:
           version: latest
           working-directory: ./
@@ -82,6 +84,7 @@ jobs:
             -E revive
             -E gofmt
             -E goimports
+            --out-format=colored-line-number
             --exclude-use-default=false
             --max-same-issues=0
             --max-issues-per-linter=0

diff --git a/kds/kds.go b/kds/kds.go
@@ -700,32 +700,43 @@ func ProductString(product *pb.SevProduct) string {
 	}
 }
 
+// DefaultProductString returns the product string of the default SEV product.
+func DefaultProductString() string {
+	return ProductString(abi.DefaultSevProduct())
+}
+
 // ProductName returns the expected productName extension value for the product associated
 // with an attestation report proto.
 func ProductName(product *pb.SevProduct) string {
 	if product == nil {
 		product = abi.DefaultSevProduct()
 	}
-	if product.Stepping > 15 {
+	// Can't produce a product name without a stepping value.
+	if product.MachineStepping == nil {
+		return "Unknown"
+	}
+	stepping := product.MachineStepping.Value
+	if stepping > 15 {
 		return "badstepping"
 	}
 	switch product.Name {
 	case pb.SevProduct_SEV_PRODUCT_MILAN:
-		if int(product.Stepping) >= len(milanSteppingVersions) {
+		if int(stepping) >= len(milanSteppingVersions) {
 			return "unmappedMilanStepping"
 		}
-		return fmt.Sprintf("Milan-%s", milanSteppingVersions[product.Stepping])
+		return fmt.Sprintf("Milan-%s", milanSteppingVersions[stepping])
 	case pb.SevProduct_SEV_PRODUCT_GENOA:
-		if int(product.Stepping) >= len(genoaSteppingVersions) {
+		if int(stepping) >= len(genoaSteppingVersions) {
 			return "unmappedGenoaStepping"
 		}
-		return fmt.Sprintf("Milan-%s", genoaSteppingVersions[product.Stepping])
+		return fmt.Sprintf("Milan-%s", genoaSteppingVersions[stepping])
 	default:
 		return "Unknown"
 	}
 }
 
-func parseProduct(product string) (pb.SevProduct_SevProductName, error) {
+// ParseProduct returns the SevProductName for a product name without the stepping suffix.
+func ParseProduct(product string) (pb.SevProduct_SevProductName, error) {
 	switch product {
 	case "Milan":
 		return pb.SevProduct_SEV_PRODUCT_MILAN, nil
@@ -748,7 +759,7 @@ func ParseProductName(productName string, key abi.ReportSigner) (*pb.SevProduct,
 		return product, nil
 	case abi.VlekReportSigner:
 		// VLEK certificates don't carry the stepping value in productName.
-		name, err := parseProduct(productName)
+		name, err := ParseProduct(productName)
 		if err != nil {
 			return nil, err
 		}

diff --git a/proto/check.proto b/proto/check.proto
@@ -18,6 +18,7 @@ syntax = "proto3";
 package check;
 
 import "google/protobuf/wrappers.proto";
+import "sevsnp.proto";
 
 option go_package = "github.com/google/go-sev-guest/proto/check";
 
@@ -50,6 +51,8 @@ message Policy {
   repeated bytes trusted_author_key_hashes = 21;
   repeated bytes trusted_id_keys = 22;
   repeated bytes trusted_id_key_hashes = 23;
+  // The expected product that generated the attestation report. Stepping optional.
+  sevsnp.SevProduct product = 24;
 }
 
 // RootOfTrust represents configuration for which hardware root of trust

diff --git a/proto/check/check.pb.go b/proto/check/check.pb.go
diff --git a/proto/sevsnp.proto b/proto/sevsnp.proto
@@ -14,6 +14,8 @@
 
 syntax = "proto3";
 
+import "google/protobuf/wrappers.proto";
+
 // Package sevsnp represents an SEV-SNP attestation report and its certificate
 // chain.
 package sevsnp;
@@ -90,7 +92,8 @@ message SevProduct {
   }
 
   SevProductName name = 1;
-  uint32 stepping = 2;  // Must be a 4-bit number
+  uint32 stepping = 2 [deprecated = true];  // Must be a 4-bit number
+  google.protobuf.UInt32Value machine_stepping = 3;
 }
 
 message Attestation {

diff --git a/proto/sevsnp/sevsnp.pb.go b/proto/sevsnp/sevsnp.pb.go
diff --git a/testing/fake_certs.go b/testing/fake_certs.go
@@ -23,8 +23,10 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
+	"flag"
 	"fmt"
 	"math/big"
+	"strings"
 
 	// Insecure randomness for faster testing.
 	"math/rand"
@@ -47,6 +49,19 @@ const (
 	askRsaBits          = 4096
 )
 
+// ProductName decides the fake certificates' product name. It must be parsable by
+// kds.ParseProductName. The flag may also be used to direct the hardware verification options.
+var ProductName = flag.String("product_name", "",
+	"The product name for the SEV-SNP machine tested on.")
+
+// GetProductName returns the --product_name flag value or a valid Default.
+func GetProductName() string {
+	if *ProductName == "" {
+		return kds.ProductName(abi.DefaultSevProduct())
+	}
+	return *ProductName
+}
+
 // AmdSigner encapsulates a key and certificate chain following the format of AMD-SP's VCEK for
 // signing attestation reports.
 type AmdSigner struct {
@@ -353,10 +368,11 @@ func CustomExtensions(tcb kds.TCBParts, hwid []byte, cspid string) []pkix.Extens
 	var productName []byte
 	asn1Zero, _ := asn1.Marshal(0)
 	if hwid != nil {
-		productName, _ = asn1.MarshalWithParams("Milan-B0", "ia5")
+		productName, _ = asn1.MarshalWithParams(GetProductName(), "ia5")
 	} else {
+		parts := strings.SplitN(GetProductName(), "-", 2)
 		// VLEK doesn't have a -stepping component to its productName.
-		productName, _ = asn1.MarshalWithParams("Milan", "ia5")
+		productName, _ = asn1.MarshalWithParams(parts[0], "ia5")
 	}
 	blSpl, _ := asn1.Marshal(int(tcb.BlSpl))
 	teeSpl, _ := asn1.Marshal(int(tcb.TeeSpl))
@@ -440,7 +456,7 @@ func (b *AmdSignerBuilder) certifyVlek() error {
 // TestOnlyCertChain creates a test-only certificate chain from the keys and configurables in b.
 func (b *AmdSignerBuilder) TestOnlyCertChain() (*AmdSigner, error) {
 	if b.Product == "" {
-		b.Product = "Milan" // For terse tests.
+		b.Product = kds.DefaultProductString()
 	}
 	if b.Keys == nil {
 		keys, err := DefaultAmdKeys()

diff --git a/testing/test_cases.go b/testing/test_cases.go
@@ -22,6 +22,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"
 )
 
 // userZeros defines a ReportData example that is all zeros
@@ -158,7 +159,7 @@ type DeviceOptions struct {
 func makeTestCerts(opts *DeviceOptions) ([]byte, *AmdSigner, error) {
 	signer := opts.Signer
 	if signer == nil {
-		s, err := DefaultTestOnlyCertChain("Milan", opts.Now)
+		s, err := DefaultTestOnlyCertChain(kds.DefaultProductString(), opts.Now)
 		if err != nil {
 			return nil, nil, err
 		}