snp: ensure we never use ARK supplied by Issuer (#3025)

* Make sure SNP ARK is always loaded from config, or fetched from AMD KDS
* GCP: Set validator `reportData` correctly

---------

Signed-off-by: Daniel Weiße <dw@edgeless.systems>
Co-authored-by: Moritz Sanft <58110325+msanft@users.noreply.github.com>

Author: daniel-weisse

internal/attestation/aws/snp/validator.go

@@ -191,11 +191,11 @@ func (a *awsValidator) validate(attestation vtpm.AttestationDocument, ask *x509.
 func getVerifyOpts(att *sevsnp.Attestation) (*verify.Options, error) {
 	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert)
 	if err != nil {
-		return &verify.Options{}, fmt.Errorf("parsing VLEK certificate: %w", err)
+		return nil, fmt.Errorf("parsing ASK certificate: %w", err)
 	}
 	ark, err := x509.ParseCertificate(att.CertificateChain.ArkCert)
 	if err != nil {
-		return &verify.Options{}, fmt.Errorf("parsing VLEK certificate: %w", err)
+		return nil, fmt.Errorf("parsing ARK certificate: %w", err)
 	}
 
 	verifyOpts := &verify.Options{

internal/attestation/azure/snp/validator.go

@@ -116,25 +116,11 @@ func (v *Validator) getTrustedKey(ctx context.Context, attDoc vtpm.AttestationDo
 		return nil, fmt.Errorf("parsing attestation report: %w", err)
 	}
 
-	// ASK, as cached in joinservice or reported from THIM / KDS.
-	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert)
+	verifyOpts, err := getVerifyOpts(att)
 	if err != nil {
-		return nil, fmt.Errorf("parsing ASK certificate: %w", err)
+		return nil, fmt.Errorf("getting verify options: %w", err)
 	}
 
-	verifyOpts := &verify.Options{
-		TrustedRoots: map[string][]*trust.AMDRootCerts{
-			"Milan": {
-				{
-					Product: "Milan",
-					ProductCerts: &trust.ProductCerts{
-						Ask: ask,
-						Ark: trustedArk,
-					},
-				},
-			},
-		},
-	}
 	if err := v.attestationVerifier.SNPAttestation(att, verifyOpts); err != nil {
 		return nil, fmt.Errorf("verifying SNP attestation: %w", err)
 	}
@@ -252,3 +238,31 @@ type maaValidator interface {
 type hclAkValidator interface {
 	Validate(runtimeDataRaw []byte, reportData []byte, rsaParameters *tpm2.RSAParams) error
 }
+
+func getVerifyOpts(att *spb.Attestation) (*verify.Options, error) {
+	// ASK, as cached in joinservice or reported from THIM / KDS.
+	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert)
+	if err != nil {
+		return nil, fmt.Errorf("parsing ASK certificate: %w", err)
+	}
+	ark, err := x509.ParseCertificate(att.CertificateChain.ArkCert)
+	if err != nil {
+		return nil, fmt.Errorf("parsing ARK certificate: %w", err)
+	}
+
+	verifyOpts := &verify.Options{
+		TrustedRoots: map[string][]*trust.AMDRootCerts{
+			"Milan": {
+				{
+					Product: "Milan",
+					ProductCerts: &trust.ProductCerts{
+						Ask: ask,
+						Ark: ark,
+					},
+				},
+			},
+		},
+	}
+
+	return verifyOpts, nil
+}

internal/attestation/gcp/snp/BUILD.bazel

@@ -1,5 +1,4 @@
 load("@io_bazel_rules_go//go:def.bzl", "go_library")
-load("//bazel/go:go_test.bzl", "go_test")
 
 go_library(
     name = "snp",
@@ -24,27 +23,7 @@ go_library(
         "@com_github_google_go_sev_guest//validate",
         "@com_github_google_go_sev_guest//verify",
         "@com_github_google_go_sev_guest//verify/trust",
-        "@com_github_google_go_tpm//legacy/tpm2",
         "@com_github_google_go_tpm_tools//client",
         "@com_github_google_go_tpm_tools//proto/attest",
     ],
 )
-
-go_test(
-    name = "snp_test",
-    srcs = ["validator_test.go"],
-    embed = [":snp"],
-    # keep
-    gotags = select({
-        "//bazel/settings:tpm_simulator_enabled": [],
-        "//conditions:default": ["disable_tpm_simulator"],
-    }),
-    deps = [
-        "//internal/attestation",
-        "//internal/attestation/vtpm",
-        "//internal/config",
-        "@com_github_google_go_tpm_tools//proto/attest",
-        "@com_github_stretchr_testify//assert",
-        "@com_github_stretchr_testify//require",
-    ],
-)

internal/attestation/gcp/snp/issuer.go

@@ -58,7 +58,7 @@ func getAttestationKey(tpm io.ReadWriter) (*tpmclient.Key, error) {
 // getInstanceInfo generates an extended SNP report, i.e. the report and any loaded certificates.
 // Report generation is triggered by sending ioctl syscalls to the SNP guest device, the AMD PSP generates the report.
 // The returned bytes will be written into the attestation document.
-func getInstanceInfo(_ context.Context, tpm io.ReadWriteCloser, extraData []byte) ([]byte, error) {
+func getInstanceInfo(_ context.Context, _ io.ReadWriteCloser, extraData []byte) ([]byte, error) {
 	if len(extraData) > 64 {
 		return nil, fmt.Errorf("extra data too long: %d, should be 64 bytes at most", len(extraData))
 	}
@@ -76,7 +76,7 @@ func getInstanceInfo(_ context.Context, tpm io.ReadWriteCloser, extraData []byte
 		return nil, fmt.Errorf("getting extended report: %w", err)
 	}
 
-	vcek, err := pemEncodedVCEK(certs)
+	vcek, certChain, err := parseSNPCertTable(certs)
 	if err != nil {
 		return nil, fmt.Errorf("parsing vcek: %w", err)
 	}
@@ -89,6 +89,7 @@ func getInstanceInfo(_ context.Context, tpm io.ReadWriteCloser, extraData []byte
 	raw, err := json.Marshal(snp.InstanceInfo{
 		AttestationReport: report,
 		ReportSigner:      vcek,
+		CertChain:         certChain,
 		GCP:               gceInstanceInfo,
 	})
 	if err != nil {
@@ -124,30 +125,44 @@ func gceInstanceInfo() (*attest.GCEInstanceInfo, error) {
 	}, nil
 }
 
-// pemEncodedVCEK takes a marshalled SNP certificate table and returns the PEM-encoded VCEK certificate.
+// parseSNPCertTable takes a marshalled SNP certificate table and returns the PEM-encoded VCEK certificate and,
+// if present, the ASK of the SNP certificate chain.
 // AMD documentation on certificate tables can be found in section 4.1.8.1, revision 2.03 "SEV-ES Guest-Hypervisor Communication Block Standardization".
 // https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
-func pemEncodedVCEK(certs []byte) ([]byte, error) {
+func parseSNPCertTable(certs []byte) (vcekPEM []byte, certChain []byte, err error) {
 	certTable := abi.CertTable{}
 	if err := certTable.Unmarshal(certs); err != nil {
-		return nil, fmt.Errorf("unmarshalling SNP certificate table: %w", err)
+		return nil, nil, fmt.Errorf("unmarshalling SNP certificate table: %w", err)
 	}
 
 	vcekRaw, err := certTable.GetByGUIDString(abi.VcekGUID)
 	if err != nil {
-		return nil, fmt.Errorf("getting VCEK certificate: %w", err)
+		return nil, nil, fmt.Errorf("getting VCEK certificate: %w", err)
 	}
 
 	// An optional check for certificate well-formedness. vcekRaw == cert.Raw.
-	cert, err := x509.ParseCertificate(vcekRaw)
+	vcek, err := x509.ParseCertificate(vcekRaw)
 	if err != nil {
-		return nil, fmt.Errorf("parsing certificate: %w", err)
+		return nil, nil, fmt.Errorf("parsing certificate: %w", err)
 	}
 
-	certPEM := pem.EncodeToMemory(&pem.Block{
+	vcekPEM = pem.EncodeToMemory(&pem.Block{
 		Type:  "CERTIFICATE",
-		Bytes: cert.Raw,
+		Bytes: vcek.Raw,
 	})
 
-	return certPEM, nil
+	var askPEM []byte
+	if askRaw, err := certTable.GetByGUIDString(abi.AskGUID); err == nil {
+		ask, err := x509.ParseCertificate(askRaw)
+		if err != nil {
+			return nil, nil, fmt.Errorf("parsing ASK certificate: %w", err)
+		}
+
+		askPEM = pem.EncodeToMemory(&pem.Block{
+			Type:  "CERTIFICATE",
+			Bytes: ask.Raw,
+		})
+	}
+
+	return vcekPEM, askPEM, nil
 }

internal/attestation/gcp/snp/validator.go

@@ -9,7 +9,6 @@ package snp
 import (
 	"context"
 	"crypto"
-	"crypto/sha512"
 	"crypto/x509"
 	"encoding/json"
 	"fmt"
@@ -27,7 +26,6 @@ import (
 	"github.com/google/go-sev-guest/verify"
 	"github.com/google/go-sev-guest/verify/trust"
 	"github.com/google/go-tpm-tools/proto/attest"
-	"github.com/google/go-tpm/legacy/tpm2"
 )
 
 // Validator for GCP SEV-SNP / TPM attestation.
@@ -62,53 +60,30 @@ func NewValidator(cfg *config.GCPSEVSNP, log attestation.Logger) (*Validator, er
 	v.Validator = vtpm.NewValidator(
 		cfg.Measurements,
 		v.getTrustedKey,
-		v.validateCVM,
+		func(_ vtpm.AttestationDocument, _ *attest.MachineState) error { return nil },
 		log,
 	)
 	return v, nil
 }
 
 // getTrustedKey returns TPM endorsement key provided through the GCE metadata API.
-func (v *Validator) getTrustedKey(ctx context.Context, attDoc vtpm.AttestationDocument, _ []byte) (crypto.PublicKey, error) {
+func (v *Validator) getTrustedKey(ctx context.Context, attDoc vtpm.AttestationDocument, extraData []byte) (crypto.PublicKey, error) {
 	ekPub, err := v.gceKeyGetter(ctx, attDoc, nil)
 	if err != nil {
 		return nil, fmt.Errorf("getting TPM endorsement key: %w", err)
 	}
 
-	return ekPub, nil
-}
-
-// validateCVM validates the SEV-SNP attestation document.
-func (v *Validator) validateCVM(attDoc vtpm.AttestationDocument, _ *attest.MachineState) error {
-	pubArea, err := tpm2.DecodePublic(attDoc.Attestation.AkPub)
-	if err != nil {
-		return fmt.Errorf("decoding public area: %w", err)
-	}
-
-	pubKey, err := pubArea.Key()
-	if err != nil {
-		return fmt.Errorf("getting public key: %w", err)
-	}
-
-	akDigest, err := sha512sum(pubKey)
-	if err != nil {
-		return fmt.Errorf("calculating hash of attestation key: %w", err)
-	}
-
-	if err := v.reportValidator.validate(attDoc, (*x509.Certificate)(&v.cfg.AMDSigningKey), (*x509.Certificate)(&v.cfg.AMDRootKey), akDigest, v.cfg, v.log); err != nil {
-		return fmt.Errorf("validating SNP report: %w", err)
+	if len(extraData) > 64 {
+		return nil, fmt.Errorf("extra data too long: %d, should be 64 bytes at most", len(extraData))
 	}
-	return nil
-}
+	extraData64 := make([]byte, 64)
+	copy(extraData64, extraData)
 
-// sha512sum PEM-encodes a public key and calculates the SHA512 hash of the encoded key.
-func sha512sum(key crypto.PublicKey) ([64]byte, error) {
-	pub, err := x509.MarshalPKIXPublicKey(key)
-	if err != nil {
-		return [64]byte{}, fmt.Errorf("marshalling public key: %w", err)
+	if err := v.reportValidator.validate(attDoc, (*x509.Certificate)(&v.cfg.AMDSigningKey), (*x509.Certificate)(&v.cfg.AMDRootKey), [64]byte(extraData64), v.cfg, v.log); err != nil {
+		return nil, fmt.Errorf("validating SNP report: %w", err)
 	}
 
-	return sha512.Sum512(pub), nil
+	return ekPub, nil
 }
 
 // snpReportValidator validates a given SNP report.
@@ -146,7 +121,7 @@ func (r *reportVerifierImpl) SnpAttestation(att *sevsnp.Attestation, opts *verif
 // validate the report by checking if it has a valid VCEK signature.
 // The certificate chain ARK -> ASK -> VCEK is also validated.
 // Checks that the report's userData matches the connection's userData.
-func (a *gcpValidator) validate(attestation vtpm.AttestationDocument, ask *x509.Certificate, ark *x509.Certificate, akDigest [64]byte, config *config.GCPSEVSNP, log attestation.Logger) error {
+func (a *gcpValidator) validate(attestation vtpm.AttestationDocument, ask *x509.Certificate, ark *x509.Certificate, reportData [64]byte, config *config.GCPSEVSNP, log attestation.Logger) error {
 	var info snp.InstanceInfo
 	if err := json.Unmarshal(attestation.InstanceInfo, &info); err != nil {
 		return fmt.Errorf("unmarshalling instance info: %w", err)
@@ -170,7 +145,7 @@ func (a *gcpValidator) validate(attestation vtpm.AttestationDocument, ask *x509.
 
 	validateOpts := &validate.Options{
 		// Check that the attestation key's digest is included in the report.
-		ReportData: akDigest[:],
+		ReportData: reportData[:],
 		GuestPolicy: abi.SnpPolicy{
 			Debug: false, // Debug means the VM can be decrypted by the host for debugging purposes and thus is not allowed.
 			SMT:   true,  // Allow Simultaneous Multi-Threading (SMT). Normally, we would want to disable SMT
@@ -205,11 +180,11 @@ func (a *gcpValidator) validate(attestation vtpm.AttestationDocument, ask *x509.
 func getVerifyOpts(att *sevsnp.Attestation) (*verify.Options, error) {
 	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert)
 	if err != nil {
-		return &verify.Options{}, fmt.Errorf("parsing ASK certificate: %w", err)
+		return nil, fmt.Errorf("parsing ASK certificate: %w", err)
 	}
 	ark, err := x509.ParseCertificate(att.CertificateChain.ArkCert)
 	if err != nil {
-		return &verify.Options{}, fmt.Errorf("parsing ARK certificate: %w", err)
+		return nil, fmt.Errorf("parsing ARK certificate: %w", err)
 	}
 
 	verifyOpts := &verify.Options{