From e82db71837a3cae9deb7019f451554deb6b7c1a8 Mon Sep 17 00:00:00 2001 From: Jason Matthyser Date: Thu, 17 Oct 2024 16:21:45 +0200 Subject: [PATCH] Replace Guardian Key with abstracted Guardian Signer (#4120) * node: add guardiansigner node/pkg * node: replace use of guardian key with guardian signer * node: replace use of vaa.AddSigner with guardian signer * node: add nolint for armor import and fix test * node: handle error returned from signing * apply draft review suggestions * apply pr reviews * apply pr reviews * apply pr reviews * apply pr reviews --------- Co-authored-by: pleasew8t --- node/cmd/guardiand/adminclient.go | 15 +-- node/cmd/guardiand/node.go | 49 ++++++--- node/hack/accountant/send_obs.go | 45 ++++----- node/pkg/accountant/accountant.go | 10 +- node/pkg/accountant/accountant_test.go | 9 +- node/pkg/accountant/submit_obs.go | 10 +- node/pkg/adminrpc/adminserver.go | 22 ++++- node/pkg/adminrpc/adminserver_test.go | 67 ++++++++----- node/pkg/governor/governor_monitoring.go | 16 +-- node/pkg/guardiansigner/filesigner.go | 99 +++++++++++++++++++ node/pkg/guardiansigner/generatedsigner.go | 62 ++++++++++++ node/pkg/guardiansigner/guardiansigner.go | 68 +++++++++++++ .../pkg/guardiansigner/guardiansigner_test.go | 98 ++++++++++++++++++ node/pkg/node/adminServiceRunnable.go | 8 +- node/pkg/node/node.go | 12 +-- node/pkg/node/node_test.go | 14 +-- node/pkg/node/options.go | 8 +- node/pkg/p2p/ccq_p2p.go | 11 +-- node/pkg/p2p/p2p.go | 26 ++--- node/pkg/p2p/p2p_test.go | 27 +++-- node/pkg/p2p/run_params.go | 16 +-- node/pkg/p2p/run_params_test.go | 10 +- node/pkg/p2p/watermark_test.go | 13 ++- node/pkg/processor/benchmark_test.go | 32 +++--- node/pkg/processor/message.go | 5 +- node/pkg/processor/processor.go | 12 +-- 26 files changed, 568 insertions(+), 196 deletions(-) create mode 100644 node/pkg/guardiansigner/filesigner.go create mode 100644 node/pkg/guardiansigner/generatedsigner.go create mode 100644 node/pkg/guardiansigner/guardiansigner.go create mode 100644 node/pkg/guardiansigner/guardiansigner_test.go diff --git a/node/cmd/guardiand/adminclient.go b/node/cmd/guardiand/adminclient.go index 0a1aa381d9..ee087431aa 100644 --- a/node/cmd/guardiand/adminclient.go +++ b/node/cmd/guardiand/adminclient.go @@ -22,7 +22,7 @@ import ( "github.com/spf13/pflag" "golang.org/x/crypto/sha3" - "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" "github.com/wormhole-foundation/wormhole/sdk" @@ -102,7 +102,7 @@ var AdminCmd = &cobra.Command{ } var AdminClientSignWormchainAddress = &cobra.Command{ - Use: "sign-wormchain-address [/path/to/guardianKey] [wormchain-validator-address]", + Use: "sign-wormchain-address [vaa-signer-uri] [wormchain-validator-address]", Short: "Sign a wormchain validator address. Only sign the address that you control the key for and will be for your validator.", RunE: runSignWormchainValidatorAddress, Args: cobra.ExactArgs(2), @@ -236,22 +236,25 @@ func getPublicRPCServiceClient(ctx context.Context, addr string) (*grpc.ClientCo } func runSignWormchainValidatorAddress(cmd *cobra.Command, args []string) error { - guardianKeyPath := args[0] + guardianSignerUri := args[0] wormchainAddress := args[1] if !strings.HasPrefix(wormchainAddress, "wormhole") || strings.HasPrefix(wormchainAddress, "wormholeval") { return errors.New("must provide a bech32 address that has 'wormhole' prefix") } - gk, err := common.LoadGuardianKey(guardianKeyPath, *unsafeDevnetMode) + + guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(guardianSignerUri, *unsafeDevnetMode) if err != nil { - return fmt.Errorf("failed to load guardian key: %w", err) + return fmt.Errorf("failed to create new guardian signer from uri: %w", err) } + addr, err := types.GetFromBech32(wormchainAddress, "wormhole") if err != nil { return fmt.Errorf("failed to decode wormchain address: %w", err) } + // Hash and sign address addrHash := crypto.Keccak256Hash(sdk.SignedWormchainAddressPrefix, addr) - sig, err := crypto.Sign(addrHash[:], gk) + sig, err := guardianSigner.Sign(addrHash.Bytes()) if err != nil { return fmt.Errorf("failed to sign wormchain address: %w", err) } diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go index e0362dffaf..63b7ba24d8 100644 --- a/node/cmd/guardiand/node.go +++ b/node/cmd/guardiand/node.go @@ -13,6 +13,7 @@ import ( "syscall" "time" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/watchers" "github.com/certusone/wormhole/node/pkg/watchers/ibc" ethcrypto "github.com/ethereum/go-ethereum/crypto" @@ -62,8 +63,9 @@ var ( statusAddr *string - guardianKeyPath *string - solanaContract *string + guardianKeyPath *string + guardianSignerUri *string + solanaContract *string ethRPC *string ethContract *string @@ -269,7 +271,8 @@ func init() { dataDir = NodeCmd.Flags().String("dataDir", "", "Data directory") - guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key (required)") + guardianKeyPath = NodeCmd.Flags().String("guardianKey", "", "Path to guardian key") + guardianSignerUri = NodeCmd.Flags().String("guardianSignerUri", "", "Guardian signer URI") solanaContract = NodeCmd.Flags().String("solanaContract", "", "Address of the Solana program (required)") ethRPC = node.RegisterFlagWithValidationOrFail(NodeCmd, "ethRPC", "Ethereum RPC URL", "ws://eth-devnet:8545", []string{"ws", "wss"}) @@ -628,7 +631,22 @@ func runNode(cmd *cobra.Command, args []string) { logger.Fatal("Please specify --nodeKey") } if *guardianKeyPath == "" { - logger.Fatal("Please specify --guardianKey") + // This if-statement is nested, since checking if both are empty at once will always result in the else-branch + // being executed if at least one is specified. For example, in the case where the signer URI is specified and + // the guardianKeyPath not, then the else-statement will create an empty `file://` URI. + if *guardianSignerUri == "" { + logger.Fatal("Please specify --guardianKey or --guardianSignerUri") + } + } else { + // To avoid confusion, require that only guardianKey or guardianSignerUri can be specified + if *guardianSignerUri != "" { + logger.Fatal("Please only specify --guardianKey or --guardianSignerUri") + } + + // If guardianKeyPath is set, set guardianSignerUri to the file signer URI, pointing to guardianKeyPath. + // This ensures that the signer-abstracted guardian has backwards compatibility with guardians that would + // just like to ignore the new guardianSignerUri altogether. + *guardianSignerUri = fmt.Sprintf("file://%s", *guardianKeyPath) } if *adminSocketPath == "" { logger.Fatal("Please specify --adminSocket") @@ -650,20 +668,23 @@ func runNode(cmd *cobra.Command, args []string) { // In devnet mode, we generate a deterministic guardian key and write it to disk. if env == common.UnsafeDevNet { - err := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath) - if err != nil { - logger.Fatal("failed to generate devnet guardian key", zap.Error(err)) + // Only if the signer is file-based should we generate the deterministic key and write it to disk + if st, _, _ := guardiansigner.ParseSignerUri(*guardianSignerUri); st == guardiansigner.FileSignerType { + err := devnet.GenerateAndStoreDevnetGuardianKey(*guardianKeyPath) + if err != nil { + logger.Fatal("failed to generate devnet guardian key", zap.Error(err)) + } } } - // Load guardian key - gk, err := common.LoadGuardianKey(*guardianKeyPath, env == common.UnsafeDevNet) + // Create the Guardian Signer + guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(*guardianSignerUri, env == common.UnsafeDevNet) if err != nil { - logger.Fatal("failed to load guardian key", zap.Error(err)) + logger.Fatal("failed to create a new guardian signer", zap.Error(err)) } logger.Info("Loaded guardian key", zap.String( - "address", ethcrypto.PubkeyToAddress(gk.PublicKey).String())) + "address", ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String())) // Load p2p private key var p2pKey libp2p_crypto.PrivKey @@ -719,7 +740,7 @@ func runNode(cmd *cobra.Command, args []string) { labels := map[string]string{ "node_name": *nodeName, "node_key": peerID.String(), - "guardian_addr": ethcrypto.PubkeyToAddress(gk.PublicKey).String(), + "guardian_addr": ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String(), "network": *p2pNetworkID, "version": version.Version(), } @@ -1060,7 +1081,7 @@ func runNode(cmd *cobra.Command, args []string) { info.PromRemoteURL = *promRemoteURL info.Labels = map[string]string{ "node_name": *nodeName, - "guardian_addr": ethcrypto.PubkeyToAddress(gk.PublicKey).String(), + "guardian_addr": ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()).String(), "network": *p2pNetworkID, "version": version.Version(), "product": "wormhole", @@ -1585,7 +1606,7 @@ func runNode(cmd *cobra.Command, args []string) { guardianNode := node.NewGuardianNode( env, - gk, + guardianSigner, ) guardianOptions := []*node.GuardianOption{ diff --git a/node/hack/accountant/send_obs.go b/node/hack/accountant/send_obs.go index 70382899d1..63bcb8d94c 100644 --- a/node/hack/accountant/send_obs.go +++ b/node/hack/accountant/send_obs.go @@ -5,12 +5,12 @@ package main import ( "context" - "crypto/ecdsa" "encoding/hex" "time" "github.com/certusone/wormhole/node/pkg/accountant" "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/wormconn" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -26,7 +26,7 @@ func main() { wormchainURL := string("localhost:9090") wormchainKeyPath := string("./dev.wormchain.key") contract := "wormhole14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9srrg465" - guardianKeyPath := string("./dev.guardian.key") + guardianSignerUri := string("file://dev.guardian.key") wormchainKey, err := wormconn.LoadWormchainPrivKey(wormchainKeyPath, "test0000") if err != nil { @@ -44,8 +44,9 @@ func main() { zap.String("senderAddress", wormchainConn.SenderAddress()), ) - logger.Info("Loading guardian key", zap.String("guardianKeyPath", guardianKeyPath)) - gk, err := common.LoadGuardianKey(guardianKeyPath, true) + logger.Info("Initializing guardian signer", zap.String("guardianSignerUri", guardianSignerUri)) + guardianSigner, err := guardiansigner.NewGuardianSignerFromUri(guardianSignerUri, true) + if err != nil { logger.Fatal("failed to load guardian key", zap.Error(err)) } @@ -53,31 +54,31 @@ func main() { sequence := uint64(time.Now().Unix()) timestamp := time.Now() - if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Submit should succeed") { + if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Submit should succeed") { return } - if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Already commited should succeed") { + if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16", timestamp, sequence, true, "Already commited should succeed") { return } sequence += 10 - if !testSubmit(ctx, logger, gk, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c17", timestamp, sequence, false, "Bad emitter address should fail") { + if !testSubmit(ctx, logger, guardianSigner, wormchainConn, contract, "0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c17", timestamp, sequence, false, "Bad emitter address should fail") { return } sequence += 10 - if !testBatch(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) { + if !testBatch(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) { return } sequence += 10 - if !testBatchWithcommitted(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) { + if !testBatchWithcommitted(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) { return } sequence += 10 - if !testBatchWithDigestError(ctx, logger, gk, wormchainConn, contract, timestamp, sequence) { + if !testBatchWithDigestError(ctx, logger, guardianSigner, wormchainConn, contract, timestamp, sequence) { return } @@ -87,7 +88,7 @@ func main() { func testSubmit( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, wormchainConn *wormconn.ClientConn, contract string, emitterAddressStr string, @@ -112,7 +113,7 @@ func testSubmit( } msgs := []*common.MessagePublication{&msg} - txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to broadcast Observation request", zap.String("test", tag), zap.Error(err)) return false @@ -151,7 +152,7 @@ func testSubmit( func testBatch( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, wormchainConn *wormconn.ClientConn, contract string, timestamp time.Time, @@ -191,7 +192,7 @@ func testBatch( } msgs = append(msgs, &msg2) - txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to broadcast Observation request", zap.Error(err)) return false @@ -229,7 +230,7 @@ func testBatch( func testBatchWithcommitted( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, wormchainConn *wormconn.ClientConn, contract string, timestamp time.Time, @@ -256,7 +257,7 @@ func testBatchWithcommitted( } msgs = append(msgs, &msg1) - _, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + _, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to submit initial observation that should work", zap.Error(err)) return false @@ -283,7 +284,7 @@ func testBatchWithcommitted( msg3 := msg1 msgs = append(msgs, &msg3) - txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to broadcast Observation request", zap.Error(err)) return false @@ -321,7 +322,7 @@ func testBatchWithcommitted( func testBatchWithDigestError( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, wormchainConn *wormconn.ClientConn, contract string, timestamp time.Time, @@ -348,7 +349,7 @@ func testBatchWithDigestError( } msgs = append(msgs, &msg1) - _, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + _, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to submit initial observation that should work", zap.Error(err)) return false @@ -376,7 +377,7 @@ func testBatchWithDigestError( msg3.Nonce = msg3.Nonce + 1 msgs = append(msgs, &msg3) - txResp, err := submit(ctx, logger, gk, wormchainConn, contract, msgs) + txResp, err := submit(ctx, logger, guardianSigner, wormchainConn, contract, msgs) if err != nil { logger.Error("failed to submit second observation that should work", zap.Error(err)) return false @@ -429,7 +430,7 @@ func testBatchWithDigestError( func submit( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, wormchainConn *wormconn.ClientConn, contract string, msgs []*common.MessagePublication, @@ -437,5 +438,5 @@ func submit( gsIndex := uint32(0) guardianIndex := uint32(0) - return accountant.SubmitObservationsToContract(ctx, logger, gk, gsIndex, guardianIndex, wormchainConn, contract, accountant.SubmitObservationPrefix, msgs) + return accountant.SubmitObservationsToContract(ctx, logger, guardianSigner, gsIndex, guardianIndex, wormchainConn, contract, accountant.SubmitObservationPrefix, msgs) } diff --git a/node/pkg/accountant/accountant.go b/node/pkg/accountant/accountant.go index 5c8a5cb844..b2eb99f97b 100644 --- a/node/pkg/accountant/accountant.go +++ b/node/pkg/accountant/accountant.go @@ -8,13 +8,13 @@ package accountant import ( "context" - "crypto/ecdsa" "fmt" "sync" "time" "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/supervisor" sdktypes "github.com/cosmos/cosmos-sdk/types" @@ -83,7 +83,7 @@ type Accountant struct { wsUrl string wormchainConn AccountantWormchainConn enforceFlag bool - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner gst *common.GuardianSetState guardianAddr ethCommon.Address msgChan chan<- *common.MessagePublication @@ -120,7 +120,7 @@ func NewAccountant( enforceFlag bool, // whether or not accountant should be enforced nttContract string, // the address of the NTT smart contract on wormchain nttWormchainConn AccountantWormchainConn, // used for communicating with the NTT smart contract - gk *ecdsa.PrivateKey, // the guardian key used for signing observation requests + guardianSigner guardiansigner.GuardianSigner, // the guardian signer used for signing observation requests gst *common.GuardianSetState, // used to get the current guardian set index when sending observation requests msgChan chan<- *common.MessagePublication, // the channel where transfers received by the accountant runnable should be published env common.Environment, // Controls the set of token bridges to be monitored @@ -134,9 +134,9 @@ func NewAccountant( wsUrl: wsUrl, wormchainConn: wormchainConn, enforceFlag: enforceFlag, - gk: gk, + guardianSigner: guardianSigner, gst: gst, - guardianAddr: ethCrypto.PubkeyToAddress(gk.PublicKey), + guardianAddr: ethCrypto.PubkeyToAddress(guardianSigner.PublicKey()), msgChan: msgChan, tokenBridges: make(validEmitters), pendingTransfers: make(map[string]*pendingEntry), diff --git a/node/pkg/accountant/accountant_test.go b/node/pkg/accountant/accountant_test.go index ce01849053..aae4d50b71 100644 --- a/node/pkg/accountant/accountant_test.go +++ b/node/pkg/accountant/accountant_test.go @@ -21,6 +21,7 @@ import ( "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/devnet" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/wormhole-foundation/wormhole/sdk/vaa" "go.uber.org/zap" @@ -101,7 +102,9 @@ func newAccountantForTest( ) *Accountant { var db db.MockAccountantDB - gk := devnet.InsecureDeterministicEcdsaKeyByIndex(ethCrypto.S256(), uint64(0)) + pk := devnet.InsecureDeterministicEcdsaKeyByIndex(ethCrypto.S256(), uint64(0)) + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(pk) + require.NoError(t, err) gst := common.NewGuardianSetState(nil) gs := &common.GuardianSet{Keys: []ethCommon.Address{ethCommon.HexToAddress("0xbeFA429d57cD18b7F8A4d91A2da9AB4AF05d0FBe")}} @@ -123,13 +126,13 @@ func newAccountantForTest( accountantCheckEnabled, "", nil, - gk, + guardianSigner, gst, acctWriteC, env, ) - err := acct.Start(ctx) + err = acct.Start(ctx) require.NoError(t, err) return acct } diff --git a/node/pkg/accountant/submit_obs.go b/node/pkg/accountant/submit_obs.go index 7da1695170..2a4658f6f3 100644 --- a/node/pkg/accountant/submit_obs.go +++ b/node/pkg/accountant/submit_obs.go @@ -2,7 +2,6 @@ package accountant import ( "context" - "crypto/ecdsa" "encoding/hex" "encoding/json" "errors" @@ -11,10 +10,9 @@ import ( "time" "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/wormhole-foundation/wormhole/sdk/vaa" - ethCrypto "github.com/ethereum/go-ethereum/crypto" - wasmdtypes "github.com/CosmWasm/wasmd/x/wasm/types" sdktypes "github.com/cosmos/cosmos-sdk/types" sdktx "github.com/cosmos/cosmos-sdk/types/tx" @@ -200,7 +198,7 @@ func (sb SignatureBytes) MarshalJSON() ([]byte, error) { // submitObservationsToContract makes a call to the smart contract to submit a batch of observation requests. // It should be called from a go routine because it can block. func (acct *Accountant) submitObservationsToContract(msgs []*common.MessagePublication, gsIndex uint32, guardianIndex uint32, wormchainConn AccountantWormchainConn, contract string, prefix []byte, tag string) { - txResp, err := SubmitObservationsToContract(acct.ctx, acct.logger, acct.gk, gsIndex, guardianIndex, wormchainConn, contract, prefix, msgs) + txResp, err := SubmitObservationsToContract(acct.ctx, acct.logger, acct.guardianSigner, gsIndex, guardianIndex, wormchainConn, contract, prefix, msgs) if err != nil { // This means the whole batch failed. They will all get retried the next audit cycle. acct.logger.Error(fmt.Sprintf("failed to submit any observations in batch to %s", tag), zap.Int("numMsgs", len(msgs)), zap.Error(err)) @@ -299,7 +297,7 @@ func (acct *Accountant) handleTransferError(msgId string, errText string, logTex func SubmitObservationsToContract( ctx context.Context, logger *zap.Logger, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, gsIndex uint32, guardianIndex uint32, wormchainConn AccountantWormchainConn, @@ -344,7 +342,7 @@ func SubmitObservationsToContract( return nil, fmt.Errorf("failed to sign accountant Observation request: %w", err) } - sigBytes, err := ethCrypto.Sign(digest.Bytes(), gk) + sigBytes, err := guardianSigner.Sign(digest.Bytes()) if err != nil { return nil, fmt.Errorf("failed to sign accountant Observation request: %w", err) } diff --git a/node/pkg/adminrpc/adminserver.go b/node/pkg/adminrpc/adminserver.go index 76426a556d..85e2b68b70 100644 --- a/node/pkg/adminrpc/adminserver.go +++ b/node/pkg/adminrpc/adminserver.go @@ -3,7 +3,6 @@ package adminrpc import ( "bytes" "context" - "crypto/ecdsa" "encoding/base64" "encoding/hex" "encoding/json" @@ -19,6 +18,7 @@ import ( "sync" "time" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/watchers/evm/connectors" "github.com/holiman/uint256" "github.com/prometheus/client_golang/prometheus" @@ -42,6 +42,7 @@ import ( ) const maxResetReleaseTimerDays = 7 +const ecdsaSignatureLength = 65 var ( vaaInjectionsTotal = promauto.NewCounter( @@ -61,7 +62,7 @@ type nodePrivilegedService struct { governor *governor.ChainGovernor evmConnector connectors.Connector gsCache sync.Map - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner guardianAddress ethcommon.Address rpcMap map[string]string } @@ -74,7 +75,7 @@ func NewPrivService( signedInC chan<- *gossipv1.SignedVAAWithQuorum, governor *governor.ChainGovernor, evmConnector connectors.Connector, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, guardianAddress ethcommon.Address, rpcMap map[string]string, @@ -87,7 +88,7 @@ func NewPrivService( signedInC: signedInC, governor: governor, evmConnector: evmConnector, - gk: gk, + guardianSigner: guardianSigner, guardianAddress: guardianAddress, rpcMap: rpcMap, } @@ -1162,7 +1163,18 @@ func (s *nodePrivilegedService) SignExistingVAA(ctx context.Context, req *nodev1 } // Add local signature - newVAA.AddSignature(s.gk, uint8(localGuardianIndex)) + sig, err := s.guardianSigner.Sign(v.SigningDigest().Bytes()) + if err != nil { + panic(err) + } + + signature := [ecdsaSignatureLength]byte{} + copy(signature[:], sig) + + newVAA.Signatures = append(v.Signatures, &vaa.Signature{ + Index: uint8(localGuardianIndex), + Signature: signature, + }) // Sort VAA signatures by guardian ID slices.SortFunc(newVAA.Signatures, func(a, b *vaa.Signature) int { diff --git a/node/pkg/adminrpc/adminserver_test.go b/node/pkg/adminrpc/adminserver_test.go index f1167f71c1..f5f6ed6e98 100644 --- a/node/pkg/adminrpc/adminserver_test.go +++ b/node/pkg/adminrpc/adminserver_test.go @@ -4,13 +4,13 @@ package adminrpc import ( "bytes" "context" - "crypto/ecdsa" "testing" "time" wh_common "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1" "github.com/certusone/wormhole/node/pkg/watchers/evm/connectors" "github.com/certusone/wormhole/node/pkg/watchers/evm/connectors/ethabi" @@ -88,14 +88,14 @@ func (c mockEVMConnector) SubscribeNewHead(ctx context.Context, ch chan<- *types panic("unimplemented") } -func generateGS(num int) (keys []*ecdsa.PrivateKey, addrs []common.Address) { +func generateGuardianSigners(num int) (signers []guardiansigner.GuardianSigner, addrs []common.Address) { for i := 0; i < num; i++ { - key, err := ethcrypto.GenerateKey() + signer, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) if err != nil { panic(err) } - keys = append(keys, key) - addrs = append(addrs, ethcrypto.PubkeyToAddress(key.PublicKey)) + signers = append(signers, signer) + addrs = append(addrs, ethcrypto.PubkeyToAddress(signer.PublicKey())) } return } @@ -107,7 +107,8 @@ func addrsToHexStrings(addrs []common.Address) (out []string) { return } -func generateMockVAA(gsIndex uint32, gsKeys []*ecdsa.PrivateKey) []byte { +func generateMockVAA(gsIndex uint32, signers []guardiansigner.GuardianSigner, t *testing.T) []byte { + t.Helper() v := &vaa.VAA{ Version: 1, GuardianSetIndex: gsIndex, @@ -120,8 +121,20 @@ func generateMockVAA(gsIndex uint32, gsKeys []*ecdsa.PrivateKey) []byte { EmitterAddress: vaa.Address{}, Payload: []byte("test"), } - for i, key := range gsKeys { - v.AddSignature(key, uint8(i)) + for i, signer := range signers { + sig, err := signer.Sign(v.SigningDigest().Bytes()) + if err != nil { + require.NoError(t, err) + } + + signature := [ecdsaSignatureLength]byte{} + copy(signature[:], sig) + + v.Signatures = append(v.Signatures, &vaa.Signature{ + Index: uint8(i), + Signature: signature, + }) + } vBytes, err := v.Marshal() @@ -132,7 +145,7 @@ func generateMockVAA(gsIndex uint32, gsKeys []*ecdsa.PrivateKey) []byte { } func setupAdminServerForVAASigning(gsIndex uint32, gsAddrs []common.Address) *nodePrivilegedService { - gk, err := ethcrypto.GenerateKey() + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) if err != nil { panic(err) } @@ -150,8 +163,8 @@ func setupAdminServerForVAASigning(gsIndex uint32, gsAddrs []common.Address) *no signedInC: nil, governor: nil, evmConnector: connector, - gk: gk, - guardianAddress: ethcrypto.PubkeyToAddress(gk.PublicKey), + guardianSigner: guardianSigner, + guardianAddress: ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()), } } @@ -167,10 +180,10 @@ func TestSignExistingVAA_NoVAA(t *testing.T) { } func TestSignExistingVAA_NotGuardian(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys) + v := generateMockVAA(0, signers, t) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ Vaa: v, @@ -181,10 +194,10 @@ func TestSignExistingVAA_NotGuardian(t *testing.T) { } func TestSignExistingVAA_InvalidVAA(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys[:2]) + v := generateMockVAA(0, signers[:2], t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -196,10 +209,10 @@ func TestSignExistingVAA_InvalidVAA(t *testing.T) { } func TestSignExistingVAA_DuplicateGuardian(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) gsAddrs = append(gsAddrs, s.guardianAddress) @@ -212,14 +225,14 @@ func TestSignExistingVAA_DuplicateGuardian(t *testing.T) { } func TestSignExistingVAA_AlreadyGuardian(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) s.evmConnector = mockEVMConnector{ guardianAddrs: append(gsAddrs, s.guardianAddress), guardianSetIndex: 0, } - v := generateMockVAA(0, append(gsKeys, s.gk)) + v := generateMockVAA(0, append(signers, s.guardianSigner), t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -231,10 +244,10 @@ func TestSignExistingVAA_AlreadyGuardian(t *testing.T) { } func TestSignExistingVAA_NotAFutureGuardian(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys) + v := generateMockVAA(0, signers, t) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ Vaa: v, @@ -245,10 +258,10 @@ func TestSignExistingVAA_NotAFutureGuardian(t *testing.T) { } func TestSignExistingVAA_CantReachQuorum(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) _, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -260,10 +273,10 @@ func TestSignExistingVAA_CantReachQuorum(t *testing.T) { } func TestSignExistingVAA_Valid(t *testing.T) { - gsKeys, gsAddrs := generateGS(5) + signers, gsAddrs := generateGuardianSigners(5) s := setupAdminServerForVAASigning(0, gsAddrs) - v := generateMockVAA(0, gsKeys) + v := generateMockVAA(0, signers, t) gsAddrs = append(gsAddrs, s.guardianAddress) res, err := s.SignExistingVAA(context.Background(), &nodev1.SignExistingVAARequest{ @@ -273,7 +286,7 @@ func TestSignExistingVAA_Valid(t *testing.T) { }) require.NoError(t, err) - v2 := generateMockVAA(1, append(gsKeys, s.gk)) + v2 := generateMockVAA(1, append(signers, s.guardianSigner), t) require.Equal(t, v2, res.Vaa) } @@ -332,7 +345,7 @@ func newNodePrivilegedServiceForGovernorTests() *nodePrivilegedService { signedInC: nil, governor: gov, evmConnector: nil, - gk: nil, + guardianSigner: nil, guardianAddress: common.Address{}, } } diff --git a/node/pkg/governor/governor_monitoring.go b/node/pkg/governor/governor_monitoring.go index 142a90dd8e..fe7118988a 100644 --- a/node/pkg/governor/governor_monitoring.go +++ b/node/pkg/governor/governor_monitoring.go @@ -75,11 +75,11 @@ package governor import ( - "crypto/ecdsa" "fmt" "sort" "time" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -487,7 +487,7 @@ var ( }) ) -func (gov *ChainGovernor) CollectMetrics(hb *gossipv1.Heartbeat, sendC chan<- []byte, gk *ecdsa.PrivateKey, ourAddr ethCommon.Address) { +func (gov *ChainGovernor) CollectMetrics(hb *gossipv1.Heartbeat, sendC chan<- []byte, guardianSigner guardiansigner.GuardianSigner, ourAddr ethCommon.Address) { gov.mutex.Lock() defer gov.mutex.Unlock() @@ -547,12 +547,12 @@ func (gov *ChainGovernor) CollectMetrics(hb *gossipv1.Heartbeat, sendC chan<- [] metricTotalEnqueuedVAAs.Set(float64(totalPending)) if startTime.After(gov.nextConfigPublishTime) { - gov.publishConfig(hb, sendC, gk, ourAddr) + gov.publishConfig(hb, sendC, guardianSigner, ourAddr) gov.nextConfigPublishTime = startTime.Add(time.Minute * time.Duration(5)) } if startTime.After(gov.nextStatusPublishTime) { - gov.publishStatus(hb, sendC, startTime, gk, ourAddr) + gov.publishStatus(hb, sendC, startTime, guardianSigner, ourAddr) gov.nextStatusPublishTime = startTime.Add(time.Minute) } } @@ -560,7 +560,7 @@ func (gov *ChainGovernor) CollectMetrics(hb *gossipv1.Heartbeat, sendC chan<- [] var governorMessagePrefixConfig = []byte("governor_config_000000000000000000|") var governorMessagePrefixStatus = []byte("governor_status_000000000000000000|") -func (gov *ChainGovernor) publishConfig(hb *gossipv1.Heartbeat, sendC chan<- []byte, gk *ecdsa.PrivateKey, ourAddr ethCommon.Address) { +func (gov *ChainGovernor) publishConfig(hb *gossipv1.Heartbeat, sendC chan<- []byte, guardianSigner guardiansigner.GuardianSigner, ourAddr ethCommon.Address) { chains := make([]*gossipv1.ChainGovernorConfig_Chain, 0) // Iterate deterministically by accessing keys from this slice instead of the chainEntry map directly for _, cid := range gov.chainIds { @@ -600,7 +600,7 @@ func (gov *ChainGovernor) publishConfig(hb *gossipv1.Heartbeat, sendC chan<- []b digest := ethCrypto.Keccak256Hash(append(governorMessagePrefixConfig, b...)) - sig, err := ethCrypto.Sign(digest.Bytes(), gk) + sig, err := guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } @@ -620,7 +620,7 @@ func (gov *ChainGovernor) publishConfig(hb *gossipv1.Heartbeat, sendC chan<- []b sendC <- b } -func (gov *ChainGovernor) publishStatus(hb *gossipv1.Heartbeat, sendC chan<- []byte, startTime time.Time, gk *ecdsa.PrivateKey, ourAddr ethCommon.Address) { +func (gov *ChainGovernor) publishStatus(hb *gossipv1.Heartbeat, sendC chan<- []byte, startTime time.Time, guardianSigner guardiansigner.GuardianSigner, ourAddr ethCommon.Address) { chains := make([]*gossipv1.ChainGovernorStatus_Chain, 0) numEnqueued := 0 for chainId, ce := range gov.chains { @@ -685,7 +685,7 @@ func (gov *ChainGovernor) publishStatus(hb *gossipv1.Heartbeat, sendC chan<- []b digest := ethCrypto.Keccak256Hash(append(governorMessagePrefixStatus, b...)) - sig, err := ethCrypto.Sign(digest.Bytes(), gk) + sig, err := guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } diff --git a/node/pkg/guardiansigner/filesigner.go b/node/pkg/guardiansigner/filesigner.go new file mode 100644 index 0000000000..991eb4674a --- /dev/null +++ b/node/pkg/guardiansigner/filesigner.go @@ -0,0 +1,99 @@ +package guardiansigner + +import ( + "crypto/ecdsa" + "errors" + "fmt" + "io" + "os" + + "github.com/ethereum/go-ethereum/crypto" + ethcrypto "github.com/ethereum/go-ethereum/crypto" + "google.golang.org/protobuf/proto" + + nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1" + "golang.org/x/crypto/openpgp/armor" // nolint +) + +type FileSigner struct { + keyPath string + privateKey *ecdsa.PrivateKey +} + +const ( + GuardianKeyArmoredBlock = "WORMHOLE GUARDIAN PRIVATE KEY" +) + +func NewFileSigner(unsafeDevMode bool, signerKeyPath string) (*FileSigner, error) { + fileSigner := &FileSigner{ + keyPath: signerKeyPath, + } + + f, err := os.Open(signerKeyPath) + if err != nil { + return nil, fmt.Errorf("failed to open file: %w", err) + } + + p, err := armor.Decode(f) + if err != nil { + return nil, fmt.Errorf("failed to read armored file: %w", err) + } + + if p.Type != GuardianKeyArmoredBlock { + return nil, fmt.Errorf("invalid block type: %s", p.Type) + } + + b, err := io.ReadAll(p.Body) + if err != nil { + return nil, fmt.Errorf("failed to read file: %w", err) + } + + var m nodev1.GuardianKey + err = proto.Unmarshal(b, &m) + if err != nil { + return nil, fmt.Errorf("failed to deserialize protobuf: %w", err) + } + + if !unsafeDevMode && m.UnsafeDeterministicKey { + return nil, errors.New("refusing to use deterministic key in production") + } + + gk, err := ethcrypto.ToECDSA(m.Data) + if err != nil { + return nil, fmt.Errorf("failed to deserialize raw key data: %w", err) + } + + fileSigner.privateKey = gk + return fileSigner, nil +} + +func (fs *FileSigner) Sign(hash []byte) ([]byte, error) { + + // Sign the hash + sig, err := crypto.Sign(hash, fs.privateKey) + + if err != nil { + return nil, fmt.Errorf("failed to sign hash: %w", err) + } + + return sig, nil +} + +func (fs *FileSigner) PublicKey() ecdsa.PublicKey { + return fs.privateKey.PublicKey +} + +func (fs *FileSigner) Verify(sig []byte, hash []byte) (bool, error) { + + recoveredPubKey, err := ethcrypto.SigToPub(hash, sig) + + if err != nil { + return false, err + } + + // Need to use fs.privateKey.Public() instead of PublicKey to ensure + // the returned public key has the right interface for Equal() to work. + fsPubkey := fs.privateKey.Public() + + return recoveredPubKey.Equal(fsPubkey), nil +} diff --git a/node/pkg/guardiansigner/generatedsigner.go b/node/pkg/guardiansigner/generatedsigner.go new file mode 100644 index 0000000000..503fccd5ea --- /dev/null +++ b/node/pkg/guardiansigner/generatedsigner.go @@ -0,0 +1,62 @@ +package guardiansigner + +import ( + "crypto/ecdsa" + "crypto/rand" + "fmt" + + ethcrypto "github.com/ethereum/go-ethereum/crypto" +) + +// The GeneratedSigner is a signer that is intended for use in tests. It uses the private +// key supplied to GenerateSignerWithPrivatekeyUnsafe, or defaults to generating a random +// private key if no private key is supplied. +type GeneratedSigner struct { + privateKey *ecdsa.PrivateKey +} + +func NewGeneratedSigner(key *ecdsa.PrivateKey) (*GeneratedSigner, error) { + if key == nil { + privateKey, err := ecdsa.GenerateKey(ethcrypto.S256(), rand.Reader) + return &GeneratedSigner{privateKey: privateKey}, err + } else { + return &GeneratedSigner{privateKey: key}, nil + } + +} + +func (gs *GeneratedSigner) Sign(hash []byte) (sig []byte, err error) { + // Sign the hash + sig, err = ethcrypto.Sign(hash, gs.privateKey) + + if err != nil { + return nil, fmt.Errorf("failed to sign: %w", err) + } + + return sig, nil +} + +func (gs *GeneratedSigner) PublicKey() (pubKey ecdsa.PublicKey) { + return gs.privateKey.PublicKey +} + +func (gs *GeneratedSigner) Verify(sig []byte, hash []byte) (valid bool, err error) { + recoveredPubKey, err := ethcrypto.SigToPub(hash, sig) + + if err != nil { + return false, err + } + + // Need to use gs.privateKey.Public() instead of PublicKey to ensure + // the returned public key has the right interface for Equal() to work. + fsPubkey := gs.privateKey.Public() + + return recoveredPubKey.Equal(fsPubkey), nil +} + +// This function is meant to be a helper function that returns a guardian signer for tests +// that simply require a private key. The caller can specify a private key to be used, or +// pass nil to have `NewGeneratedSigner` generate a random private key. +func GenerateSignerWithPrivatekeyUnsafe(key *ecdsa.PrivateKey) (GuardianSigner, error) { + return NewGeneratedSigner(key) +} diff --git a/node/pkg/guardiansigner/guardiansigner.go b/node/pkg/guardiansigner/guardiansigner.go new file mode 100644 index 0000000000..057f9c03bf --- /dev/null +++ b/node/pkg/guardiansigner/guardiansigner.go @@ -0,0 +1,68 @@ +package guardiansigner + +import ( + "crypto/ecdsa" + "errors" + "fmt" + "strings" +) + +// The types of guardian signers that are supported +type SignerType int + +const ( + InvalidSignerType SignerType = iota + // file:// + FileSignerType +) + +// GuardianSigner interface +type GuardianSigner interface { + // Sign expects a keccak256 hash that needs to be signed. + Sign(hash []byte) (sig []byte, err error) + // PublicKey returns the ECDSA public key of the signer. Note that this should not + // be confused with the EVM address. + PublicKey() (pubKey ecdsa.PublicKey) + // Verify is a convenience function that recovers a public key from the sig/hash pair, + // and checks if the public key matches that of the guardian signer. + Verify(sig []byte, hash []byte) (valid bool, err error) +} + +func NewGuardianSignerFromUri(signerUri string, unsafeDevMode bool) (GuardianSigner, error) { + + // Get the signer type + signerType, signerKeyConfig, err := ParseSignerUri(signerUri) + + if err != nil { + return nil, err + } + + switch signerType { + case FileSignerType: + return NewFileSigner(unsafeDevMode, signerKeyConfig) + default: + return nil, errors.New("unsupported guardian signer type") + } +} + +func ParseSignerUri(signerUri string) (signerType SignerType, signerKeyConfig string, err error) { + // Split the URI using the standard "://" scheme separator + signerUriSplit := strings.Split(signerUri, "://") + + // This check is purely for ensuring that there is actually a path separator. + if len(signerUriSplit) < 2 { + return InvalidSignerType, "", errors.New("no path separator in guardian signer URI") + } + + typeStr := signerUriSplit[0] + // Rejoin the remainder of the split URI as the configuration for the guardian signer + // implementation. The remainder of the split is joined using the URI scheme separator. + keyConfig := strings.Join(signerUriSplit[1:], "://") + + switch typeStr { + case "file": + return FileSignerType, keyConfig, nil + default: + return InvalidSignerType, "", fmt.Errorf("unsupported guardian signer type: %s", typeStr) + } +} diff --git a/node/pkg/guardiansigner/guardiansigner_test.go b/node/pkg/guardiansigner/guardiansigner_test.go new file mode 100644 index 0000000000..5e660357ed --- /dev/null +++ b/node/pkg/guardiansigner/guardiansigner_test.go @@ -0,0 +1,98 @@ +package guardiansigner + +import ( + "testing" + + "github.com/ethereum/go-ethereum/crypto" + ethcrypto "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseSignerUri(t *testing.T) { + tests := []struct { + label string + path string + expectedType SignerType + }{ + {label: "RandomText", path: "RandomText", expectedType: InvalidSignerType}, + {label: "ArbitraryUriScheme", path: "arb://data", expectedType: InvalidSignerType}, + // File + {label: "FileURI", path: "file://whatever", expectedType: FileSignerType}, + {label: "FileUriNoSchemeSeparator", path: "filewhatever", expectedType: InvalidSignerType}, + {label: "FileUriMultipleSchemeSeparators", path: "file://testing://this://", expectedType: FileSignerType}, + {label: "FileUriTraversal", path: "file://../../../file", expectedType: FileSignerType}, + } + + for _, testcase := range tests { + t.Run(testcase.label, func(t *testing.T) { + signerType, _, err := ParseSignerUri(testcase.path) + + assert.Equal(t, signerType, testcase.expectedType) + + // If the signer type is Invalid, then an error should have been returned. + if testcase.expectedType == InvalidSignerType { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + }) + } +} + +func TestFileSignerNonExistentFile(t *testing.T) { + nonexistentFileUri := "file://somewhere/on/disk.key" + + // Attempt to generate signer using top-level generator + _, err := NewGuardianSignerFromUri(nonexistentFileUri, true) + assert.Error(t, err) + + // Attempt to generate signer using NewFileSigner + _, keyPath, _ := ParseSignerUri(nonexistentFileUri) + fileSigner, err := NewFileSigner(true, keyPath) + assert.Nil(t, fileSigner) + assert.Error(t, err) +} + +func TestFileSigner(t *testing.T) { + fileUri := "file://../query/dev.guardian.key" + expectedEthAddress := "0xbeFA429d57cD18b7F8A4d91A2da9AB4AF05d0FBe" + + // For each file signer generation attempt, check: + // That the signer returned is not nil + // No error is returned + // The public key returned by PublicKey(), converted to an eth address, + // matches the expected address. + + // Attempt to generate signer using top-level generator + fileSigner1, err := NewGuardianSignerFromUri(fileUri, true) + require.NoError(t, err) + assert.NotNil(t, fileSigner1) + assert.Equal(t, ethcrypto.PubkeyToAddress(fileSigner1.PublicKey()).Hex(), expectedEthAddress) + + // Attempt to generate signer using NewFileSigner + signerType, keyPath, err := ParseSignerUri(fileUri) + assert.Equal(t, signerType, FileSignerType) + require.NoError(t, err) + + fileSigner2, err := NewFileSigner(true, keyPath) + require.NoError(t, err) + assert.NotNil(t, fileSigner2) + assert.Equal(t, ethcrypto.PubkeyToAddress(fileSigner2.PublicKey()).Hex(), expectedEthAddress) + + // Sign some arbitrary data + data := crypto.Keccak256Hash([]byte("data")) + sig, err := fileSigner1.Sign(data.Bytes()) + assert.NoError(t, err) + + // Verify the signature + valid, _ := fileSigner1.Verify(sig, data.Bytes()) + assert.True(t, valid) + + // Use generated signature with incorrect hash, should fail + arbitraryHash := crypto.Keccak256Hash([]byte("arbitrary hash data")) + valid, _ = fileSigner1.Verify(sig, arbitraryHash.Bytes()) + assert.False(t, valid) + +} diff --git a/node/pkg/node/adminServiceRunnable.go b/node/pkg/node/adminServiceRunnable.go index 44e1e79bc8..60717b5ab0 100644 --- a/node/pkg/node/adminServiceRunnable.go +++ b/node/pkg/node/adminServiceRunnable.go @@ -2,7 +2,6 @@ package node import ( "context" - "crypto/ecdsa" "fmt" "net" "os" @@ -12,6 +11,7 @@ import ( "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" @@ -33,7 +33,7 @@ func adminServiceRunnable( db *db.Database, gst *common.GuardianSetState, gov *governor.ChainGovernor, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, ethRpc *string, ethContract *string, rpcMap map[string]string, @@ -89,8 +89,8 @@ func adminServiceRunnable( signedInC, gov, evmConnector, - gk, - ethcrypto.PubkeyToAddress(gk.PublicKey), + guardianSigner, + ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()), rpcMap, ) diff --git a/node/pkg/node/node.go b/node/pkg/node/node.go index 3bb934d2de..fab5bedce8 100644 --- a/node/pkg/node/node.go +++ b/node/pkg/node/node.go @@ -2,13 +2,13 @@ package node import ( "context" - "crypto/ecdsa" "fmt" "github.com/certusone/wormhole/node/pkg/accountant" "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/gwrelayer" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/query" @@ -63,8 +63,8 @@ type G struct { rootCtxCancel context.CancelFunc env common.Environment - // keys - gk *ecdsa.PrivateKey + // guardianSigner is the abstracted GuardianSigner that signs VAAs, or any other guardian-related information + guardianSigner guardiansigner.GuardianSigner // components db *db.Database @@ -110,11 +110,11 @@ type G struct { func NewGuardianNode( env common.Environment, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, ) *G { g := G{ - env: env, - gk: gk, + env: env, + guardianSigner: guardianSigner, } return &g } diff --git a/node/pkg/node/node_test.go b/node/pkg/node/node_test.go index 9c74be7af4..378f2acc0f 100644 --- a/node/pkg/node/node_test.go +++ b/node/pkg/node/node_test.go @@ -5,7 +5,6 @@ import ( "bytes" "context" "crypto/ecdsa" - "crypto/rand" "encoding/binary" "errors" "fmt" @@ -26,6 +25,7 @@ import ( "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/devnet" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/processor" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" @@ -78,7 +78,7 @@ type mockGuardian struct { p2pKey libp2p_crypto.PrivKey MockObservationC chan *common.MessagePublication MockSetC chan *common.GuardianSet - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner guardianAddr eth_common.Address ready bool config *guardianConfig @@ -111,8 +111,8 @@ func newMockGuardianSet(t testing.TB, testId uint, n int) []*mockGuardian { gs := make([]*mockGuardian, n) for i := 0; i < n; i++ { - // generate guardian key - gk, err := ecdsa.GenerateKey(eth_crypto.S256(), rand.Reader) + // generate guardian signer + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) if err != nil { panic(err) } @@ -121,8 +121,8 @@ func newMockGuardianSet(t testing.TB, testId uint, n int) []*mockGuardian { p2pKey: devnet.DeterministicP2PPrivKeyByIndex(int64(i)), MockObservationC: make(chan *common.MessagePublication), MockSetC: make(chan *common.GuardianSet), - gk: gk, - guardianAddr: eth_crypto.PubkeyToAddress(gk.PublicKey), + guardianSigner: guardianSigner, + guardianAddr: eth_crypto.PubkeyToAddress(guardianSigner.PublicKey()), config: createGuardianConfig(t, testId, uint(i)), } } @@ -201,7 +201,7 @@ func mockGuardianRunnable(t testing.TB, gs []*mockGuardian, mockGuardianIndex ui guardianNode := NewGuardianNode( env, - gs[mockGuardianIndex].gk, + gs[mockGuardianIndex].guardianSigner, ) if err = supervisor.Run(ctx, "g", guardianNode.Run(ctxCancel, guardianOptions...)); err != nil { diff --git a/node/pkg/node/options.go b/node/pkg/node/options.go index 48f2e8c5af..c5c5b014b7 100644 --- a/node/pkg/node/options.go +++ b/node/pkg/node/options.go @@ -82,7 +82,7 @@ func GuardianOptionP2P( g.rootCtxCancel, p2p.WithGuardianOptions( nodeName, - g.gk, + g.guardianSigner, g.obsvC, g.batchObsvC.writeC, signedInC, @@ -206,7 +206,7 @@ func GuardianOptionAccountant( enforcing, nttContract, nttWormchainConn, - g.gk, + g.guardianSigner, g.gst, g.acctC.writeC, g.env, @@ -502,7 +502,7 @@ func GuardianOptionAdminService(socketPath string, ethRpc *string, ethContract * g.db, g.gst, g.gov, - g.gk, + g.guardianSigner, ethRpc, ethContract, rpcMap, @@ -593,7 +593,7 @@ func GuardianOptionProcessor(networkId string) *GuardianOption { g.batchObsvC.readC, g.obsvReqSendC.writeC, g.signedInC.readC, - g.gk, + g.guardianSigner, g.gst, g.gov, g.acct, diff --git a/node/pkg/p2p/ccq_p2p.go b/node/pkg/p2p/ccq_p2p.go index 66b4acdd70..51e7056576 100644 --- a/node/pkg/p2p/ccq_p2p.go +++ b/node/pkg/p2p/ccq_p2p.go @@ -2,14 +2,13 @@ package p2p import ( "context" - "crypto/ecdsa" "errors" "fmt" "strings" "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/query" - ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "google.golang.org/protobuf/proto" @@ -71,7 +70,7 @@ func newCcqRunP2p( func (ccq *ccqP2p) run( ctx context.Context, priv crypto.PrivKey, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, p2pNetworkID string, bootstrapPeers string, port uint, @@ -174,7 +173,7 @@ func (ccq *ccqP2p) run( }) common.StartRunnable(ctx, errC, false, "ccqp2p_publisher", func(ctx context.Context) error { - return ccq.publisher(ctx, gk, queryResponseReadC) + return ccq.publisher(ctx, guardianSigner, queryResponseReadC) }) ccq.logger.Info("Node has been started", zap.String("peer_id", ccq.h.ID().String()), zap.String("addrs", fmt.Sprintf("%v", ccq.h.Addrs()))) @@ -235,7 +234,7 @@ func (ccq *ccqP2p) listener(ctx context.Context, signedQueryReqC chan<- *gossipv } } -func (ccq *ccqP2p) publisher(ctx context.Context, gk *ecdsa.PrivateKey, queryResponseReadC <-chan *query.QueryResponsePublication) error { +func (ccq *ccqP2p) publisher(ctx context.Context, guardianSigner guardiansigner.GuardianSigner, queryResponseReadC <-chan *query.QueryResponsePublication) error { for { select { case <-ctx.Done(): @@ -247,7 +246,7 @@ func (ccq *ccqP2p) publisher(ctx context.Context, gk *ecdsa.PrivateKey, queryRes continue } digest := query.GetQueryResponseDigestFromBytes(msgBytes) - sig, err := ethcrypto.Sign(digest.Bytes(), gk) + sig, err := guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } diff --git a/node/pkg/p2p/p2p.go b/node/pkg/p2p/p2p.go index 486496e833..fc6f2d8c98 100644 --- a/node/pkg/p2p/p2p.go +++ b/node/pkg/p2p/p2p.go @@ -2,7 +2,6 @@ package p2p import ( "context" - "crypto/ecdsa" "encoding/hex" "errors" "fmt" @@ -11,6 +10,7 @@ import ( "time" "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/version" eth_common "github.com/ethereum/go-ethereum/common" ethcrypto "github.com/ethereum/go-ethereum/crypto" @@ -466,7 +466,7 @@ func Run(params *RunParams) func(ctx context.Context) error { if params.ccqEnabled { ccqErrC := make(chan error) ccq := newCcqRunP2p(logger, params.ccqAllowedPeers, params.components) - if err := ccq.run(ctx, params.priv, params.gk, params.networkID, params.ccqBootstrapPeers, params.ccqPort, params.signedQueryReqC, params.queryResponseReadC, ccqErrC); err != nil { + if err := ccq.run(ctx, params.priv, params.guardianSigner, params.networkID, params.ccqBootstrapPeers, params.ccqPort, params.signedQueryReqC, params.queryResponseReadC, ccqErrC); err != nil { return fmt.Errorf("failed to start p2p for CCQ: %w", err) } defer ccq.close() @@ -501,7 +501,7 @@ func Run(params *RunParams) func(ctx context.Context) error { // Start up heartbeating if it is enabled. if params.nodeName != "" { go func() { - ourAddr := ethcrypto.PubkeyToAddress(params.gk.PublicKey) + ourAddr := ethcrypto.PubkeyToAddress(params.guardianSigner.PublicKey()) ctr := int64(0) // Guardians should send out their first heartbeat immediately to speed up test runs. @@ -581,12 +581,12 @@ func Run(params *RunParams) func(ctx context.Context) error { collectNodeMetrics(ourAddr, h.ID(), heartbeat) if params.gov != nil { - params.gov.CollectMetrics(heartbeat, params.gossipControlSendC, params.gk, ourAddr) + params.gov.CollectMetrics(heartbeat, params.gossipControlSendC, params.guardianSigner, ourAddr) } msg := gossipv1.GossipMessage{ Message: &gossipv1.GossipMessage_SignedHeartbeat{ - SignedHeartbeat: createSignedHeartbeat(params.gk, heartbeat), + SignedHeartbeat: createSignedHeartbeat(params.guardianSigner, heartbeat), }, } @@ -679,7 +679,7 @@ func Run(params *RunParams) func(ctx context.Context) error { // Sign the observation request using our node's guardian key. digest := signedObservationRequestDigest(b) - sig, err := ethcrypto.Sign(digest.Bytes(), params.gk) + sig, err := params.guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } @@ -687,7 +687,7 @@ func Run(params *RunParams) func(ctx context.Context) error { sReq := &gossipv1.SignedObservationRequest{ ObservationRequest: b, Signature: sig, - GuardianAddr: ethcrypto.PubkeyToAddress(params.gk.PublicKey).Bytes(), + GuardianAddr: ethcrypto.PubkeyToAddress(params.guardianSigner.PublicKey()).Bytes(), } envelope := &gossipv1.GossipMessage{ @@ -807,7 +807,7 @@ func Run(params *RunParams) func(ctx context.Context) error { zap.String("from", envelope.GetFrom().String())) } else { guardianAddr := eth_common.BytesToAddress(s.GuardianAddr) - if params.gk == nil || guardianAddr != ethcrypto.PubkeyToAddress(params.gk.PublicKey) { + if params.guardianSigner == nil || guardianAddr != ethcrypto.PubkeyToAddress(params.guardianSigner.PublicKey()) { prevPeerId, ok := params.components.ProtectedHostByGuardianKey[guardianAddr] if ok { if prevPeerId != peerId { @@ -1034,7 +1034,7 @@ func Run(params *RunParams) func(ctx context.Context) error { zap.String("from", envelope.GetFrom().String())) } else { guardianAddr := eth_common.BytesToAddress(s.GuardianAddr) - if params.gk == nil || guardianAddr != ethcrypto.PubkeyToAddress(params.gk.PublicKey) { + if params.guardianSigner == nil || guardianAddr != ethcrypto.PubkeyToAddress(params.guardianSigner.PublicKey()) { prevPeerId, ok := params.components.ProtectedHostByGuardianKey[guardianAddr] if ok { if prevPeerId != peerId { @@ -1161,17 +1161,17 @@ func Run(params *RunParams) func(ctx context.Context) error { } } -func createSignedHeartbeat(gk *ecdsa.PrivateKey, heartbeat *gossipv1.Heartbeat) *gossipv1.SignedHeartbeat { - ourAddr := ethcrypto.PubkeyToAddress(gk.PublicKey) +func createSignedHeartbeat(guardianSigner guardiansigner.GuardianSigner, heartbeat *gossipv1.Heartbeat) *gossipv1.SignedHeartbeat { + ourAddr := ethcrypto.PubkeyToAddress(guardianSigner.PublicKey()) b, err := proto.Marshal(heartbeat) if err != nil { panic(err) } - // Sign the heartbeat using our node's guardian key. + // Sign the heartbeat using our node's guardian signer. digest := heartbeatDigest(b) - sig, err := ethcrypto.Sign(digest.Bytes(), gk) + sig, err := guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } diff --git a/node/pkg/p2p/p2p_test.go b/node/pkg/p2p/p2p_test.go index e65670eed8..c77aa1eaf6 100644 --- a/node/pkg/p2p/p2p_test.go +++ b/node/pkg/p2p/p2p_test.go @@ -1,8 +1,6 @@ package p2p import ( - "crypto/ecdsa" - "crypto/rand" "testing" "time" @@ -10,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" node_common "github.com/certusone/wormhole/node/pkg/common" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" @@ -19,7 +18,7 @@ func TestSignedHeartbeat(t *testing.T) { type testCase struct { timestamp int64 - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner heartbeatGuardianAddr string fromP2pId peer.ID p2pNodeId []byte @@ -28,17 +27,17 @@ func TestSignedHeartbeat(t *testing.T) { // define the tests - gk, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) assert.NoError(t, err) - gAddr := crypto.PubkeyToAddress(gk.PublicKey) + gAddr := crypto.PubkeyToAddress(guardianSigner.PublicKey()) fromP2pId, err := peer.Decode("12D3KooWSgMXkhzTbKTeupHYmyG7sFJ5LpVreQcwVnX8RD7LBpy9") assert.NoError(t, err) p2pNodeId, err := fromP2pId.Marshal() assert.NoError(t, err) - gk2, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + guardianSigner2, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) assert.NoError(t, err) - gAddr2 := crypto.PubkeyToAddress(gk2.PublicKey) + gAddr2 := crypto.PubkeyToAddress(guardianSigner2.PublicKey()) fromP2pId2, err := peer.Decode("12D3KooWDZVv7BhZ8yFLkarNdaSWaB43D6UbQwExJ8nnGAEmfHcU") assert.NoError(t, err) p2pNodeId2, err := fromP2pId2.Marshal() @@ -48,7 +47,7 @@ func TestSignedHeartbeat(t *testing.T) { // happy case { timestamp: time.Now().UnixNano(), - gk: gk, + guardianSigner: guardianSigner, heartbeatGuardianAddr: gAddr.String(), fromP2pId: fromP2pId, p2pNodeId: p2pNodeId, @@ -57,7 +56,7 @@ func TestSignedHeartbeat(t *testing.T) { // guardian signed a heartbeat for another guardian { timestamp: time.Now().UnixNano(), - gk: gk, + guardianSigner: guardianSigner, heartbeatGuardianAddr: gAddr2.String(), fromP2pId: fromP2pId, p2pNodeId: p2pNodeId, @@ -66,7 +65,7 @@ func TestSignedHeartbeat(t *testing.T) { // old heartbeat { timestamp: time.Now().Add(-time.Hour).UnixNano(), - gk: gk, + guardianSigner: guardianSigner, heartbeatGuardianAddr: gAddr.String(), fromP2pId: fromP2pId, p2pNodeId: p2pNodeId, @@ -75,7 +74,7 @@ func TestSignedHeartbeat(t *testing.T) { // heartbeat from the distant future { timestamp: time.Now().Add(time.Hour).UnixNano(), - gk: gk, + guardianSigner: guardianSigner, heartbeatGuardianAddr: gAddr.String(), fromP2pId: fromP2pId, p2pNodeId: p2pNodeId, @@ -84,7 +83,7 @@ func TestSignedHeartbeat(t *testing.T) { // mismatched peer id { timestamp: time.Now().UnixNano(), - gk: gk, + guardianSigner: guardianSigner, heartbeatGuardianAddr: gAddr.String(), fromP2pId: fromP2pId, p2pNodeId: p2pNodeId2, @@ -95,7 +94,7 @@ func TestSignedHeartbeat(t *testing.T) { testFunc := func(t *testing.T, tc testCase) { - addr := crypto.PubkeyToAddress(gk.PublicKey) + addr := crypto.PubkeyToAddress(guardianSigner.PublicKey()) heartbeat := &gossipv1.Heartbeat{ NodeName: "someNode", @@ -109,7 +108,7 @@ func TestSignedHeartbeat(t *testing.T) { P2PNodeId: tc.p2pNodeId, } - s := createSignedHeartbeat(gk, heartbeat) + s := createSignedHeartbeat(guardianSigner, heartbeat) gs := &node_common.GuardianSet{ Keys: []common.Address{addr}, Index: 1, diff --git a/node/pkg/p2p/run_params.go b/node/pkg/p2p/run_params.go index 558117d6fa..4cdd05539a 100644 --- a/node/pkg/p2p/run_params.go +++ b/node/pkg/p2p/run_params.go @@ -2,12 +2,12 @@ package p2p import ( "context" - "crypto/ecdsa" "errors" "github.com/certusone/wormhole/node/pkg/accountant" "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/query" "github.com/libp2p/go-libp2p/core/crypto" @@ -46,7 +46,7 @@ type ( // The following options are guardian specific. Set with `WithGuardianOptions`. nodeName string - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner gossipControlSendC chan []byte gossipAttestationSendC chan []byte gossipVaaSendC chan []byte @@ -176,7 +176,7 @@ func WithDisableHeartbeatVerify(disableHeartbeatVerify bool) RunOpt { // WithGuardianOptions is used to set options that are only meaningful to the guardian. func WithGuardianOptions( nodeName string, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, obsvRecvC chan<- *common.MsgWithTimeStamp[gossipv1.SignedObservation], batchObsvRecvC chan<- *common.MsgWithTimeStamp[gossipv1.SignedObservationBatch], signedIncomingVaaRecvC chan<- *gossipv1.SignedVAAWithQuorum, @@ -200,7 +200,7 @@ func WithGuardianOptions( ) RunOpt { return func(p *RunParams) error { p.nodeName = nodeName - p.gk = gk + p.guardianSigner = guardianSigner p.obsvRecvC = obsvRecvC p.batchObsvRecvC = batchObsvRecvC p.signedIncomingVaaRecvC = signedIncomingVaaRecvC @@ -243,13 +243,13 @@ func (p *RunParams) verify() error { return errors.New("rootCtxCancel may not be nil") } if p.nodeName != "" { // Heartbeating is enabled. - if p.gk == nil { - return errors.New("if heart beating is enabled, gk may not be nil") + if p.guardianSigner == nil { + return errors.New("if heart beating is enabled, guardianSigner may not be nil") } } if p.obsvReqSendC != nil { - if p.gk == nil { - return errors.New("if obsvReqSendC is not nil, gk may not be nil") + if p.guardianSigner == nil { + return errors.New("if obsvReqSendC is not nil, vs may not be nil") } } return nil diff --git a/node/pkg/p2p/run_params_test.go b/node/pkg/p2p/run_params_test.go index 46ff215419..99903d79fc 100644 --- a/node/pkg/p2p/run_params_test.go +++ b/node/pkg/p2p/run_params_test.go @@ -2,16 +2,14 @@ package p2p import ( "context" - "crypto/ecdsa" - "crypto/rand" "testing" "github.com/certusone/wormhole/node/pkg/accountant" "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/query" - "github.com/ethereum/go-ethereum/crypto" p2pcrypto "github.com/libp2p/go-libp2p/core/crypto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -136,9 +134,9 @@ func TestRunParamsWithGuardianOptions(t *testing.T) { _, rootCtxCancel := context.WithCancel(context.Background()) defer rootCtxCancel() - gk, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) require.NoError(t, err) - require.NotNil(t, gk) + require.NotNil(t, guardianSigner) obsvC := make(chan<- *common.MsgWithTimeStamp[gossipv1.SignedObservation], 42) batchObsvC := make(chan<- *common.MsgWithTimeStamp[gossipv1.SignedObservationBatch], 42) @@ -171,7 +169,7 @@ func TestRunParamsWithGuardianOptions(t *testing.T) { rootCtxCancel, WithGuardianOptions( nodeName, - gk, + guardianSigner, obsvC, batchObsvC, signedInC, diff --git a/node/pkg/p2p/watermark_test.go b/node/pkg/p2p/watermark_test.go index 5ca92e38fc..ce4dfadaca 100644 --- a/node/pkg/p2p/watermark_test.go +++ b/node/pkg/p2p/watermark_test.go @@ -2,8 +2,6 @@ package p2p import ( "context" - "crypto/ecdsa" - "crypto/rand" "fmt" "testing" "time" @@ -14,6 +12,7 @@ import ( "github.com/certusone/wormhole/node/pkg/accountant" node_common "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/supervisor" "github.com/ethereum/go-ethereum/crypto" @@ -36,7 +35,7 @@ type G struct { vaaSendC chan []byte signedInC chan *gossipv1.SignedVAAWithQuorum priv p2pcrypto.PrivKey - gk *ecdsa.PrivateKey + guardianSigner guardiansigner.GuardianSigner gst *node_common.GuardianSetState networkID string bootstrapPeers string @@ -59,7 +58,7 @@ func NewG(t *testing.T, nodeName string) *G { panic(err) } - guardianpriv, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) if err != nil { panic(err) } @@ -76,7 +75,7 @@ func NewG(t *testing.T, nodeName string) *G { vaaSendC: make(chan []byte, cs), signedInC: make(chan *gossipv1.SignedVAAWithQuorum, cs), priv: p2ppriv, - gk: guardianpriv, + guardianSigner: guardianSigner, gst: node_common.NewGuardianSetState(nil), nodeName: nodeName, disableHeartbeatVerify: false, @@ -120,7 +119,7 @@ func TestWatermark(t *testing.T) { gs[i].components.Port = uint(LOCAL_P2P_PORTRANGE_START + i) gs[i].networkID = "/wormhole/localdev" - guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].gk.PublicKey)) + guardianset.Keys = append(guardianset.Keys, crypto.PubkeyToAddress(gs[i].guardianSigner.PublicKey())) id, err := p2ppeer.IDFromPublicKey(gs[0].priv.GetPublic()) require.NoError(t, err) @@ -182,7 +181,7 @@ func startGuardian(t *testing.T, ctx context.Context, g *G) { g.rootCtxCancel, WithGuardianOptions( g.nodeName, - g.gk, + g.guardianSigner, g.obsvC, g.batchObsvC, g.signedInC, diff --git a/node/pkg/processor/benchmark_test.go b/node/pkg/processor/benchmark_test.go index 4acf840fea..0995317940 100644 --- a/node/pkg/processor/benchmark_test.go +++ b/node/pkg/processor/benchmark_test.go @@ -2,8 +2,6 @@ package processor import ( "context" - "crypto/ecdsa" - "crypto/rand" "fmt" "os" "runtime/pprof" @@ -12,6 +10,7 @@ import ( "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/gwrelayer" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" @@ -123,7 +122,7 @@ type ProcessorData struct { gossipVaaSendC chan []byte emitterChain vaa.ChainID emitterAddress vaa.Address - guardianKeys []*ecdsa.PrivateKey + guardianSigners []guardiansigner.GuardianSigner guardianAddrs [][]byte } @@ -136,19 +135,19 @@ func createProcessorForTest(b *testing.B, numVAAs int, ctx context.Context, db * b.Helper() logger := zap.NewNop() - var ourKey *ecdsa.PrivateKey + var ourSigner guardiansigner.GuardianSigner keys := []ethCommon.Address{} - guardianKeys := []*ecdsa.PrivateKey{} + guardianSigners := []guardiansigner.GuardianSigner{} guardianAddrs := [][]byte{} for count := 0; count < 19; count++ { - gk, err := ecdsa.GenerateKey(crypto.S256(), rand.Reader) + guardianSigner, err := guardiansigner.GenerateSignerWithPrivatekeyUnsafe(nil) require.NoError(b, err) - keys = append(keys, crypto.PubkeyToAddress(gk.PublicKey)) - guardianKeys = append(guardianKeys, gk) - guardianAddrs = append(guardianAddrs, crypto.PubkeyToAddress(gk.PublicKey).Bytes()) - if ourKey == nil { - ourKey = gk + keys = append(keys, crypto.PubkeyToAddress(guardianSigner.PublicKey())) + guardianSigners = append(guardianSigners, guardianSigner) + guardianAddrs = append(guardianAddrs, crypto.PubkeyToAddress(guardianSigner.PublicKey()).Bytes()) + if count == 0 { + ourSigner = guardianSigner } } @@ -167,20 +166,20 @@ func createProcessorForTest(b *testing.B, numVAAs int, ctx context.Context, db * gossipVaaSendC: make(chan []byte, numVAAs+100), emitterChain: vaa.ChainIDEthereum, emitterAddress: emitterAddress, - guardianKeys: guardianKeys, + guardianSigners: guardianSigners, guardianAddrs: guardianAddrs, } p := &Processor{ gossipAttestationSendC: pd.gossipAttestationSendC, gossipVaaSendC: pd.gossipVaaSendC, - gk: ourKey, + guardianSigner: ourSigner, gs: gs, gst: gst, db: db, logger: logger, state: &aggregationState{observationMap{}}, - ourAddr: crypto.PubkeyToAddress(ourKey.PublicKey), + ourAddr: crypto.PubkeyToAddress(ourSigner.PublicKey()), pythnetVaas: make(map[string]PythNetVaaEntry), updatedVAAs: make(map[string]*updateVaaEntry), gatewayRelayer: gwRelayer, @@ -230,8 +229,9 @@ func (pd *ProcessorData) createObservation(b *testing.B, guardianIdx int, k *com // Generate digest of the unsigned VAA. digest := v.SigningDigest() - // Sign the digest using our node's guardian key. - signature, err := crypto.Sign(digest.Bytes(), pd.guardianKeys[guardianIdx]) + // Sign the digest using our node's guardian signer + guardianSigner := pd.guardianSigners[guardianIdx] + signature, err := guardianSigner.Sign(digest.Bytes()) require.NoError(b, err) return &gossipv1.Observation{ diff --git a/node/pkg/processor/message.go b/node/pkg/processor/message.go index e58f7dfc04..ed5946a330 100644 --- a/node/pkg/processor/message.go +++ b/node/pkg/processor/message.go @@ -10,7 +10,6 @@ import ( "github.com/prometheus/client_golang/prometheus/promauto" ethCommon "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/crypto" "go.uber.org/zap" "go.uber.org/zap/zapcore" @@ -69,8 +68,8 @@ func (p *Processor) handleMessage(k *common.MessagePublication) { digest := v.SigningDigest() hash := hex.EncodeToString(digest.Bytes()) - // Sign the digest using our node's guardian key. - signature, err := crypto.Sign(digest.Bytes(), p.gk) + // Sign the digest using the node's GuardianSigner + signature, err := p.guardianSigner.Sign(digest.Bytes()) if err != nil { panic(err) } diff --git a/node/pkg/processor/processor.go b/node/pkg/processor/processor.go index 1a2ad16438..e483996505 100644 --- a/node/pkg/processor/processor.go +++ b/node/pkg/processor/processor.go @@ -2,7 +2,6 @@ package processor import ( "context" - "crypto/ecdsa" "encoding/hex" "fmt" "sync" @@ -10,6 +9,7 @@ import ( "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/governor" + "github.com/certusone/wormhole/node/pkg/guardiansigner" "github.com/certusone/wormhole/node/pkg/p2p" ethcommon "github.com/ethereum/go-ethereum/common" @@ -126,8 +126,8 @@ type Processor struct { // signedInC is a channel of inbound signed VAA observations from p2p signedInC <-chan *gossipv1.SignedVAAWithQuorum - // gk is the node's guardian private key - gk *ecdsa.PrivateKey + // guardianSigner is the guardian node's signer + guardianSigner guardiansigner.GuardianSigner logger *zap.Logger @@ -229,7 +229,7 @@ func NewProcessor( batchObsvC <-chan *common.MsgWithTimeStamp[gossipv1.SignedObservationBatch], obsvReqSendC chan<- *gossipv1.ObservationRequest, signedInC <-chan *gossipv1.SignedVAAWithQuorum, - gk *ecdsa.PrivateKey, + guardianSigner guardiansigner.GuardianSigner, gst *common.GuardianSetState, g *governor.ChainGovernor, acct *accountant.Accountant, @@ -247,13 +247,13 @@ func NewProcessor( batchObsvC: batchObsvC, obsvReqSendC: obsvReqSendC, signedInC: signedInC, - gk: gk, + guardianSigner: guardianSigner, gst: gst, db: db, logger: supervisor.Logger(ctx), state: &aggregationState{observationMap{}}, - ourAddr: crypto.PubkeyToAddress(gk.PublicKey), + ourAddr: crypto.PubkeyToAddress(guardianSigner.PublicKey()), governor: g, acct: acct, acctReadC: acctReadC,