Add public key hash check in Signed Note verification (#2214)

* Update Verify method to perform PK hash check

Signed-off-by: Jang <[email protected]>

* Update unit test file

Update existing tests and ensure they work as expected and add a unit test case
to check for public key mismatch.

Signed-off-by: Jang <[email protected]>

* Make getPublicKeyHash an unexported function

Signed-off-by: Jang <[email protected]>

---------

Signed-off-by: Jang <[email protected]>

pkg/util/checkpoint_test.go

@@ -355,6 +355,8 @@ func TestSigningRoundtripCheckpoint(t *testing.T) {
 
 func TestInvalidSigVerification(t *testing.T) {
 	ecdsaKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	pubKeyHash, _ := getPublicKeyHash(ecdsaKey.Public())
+
 	for _, test := range []struct {
 		checkpoint     Checkpoint
 		s              []note.Signature
@@ -382,12 +384,27 @@ func TestInvalidSigVerification(t *testing.T) {
 			s: []note.Signature{
 				{
 					Name:   "something",
-					Hash:   1234,
+					Hash:   pubKeyHash,
 					Base64: "not_base 64 string",
 				},
 			},
 			expectedResult: false,
 		},
+		{
+			checkpoint: Checkpoint{
+				Origin: "Log Checkpoint v0 public key mismatch",
+				Size:   123,
+				Hash:   []byte("bananas"),
+			},
+			pubKey: ecdsaKey.Public(),
+			s: []note.Signature{
+				{
+					Name:   "something",
+					Hash:   123,
+					Base64: "bm90IGEgc2ln", // not a valid signature; public key mismatch happens first
+				},
+			},
+		},
 		{
 			checkpoint: Checkpoint{
 				Origin: "Log Checkpoint v0 invalid signature",
@@ -398,7 +415,7 @@ func TestInvalidSigVerification(t *testing.T) {
 			s: []note.Signature{
 				{
 					Name:   "someone",
-					Hash:   142,
+					Hash:   pubKeyHash,
 					Base64: "bm90IGEgc2ln", // valid base64, not a valid signature
 				},
 			},

pkg/util/signed_note.go

@@ -18,6 +18,7 @@ package util
 import (
 	"bufio"
 	"bytes"
+	"crypto"
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/rsa"
@@ -53,16 +54,14 @@ func (s *SignedNote) Sign(identity string, signer signature.Signer, opts signatu
 	if err != nil {
 		return nil, fmt.Errorf("retrieving public key: %w", err)
 	}
-	pubKeyBytes, err := x509.MarshalPKIXPublicKey(pk)
+	pkHash, err := getPublicKeyHash(pk)
 	if err != nil {
-		return nil, fmt.Errorf("marshalling public key: %w", err)
+		return nil, err
 	}
 
-	pkSha := sha256.Sum256(pubKeyBytes)
-
 	signature := note.Signature{
 		Name:   identity,
-		Hash:   binary.BigEndian.Uint32(pkSha[:]),
+		Hash:   pkHash,
 		Base64: base64.StdEncoding.EncodeToString(sig),
 	}
 
@@ -80,15 +79,25 @@ func (s SignedNote) Verify(verifier signature.Verifier) bool {
 	msg := []byte(s.Note)
 	digest := sha256.Sum256(msg)
 
+	pk, err := verifier.PublicKey()
+	if err != nil {
+		return false
+	}
+	verifierPkHash, err := getPublicKeyHash(pk)
+	if err != nil {
+		return false
+	}
+
 	for _, s := range s.Signatures {
 		sigBytes, err := base64.StdEncoding.DecodeString(s.Base64)
 		if err != nil {
 			return false
 		}
-		pk, err := verifier.PublicKey()
-		if err != nil {
+
+		if s.Hash != verifierPkHash {
 			return false
 		}
+
 		opts := []signature.VerifyOption{}
 		switch pk.(type) {
 		case *rsa.PublicKey, *ecdsa.PublicKey:
@@ -190,3 +199,13 @@ func SignedNoteValidator(strToValidate string) bool {
 	s := SignedNote{}
 	return s.UnmarshalText([]byte(strToValidate)) == nil
 }
+
+func getPublicKeyHash(publicKey crypto.PublicKey) (uint32, error) {
+	pubKeyBytes, err := x509.MarshalPKIXPublicKey(publicKey)
+	if err != nil {
+		return 0, fmt.Errorf("marshalling public key: %w", err)
+	}
+	pkSha := sha256.Sum256(pubKeyBytes)
+	hash := binary.BigEndian.Uint32(pkSha[:])
+	return hash, nil
+}