Revert "Add HashNonce flag to Attest and VerifyAttestation (google#585)" (google#601)

This reverts commit 215e2ab.

Author: alexmwu

client/attest.go

@@ -59,11 +59,6 @@ type AttestOpts struct {
 	// depending on the technology's size. Leaving this nil is not recommended. If
 	// nil, then TEEDevice must be nil.
 	TEENonce []byte
-	// HashNonce will apply the attestation key's signing scheme hash algorithm
-	// to the input Nonce field and use the resulting digest in place of the
-	// original Nonce.
-	// Nonce must still be unique and application-specific.
-	HashNonce bool
 }
 
 // SevSnpQuoteProvider encapsulates the SEV-SNP attestation device to add its attestation report
@@ -291,16 +286,8 @@ func (k *Key) Attest(opts AttestOpts) (*pb.Attestation, error) {
 		return nil, fmt.Errorf("failed to encode public area: %w", err)
 	}
 	attestation.AkCert = k.CertDERBytes()
-	extraData := opts.Nonce
-	if opts.HashNonce {
-		var err error
-		extraData, err = internal.HashNonce(k.PublicArea(), extraData)
-		if err != nil {
-			return nil, fmt.Errorf("failed to hash the input nonce: %w", err)
-		}
-	}
 	for _, sel := range sels {
-		quote, err := k.Quote(sel, extraData)
+		quote, err := k.Quote(sel, opts.Nonce)
 		if err != nil {
 			return nil, err
 		}

internal/quote.go

@@ -133,20 +133,3 @@ func validatePCRDigest(quoteInfo *tpm2.QuoteInfo, pcrs *pb.PCRs, hash crypto.Has
 	}
 	return nil
 }
-
-// HashNonce takes an arbitrary-sized nonce and ensures it can fit in the TPM's
-// extraData field by applying the given key's signing hash algorithm to the
-// nonce.
-func HashNonce(pubArea tpm2.Public, nonce []byte) ([]byte, error) {
-	sigHash, err := GetSigningHashAlg(pubArea)
-	if err != nil {
-		return nil, err
-	}
-	chash, err := sigHash.Hash()
-	if err != nil {
-		return nil, err
-	}
-	hasher := chash.New()
-	hasher.Write(nonce)
-	return hasher.Sum(nil), nil
-}

server/verify.go

@@ -66,10 +66,6 @@ type VerifyOpts struct {
 	// "Calling EFI Application from Boot Option". This option is useful when
 	// the host platform loads EFI Applications unrelated to OS boot.
 	AllowEFIAppBeforeCallingEvent bool
-	// HashNonce will apply the attestation key's signing scheme hash algorithm
-	// to the input Nonce field and use the resulting digest in place of the
-	// original Nonce.
-	HashNonce bool
 }
 
 // Bootloader refers to the second-stage bootloader that loads and transfers
@@ -118,24 +114,16 @@ func VerifyAttestation(attestation *pb.Attestation, opts VerifyOpts) (*pb.Machin
 		return nil, fmt.Errorf("bad options: %w", err)
 	}
 
-	machineState, akPub, akPubKey, err := validateAK(attestation, opts)
+	machineState, akPubKey, err := validateAK(attestation, opts)
 	if err != nil {
 		return nil, fmt.Errorf("failed to parse and validate AK: %w", err)
 	}
-	extraData := opts.Nonce
-	if opts.HashNonce {
-		var err error
-		extraData, err = internal.HashNonce(akPub, extraData)
-		if err != nil {
-			return nil, fmt.Errorf("failed to hash the input nonce: %w", err)
-		}
-	}
 
 	// Attempt to replay the log against our PCRs in order of hash preference
 	var lastErr error
 	for _, quote := range supportedQuotes(attestation.GetQuotes()) {
 		// Verify the Quote
-		if err := internal.VerifyQuote(quote, akPubKey, extraData); err != nil {
+		if err := internal.VerifyQuote(quote, akPubKey, opts.Nonce); err != nil {
 			lastErr = fmt.Errorf("failed to verify quote: %w", err)
 			continue
 		}
@@ -170,48 +158,44 @@ func VerifyAttestation(attestation *pb.Attestation, opts VerifyOpts) (*pb.Machin
 
 // validateAK validates AK cert in the attestation, and returns AK cert (if exists) and public key.
 // It also pulls out the GCE Instance Info if it exists.
-func validateAK(attestation *pb.Attestation, opts VerifyOpts) (*pb.MachineState, tpm2.Public, crypto.PublicKey, error) {
-	// If the AK Cert is not in the attestation, use the AK Public Area.
-	akPubArea, err := tpm2.DecodePublic(attestation.GetAkPub())
-	if err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("failed to decode AK public area: %w", err)
-	}
-	akPubKey, err := akPubArea.Key()
-	if err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("failed to get AK public key: %w", err)
-	}
+func validateAK(attestation *pb.Attestation, opts VerifyOpts) (*pb.MachineState, crypto.PublicKey, error) {
 	if len(attestation.GetAkCert()) == 0 || len(opts.TrustedRootCerts) == 0 {
+		// If the AK Cert is not in the attestation, use the AK Public Area.
+		akPubArea, err := tpm2.DecodePublic(attestation.GetAkPub())
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to decode AK public area: %w", err)
+		}
+		akPubKey, err := akPubArea.Key()
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to get AK public key: %w", err)
+		}
 		if err := validateAKPub(akPubKey, opts); err != nil {
-			return nil, tpm2.Public{}, nil, fmt.Errorf("failed to validate AK public key: %w", err)
+			return nil, nil, fmt.Errorf("failed to validate AK public key: %w", err)
 		}
-		return &pb.MachineState{}, akPubArea, akPubKey, nil
+		return &pb.MachineState{}, akPubKey, nil
 	}
 
 	// If AK Cert is presented, ignore the AK Public Area.
 	akCert, err := x509.ParseCertificate(attestation.GetAkCert())
-	certPubKey := akCert.PublicKey.(crypto.PublicKey) // This cast cannot fail
-	if !internal.PubKeysEqual(certPubKey, akPubKey) {
-		return nil, tpm2.Public{}, nil, errors.New("AK certificate does not match key")
-	}
 	if err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("failed to parse AK certificate: %w", err)
+		return nil, nil, fmt.Errorf("failed to parse AK certificate: %w", err)
 	}
 	// Use intermediate certs from the attestation if they exist.
 	certs, err := parseCerts(attestation.IntermediateCerts)
 	if err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("attestation intermediates: %w", err)
+		return nil, nil, fmt.Errorf("attestation intermediates: %w", err)
 	}
 	opts.IntermediateCerts = append(opts.IntermediateCerts, certs...)
 
 	if err := VerifyAKCert(akCert, opts.TrustedRootCerts, opts.IntermediateCerts); err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("failed to validate AK certificate: %w", err)
