diff --git a/pkg/util/checkpoint_test.go b/pkg/util/checkpoint_test.go index bbfc53c59..7f8c35279 100644 --- a/pkg/util/checkpoint_test.go +++ b/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 }, }, diff --git a/pkg/util/signed_note.go b/pkg/util/signed_note.go index 85063497c..f4bfdfb67 100644 --- a/pkg/util/signed_note.go +++ b/pkg/util/signed_note.go @@ -54,7 +54,7 @@ func (s *SignedNote) Sign(identity string, signer signature.Signer, opts signatu if err != nil { return nil, fmt.Errorf("retrieving public key: %w", err) } - pkHash, err := getPublicKeyHash(pk) + pkHash, err := GetPublicKeyHash(pk) if err != nil { return nil, err } @@ -79,21 +79,11 @@ func (s SignedNote) Verify(verifier signature.Verifier) bool { msg := []byte(s.Note) digest := sha256.Sum256(msg) - /* - Clarifications required: - 0. Am I understanding this correctly? calculate the hash of the public key in every loop? Isn't the PK constant throughout this function call? - 1. Would it still suffice to still only return `false` upon error? - 2. Should I write a function to reuse the logic for generating hash of the PK? - 3. How can I test this? - - unit tests - - end-to-end behavior test - */ - pk, err := verifier.PublicKey() if err != nil { return false } - verifierPkHash, err := getPublicKeyHash(pk) + verifierPkHash, err := GetPublicKeyHash(pk) if err != nil { return false } @@ -210,7 +200,7 @@ func SignedNoteValidator(strToValidate string) bool { return s.UnmarshalText([]byte(strToValidate)) == nil } -func getPublicKeyHash(publicKey crypto.PublicKey) (uint32, error) { +func GetPublicKeyHash(publicKey crypto.PublicKey) (uint32, error) { pubKeyBytes, err := x509.MarshalPKIXPublicKey(publicKey) if err != nil { return 0, fmt.Errorf("marshalling public key: %w", err)