web3signer: url parsing bug (#10278)

* adding in fixes for url

* fixing gazelle

* fixing wrong keymanager kind

* adding required scheme to urls

* fixing another unit test

* removing unused file

* adding new commit to retrigger deepsource ci
This commit is contained in:
james-prysm 2022-02-24 11:24:11 -05:00 committed by GitHub
parent 02a088d93c
commit 01e9125761
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 153 additions and 23 deletions

View File

@ -42,10 +42,13 @@ type ApiClient struct {
// NewApiClient method instantiates a new ApiClient object.
func NewApiClient(baseEndpoint string) (*ApiClient, error) {
u, err := url.Parse(baseEndpoint)
u, err := url.ParseRequestURI(baseEndpoint)
if err != nil {
return nil, errors.Wrap(err, "invalid format, unable to parse url")
}
if u.Scheme == "" || u.Host == "" {
return nil, fmt.Errorf("web3signer url must be in the format of http(s)://host:port url used: %v", baseEndpoint)
}
return &ApiClient{
BaseURL: u,
RestClient: &http.Client{},

View File

@ -78,7 +78,7 @@ func (km *Keymanager) FetchValidatingPublicKeys(ctx context.Context) ([][fieldpa
providedPublicKeys, err := km.client.GetPublicKeys(ctx, km.publicKeysURL)
if err != nil {
erroredResponsesTotal.Inc()
return nil, err
return nil, errors.Wrap(err, fmt.Sprintf("could not get public keys from remote server url: %v", km.publicKeysURL))
}
km.providedPublicKeys = providedPublicKeys
}

View File

@ -55,9 +55,9 @@ func TestKeymanager_Sign(t *testing.T) {
fmt.Printf("error: %v", err)
}
config := &SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
PublicKeysURL: "example2.com/api/v1/eth2/publicKeys",
PublicKeysURL: "http://example2.com/api/v1/eth2/publicKeys",
}
km, err := NewKeymanager(ctx, config)
if err != nil {
@ -189,7 +189,7 @@ func TestKeymanager_FetchValidatingPublicKeys_HappyPath_WithKeyList(t *testing.T
fmt.Printf("error: %v", err)
}
config := &SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
ProvidedPublicKeys: keys,
}
@ -223,9 +223,9 @@ func TestKeymanager_FetchValidatingPublicKeys_HappyPath_WithExternalURL(t *testi
fmt.Printf("error: %v", err)
}
config := &SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
PublicKeysURL: "example2.com/api/v1/eth2/publicKeys",
PublicKeysURL: "http://example2.com/api/v1/eth2/publicKeys",
}
km, err := NewKeymanager(ctx, config)
if err != nil {
@ -252,9 +252,9 @@ func TestKeymanager_FetchValidatingPublicKeys_WithExternalURL_ThrowsError(t *tes
fmt.Printf("error: %v", err)
}
config := &SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
PublicKeysURL: "example2.com/api/v1/eth2/publicKeys",
PublicKeysURL: "http://example2.com/api/v1/eth2/publicKeys",
}
km, err := NewKeymanager(ctx, config)
if err != nil {
@ -264,5 +264,5 @@ func TestKeymanager_FetchValidatingPublicKeys_WithExternalURL_ThrowsError(t *tes
resp, err := km.FetchValidatingPublicKeys(ctx)
assert.NotNil(t, err)
assert.Nil(t, resp)
assert.Equal(t, fmt.Errorf("mock error"), err)
assert.Equal(t, "could not get public keys from remote server url: http://example2.com/api/v1/eth2/publicKeys: mock error", fmt.Sprintf("%v", err))
}

View File

@ -7,10 +7,13 @@ go_test(
embed = [":go_default_library"],
deps = [
"//cmd/validator/flags:go_default_library",
"//encoding/bytesutil:go_default_library",
"//testing/require:go_default_library",
"//validator/accounts:go_default_library",
"//validator/accounts/wallet:go_default_library",
"//validator/keymanager:go_default_library",
"//validator/keymanager/remote-web3signer:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
"@com_github_sirupsen_logrus//hooks/test:go_default_library",
"@com_github_urfave_cli_v2//:go_default_library",
],

View File

@ -429,26 +429,26 @@ func web3SignerConfig(cliCtx *cli.Context) (*remote_web3signer.SetupConfig, erro
if cliCtx.IsSet(flags.Web3SignerURLFlag.Name) && cliCtx.IsSet(flags.Web3SignerPublicValidatorKeysFlag.Name) {
urlStr := cliCtx.String(flags.Web3SignerURLFlag.Name)
publicKeysStr := cliCtx.String(flags.Web3SignerPublicValidatorKeysFlag.Name)
u, err := url.Parse(urlStr)
u, err := url.ParseRequestURI(urlStr)
if err != nil {
return nil, errors.Wrapf(err, "web3signer url %s is invalid", urlStr)
}
if u.Scheme == "" || u.Host == "" {
return nil, fmt.Errorf("web3signer url must be in the format of http(s)://host:port url used: %v", urlStr)
}
web3signerConfig = &remote_web3signer.SetupConfig{
BaseEndpoint: u.String(),
GenesisValidatorsRoot: nil,
}
u, err = url.Parse(publicKeysStr)
if err != nil {
return nil, errors.Wrapf(err, "could not parse %s as a URL for web3signer remote public keys", publicKeysStr)
}
if u != nil {
pURL, err := url.ParseRequestURI(publicKeysStr)
if err == nil && pURL.Scheme != "" && pURL.Host != "" {
web3signerConfig.PublicKeysURL = publicKeysStr
} else {
var validatorKeys [][48]byte
for _, key := range strings.Split(publicKeysStr, ",") {
decodedKey, err := hexutil.Decode(key)
if err != nil {
return nil, errors.Wrapf(err, "could not decode public key for web3signer: %s", key)
decodedKey, decodeErr := hexutil.Decode(key)
if decodeErr != nil {
return nil, errors.Wrapf(decodeErr, "could not decode public key for web3signer: %s", key)
}
validatorKeys = append(validatorKeys, bytesutil.ToBytes48(decodedKey))
}

View File

@ -3,16 +3,20 @@ package node
import (
"context"
"flag"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/prysmaticlabs/prysm/cmd/validator/flags"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
"github.com/prysmaticlabs/prysm/testing/require"
"github.com/prysmaticlabs/prysm/validator/accounts"
"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
"github.com/prysmaticlabs/prysm/validator/keymanager"
remote_web3signer "github.com/prysmaticlabs/prysm/validator/keymanager/remote-web3signer"
logTest "github.com/sirupsen/logrus/hooks/test"
"github.com/urfave/cli/v2"
)
@ -60,3 +64,123 @@ func TestClearDB(t *testing.T) {
require.NoError(t, clearDB(context.Background(), tmp, true))
require.LogsContain(t, hook, "Removing database")
}
// TestWeb3SignerConfig tests the web3 signer config returns the correct values.
func TestWeb3SignerConfig(t *testing.T) {
pubkey1decoded, err := hexutil.Decode("0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c")
require.NoError(t, err)
bytepubkey1 := bytesutil.ToBytes48(pubkey1decoded)
pubkey2decoded, err := hexutil.Decode("0xb89bebc699769726a318c8e9971bd3171297c61aea4a6578a7a4f94b547dcba5bac16a89108b6b6a1fe3695d1a874a0b")
require.NoError(t, err)
bytepubkey2 := bytesutil.ToBytes48(pubkey2decoded)
type args struct {
baseURL string
publicKeysOrURL string
}
tests := []struct {
name string
args args
want *remote_web3signer.SetupConfig
wantErrMsg string
}{
{
name: "happy path with public keys",
args: args{
baseURL: "http://localhost:8545",
publicKeysOrURL: "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c," +
"0xb89bebc699769726a318c8e9971bd3171297c61aea4a6578a7a4f94b547dcba5bac16a89108b6b6a1fe3695d1a874a0b",
},
want: &remote_web3signer.SetupConfig{
BaseEndpoint: "http://localhost:8545",
GenesisValidatorsRoot: nil,
PublicKeysURL: "",
ProvidedPublicKeys: [][48]byte{
bytepubkey1,
bytepubkey2,
},
},
},
{
name: "happy path with external url",
args: args{
baseURL: "http://localhost:8545",
publicKeysOrURL: "http://localhost:8545/api/v1/eth2/publicKeys",
},
want: &remote_web3signer.SetupConfig{
BaseEndpoint: "http://localhost:8545",
GenesisValidatorsRoot: nil,
PublicKeysURL: "http://localhost:8545/api/v1/eth2/publicKeys",
ProvidedPublicKeys: nil,
},
},
{
name: "Bad base URL",
args: args{
baseURL: "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88,",
publicKeysOrURL: "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c," +
"0xb89bebc699769726a318c8e9971bd3171297c61aea4a6578a7a4f94b547dcba5bac16a89108b6b6a1fe3695d1a874a0b",
},
want: nil,
wantErrMsg: "web3signer url 0xa99a76ed7796f7be22d5b7e85deeb7c5677e88, is invalid: parse \"0xa99a76ed7796f7be22d5b7e85deeb7c5677e88,\": invalid URI for request",
},
{
name: "Bad publicKeys",
args: args{
baseURL: "http://localhost:8545",
publicKeysOrURL: "0xa99a76ed7796f7be22c," +
"0xb89bebc699769726a318c8e9971bd3171297c61aea4a6578a7a4f94b547dcba5bac16a89108b6b6a1fe3695d1a874a0b",
},
want: nil,
wantErrMsg: "could not decode public key for web3signer: 0xa99a76ed7796f7be22c: hex string of odd length",
},
{
name: "Bad publicKeysURL",
args: args{
baseURL: "http://localhost:8545",
publicKeysOrURL: "localhost",
},
want: nil,
wantErrMsg: "could not decode public key for web3signer: localhost: hex string without 0x prefix",
},
{
name: "Base URL missing scheme or host",
args: args{
baseURL: "localhost:8545",
publicKeysOrURL: "localhost",
},
want: nil,
wantErrMsg: "web3signer url must be in the format of http(s)://host:port url used: localhost:8545",
},
{
name: "Public Keys URL missing scheme or host",
args: args{
baseURL: "http://localhost:8545",
publicKeysOrURL: "localhost:8545",
},
want: nil,
wantErrMsg: "could not decode public key for web3signer: localhost:8545: hex string without 0x prefix",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := web3SignerConfig(newWeb3SignerCli(t, tt.args.baseURL, tt.args.publicKeysOrURL))
if (tt.wantErrMsg != "") && (tt.wantErrMsg != fmt.Sprintf("%v", err)) {
t.Errorf("web3SignerConfig error = %v, wantErrMsg = %v", err, tt.wantErrMsg)
return
}
require.DeepEqual(t, got, tt.want)
})
}
}
func newWeb3SignerCli(t *testing.T, baseUrl string, publicKeysOrURL string) *cli.Context {
app := cli.App{}
set := flag.NewFlagSet("test", 0)
set.String("validators-external-signer-url", baseUrl, "baseUrl")
set.String("validators-external-signer-public-keys", publicKeysOrURL, "publicKeys or URL")
require.NoError(t, set.Set(flags.Web3SignerURLFlag.Name, baseUrl))
require.NoError(t, set.Set(flags.Web3SignerPublicValidatorKeysFlag.Name, publicKeysOrURL))
return cli.NewContext(&app, set, nil)
}

View File

@ -239,9 +239,9 @@ func TestServer_ImportKeystores_WrongKeymanagerKind(t *testing.T) {
root := make([]byte, fieldparams.RootLength)
root[0] = 1
km, err := w.InitializeKeymanager(ctx, iface.InitKeymanagerConfig{ListenForChanges: false, Web3SignerConfig: &remote_web3signer.SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
PublicKeysURL: "example.com/public_keys",
PublicKeysURL: "http://example.com/public_keys",
}})
require.NoError(t, err)
vs, err := client.NewValidatorService(ctx, &client.Config{
@ -455,9 +455,9 @@ func TestServer_DeleteKeystores_WrongKeymanagerKind(t *testing.T) {
root[0] = 1
km, err := w.InitializeKeymanager(ctx, iface.InitKeymanagerConfig{ListenForChanges: false,
Web3SignerConfig: &remote_web3signer.SetupConfig{
BaseEndpoint: "example.com",
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
PublicKeysURL: "example.com/public_keys",
PublicKeysURL: "http://example.com/public_keys",
}})
require.NoError(t, err)
vs, err := client.NewValidatorService(ctx, &client.Config{