snp: ensure we never use ARK supplied by Issuer

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

@@ -24,7 +24,6 @@ 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",
     ],
@@ -45,6 +44,5 @@ go_test(
         "//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{

internal/attestation/gcp/snp/validator_test.go

@@ -7,20 +7,16 @@ SPDX-License-Identifier: AGPL-3.0-only
 package snp
 
 import (
-	"bytes"
 	"context"
 	"crypto"
 	"crypto/x509"
-	"encoding/hex"
-	"fmt"
 	"testing"
 
 	"github.com/edgelesssys/constellation/v2/internal/attestation"
 	"github.com/edgelesssys/constellation/v2/internal/attestation/vtpm"
 	"github.com/edgelesssys/constellation/v2/internal/config"
 	"github.com/google/go-tpm-tools/proto/attest"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestGetTrustedKey(t *testing.T) {
@@ -64,58 +60,6 @@ func TestGetTrustedKey(t *testing.T) {
 	}
 }
 
-func TestSha512sum(t *testing.T) {
-	testCases := map[string]struct {
-		key   string
-		hash  string
-		match bool
-	}{
-		"success": {
-			// Generated using: rsa.GenerateKey(rand.Reader, 1024).
-			key:   "30819f300d06092a864886f70d010101050003818d0030818902818100d4b2f072a32fa98456eb7f5938e2ff361fb64d698ea91e003d34bfc5374b814c16ba9ae3ec392ef6d48cf79b63067e338aa941219a7bcdf18aa43cd38bbe5567504838a3b1dca482035458853c5a171709dfae9df551815010bdfbc6df733cde84c4f7a5b0591d9cda9db087fb411ee3e2a4f19ad50c8331712ecdc5dd7ce34b0203010001",
-			hash:  "2d6fe5ec59d7240b8a4c27c2ff27ba1071105fa50d45543768fcbabf9ee3cb8f8fa0afa51e08e053af30f6d11066ebfd47e75bda5ccc085c115d7e1896f3c62f",
-			match: true,
-		},
-		"mismatching hash": {
-			key:   "30819f300d06092a864886f70d010101050003818d0030818902818100d4b2f072a32fa98456eb7f5938e2ff361fb64d698ea91e003d34bfc5374b814c16ba9ae3ec392ef6d48cf79b63067e338aa941219a7bcdf18aa43cd38bbe5567504838a3b1dca482035458853c5a171709dfae9df551815010bdfbc6df733cde84c4f7a5b0591d9cda9db087fb411ee3e2a4f19ad50c8331712ecdc5dd7ce34b0203010001",
-			hash:  "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
-			match: false,
-		},
-	}
-
-	for name, tc := range testCases {
-		t.Run(name, func(t *testing.T) {
-			assert := assert.New(t)
-			require := require.New(t)
-
-			newKey, err := loadKeyFromHex(tc.key)
-			require.NoError(err)
-
-			// Function under test:
-			hash, err := sha512sum(newKey)
-			assert.NoError(err)
-
-			expected, err := hex.DecodeString(tc.hash)
-			require.NoError(err)
-
-			if tc.match {
-				assert.True(bytes.Equal(expected, hash[:]), fmt.Sprintf("expected hash %x, got %x", expected, hash))
-			} else {
-				assert.False(bytes.Equal(expected, hash[:]), fmt.Sprintf("expected mismatching hashes, got %x", hash))
-			}
-		})
-	}
-}
-
-func loadKeyFromHex(key string) (crypto.PublicKey, error) {
-	decoded, err := hex.DecodeString(key)
-	if err != nil {
-		return nil, err
-	}
-
-	return x509.ParsePKIXPublicKey(decoded)
-}
-
 type stubGCPValidator struct{}
 
 func (stubGCPValidator) validate(_ vtpm.AttestationDocument, _ *x509.Certificate, _ *x509.Certificate, _ [64]byte, _ *config.GCPSEVSNP, _ attestation.Logger) error {

internal/attestation/snp/BUILD.bazel

@@ -8,7 +8,6 @@ go_library(
     visibility = ["//:__subpackages__"],
     deps = [
         "//internal/attestation",
-        "//internal/constants",
         "@com_github_google_go_sev_guest//abi",
         "@com_github_google_go_sev_guest//kds",
         "@com_github_google_go_sev_guest//proto/sevsnp",