diff --git a/WORKSPACE b/WORKSPACE index dfe700190fc8..105a176f80ca 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1654,8 +1654,8 @@ 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:Fs0GfrdhboBKW7zaMvIvUHJaOB1ibpAmRG3lkB53in4=", + version = "v1.3.0", ) go_repository( diff --git a/validator/client/validator_aggregate.go b/validator/client/validator_aggregate.go index c90ce75cb395..03109f368fff 100644 --- a/validator/client/validator_aggregate.go +++ b/validator/client/validator_aggregate.go @@ -165,11 +165,7 @@ func (v *validator) aggregateAndProofSig(ctx context.Context, pubKey [48]byte, a if err != nil { return nil, err } - signedRoot, err := helpers.ComputeSigningRoot(agg, d.SignatureDomain) - if err != nil { - return nil, err - } - sig, err := v.keyManager.Sign(pubKey, signedRoot) + sig, err := v.signObject(pubKey, agg, d.SignatureDomain) if err != nil { return nil, err } diff --git a/validator/client/validator_propose.go b/validator/client/validator_propose.go index 336f7c903ff4..17a988cf63cb 100644 --- a/validator/client/validator_propose.go +++ b/validator/client/validator_propose.go @@ -193,10 +193,11 @@ func (v *validator) signBlock(ctx context.Context, pubKey [48]byte, epoch uint64 return nil, errors.Wrap(err, "could not get signing root") } blockHeader := ðpb.BeaconBlockHeader{ - Slot: b.Slot, - StateRoot: b.StateRoot, - ParentRoot: b.ParentRoot, - BodyRoot: bodyRoot[:], + Slot: b.Slot, + ProposerIndex: b.ProposerIndex, + StateRoot: b.StateRoot, + ParentRoot: b.ParentRoot, + BodyRoot: bodyRoot[:], } sig, err = protectingKeymanager.SignProposal(pubKey, bytesutil.ToBytes32(domain.SignatureDomain), blockHeader) if err != nil { diff --git a/validator/keymanager/BUILD.bazel b/validator/keymanager/BUILD.bazel index 384ded3119c3..4540021ec454 100644 --- a/validator/keymanager/BUILD.bazel +++ b/validator/keymanager/BUILD.bazel @@ -39,6 +39,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..a4b4feb03baa 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...) @@ -129,7 +139,7 @@ func NewRemoteWallet(input string) (KeyManager, string, error) { err = km.RefreshValidatingKeys() if err != nil { - return nil, remoteOptsHelp, errors.New("failed to fetch accounts from remote wallet") + return nil, remoteOptsHelp, errors.Wrap(err, "failed to fetch accounts from remote wallet") } return km, remoteOptsHelp, nil @@ -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) @@ -187,10 +197,11 @@ func (km *Remote) SignProposal(pubKey [48]byte, domain [32]byte, data *ethpb.Bea Id: &pb.SignBeaconProposalRequest_Account{Account: accountInfo.Name}, Domain: domain[:], Data: &pb.BeaconBlockHeader{ - Slot: data.Slot, - ParentRoot: data.ParentRoot, - StateRoot: data.StateRoot, - BodyRoot: data.BodyRoot, + Slot: data.Slot, + ProposerIndex: data.ProposerIndex, + ParentRoot: data.ParentRoot, + StateRoot: data.StateRoot, + BodyRoot: data.BodyRoot, }, } resp, err := client.SignBeaconProposal(context.Background(), req) @@ -198,9 +209,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 +247,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 +261,31 @@ 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 +295,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()) + } + } + }) + } +}