diff --git a/node/pkg/processor/observation.go b/node/pkg/processor/observation.go index 193bb5690..d5434d350 100644 --- a/node/pkg/processor/observation.go +++ b/node/pkg/processor/observation.go @@ -270,6 +270,25 @@ func (p *Processor) handleInboundSignedVAAWithQuorum(ctx context.Context, m *gos return } + // Check if guardianSet doesn't have any keys + if len(p.gs.Keys) == 0 { + p.logger.Warn("dropping SignedVAAWithQuorum message since we have a guardian set without keys", + zap.String("digest", hash), + zap.Any("message", m), + ) + return + } + + // Check if VAA doesn't have any signatures + if len(v.Signatures) == 0 { + p.logger.Warn("received SignedVAAWithQuorum message with no VAA signatures", + zap.String("digest", hash), + zap.Any("message", m), + zap.Any("vaa", v), + ) + return + } + // Verify VAA signature to prevent a DoS attack on our local store. if !v.VerifySignatures(p.gs.Keys) { p.logger.Warn("received SignedVAAWithQuorum message with invalid VAA signatures", diff --git a/node/pkg/processor/observation_test.go b/node/pkg/processor/observation_test.go new file mode 100644 index 000000000..86016365b --- /dev/null +++ b/node/pkg/processor/observation_test.go @@ -0,0 +1,124 @@ +package processor + +import ( + "context" + "crypto/ecdsa" + "crypto/rand" + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/vaa" + ethcommon "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + "testing" + "time" +) + +func getVAA() vaa.VAA { + var payload = []byte{97, 97, 97, 97, 97, 97} + var governanceEmitter = vaa.Address{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4} + + vaa := vaa.VAA{ + Version: uint8(1), + GuardianSetIndex: uint32(1), + Signatures: nil, + Timestamp: time.Unix(0, 0), + Nonce: uint32(1), + Sequence: uint64(1), + ConsistencyLevel: uint8(32), + EmitterChain: vaa.ChainIDSolana, + EmitterAddress: governanceEmitter, + Payload: payload, + } + + return vaa +} + +func TestHandleInboundSignedVAAWithQuorum_NilGuardianSet(t *testing.T) { + vaa := getVAA() + marshalVAA, _ := vaa.Marshal() + + // Stub out the minimum to get processor to dance + observedZapCore, observedLogs := observer.New(zap.InfoLevel) + observedLogger := zap.New(observedZapCore) + + ctx := context.Background() + signedVAAWithQuorum := &gossipv1.SignedVAAWithQuorum{Vaa: marshalVAA} + processor := Processor{} + processor.logger = observedLogger + + processor.handleInboundSignedVAAWithQuorum(ctx, signedVAAWithQuorum) + + // Check to see if we got an error, which we should have, + // because a `gs` is not defined on processor + assert.Equal(t, 1, observedLogs.Len()) + firstLog := observedLogs.All()[0] + errorString := "dropping SignedVAAWithQuorum message since we haven't initialized our guardian set yet" + assert.Equal(t, errorString, firstLog.Message) +} + +func TestHandleInboundSignedVAAWithQuorum(t *testing.T) { + goodPrivateKey1, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + goodAddr1 := crypto.PubkeyToAddress(goodPrivateKey1.PublicKey) + badPrivateKey1, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + + tests := []struct { + label string + keyOrder []*ecdsa.PrivateKey + indexOrder []uint8 + addrs []ethcommon.Address + errString string + }{ + {label: "GuardianSetNoKeys", keyOrder: []*ecdsa.PrivateKey{}, indexOrder: []uint8{}, addrs: []ethcommon.Address{}, + errString: "dropping SignedVAAWithQuorum message since we have a guardian set without keys"}, + {label: "VAANoSignatures", keyOrder: []*ecdsa.PrivateKey{}, indexOrder: []uint8{0}, addrs: []ethcommon.Address{goodAddr1}, + errString: "received SignedVAAWithQuorum message with no VAA signatures"}, + {label: "VAAInvalidSignatures", keyOrder: []*ecdsa.PrivateKey{badPrivateKey1}, indexOrder: []uint8{0}, addrs: []ethcommon.Address{goodAddr1}, + errString: "received SignedVAAWithQuorum message with invalid VAA signatures"}, + {label: "DuplicateGoodSignaturesNonMonotonic", keyOrder: []*ecdsa.PrivateKey{goodPrivateKey1, goodPrivateKey1, goodPrivateKey1, goodPrivateKey1}, indexOrder: []uint8{0, 0, 0, 0}, addrs: []ethcommon.Address{goodAddr1}, + errString: "received SignedVAAWithQuorum message with invalid VAA signatures"}, + {label: "DuplicateGoodSignaturesMonotonic", keyOrder: []*ecdsa.PrivateKey{goodPrivateKey1, goodPrivateKey1, goodPrivateKey1, goodPrivateKey1}, indexOrder: []uint8{0, 1, 2, 3}, addrs: []ethcommon.Address{goodAddr1}, + errString: "received SignedVAAWithQuorum message with invalid VAA signatures"}, + } + + for _, tc := range tests { + t.Run(tc.label, func(t *testing.T) { + vaa := getVAA() + + // Define a GuardianSet from test addrs + guardianSet := common.GuardianSet{ + Keys: tc.addrs, + Index: 1, + } + + // Sign with the keys at the proper index + for i, key := range tc.keyOrder { + vaa.AddSignature(key, tc.indexOrder[i]) + } + + marshalVAA, err := vaa.Marshal() + if err != nil { + panic(err) + } + + // Stub out the minimum to get processor to dance + observedZapCore, observedLogs := observer.New(zap.InfoLevel) + observedLogger := zap.New(observedZapCore) + + ctx := context.Background() + signedVAAWithQuorum := &gossipv1.SignedVAAWithQuorum{Vaa: marshalVAA} + processor := Processor{} + processor.gs = &guardianSet + processor.logger = observedLogger + + processor.handleInboundSignedVAAWithQuorum(ctx, signedVAAWithQuorum) + + // Check to see if we got an error, which we should have + assert.Equal(t, 1, observedLogs.Len()) + firstLog := observedLogs.All()[0] + assert.Equal(t, tc.errString, firstLog.Message) + }) + } +} diff --git a/node/pkg/vaa/structs.go b/node/pkg/vaa/structs.go index 38143e807..7922517fd 100644 --- a/node/pkg/vaa/structs.go +++ b/node/pkg/vaa/structs.go @@ -291,20 +291,39 @@ func (v *VAA) VerifySignatures(addresses []common.Address) bool { h := v.SigningMsg() + last_index := -1 + signing_addresses := []common.Address{} + for _, sig := range v.Signatures { if int(sig.Index) >= len(addresses) { return false } + // Ensure increasing indexes + if int(sig.Index) <= last_index { + return false + } + last_index = int(sig.Index) + + // Get pubKey to determine who signers address pubKey, err := crypto.Ecrecover(h.Bytes(), sig.Signature[:]) if err != nil { return false } addr := common.BytesToAddress(crypto.Keccak256(pubKey[1:])[12:]) + // Ensure this signer is at the correct positional index if addr != addresses[sig.Index] { return false } + + // Ensure we never see the same signer twice + for _, signing_address := range signing_addresses { + if signing_address == addr { + return false + } + } + signing_addresses = append(signing_addresses, addr) } return true diff --git a/node/pkg/vaa/structs_test.go b/node/pkg/vaa/structs_test.go index 7c8322c3c..37a9ffee7 100644 --- a/node/pkg/vaa/structs_test.go +++ b/node/pkg/vaa/structs_test.go @@ -9,30 +9,11 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "reflect" "testing" "time" ) -func getVAA() *VAA { - var payload = []byte{97, 97, 97, 97, 97, 97} - var governanceEmitter = Address{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4} - - vaa := &VAA{ - Version: uint8(1), - GuardianSetIndex: uint32(1), - Signatures: nil, - Timestamp: time.Unix(0, 0), - Nonce: uint32(1), - Sequence: uint64(1), - ConsistencyLevel: uint8(32), - EmitterChain: ChainIDSolana, - EmitterAddress: governanceEmitter, - Payload: payload, - } - - return vaa -} - func TestChainIDFromString(t *testing.T) { type test struct { input string @@ -160,8 +141,26 @@ func TestChainId_String(t *testing.T) { } } +func getVaa() VAA { + var payload = []byte{97, 97, 97, 97, 97, 97} + var governanceEmitter = Address{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4} + + return VAA{ + Version: uint8(1), + GuardianSetIndex: uint32(1), + Signatures: nil, + Timestamp: time.Unix(0, 0), + Nonce: uint32(1), + Sequence: uint64(1), + ConsistencyLevel: uint8(32), + EmitterChain: ChainIDSolana, + EmitterAddress: governanceEmitter, + Payload: payload, + } +} + func TestAddSignature(t *testing.T) { - vaa := getVAA() + vaa := getVaa() // Generate a random private key to sign with key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -173,57 +172,309 @@ func TestAddSignature(t *testing.T) { } func TestSerializeBody(t *testing.T) { - vaa := getVAA() + vaa := getVaa() expected := []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x20, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61} assert.Equal(t, vaa.serializeBody(), expected) } func TestSigningBody(t *testing.T) { - vaa := getVAA() + vaa := getVaa() expected := []byte{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x20, 0x61, 0x61, 0x61, 0x61, 0x61, 0x61} assert.Equal(t, vaa.signingBody(), expected) } func TestSigningMsg(t *testing.T) { - vaa := getVAA() + vaa := getVaa() expected := common.HexToHash("4fae136bb1fd782fe1b5180ba735cdc83bcece3f9b7fd0e5e35300a61c8acd8f") assert.Equal(t, vaa.SigningMsg(), expected) } func TestMessageID(t *testing.T) { - vaa := getVAA() + vaa := getVaa() expected := "1/0000000000000000000000000000000000000000000000000000000000000004/1" assert.Equal(t, vaa.MessageID(), expected) } func TestHexDigest(t *testing.T) { - vaa := getVAA() + vaa := getVaa() expected := "4fae136bb1fd782fe1b5180ba735cdc83bcece3f9b7fd0e5e35300a61c8acd8f" assert.Equal(t, vaa.HexDigest(), expected) } func TestVerifySignatures(t *testing.T) { - vaa := getVAA() + // Generate some random private keys to sign with + privKey1, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey2, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey3, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey4, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) - // Generate a random private key to sign with - privKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - assert.Nil(t, vaa.Signatures) + // Give a fixed order of trusted addresses + addrs := []common.Address{} + addrs = append(addrs, crypto.PubkeyToAddress(privKey1.PublicKey)) + addrs = append(addrs, crypto.PubkeyToAddress(privKey2.PublicKey)) + addrs = append(addrs, crypto.PubkeyToAddress(privKey3.PublicKey)) - // Add a signature and make sure it's added - vaa.AddSignature(privKey, 0) - assert.Equal(t, len(vaa.Signatures), 1) + type test struct { + label string + keyOrder []*ecdsa.PrivateKey + addrs []common.Address + indexOrder []uint8 + result bool + } - // Generate a public key to compare to from our private key - h := vaa.SigningMsg() - pubKey, err := crypto.Ecrecover(h.Bytes(), vaa.Signatures[0].Signature[:]) - assert.Nil(t, err) - assert.NotNil(t, pubKey) + tests := []test{ + {label: "NoSignerZero", + keyOrder: []*ecdsa.PrivateKey{}, + addrs: addrs, + indexOrder: []uint8{0}, + result: true}, + {label: "NoSignerOne", + keyOrder: []*ecdsa.PrivateKey{}, + addrs: addrs, + indexOrder: []uint8{1}, + result: true}, + {label: "SingleZero", + keyOrder: []*ecdsa.PrivateKey{privKey1}, + addrs: addrs, + indexOrder: []uint8{0}, + result: true}, + {label: "RogueSingleOne", + keyOrder: []*ecdsa.PrivateKey{privKey4}, + addrs: addrs, + indexOrder: []uint8{0}, + result: false}, + {label: "RogueSingleZero", + keyOrder: []*ecdsa.PrivateKey{privKey4}, + addrs: addrs, + indexOrder: []uint8{0}, + result: false}, + {label: "SingleOne", + keyOrder: []*ecdsa.PrivateKey{privKey1}, + addrs: addrs, + indexOrder: []uint8{0}, + result: true}, + {label: "MultiUniqSignerMonotonicIndex", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, + addrs: addrs, + indexOrder: []uint8{0, 1, 2}, + result: true}, + {label: "MultiMisOrderedSignerMonotonicIndex", + keyOrder: []*ecdsa.PrivateKey{privKey3, privKey2, privKey1}, + addrs: addrs, + indexOrder: []uint8{0, 1, 2}, result: false}, + {label: "MultiUniqSignerNonMonotonic", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, + addrs: addrs, + indexOrder: []uint8{0, 2, 1}, + result: false}, + {label: "MultiUniqSignerFullSameIndex0", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, + addrs: addrs, + indexOrder: []uint8{0, 0, 0}, + result: false}, + {label: "MultiUniqSignerFullSameIndex1", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, + addrs: addrs, + indexOrder: []uint8{0, 0, 0}, + result: false}, + {label: "MultiUniqSignerPartialSameIndex", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, + addrs: addrs, + indexOrder: []uint8{0, 1, 1}, + result: false}, + {label: "MultiSameSignerPartialSameIndex", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey2}, + addrs: addrs, + indexOrder: []uint8{0, 1, 1}, + result: false}, + {label: "MultiSameSignerNonMonotonic", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey2, privKey2}, + addrs: addrs, + indexOrder: []uint8{0, 2, 1}, + result: false}, + {label: "MultiSameSignerFullSameIndex", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey1, privKey1}, + addrs: addrs, + indexOrder: []uint8{0, 0, 0}, + result: false}, + {label: "MultiSameSignerMonotonic", + keyOrder: []*ecdsa.PrivateKey{privKey1, privKey1, privKey1}, + addrs: addrs, + indexOrder: []uint8{0, 1, 2}, + result: false}, + } - // Translate that public key back to an address - addr := common.BytesToAddress(crypto.Keccak256(pubKey[1:])[12:]) + for _, tc := range tests { + t.Run(tc.label, func(t *testing.T) { + vaa := getVaa() - // Make sure that it verifies - assert.True(t, vaa.VerifySignatures([]common.Address{addr})) + for i, key := range tc.keyOrder { + vaa.AddSignature(key, tc.indexOrder[i]) + } + + assert.Equal(t, tc.result, vaa.VerifySignatures(tc.addrs)) + }) + } +} + +func TestVerifySignaturesFuzz(t *testing.T) { + // Generate some random trusted private keys to sign with + privKey1, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey2, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey3, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + + // Generate some random untrusted private keys to sign with + privKey4, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey5, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + privKey6, _ := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + + // Give a fixed order of trusted addresses (we intentionally omit privKey4, privKey5, privKey6) + addrs := []common.Address{} + addrs = append(addrs, crypto.PubkeyToAddress(privKey1.PublicKey)) + addrs = append(addrs, crypto.PubkeyToAddress(privKey2.PublicKey)) + addrs = append(addrs, crypto.PubkeyToAddress(privKey3.PublicKey)) + + // key space for fuzz tests + keys := []*ecdsa.PrivateKey{} + keys = append(keys, privKey1) + keys = append(keys, privKey2) + keys = append(keys, privKey3) + keys = append(keys, privKey4) + keys = append(keys, privKey5) + keys = append(keys, privKey6) + + // index space for fuzz tests + indexes := []uint8{0, 1, 2, 3, 4, 5} + + type test struct { + label string + keyOrder []*ecdsa.PrivateKey + addrs []common.Address + indexOrder []uint8 + result bool + } + + type allow struct { + keyPair []*ecdsa.PrivateKey + indexPair []uint8 + } + + // Known good cases where we should have a verified result for + allows := []allow{ + {keyPair: []*ecdsa.PrivateKey{}, indexPair: []uint8{}}, + {keyPair: []*ecdsa.PrivateKey{privKey1}, indexPair: []uint8{0}}, + {keyPair: []*ecdsa.PrivateKey{privKey2}, indexPair: []uint8{1}}, + {keyPair: []*ecdsa.PrivateKey{privKey3}, indexPair: []uint8{2}}, + {keyPair: []*ecdsa.PrivateKey{privKey1, privKey2}, indexPair: []uint8{0, 1}}, + {keyPair: []*ecdsa.PrivateKey{privKey1, privKey3}, indexPair: []uint8{0, 2}}, + {keyPair: []*ecdsa.PrivateKey{privKey2, privKey3}, indexPair: []uint8{1, 2}}, + {keyPair: []*ecdsa.PrivateKey{privKey1, privKey2, privKey3}, indexPair: []uint8{0, 1, 2}}, + } + + tests := []test{} + keyPairs := [][]*ecdsa.PrivateKey{} + indexPairs := [][]uint8{} + + // Build empty keyPair + keyPairs = append(keyPairs, []*ecdsa.PrivateKey{}) + + // Build single keyPairs + for _, key := range keys { + keyPairs = append(keyPairs, []*ecdsa.PrivateKey{key}) + } + + // Build double keyPairs + for _, key_i := range keys { + for _, key_j := range keys { + keyPairs = append(keyPairs, []*ecdsa.PrivateKey{key_i, key_j}) + } + } + + // Build triple keyPairs + for _, key_i := range keys { + for _, key_j := range keys { + for _, key_k := range keys { + keyPairs = append(keyPairs, []*ecdsa.PrivateKey{key_i, key_j, key_k}) + } + } + } + + // Build empty indexPairs + indexPairs = append(indexPairs, []uint8{}) + + // Build single indexPairs + for _, ind := range indexes { + indexPairs = append(indexPairs, []uint8{ind}) + } + + // Build double indexPairs + for _, ind_i := range indexes { + for _, ind_j := range indexes { + indexPairs = append(indexPairs, []uint8{ind_i, ind_j}) + } + } + + // Build triple keyPairs + for _, ind_i := range indexes { + for _, ind_j := range indexes { + for _, ind_k := range indexes { + indexPairs = append(indexPairs, []uint8{ind_i, ind_j, ind_k}) + } + } + } + + // Build out the fuzzTest cases + for _, keyPair := range keyPairs { + for _, indexPair := range indexPairs { + if len(keyPair) == len(indexPair) { + result := false + + for _, allow := range allows { + if reflect.DeepEqual(allow.indexPair, indexPair) && reflect.DeepEqual(allow.keyPair, keyPair) { + result = true + break + } + } + + test := test{label: "A", keyOrder: keyPair, addrs: addrs, indexOrder: indexPair, result: result} + tests = append(tests, test) + } + } + } + + // Run the fuzzTest cases + for _, tc := range tests { + t.Run(tc.label, func(t *testing.T) { + vaa := getVaa() + + for i, key := range tc.keyOrder { + vaa.AddSignature(key, tc.indexOrder[i]) + } + + /* Fuzz Debugging + * Tell us what keys and indexes were used (for debug when/if we have a failure case) + */ + if vaa.VerifySignatures(tc.addrs) != tc.result { + if len(tc.keyOrder) == 0 { + t.Logf("Key Order %v\n", tc.keyOrder) + } else { + keyIndex := []uint8{} + for i, key_i := range keys { + for _, key_k := range tc.keyOrder { + if key_i == key_k { + keyIndex = append(keyIndex, uint8(i)) + } + } + } + t.Logf("Key Order %v\n", keyIndex) + } + t.Logf("Index Order %v\n", tc.indexOrder) + + } + + assert.Equal(t, tc.result, vaa.VerifySignatures(tc.addrs)) + }) + } } func TestStringToAddress(t *testing.T) {