Merge pull request from GHSA-99cg-575x-774p

* AKPublic.Verify: Return an error if a provided PCR of the correct
   digest was not included in the quote.
 * AKPublic.VerifyAll: Implement VerifyAll method, which can cross-check
   that provided PCRs were covered by quotes across PCR banks.
 * PCR.QuoteVerified(): Introduce getter method to expose whether a
   PCR value was covered during quote verification.

attest/attest.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/google/certificate-transparency-go/x509"
 	"github.com/google/go-tpm/tpm"
@@ -179,6 +180,16 @@ type PCR struct {
 	Index     int
 	Digest    []byte
 	DigestAlg crypto.Hash
+
+	// quoteVerified is true if the PCR was verified against a quote
+	// in a call to AKPublic.Verify or AKPublic.VerifyAll.
+	quoteVerified bool
+}
+
+// QuoteVerified returns true if the value of this PCR was previously
+// verified against a Quote, in a call to AKPublic.Verify or AKPublic.VerifyAll.
+func (p *PCR) QuoteVerified() bool {
+	return p.quoteVerified
 }
 
 // EK is a burned-in endorcement key bound to a TPM. This optionally contains
@@ -290,7 +301,12 @@ func ParseAKPublic(version TPMVersion, public []byte) (*AKPublic, error) {
 
 // Verify is used to prove authenticity of the PCR measurements. It ensures that
 // the quote was signed by the AK, and that its contents matches the PCR and
-// nonce combination.
+// nonce combination. An error is returned if a provided PCR index was not part
+// of the quote. QuoteVerified() will return true on PCRs which were verified
+// by a quote.
+//
+// Do NOT use this method if you have multiple quotes to verify: Use VerifyAll
+// instead.
 //
 // The nonce is used to prevent replays of Quote and PCRs and is signed by the
 // quote. Some TPMs don't support nonces longer than 20 bytes, and if the
@@ -307,6 +323,27 @@ func (a *AKPublic) Verify(quote Quote, pcrs []PCR, nonce []byte) error {
 	}
 }
 
+// VerifyAll uses multiple quotes to verify the authenticity of all PCR
+// measurements. See documentation on Verify() for semantics.
+func (a *AKPublic) VerifyAll(quotes []Quote, pcrs []PCR, nonce []byte) error {
+	for i, quote := range quotes {
+		if err := a.Verify(quote, pcrs, nonce); err != nil {
+			return fmt.Errorf("quote %d: %v", i, err)
+		}
+	}
+
+	var errPCRs []string
+	for _, p := range pcrs {
+		if !p.QuoteVerified() {
+			errPCRs = append(errPCRs, fmt.Sprintf("%d (%s)", p.Index, p.DigestAlg))
+		}
+	}
+	if len(errPCRs) > 0 {
+		return fmt.Errorf("some PCRs were not covered by a quote: %s", strings.Join(errPCRs, ", "))
+	}
+	return nil
+}
+
 // HashAlg identifies a hashing Algorithm.
 type HashAlg uint8
 

attest/attest_fuzz.go

@@ -12,6 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
+//go:build gofuzz
 // +build gofuzz
 
 package attest

attest/attest_simulated_tpm20_test.go

@@ -149,7 +149,7 @@ func TestParseAKPublic20(t *testing.T) {
 	}
 }
 