+		return nil, nil, fmt.Errorf("failed to validate AK certificate: %w", err)
 	}
 	instanceInfo, err := getInstanceInfoFromExtensions(akCert.Extensions)
 	if err != nil {
-		return nil, tpm2.Public{}, nil, fmt.Errorf("error getting instance info: %v", err)
+		return nil, nil, fmt.Errorf("error getting instance info: %v", err)
 	}
 
-	return &pb.MachineState{Platform: &pb.PlatformState{InstanceInfo: instanceInfo}}, akPubArea, akCert.PublicKey, nil
+	return &pb.MachineState{Platform: &pb.PlatformState{InstanceInfo: instanceInfo}}, akCert.PublicKey, nil
 }
 
 // GetGCEInstanceInfo takes a GCE-issued x509 EK/AK certificate and tries to

server/verify_test.go

@@ -13,7 +13,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"strconv"
 	"testing"
 
 	"github.com/google/go-tpm-tools/client"
@@ -221,47 +220,6 @@ func TestVerifyWithTrustedAK(t *testing.T) {
 	}
 }
 
-func TestVerifyHashNonce(t *testing.T) {
-	rwc := test.GetTPM(t)
-	defer client.CheckedClose(t, rwc)
-
-	ak, err := client.AttestationKeyRSA(rwc)
-	if err != nil {
-		t.Fatalf("failed to generate AK: %v", err)
-	}
-	defer ak.Close()
-	tests := []struct {
-		attHash bool
-		verHash bool
-		wantErr bool
-	}{
-		{true, true, false},
-		{false, false, false},
-		{true, false, true},
-		{false, true, true},
-	}
-	nonce := []byte("super secret nonce")
-
-	for _, test := range tests {
-		t.Run("attest hash "+strconv.FormatBool(test.attHash)+" verify hash "+strconv.FormatBool(test.verHash), func(t *testing.T) {
-			attestation, err := ak.Attest(client.AttestOpts{Nonce: nonce, HashNonce: test.attHash})
-			if err != nil {
-				t.Fatalf("failed to attest: %v", err)
-			}
-
-			opts := VerifyOpts{
-				Nonce:      nonce,
-				TrustedAKs: []crypto.PublicKey{ak.PublicKey()},
-				HashNonce:  test.verHash,
-			}
-			_, err = VerifyAttestation(attestation, opts)
-			if test.wantErr != (err != nil) {
-				t.Errorf("Attest(HashNonce %v), Verify(HashNonce %v): got %v wantErr %v", test.attHash, test.verHash, err, test.wantErr)
-			}
-		})
-	}
-}
-
 func TestVerifySHA1Attestation(t *testing.T) {
 	rwc := test.GetTPM(t)
 	defer client.CheckedClose(t, rwc)
@@ -481,7 +439,7 @@ func TestValidateAK(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			_, _, _, err := validateAK(tc.att(), tc.opts)
+			_, _, err := validateAK(tc.att(), tc.opts)
 			if gotPass := (err == nil); gotPass != tc.wantPass {
 				t.Errorf("ValidateAK failed, got pass %v, but want %v", gotPass, tc.wantPass)
 			}