Merge pull request #85 from deeglaze/stepping

Fix "modelstepping" handling in verification

Author: deeglaze

abi/abi.go

@@ -818,11 +818,12 @@ func SevProduct() *pb.SevProduct {
 	// 15:14 reserved
 	// 11:8 Family ID
 	family := (eax >> 8) & 0xf
-	// 7:4 Model, 3:0 Stepping
-	modelStepping := eax & 0xff
+	// 3:0 Stepping
+	stepping := eax & 0xf
 	// Ah, Fh, {0h,1h} values from the KDS specification,
 	// section "Determining the Product Name".
 	var productName pb.SevProduct_SevProductName
+	// Product information specified by processor programming reference publications.
 	if extendedFamily == 0xA && family == 0xF {
 		switch extendedModel {
 		case 0:
@@ -834,12 +835,12 @@ func SevProduct() *pb.SevProduct {
 		}
 	}
 	return &pb.SevProduct{
-		Name:          productName,
-		ModelStepping: modelStepping,
+		Name:     productName,
+		Stepping: stepping,
 	}
 }
 
 // DefaultSevProduct returns the initial product version for a commercially available AMD SEV-SNP chip.
 func DefaultSevProduct() *pb.SevProduct {
-	return &pb.SevProduct{Name: pb.SevProduct_SEV_PRODUCT_MILAN, ModelStepping: 0xB0}
+	return &pb.SevProduct{Name: pb.SevProduct_SEV_PRODUCT_MILAN, Stepping: 1}
 }

kds/kds.go

@@ -84,6 +84,20 @@ var (
 	kdsBaseURL  = "https://" + kdsHostname
 	kdsVcekPath = "/vcek/v1/"
 	kdsVlekPath = "/vlek/v1/"
+
+	// Chip manufacturers assign stepping versions strings that are <letter><number>
+	// to describe a stepping number for a particular model chip. There is no way
+	// other than documentation to map a stepping number to a stepping version and
+	// vice versa.
+	steppingDecoder = map[string]*pb.SevProduct{
+		"Milan-B0": {Name: pb.SevProduct_SEV_PRODUCT_MILAN, Stepping: 0},
+		"Milan-B1": {Name: pb.SevProduct_SEV_PRODUCT_MILAN, Stepping: 1},
+		"Genoa-B0": {Name: pb.SevProduct_SEV_PRODUCT_GENOA, Stepping: 0},
+		"Genoa-B1": {Name: pb.SevProduct_SEV_PRODUCT_GENOA, Stepping: 1},
+		"Genoa-B2": {Name: pb.SevProduct_SEV_PRODUCT_GENOA, Stepping: 2},
+	}
+	milanSteppingVersions = []string{"B0", "B1"}
+	genoaSteppingVersions = []string{"B0", "B1", "B2"}
 )
 
 // TCBVersion is a 64-bit bitfield of different security patch levels of AMD firmware and microcode.
@@ -692,45 +706,55 @@ func ProductName(product *pb.SevProduct) string {
 	if product == nil {
 		product = abi.DefaultSevProduct()
 	}
-	return fmt.Sprintf("%s-%02X", ProductString(product), product.ModelStepping)
+	if product.Stepping > 15 {
+		return "badstepping"
+	}
+	switch product.Name {
+	case pb.SevProduct_SEV_PRODUCT_MILAN:
+		if int(product.Stepping) >= len(milanSteppingVersions) {
+			return "unmappedMilanStepping"
+		}
+		return fmt.Sprintf("Milan-%s", milanSteppingVersions[product.Stepping])
+	case pb.SevProduct_SEV_PRODUCT_GENOA:
+		if int(product.Stepping) >= len(genoaSteppingVersions) {
+			return "unmappedGenoaStepping"
+		}
+		return fmt.Sprintf("Milan-%s", genoaSteppingVersions[product.Stepping])
+	default:
+		return "Unknown"
+	}
+}
+
+func parseProduct(product string) (pb.SevProduct_SevProductName, error) {
+	switch product {
+	case "Milan":
+		return pb.SevProduct_SEV_PRODUCT_MILAN, nil
+	case "Genoa":
+		return pb.SevProduct_SEV_PRODUCT_GENOA, nil
+	default:
+		return pb.SevProduct_SEV_PRODUCT_UNKNOWN, fmt.Errorf("unknown AMD SEV product: %q", product)
+	}
 }
 
 // ParseProductName returns the KDS project input value, and the model, stepping numbers represented
 // by a given V[CL]EK productName extension value, or an error.
 func ParseProductName(productName string, key abi.ReportSigner) (*pb.SevProduct, error) {
-	var product, stepping string
-	var needStepping bool
 	switch key {
 	case abi.VcekReportSigner:
-		subs := strings.SplitN(productName, "-", 2)
-		if len(subs) != 2 {
-			return nil, fmt.Errorf("productName value %q does not match the VCEK expected Name-ModelStepping format", productName)
+		product, ok := steppingDecoder[productName]
+		if !ok {
+			return nil, fmt.Errorf("unknown product name (new stepping published?): %q", productName)
 		}
-		product = subs[0]
-		stepping = subs[1]
-		needStepping = true
+		return product, nil
 	case abi.VlekReportSigner:
 		// VLEK certificates don't carry the stepping value in productName.
-		product = productName
-	}
-	var name pb.SevProduct_SevProductName
-	switch product {
-	case "Milan":
-		name = pb.SevProduct_SEV_PRODUCT_MILAN
-	case "Genoa":
-		name = pb.SevProduct_SEV_PRODUCT_GENOA
-	default:
-		return nil, fmt.Errorf("unknown AMD SEV product: %q", product)
-	}
-	var modelStepping uint64
-	if needStepping {
-		var err error
-		modelStepping, err = strconv.ParseUint(stepping, 16, 8)
+		name, err := parseProduct(productName)
 		if err != nil {
-			return nil, fmt.Errorf("model stepping in productName is not a hexadecimal byte: %q", stepping)
+			return nil, err
 		}
+		return &pb.SevProduct{Name: name}, nil
 	}
-	return &pb.SevProduct{Name: name, ModelStepping: uint32(modelStepping)}, nil
+	return nil, fmt.Errorf("internal: unhandled reportSigner %v", key)
 }
 
 // CrlLinkByKey returns the CRL distribution point for the given key type's

kds/kds_test.go

@@ -185,29 +185,29 @@ func TestProductName(t *testing.T) {
 	}{
 		{
 			name: "nil",
-			want: "Milan-B0",
+			want: "Milan-B1",
 		},
 		{
 			name: "unknown",
 			input: &pb.SevProduct{
-				ModelStepping: 0x1A,
+				Stepping: 0x1A,
 			},
-			want: "Unknown-1A",
+			want: "badstepping",
 		},
 		{
-			name: "Milan-00",
+			name: "Milan-B0",
 			input: &pb.SevProduct{
 				Name: pb.SevProduct_SEV_PRODUCT_MILAN,
 			},
-			want: "Milan-00",
+			want: "Milan-B0",
 		},
 		{
 			name: "Genoa-FF",
 			input: &pb.SevProduct{
-				Name:          pb.SevProduct_SEV_PRODUCT_GENOA,
-				ModelStepping: 0xFF,
+				Name:     pb.SevProduct_SEV_PRODUCT_GENOA,
+				Stepping: 0xFF,
 			},
-			want: "Genoa-FF",
+			want: "badstepping",
 		},
 	}
 	for _, tc := range tcs {
@@ -229,36 +229,26 @@ func TestParseProductName(t *testing.T) {
 	}{
 		{
 			name:    "empty",
-			wantErr: "does not match",
-		},
-		{
-			name:    "Too much",
-			input:   "Milan-B0-and some extra",
-			wantErr: "not a hexadecimal byte: \"B0-and some extra\"",
-		},
-		{
-			name:    "start-",
-			input:   "-00",
-			wantErr: "unknown AMD SEV product: \"\"",
-		},
-		{
-			name:    "end-",
-			input:   "Milan-",
-			wantErr: "model stepping in productName is not a hexadecimal byte: \"\"",
+			wantErr: "unknown product name",
 		},
 		{
 			name:    "Too big",
 			input:   "Milan-100",
-			wantErr: "model stepping in productName is not a hexadecimal byte: \"100\"",
+			wantErr: "unknown product name",
 		},
 		{
-			name:  "happy path",
-			input: "Genoa-9C",
+			name:  "happy path Genoa",
+			input: "Genoa-B1",
 			want: &pb.SevProduct{
-				Name:          pb.SevProduct_SEV_PRODUCT_GENOA,
-				ModelStepping: 0x9C,
+				Name:     pb.SevProduct_SEV_PRODUCT_GENOA,
+				Stepping: 1,
 			},
 		},
+		{
+			name:    "bad revision Milan",
+			input:   "Milan-A1",
+			wantErr: "unknown product name",
+		},
 		{
 			name:  "vlek products have no stepping",
 			input: "Genoa",

proto/sevsnp.proto

@@ -90,7 +90,7 @@ message SevProduct {
   }
 
   SevProductName name = 1;
-  uint32 model_stepping = 2;  // Must be a byte
+  uint32 stepping = 2;  // Must be a 4-bit number
 }
 
 message Attestation {

proto/sevsnp/sevsnp.pb.go

@@ -144,8 +144,8 @@ func (d *Device) Ioctl(command uintptr, req any) (uintptr, error) {
 func (d *Device) Product() *spb.SevProduct {
 	if d.SevProduct == nil {
 		return &spb.SevProduct{
-			Name:          spb.SevProduct_SEV_PRODUCT_MILAN,
-			ModelStepping: 0xB0,
+			Name:     spb.SevProduct_SEV_PRODUCT_MILAN,
+			Stepping: 0, // Milan-B0 is the product name we fake.
 		}
 	}
 	return d.SevProduct

testing/mocks.go

@@ -451,9 +451,9 @@ func checkProductName(got, want *spb.SevProduct, key abi.ReportSigner) error {
 		return fmt.Errorf("%v cert product name %v is not %v", key, got, want)
 	}
 	// The model stepping number is only part of the VLEK product name, not VLEK's.
-	if key == abi.VcekReportSigner && got.ModelStepping != want.ModelStepping {
-		return fmt.Errorf("%v cert product model-stepping number %02X is not %02X",
-			key, got.ModelStepping, want.ModelStepping)
+	if key == abi.VcekReportSigner && got.Stepping != want.Stepping {
+		return fmt.Errorf("%v cert product stepping number %02X is not %02X",
+			key, got.Stepping, want.Stepping)
 	}
 	return nil
 }
@@ -483,6 +483,7 @@ func decodeCerts(chain *spb.CertificateChain, key abi.ReportSigner, options *Opt
 	if err != nil {
 		return nil, nil, err
 	}
+	fmt.Println("product", exts.ProductName, product)
 	productName := kds.ProductString(product)
 	// Ensure the extension product info matches expectations.
 	if err := checkProductName(product, options.Product, key); err != nil {
@@ -650,15 +651,13 @@ func SnpAttestation(attestation *spb.Attestation, options *Options) error {
 // fillInAttestation uses AMD's KDS to populate any empty certificate field in the attestation's
 // certificate chain.
 func fillInAttestation(attestation *spb.Attestation, options *Options) error {
+	var productOverridden bool
 	if options.Product != nil {
 		attestation.Product = options.Product
-	}
-	if attestation.Product == nil {
-		// The default product is the first launched SEV-SNP product value.
-		attestation.Product = &spb.SevProduct{
-			Name:          spb.SevProduct_SEV_PRODUCT_MILAN,
-			ModelStepping: 0xB0,
-		}
+		productOverridden = true
+	} else if attestation.Product == nil {
+		attestation.Product = abi.DefaultSevProduct()
+		productOverridden = true
 	}
 	if options.DisableCertFetching {
 		return nil
@@ -702,6 +701,21 @@ func fillInAttestation(attestation *spb.Attestation, options *Options) error {
 				}
 			}
 			chain.VcekCert = vcek
+			if productOverridden {
+				cert, err := x509.ParseCertificate(vcek)
+				if err != nil {
+					return err
+				}
+				exts, err := kds.VcekCertificateExtensions(cert)
+				if err != nil {
+					return err
+				}
+				attestation.Product, err = kds.ParseProductName(exts.ProductName, abi.VcekReportSigner)
+				fmt.Printf("filled in product with %v\n", attestation.Product)
+				if err != nil {
+					return err
+				}
+			}
 		}
 	case abi.VlekReportSigner:
 		// We can't lazily ask KDS for the certificate as a user. The CSP must cache their provisioned