-func TestSimTPM20QuoteAndVerify(t *testing.T) {
+func TestSimTPM20QuoteAndVerifyAll(t *testing.T) {
 	sim, tpm := setupSimulatedTPM(t)
 	defer sim.Close()
 
@@ -160,9 +160,13 @@ func TestSimTPM20QuoteAndVerify(t *testing.T) {
 	defer ak.Close(tpm)
 
 	nonce := []byte{1, 2, 3, 4, 5, 6, 7, 8}
-	quote, err := ak.Quote(tpm, nonce, HashSHA256)
+	quote256, err := ak.Quote(tpm, nonce, HashSHA256)
 	if err != nil {
-		t.Fatalf("ak.Quote() failed: %v", err)
+		t.Fatalf("ak.Quote(SHA256) failed: %v", err)
+	}
+	quote1, err := ak.Quote(tpm, nonce, HashSHA1)
+	if err != nil {
+		t.Fatalf("ak.Quote(SHA1) failed: %v", err)
 	}
 
 	// Providing both PCR banks to AKPublic.Verify() ensures we can handle
@@ -180,7 +184,14 @@ func TestSimTPM20QuoteAndVerify(t *testing.T) {
 	if err != nil {
 		t.Fatalf("ParseAKPublic() failed: %v", err)
 	}
-	if err := pub.Verify(*quote, pcrs, nonce); err != nil {
+
+	// Ensure VerifyAll fails if a quote is missing and hence not all PCR
+	// banks are covered.
+	if err := pub.VerifyAll([]Quote{*quote256}, pcrs, nonce); err == nil {
+		t.Error("VerifyAll().err returned nil, expected failure")
+	}
+
+	if err := pub.VerifyAll([]Quote{*quote256, *quote1}, pcrs, nonce); err != nil {
 		t.Errorf("quote verification failed: %v", err)
 	}
 }
@@ -205,10 +216,8 @@ func TestSimTPM20AttestPlatform(t *testing.T) {
 	if err != nil {
 		t.Fatalf("ParseAKPublic() failed: %v", err)
 	}
-	for i, q := range attestation.Quotes {
-		if err := pub.Verify(q, attestation.PCRs, nonce); err != nil {
-			t.Errorf("quote[%d] verification failed: %v", i, err)
-		}
+	if err := pub.VerifyAll(attestation.Quotes, attestation.PCRs, nonce); err != nil {
+		t.Errorf("quote verification failed: %v", err)
 	}
 }
 

attest/eventlog.go

@@ -320,6 +320,13 @@ func (a *AKPublic) validate12Quote(quote Quote, pcrs []PCR, nonce []byte) error
 	if att.Digest != sha1.Sum(composite) {
 		return fmt.Errorf("PCRs passed didn't match quote: %v", err)
 	}
+
+	// All provided PCRs are used to construct the composite hash which
+	// is verified against the quote (for TPM 1.2), so if we got this far,
+	// all PCR values are verified.
+	for i := range pcrs {
+		pcrs[i].quoteVerified = true
+	}
 	return nil
 }
 
@@ -366,17 +373,33 @@ func (a *AKPublic) validate20Quote(quote Quote, pcrs []PCR, nonce []byte) error
 	}
 
 	sigHash.Reset()
+	quotePCRs := make(map[int]struct{}, len(att.AttestedQuoteInfo.PCRSelection.PCRs))
 	for _, index := range att.AttestedQuoteInfo.PCRSelection.PCRs {
 		digest, ok := pcrByIndex[index]
 		if !ok {
 			return fmt.Errorf("quote was over PCR %d which wasn't provided", index)
 		}
+		quotePCRs[index] = struct{}{}
 		sigHash.Write(digest)
 	}
 
+	for index, _ := range pcrByIndex {
+		if _, exists := quotePCRs[index]; !exists {
+			return fmt.Errorf("provided PCR %d was not included in quote", index)
+		}
+	}
+
 	if !bytes.Equal(sigHash.Sum(nil), att.AttestedQuoteInfo.PCRDigest) {
 		return fmt.Errorf("quote digest didn't match pcrs provided")
 	}
+
+	// If we got this far, all included PCRs with a digest algorithm matching that
+	// of the quote are verified. As such, we set their quoteVerified bit.
+	for i, pcr := range pcrs {
+		if _, exists := quotePCRs[pcr.Index]; exists && pcr.DigestAlg == pcrDigestAlg {
+			pcrs[i].quoteVerified = true
+		}
+	}
 	return nil
 }
 

attest/eventlog_fuzz.go

@@ -12,6 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
+//go:build gofuzz
 // +build gofuzz
 
 package attest

attest/key_linux.go

@@ -12,6 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
+//go:build linux && !gofuzz && cgo && tspi
 // +build linux,!gofuzz,cgo,tspi
 
 package attest

attest/key_windows.go

@@ -12,6 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
+//go:build windows
 // +build windows
 
 package attest

attest/pcp_windows.go

@@ -12,6 +12,7 @@
 // License for the specific language governing permissions and limitations under
 // the License.
 
+//go:build windows
 // +build windows
 
 package attest
@@ -26,9 +27,9 @@ import (
 
 	"github.com/google/certificate-transparency-go/x509"
 
-	"golang.org/x/sys/windows"
-	tpmtbs "github.com/google/go-tpm/tpmutil/tbs"
 	"github.com/google/go-tpm/tpmutil"
+	tpmtbs "github.com/google/go-tpm/tpmutil/tbs"
+	"golang.org/x/sys/windows"
 )
 
 const (

attest/secureboot.go

@@ -19,8 +19,8 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/google/go-attestation/attest/internal"
 	"github.com/google/certificate-transparency-go/x509"
+	"github.com/google/go-attestation/attest/internal"
 )
 
 // SecurebootState describes the secure boot status of a machine, as determined