From 3cad5e68314c13224b2800a23dddaf62bd13b90e Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 17 Feb 2025 12:52:29 -0500 Subject: [PATCH 1/7] Add comments to signer-config setup --- config/config.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/config.go b/config/config.go index f3a9e61516f8..f24911787424 100644 --- a/config/config.go +++ b/config/config.go @@ -675,10 +675,16 @@ func getStakingSigner(v *viper.Viper) (bls.Signer, error) { return key, nil } + // If the key is set, but a user-file isn't provided, we don't create one. + // The siging key is only stored to the default file-location if it's created + // and saved by the current application run. + if v.IsSet(StakingSignerKeyPathKey) { return nil, errMissingStakingSigningKeyFile } + // no config values explicitly set, + key, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) From 30834ff23c9e45d0036dc814ddb6bd1cf24f2e10 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 14 Feb 2025 18:01:52 -0500 Subject: [PATCH 2/7] Extend siging configuration to include RPC --- config/config.go | 95 ++++++++++++++++++++++++----------- config/config_test.go | 112 ++++++++++++++++++++++++++++++++++++++++++ config/flags.go | 1 + config/keys.go | 1 + config/node/config.go | 1 + main/main.go | 3 +- 6 files changed, 184 insertions(+), 29 deletions(-) diff --git a/config/config.go b/config/config.go index f24911787424..15e9fdbba7be 100644 --- a/config/config.go +++ b/config/config.go @@ -4,6 +4,7 @@ package config import ( + "context" "crypto/tls" "encoding/base64" "encoding/json" @@ -17,6 +18,8 @@ import ( "time" "github.com/spf13/viper" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/api/server" "github.com/ava-labs/avalanchego/chains" @@ -38,6 +41,7 @@ import ( "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/ips" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/perms" @@ -84,6 +88,7 @@ var ( errCannotReadDirectory = errors.New("cannot read directory") errUnmarshalling = errors.New("unmarshalling failed") errFileDoesNotExist = errors.New("file does not exist") + errInvalidSignerConfig = fmt.Errorf("only one of the following flags can be set: %s, %s, %s, %s", StakingEphemeralSignerEnabledKey, StakingSignerKeyContentKey, StakingSignerKeyPathKey, StakingRPCSignerKey) ) func getConsensusConfig(v *viper.Viper) snowball.Parameters { @@ -639,53 +644,84 @@ func getStakingTLSCert(v *viper.Viper) (tls.Certificate, error) { } } -func getStakingSigner(v *viper.Viper) (bls.Signer, error) { - if v.GetBool(StakingEphemeralSignerEnabledKey) { - key, err := localsigner.New() +func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { + ephemeralSignerEnabled := v.GetBool(StakingEphemeralSignerEnabledKey) + contentKeyIsSet := v.IsSet(StakingSignerKeyContentKey) + keyPathIsSet := v.IsSet(StakingSignerKeyPathKey) + rpcSignerURLIsSet := v.IsSet(StakingRPCSignerKey) + + signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) + + switch { + case ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: + signer, err := localsigner.New() if err != nil { - return nil, fmt.Errorf("couldn't generate ephemeral signing key: %w", err) + return nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) } - return key, nil - } - if v.IsSet(StakingSignerKeyContentKey) { + return signer, nil + + case !ephemeralSignerEnabled && contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: signerKeyRawContent := v.GetString(StakingSignerKeyContentKey) signerKeyContent, err := base64.StdEncoding.DecodeString(signerKeyRawContent) if err != nil { return nil, fmt.Errorf("unable to decode base64 content: %w", err) } - key, err := localsigner.FromBytes(signerKeyContent) + + signer, err := localsigner.FromBytes(signerKeyContent) if err != nil { return nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return key, nil - } - signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) - _, err := os.Stat(signingKeyPath) - if !errors.Is(err, fs.ErrNotExist) { + return signer, nil + + case !ephemeralSignerEnabled && !contentKeyIsSet && keyPathIsSet && !rpcSignerURLIsSet: + // If the key is set, but a user-file isn't provided, we don't create one. + // The siging key is only stored to the default file-location if it's created + // and saved by the current application run. + _, err := os.Stat(signingKeyPath) + + if errors.Is(err, fs.ErrNotExist) { + return nil, errMissingStakingSigningKeyFile + } + signingKeyBytes, err := os.ReadFile(signingKeyPath) if err != nil { return nil, err } - key, err := localsigner.FromBytes(signingKeyBytes) + + signer, err := localsigner.FromBytes(signingKeyBytes) if err != nil { return nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return key, nil - } - // If the key is set, but a user-file isn't provided, we don't create one. - // The siging key is only stored to the default file-location if it's created - // and saved by the current application run. + return signer, nil + + case !ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && rpcSignerURLIsSet: + rpcSignerURL := v.GetString(StakingRPCSignerKey) + + // the rpc-signer client should call a proxy server (on the same machine) that forwards + // the request to the actual signer instead of relying on tls-credentials + conn, err := grpc.NewClient(rpcSignerURL, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + signer, err := rpcsigner.NewClient(ctx, conn) + if err != nil { + conn.Close() + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + return signer, nil - if v.IsSet(StakingSignerKeyPathKey) { - return nil, errMissingStakingSigningKeyFile + case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: + return nil, errInvalidSignerConfig } // no config values explicitly set, - key, err := localsigner.New() + signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) } @@ -694,17 +730,19 @@ func getStakingSigner(v *viper.Viper) (bls.Signer, error) { return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) } - keyBytes := key.ToBytes() + keyBytes := signer.ToBytes() if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) } + if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) } - return key, nil + + return signer, nil } -func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, error) { +func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { config := node.StakingConfig{ SybilProtectionEnabled: v.GetBool(SybilProtectionEnabledKey), SybilProtectionDisabledWeight: v.GetUint64(SybilProtectionDisabledWeightKey), @@ -712,6 +750,7 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err StakingKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), StakingCertPath: getExpandedArg(v, StakingCertPathKey), StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), + StakingSignerRpc: getExpandedArg(v, StakingRPCSignerKey), } if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights @@ -726,7 +765,7 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err if err != nil { return node.StakingConfig{}, err } - config.StakingSigningKey, err = getStakingSigner(v) + config.StakingSigningKey, err = getStakingSigner(ctx, v) if err != nil { return node.StakingConfig{}, err } @@ -1250,7 +1289,7 @@ func getPluginDir(v *viper.Viper) (string, error) { return pluginDir, nil } -func GetNodeConfig(v *viper.Viper) (node.Config, error) { +func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { var ( nodeConfig node.Config err error @@ -1305,7 +1344,7 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { } // Staking - nodeConfig.StakingConfig, err = getStakingConfig(v, nodeConfig.NetworkID) + nodeConfig.StakingConfig, err = getStakingConfig(ctx, v, nodeConfig.NetworkID) if err != nil { return node.Config{}, err } diff --git a/config/config_test.go b/config/config_test.go index 68847ca4f6d5..452c8dd138f2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,22 +4,31 @@ package config import ( + "context" "encoding/base64" "encoding/json" "fmt" "log" + "net" "os" "path/filepath" + "reflect" "testing" + "google.golang.org/grpc" + "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/chains" "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/proto/pb/signer" "github.com/ava-labs/avalanchego/snow/consensus/snowball" "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" + "github.com/ava-labs/avalanchego/utils/perms" ) const chainConfigFilenameExtention = ".ex" @@ -541,6 +550,109 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { } } +type signerServer struct { + signer.UnimplementedSignerServer +} + +func (*signerServer) PublicKey(context.Context, *signer.PublicKeyRequest) (*signer.PublicKeyResponse, error) { + // for tests to pass, this must be the base64 encoding of a 32 byte public key + // but it does not need to be associated with any private key + bytes, err := base64.StdEncoding.DecodeString("j8Ndzc1I6EYWYUWAdhcwpQ1I2xX/i4fdwgJIaxbHlf9yQKMT0jlReiiLYsydgaS1") + if err != nil { + return nil, err + } + + return &signer.PublicKeyResponse{ + PublicKey: bytes, + }, nil +} + +func TestGetStakingSigner(t *testing.T) { + testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" + rpcServer := grpc.NewServer() + defer rpcServer.GracefulStop() + + signer.RegisterSignerServer(rpcServer, &signerServer{}) + + listener, err := net.Listen("tcp", "[::1]:0") + require.NoError(t, err) + + go func() { + err := rpcServer.Serve(listener) + require.NoError(t, err) + }() + + type config map[string]any + + tests := []struct { + name string + viperKeys string + config config + expectedSignerType reflect.Type + expectedErr error + }{ + { + name: "default-signer", + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "ephemeral-signer", + config: config{StakingEphemeralSignerEnabledKey: true}, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "content-key", + config: config{StakingSignerKeyContentKey: testKey}, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "file-key", + config: config{ + StakingSignerKeyPathKey: func() string { + filePath := filepath.Join(t.TempDir(), "signer.key") + bytes, err := base64.StdEncoding.DecodeString(testKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) + return filePath + }(), + }, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "rpc-signer", + config: config{StakingRPCSignerKey: listener.Addr().String()}, + expectedSignerType: reflect.TypeOf(&rpcsigner.Client{}), + }, + { + name: "multiple-configurations-set", + config: config{ + StakingEphemeralSignerEnabledKey: true, + StakingSignerKeyContentKey: testKey, + }, + expectedErr: errInvalidSignerConfig, + }, + } + + // required for proper write permissions for the default signer-key location + t.Setenv("HOME", t.TempDir()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + v := setupViperFlags() + + for key, value := range tt.config { + v.Set(key, value) + } + + signer, err := getStakingSigner(context.Background(), v) + + require.ErrorIs(err, tt.expectedErr) + require.Equal(tt.expectedSignerType, reflect.TypeOf(signer)) + }) + } +} + // setups config json file and writes content func setupConfigJSON(t *testing.T, rootPath string, value string) string { configFilePath := filepath.Join(rootPath, "config.json") diff --git a/config/flags.go b/config/flags.go index 193d1343d559..e08ccd488444 100644 --- a/config/flags.go +++ b/config/flags.go @@ -271,6 +271,7 @@ func addNodeFlags(fs *pflag.FlagSet) { fs.Bool(StakingEphemeralSignerEnabledKey, false, "If true, the node uses an ephemeral staking signer key") fs.String(StakingSignerKeyPathKey, defaultStakingSignerKeyPath, fmt.Sprintf("Path to the signer private key for staking. Ignored if %s is specified", StakingSignerKeyContentKey)) fs.String(StakingSignerKeyContentKey, "", "Specifies base64 encoded signer private key for staking") + fs.String(StakingRPCSignerKey, "", "Specifies the RPC endpoint of the staking signer") fs.Bool(SybilProtectionEnabledKey, true, "Enables sybil protection. If enabled, Network TLS is required") fs.Uint64(SybilProtectionDisabledWeightKey, 100, "Weight to provide to each peer when sybil protection is disabled") fs.Bool(PartialSyncPrimaryNetworkKey, false, "Only sync the P-chain on the Primary Network. If the node is a Primary Network validator, it will report unhealthy") diff --git a/config/keys.go b/config/keys.go index 760bee97fd77..32dc6edad831 100644 --- a/config/keys.go +++ b/config/keys.go @@ -85,6 +85,7 @@ const ( StakingEphemeralSignerEnabledKey = "staking-ephemeral-signer-enabled" StakingSignerKeyPathKey = "staking-signer-key-file" StakingSignerKeyContentKey = "staking-signer-key-file-content" + StakingRPCSignerKey = "staking-rpc-signer" SybilProtectionEnabledKey = "sybil-protection-enabled" SybilProtectionDisabledWeightKey = "sybil-protection-disabled-weight" NetworkInitialTimeoutKey = "network-initial-timeout" diff --git a/config/node/config.go b/config/node/config.go index a97c3b84df2e..4c8e20c9f566 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -82,6 +82,7 @@ type StakingConfig struct { StakingKeyPath string `json:"stakingKeyPath"` StakingCertPath string `json:"stakingCertPath"` StakingSignerPath string `json:"stakingSignerPath"` + StakingSignerRpc string `json:"stakingSignerRpc"` } type StateSyncConfig struct { diff --git a/main/main.go b/main/main.go index bdd4a5d83b23..df1d65341933 100644 --- a/main/main.go +++ b/main/main.go @@ -4,6 +4,7 @@ package main import ( + "context" "encoding/json" "errors" "fmt" @@ -52,7 +53,7 @@ func main() { os.Exit(0) } - nodeConfig, err := config.GetNodeConfig(v) + nodeConfig, err := config.GetNodeConfig(context.Background(), v) if err != nil { fmt.Printf("couldn't load node config: %s\n", err) os.Exit(1) From 3175c0d98912f391d879f6e0dc7ff35b5e23c516 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 19 Feb 2025 16:01:38 -0500 Subject: [PATCH 3/7] Add default behaviour to switch --- config/config.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/config/config.go b/config/config.go index 15e9fdbba7be..d29ad1f6fa15 100644 --- a/config/config.go +++ b/config/config.go @@ -717,29 +717,29 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: return nil, errInvalidSignerConfig - } + default: - // no config values explicitly set, + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + } - signer, err := localsigner.New() - if err != nil { - return nil, fmt.Errorf("couldn't generate new signing key: %w", err) - } + if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { + return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) + } - if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { - return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) - } + keyBytes := signer.ToBytes() + if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { + return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) + } - keyBytes := signer.ToBytes() - if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) - } + if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { + return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) + } - if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { - return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) + return signer, nil } - return signer, nil } func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { From 9551683d485a6766ea30dbb6d69024ee841c8d10 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 19 Feb 2025 16:27:42 -0500 Subject: [PATCH 4/7] Add timeout to signer instantiation --- config/config.go | 2 -- main/main.go | 5 ++++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index d29ad1f6fa15..d541a5290fa3 100644 --- a/config/config.go +++ b/config/config.go @@ -718,7 +718,6 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: return nil, errInvalidSignerConfig default: - signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) @@ -739,7 +738,6 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return signer, nil } - } func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { diff --git a/main/main.go b/main/main.go index df1d65341933..ced58a9854f2 100644 --- a/main/main.go +++ b/main/main.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "time" "github.com/spf13/pflag" "golang.org/x/term" @@ -53,7 +54,9 @@ func main() { os.Exit(0) } - nodeConfig, err := config.GetNodeConfig(context.Background(), v) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + nodeConfig, err := config.GetNodeConfig(ctx, v) + cancel() if err != nil { fmt.Printf("couldn't load node config: %s\n", err) os.Exit(1) From 8742f9e8320b6a8a00be84993e60c62810aeadac Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 20 Feb 2025 17:41:02 -0500 Subject: [PATCH 5/7] Make rpc-signer client handle the connection --- config/config.go | 14 ++---------- config/config_test.go | 3 +-- config/node/config.go | 2 +- utils/crypto/bls/signer/rpcsigner/client.go | 24 ++++++++++++++++++++- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/config/config.go b/config/config.go index d541a5290fa3..d603b2c4313c 100644 --- a/config/config.go +++ b/config/config.go @@ -18,8 +18,6 @@ import ( "time" "github.com/spf13/viper" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/api/server" "github.com/ava-labs/avalanchego/chains" @@ -700,19 +698,11 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case !ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && rpcSignerURLIsSet: rpcSignerURL := v.GetString(StakingRPCSignerKey) - // the rpc-signer client should call a proxy server (on the same machine) that forwards - // the request to the actual signer instead of relying on tls-credentials - conn, err := grpc.NewClient(rpcSignerURL, grpc.WithTransportCredentials(insecure.NewCredentials())) + signer, err := rpcsigner.NewClient(ctx, rpcSignerURL) if err != nil { return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) } - signer, err := rpcsigner.NewClient(ctx, conn) - if err != nil { - conn.Close() - return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) - } - return signer, nil case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: @@ -748,7 +738,7 @@ func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (no StakingKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), StakingCertPath: getExpandedArg(v, StakingCertPathKey), StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), - StakingSignerRpc: getExpandedArg(v, StakingRPCSignerKey), + StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), } if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights diff --git a/config/config_test.go b/config/config_test.go index 452c8dd138f2..dfb358dd944d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -15,11 +15,10 @@ import ( "reflect" "testing" - "google.golang.org/grpc" - "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" + "google.golang.org/grpc" "github.com/ava-labs/avalanchego/chains" "github.com/ava-labs/avalanchego/ids" diff --git a/config/node/config.go b/config/node/config.go index 4c8e20c9f566..8abef8328988 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -82,7 +82,7 @@ type StakingConfig struct { StakingKeyPath string `json:"stakingKeyPath"` StakingCertPath string `json:"stakingCertPath"` StakingSignerPath string `json:"stakingSignerPath"` - StakingSignerRpc string `json:"stakingSignerRpc"` + StakingSignerRPC string `json:"stakingSignerRpc"` } type StateSyncConfig struct { diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 294bb1829d81..d7c2bd5c94aa 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -5,8 +5,11 @@ package rpcsigner import ( "context" + "fmt" "google.golang.org/grpc" + "google.golang.org/grpc/backoff" + "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/utils/crypto/bls" @@ -21,17 +24,30 @@ type Client struct { pk *bls.PublicKey } -func NewClient(ctx context.Context, conn *grpc.ClientConn) (*Client, error) { +func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { + // TODO: figure out the best parameters here given the target block-time + opts := grpc.WithConnectParams(grpc.ConnectParams{ + Backoff: backoff.DefaultConfig, + }) + + // the rpc-signer client should call a proxy server (on the same machine) that forwards + // the request to the actual signer instead of relying on tls-credentials + conn, err := grpc.NewClient(rpcSignerURL, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { + conn.Close() return nil, err } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { + conn.Close() return nil, err } @@ -46,9 +62,12 @@ func (c *Client) PublicKey() *bls.PublicKey { return c.pk } +// Sign a message. The [Client] already handles transient connection errors. If this method fails, it will +// render the client in an unusable state and the client should be discarded. func (c *Client) Sign(message []byte) (*bls.Signature, error) { resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { + c.conn.Close() return nil, err } signature := resp.GetSignature() @@ -56,9 +75,12 @@ func (c *Client) Sign(message []byte) (*bls.Signature, error) { return bls.SignatureFromBytes(signature) } +// [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. +// See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { + c.conn.Close() return nil, err } signature := resp.GetSignature() From df0e4ce11c5cac4731ae45180a7e425184ac4a78 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 20 Feb 2025 23:51:21 -0500 Subject: [PATCH 6/7] Fix linter error --- config/config_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index dfb358dd944d..53beec8214ce 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -577,8 +577,7 @@ func TestGetStakingSigner(t *testing.T) { require.NoError(t, err) go func() { - err := rpcServer.Serve(listener) - require.NoError(t, err) + require.NoError(t, rpcServer.Serve(listener)) }() type config map[string]any From 980175468d14011b1ab05c4c5c5b781c2033210c Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 21 Feb 2025 00:24:03 -0500 Subject: [PATCH 7/7] Close the connection on any error --- utils/crypto/bls/signer/rpcsigner/client.go | 50 +++++++++++++++++---- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index d7c2bd5c94aa..af0a5eda150c 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -36,18 +36,22 @@ func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { if err != nil { return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) } + defer func() { + if err != nil { + conn.Close() + } + }() + client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - conn.Close() return nil, err } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - conn.Close() return nil, err } @@ -65,25 +69,55 @@ func (c *Client) PublicKey() *bls.PublicKey { // Sign a message. The [Client] already handles transient connection errors. If this method fails, it will // render the client in an unusable state and the client should be discarded. func (c *Client) Sign(message []byte) (*bls.Signature, error) { + var err error + defer func() { + if err != nil { + c.Close() + } + }() + resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { - c.conn.Close() return nil, err } - signature := resp.GetSignature() - return bls.SignatureFromBytes(signature) + sigBytes := resp.GetSignature() + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, err + } + + return sig, nil } // [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. // See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { + var err error + defer func() { + if err != nil { + c.Close() + } + }() + resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { - c.conn.Close() return nil, err } - signature := resp.GetSignature() - return bls.SignatureFromBytes(signature) + sigBytes := resp.GetSignature() + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, err + } + + return sig, nil +} + +func (c *Client) Close() error { + if c.conn == nil { + return nil + } + + return c.conn.Close() }