diff --git a/WORKSPACE b/WORKSPACE index f68ffb1420f8..c72ee31482ee 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1655,6 +1655,6 @@ go_repository( name = "com_github_wealdtech_eth2_signer_api", build_file_proto_mode = "disable_global", importpath = "github.com/wealdtech/eth2-signer-api", - sum = "h1:fqJYjKwG/FeUAJYYiZblIP6agiz3WWB+Hxpw85Fnr5I=", - version = "v1.0.1", + sum = "h1:AL4bRJDW6lyRc0ROPruVTEHt7Xs+EV2lRBPen2plOr8=", + version = "v1.2.0", ) diff --git a/validator/keymanager/BUILD.bazel b/validator/keymanager/BUILD.bazel index 1db746bd9e8d..044a18233e02 100644 --- a/validator/keymanager/BUILD.bazel +++ b/validator/keymanager/BUILD.bazel @@ -36,6 +36,7 @@ go_test( "direct_interop_test.go", "direct_test.go", "opts_test.go", + "remote_internal_test.go", "remote_test.go", "wallet_test.go", ], diff --git a/validator/keymanager/remote.go b/validator/keymanager/remote.go index 675a0acbb093..3d8f571643f8 100644 --- a/validator/keymanager/remote.go +++ b/validator/keymanager/remote.go @@ -5,7 +5,10 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" + "fmt" "io/ioutil" + "regexp" + "strings" "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" @@ -16,6 +19,11 @@ import ( "google.golang.org/grpc/credentials" ) +const ( + // maxMessageSize is the largest message that can be received over GRPC. Set to 8MB, which handles ~128K keys. + maxMessageSize = 8 * 1024 * 1024 +) + // Remote is a key manager that accesses a remote wallet daemon. type Remote struct { paths []string @@ -115,6 +123,8 @@ func NewRemoteWallet(input string) (KeyManager, string, error) { grpcOpts := []grpc.DialOption{ // Require TLS with client certificate. grpc.WithTransportCredentials(clientCreds), + // Receive large messages without erroring. + grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(maxMessageSize)), } conn, err := grpc.Dial(opts.Location, grpcOpts...) @@ -167,9 +177,9 @@ func (km *Remote) SignGeneric(pubKey [48]byte, root [32]byte, domain [32]byte) ( return nil, err } switch resp.State { - case pb.SignState_DENIED: + case pb.ResponseState_DENIED: return nil, ErrDenied - case pb.SignState_FAILED: + case pb.ResponseState_FAILED: return nil, ErrCannotSign } return bls.SignatureFromBytes(resp.Signature) @@ -198,9 +208,9 @@ func (km *Remote) SignProposal(pubKey [48]byte, domain [32]byte, data *ethpb.Bea return nil, err } switch resp.State { - case pb.SignState_DENIED: + case pb.ResponseState_DENIED: return nil, ErrDenied - case pb.SignState_FAILED: + case pb.ResponseState_FAILED: return nil, ErrCannotSign } return bls.SignatureFromBytes(resp.Signature) @@ -236,9 +246,9 @@ func (km *Remote) SignAttestation(pubKey [48]byte, domain [32]byte, data *ethpb. return nil, err } switch resp.State { - case pb.SignState_DENIED: + case pb.ResponseState_DENIED: return nil, ErrDenied - case pb.SignState_FAILED: + case pb.ResponseState_FAILED: return nil, ErrCannotSign } return bls.SignatureFromBytes(resp.Signature) @@ -250,12 +260,30 @@ func (km *Remote) RefreshValidatingKeys() error { listAccountsReq := &pb.ListAccountsRequest{ Paths: km.paths, } - accountsResp, err := listerClient.ListAccounts(context.Background(), listAccountsReq) + resp, err := listerClient.ListAccounts(context.Background(), listAccountsReq) if err != nil { - panic(err) + return err + } + if resp.State == pb.ResponseState_DENIED { + return errors.New("attempt to fetch keys denied") + } + if resp.State == pb.ResponseState_FAILED { + return errors.New("attempt to fetch keys failed") } - accounts := make(map[[48]byte]*accountInfo, len(accountsResp.Accounts)) - for _, account := range accountsResp.Accounts { + verificationRegexes := pathsToVerificationRegexes(km.paths) + accounts := make(map[[48]byte]*accountInfo, len(resp.Accounts)) + for _, account := range resp.Accounts { + verified := false + for _, verificationRegex := range verificationRegexes { + if verificationRegex.Match([]byte(account.Name)) { + verified = true + break + } + } + if !verified { + log.WithField("path", account.Name).Warn("Received unwanted account from server; ignoring") + continue + } account := &accountInfo{ Name: account.Name, PubKey: account.PublicKey, @@ -265,3 +293,35 @@ func (km *Remote) RefreshValidatingKeys() error { km.accounts = accounts return nil } + +// pathsToVerificationRegexes turns path specifiers in to regexes to ensure accounts we are given are good. +func pathsToVerificationRegexes(paths []string) []*regexp.Regexp { + regexes := make([]*regexp.Regexp, 0, len(paths)) + for _, path := range paths { + log := log.WithField("path", path) + parts := strings.Split(path, "/") + if len(parts) == 0 || len(parts[0]) == 0 { + log.Debug("Invalid path") + continue + } + if len(parts) == 1 { + parts = append(parts, ".*") + } + if strings.HasPrefix(parts[1], "^") { + parts[1] = parts[1][1:] + } + var specifier string + if strings.HasSuffix(parts[1], "$") { + specifier = fmt.Sprintf("^%s/%s", parts[0], parts[1]) + } else { + specifier = fmt.Sprintf("^%s/%s$", parts[0], parts[1]) + } + regex, err := regexp.Compile(specifier) + if err != nil { + log.WithField("specifier", specifier).WithError(err).Warn("Invalid path regex") + continue + } + regexes = append(regexes, regex) + } + return regexes +} diff --git a/validator/keymanager/remote_internal_test.go b/validator/keymanager/remote_internal_test.go new file mode 100644 index 000000000000..9d6a4dc4690e --- /dev/null +++ b/validator/keymanager/remote_internal_test.go @@ -0,0 +1,58 @@ +package keymanager + +import ( + "testing" +) + +func TestPathsToVerificationRegexes(t *testing.T) { + tests := []struct { + name string + paths []string + regexes []string + err string + }{ + { + name: "Empty", + regexes: []string{}, + }, + { + name: "IgnoreBadPaths", + paths: []string{"", "/", "/Account"}, + regexes: []string{}, + }, + { + name: "Simple", + paths: []string{"Wallet/Account"}, + regexes: []string{"^Wallet/Account$"}, + }, + { + name: "Multiple", + paths: []string{"Wallet/Account1", "Wallet/Account2"}, + regexes: []string{"^Wallet/Account1$", "^Wallet/Account2$"}, + }, + { + name: "IgnoreInvalidRegex", + paths: []string{"Wallet/Account1", "Bad/***", "Wallet/Account2"}, + regexes: []string{"^Wallet/Account1$", "^Wallet/Account2$"}, + }, + { + name: "TidyExistingAnchors", + paths: []string{"Wallet/^.*$", "Wallet/Foo.*Bar$", "Wallet/^Account"}, + regexes: []string{"^Wallet/.*$", "^Wallet/Foo.*Bar$", "^Wallet/Account$"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + regexes := pathsToVerificationRegexes(test.paths) + if len(regexes) != len(test.regexes) { + t.Fatalf("Unexpected number of regexes: expected %v, received %v", len(test.regexes), len(regexes)) + } + for i := range regexes { + if regexes[i].String() != test.regexes[i] { + t.Fatalf("Unexpected regex %d: expected %v, received %v", i, test.regexes[i], regexes[i].String()) + } + } + }) + } +}