Accounts V2: Derived Keymanager Sign (#6667)

* amend derived with secret keys cache
* all tests for sign
* Merge branch 'master' into sign-derived
* formatting
* Merge branch 'sign-derived' of github.com:prysmaticlabs/prysm into sign-derived
* initialize
* use seed
* fix build
* Merge refs/heads/master into sign-derived
* Merge refs/heads/master into sign-derived
* Update validator/keymanager/v2/derived/derived.go

nogo_config.json

@@ -107,8 +107,7 @@
     "exclude_files": {
       ".*/.*_test\\.go": "Tests are OK to use weak crypto",
       "shared/rand/rand\\.go": "Abstracts CSPRNGs for common use",
-      "shared/aggregation/testing/bitlistutils.go": "Test-only package",
-      "shared/petnames/names.go": "Needs deterministic randomness"
+      "shared/aggregation/testing/bitlistutils.go": "Test-only package"
     }
   }
 }

shared/petnames/BUILD.bazel

@@ -5,5 +5,8 @@ go_library(
     srcs = ["names.go"],
     importpath = "github.com/prysmaticlabs/prysm/shared/petnames",
     visibility = ["//visibility:public"],
-    deps = ["//shared/hashutil:go_default_library"],
+    deps = [
+        "//shared/hashutil:go_default_library",
+        "//shared/rand:go_default_library",
+    ],
 )

shared/petnames/names.go

@@ -1,10 +1,10 @@
 package petnames
 
 import (
-	"math/rand"
 	"strings"
 
 	"github.com/prysmaticlabs/prysm/shared/hashutil"
+	"github.com/prysmaticlabs/prysm/shared/rand"
 )
 
 var (
@@ -16,11 +16,12 @@ var (
 // DeterministicName returns a deterministic triple of adverb-adjective-name
 // given a random seed for initialization.
 func DeterministicName(seed []byte, separator string) string {
+	rng := rand.NewDeterministicGenerator()
 	hashedValue := hashutil.FastSum64(seed)
-	rand.Seed(int64(hashedValue))
-	adverb := adverbs[rand.Intn(len(adverbs)-1)]
-	adjective := adjectives[rand.Intn(len(adjectives)-1)]
-	name := names[rand.Intn(len(names)-1)]
+	rng.Seed(int64(hashedValue))
+	adverb := adverbs[rng.Intn(len(adverbs)-1)]
+	adjective := adjectives[rng.Intn(len(adjectives)-1)]
+	name := names[rng.Intn(len(names)-1)]
 	petname := []string{adverb, adjective, name}
 	return strings.Join(petname, separator)
 }

validator/accounts/v2/list.go

@@ -160,7 +160,7 @@ func listDerivedKeymanagerAccounts(
 	if nextAccountNumber > 0 {
 		currentAccountNumber--
 	}
-	accountNames, err := keymanager.AccountNames(ctx)
+	accountNames, err := keymanager.ValidatingAccountNames(ctx)
 	if err != nil {
 		return err
 	}

validator/accounts/v2/list_test.go

@@ -166,7 +166,7 @@ func TestListAccounts_DerivedKeymanager(t *testing.T) {
 		t.Errorf("Did not find accounts path %s in output", wallet.accountsPath)
 	}
 
-	accountNames, err := keymanager.AccountNames(ctx)
+	accountNames, err := keymanager.ValidatingAccountNames(ctx)
 	require.NoError(t, err)
 	pubKeys, err := keymanager.FetchValidatingPublicKeys(ctx)
 	require.NoError(t, err)

validator/keymanager/v2/derived/BUILD.bazel

@@ -45,7 +45,9 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
+        "//proto/validator/accounts/v2:go_default_library",
         "//shared/bls:go_default_library",
+        "//shared/bytesutil:go_default_library",
         "//shared/testutil:go_default_library",
         "//shared/testutil/assert:go_default_library",
         "//shared/testutil/require:go_default_library",

validator/keymanager/v2/derived/derived.go

@@ -127,8 +127,15 @@ func NewKeymanager(
 		mnemonicGenerator: &EnglishMnemonicGenerator{
 			skipMnemonicConfirm: skipMnemonicConfirm,
 		},
-		seedCfg: seedConfig,
-		seed:    seed,
+		seedCfg:   seedConfig,
+		seed:      seed,
+		keysCache: make(map[[48]byte]bls.SecretKey),
+	}
+	// We initialize a cache of public key -> secret keys
+	// used to retrieve secrets keys for the accounts via the unlocked wallet.
+	// This cache is needed to process Sign requests using a validating public key.
+	if err := k.initializeSecretKeysCache(); err != nil {
+		return nil, errors.Wrap(err, "could not initialize secret keys cache")
 	}
 	return k, nil
 }
@@ -201,7 +208,7 @@ func MarshalEncryptedSeedFile(ctx context.Context, seedCfg *SeedConfig) ([]byte,
 	return json.MarshalIndent(seedCfg, "", "\t")
 }
 
-// Config --
+// Config returns the derived keymanager configuration.
 func (dr *Keymanager) Config() *Config {
 	return dr.cfg
 }
@@ -211,8 +218,8 @@ func (dr *Keymanager) NextAccountNumber(ctx context.Context) uint64 {
 	return dr.seedCfg.NextAccount
 }
 
-// AccountNames --
-func (dr *Keymanager) AccountNames(ctx context.Context) ([]string, error) {
+// ValidatingAccountNames for the derived keymanager.
+func (dr *Keymanager) ValidatingAccountNames(ctx context.Context) ([]string, error) {
 	names := make([]string, 0)
 	for i := uint64(0); i < dr.seedCfg.NextAccount; i++ {
 		withdrawalKeyPath := fmt.Sprintf(WithdrawalKeyDerivationPathTemplate, i)
@@ -319,12 +326,34 @@ func (dr *Keymanager) CreateAccount(ctx context.Context, password string) (strin
 
 // Sign signs a message using a validator key.
 func (dr *Keymanager) Sign(ctx context.Context, req *validatorpb.SignRequest) (bls.Signature, error) {
-	return nil, errors.New("unimplemented")
+	rawPubKey := req.PublicKey
+	if rawPubKey == nil {
+		return nil, errors.New("nil public key in request")
+	}
+	dr.lock.RLock()
+	defer dr.lock.RUnlock()
+	secretKey, ok := dr.keysCache[bytesutil.ToBytes48(rawPubKey)]
+	if !ok {
+		return nil, errors.New("no signing key found in keys cache")
+	}
+	return secretKey.Sign(req.SigningRoot), nil
 }
 
 // FetchValidatingPublicKeys fetches the list of validating public keys from the keymanager.
 func (dr *Keymanager) FetchValidatingPublicKeys(ctx context.Context) ([][48]byte, error) {
-	publicKeys := make([][48]byte, 0)
+	// Return the public keys from the cache if they match the
+	// number of accounts from the wallet.
+	publicKeys := make([][48]byte, dr.seedCfg.NextAccount)
+	dr.lock.RLock()
+	defer dr.lock.RUnlock()
+	if dr.keysCache != nil && uint64(len(dr.keysCache)) == dr.seedCfg.NextAccount {
+		var i int
+		for k := range dr.keysCache {
+			publicKeys[i] = k
+			i++
+		}
+		return publicKeys, nil
+	}
 	for i := uint64(0); i < dr.seedCfg.NextAccount; i++ {
 		validatingKeyPath := fmt.Sprintf(ValidatingKeyDerivationPathTemplate, i)
 		validatingKeystore, err := dr.wallet.ReadFileAtPath(ctx, validatingKeyPath, KeystoreFileName)
@@ -366,6 +395,31 @@ func (dr *Keymanager) FetchWithdrawalPublicKeys(ctx context.Context) ([][48]byte
 	return publicKeys, nil
 }
 
+func (dr *Keymanager) initializeSecretKeysCache() error {
+	dr.lock.Lock()
+	defer dr.lock.Unlock()
+	for i := uint64(0); i < dr.seedCfg.NextAccount; i++ {
+		validatingKeyPath := fmt.Sprintf(ValidatingKeyDerivationPathTemplate, i)
+		derivedKey, err := util.PrivateKeyFromSeedAndPath(dr.seed, validatingKeyPath)
+		if err != nil {
+			return errors.Wrapf(err, "failed to derive validating key for account %s", validatingKeyPath)
+		}
+		validatorSigningKey, err := bls.SecretKeyFromBytes(derivedKey.Marshal())
+		if err != nil {
+			return errors.Wrapf(
+				err,
+				"could not instantiate bls secret key from bytes for account: %s",
+				validatingKeyPath,
+			)
+		}
+
+		// Update a simple cache of public key -> secret key utilized
+		// for fast signing access in the keymanager.
+		dr.keysCache[bytesutil.ToBytes48(validatorSigningKey.PublicKey().Marshal())] = validatorSigningKey
+	}
+	return nil
+}
+
 func (dr *Keymanager) generateKeystoreFile(privateKey []byte, publicKey []byte, password string) ([]byte, error) {
 	encryptor := keystorev4.New()
 	cryptoFields, err := encryptor.Encrypt(privateKey, []byte(password))

validator/keymanager/v2/derived/derived_test.go

@@ -2,12 +2,16 @@ package derived
 
 import (
 	"context"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"strings"
 	"testing"
 
+	validatorpb "github.com/prysmaticlabs/prysm/proto/validator/accounts/v2"
 	"github.com/prysmaticlabs/prysm/shared/bls"
+	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/testutil"
 	"github.com/prysmaticlabs/prysm/shared/testutil/assert"
 	"github.com/prysmaticlabs/prysm/shared/testutil/require"
@@ -92,3 +96,127 @@ func TestDerivedKeymanager_CreateAccount(t *testing.T) {
 	testutil.AssertLogsContain(t, hook, fmt.Sprintf("%#x", validatingKey.PublicKey().Marshal()))
 	testutil.AssertLogsContain(t, hook, fmt.Sprintf("%#x", withdrawalKey.PublicKey().Marshal()))
 }
+
+func TestDerivedKeymanager_FetchValidatingPublicKeys(t *testing.T) {
+	wallet := &mock.Wallet{
+		Files:            make(map[string]map[string][]byte),
+		AccountPasswords: make(map[string]string),
+	}
+	dr := &Keymanager{
+		wallet:    wallet,
+		keysCache: make(map[[48]byte]bls.SecretKey),
+		seedCfg: &SeedConfig{
+			NextAccount: 0,
+		},
+		seed: make([]byte, 32),
+	}
+	// First, generate accounts and their keystore.json files.
+	ctx := context.Background()
+	numAccounts := 20
+	password := "hello world"
+	wantedPublicKeys := make([][48]byte, numAccounts)
+	var err error
+	var accountName string
+	for i := 0; i < numAccounts; i++ {
+		accountName, err = dr.CreateAccount(ctx, password)
+		require.NoError(t, err)
+		validatingKeyPath := fmt.Sprintf(ValidatingKeyDerivationPathTemplate, i)
+		enc, err := wallet.ReadFileAtPath(ctx, validatingKeyPath, KeystoreFileName)
+		require.NoError(t, err)
+		keystore := &v2keymanager.Keystore{}
+		require.NoError(t, json.Unmarshal(enc, keystore))
+		pubKey, err := hex.DecodeString(keystore.Pubkey)
+		require.NoError(t, err)
+		wantedPublicKeys[i] = bytesutil.ToBytes48(pubKey)
+	}
+	assert.Equal(t, fmt.Sprintf("%d", numAccounts-1), accountName)
+
+	publicKeys, err := dr.FetchValidatingPublicKeys(ctx)
+	require.NoError(t, err)
+
+	// The results are not guaranteed to be ordered, so we ensure each
+	// key we expect exists in the results via a map.
+	keysMap := make(map[[48]byte]bool)
+	for _, key := range publicKeys {
+		keysMap[key] = true
+	}
+	for _, wanted := range wantedPublicKeys {
+		if _, ok := keysMap[wanted]; !ok {
+			t.Errorf("Could not find expected public key %#x in results", wanted)
+		}
+	}
+}
+
+func TestDerivedKeymanager_Sign(t *testing.T) {
+	wallet := &mock.Wallet{
+		Files:            make(map[string]map[string][]byte),
+		AccountPasswords: make(map[string]string),
+	}
+	seed := make([]byte, 32)
+	copy(seed, "hello world")
+	dr := &Keymanager{
+		wallet:    wallet,
+		seed:      seed,
+		keysCache: make(map[[48]byte]bls.SecretKey),
+		seedCfg: &SeedConfig{
+			NextAccount: 0,
+		},
+	}
+
+	// First, generate some accounts.
+	numAccounts := 2
+	ctx := context.Background()
+	password := "hello world"
+	var err error
+	var accountName string
+	for i := 0; i < numAccounts; i++ {
+		accountName, err = dr.CreateAccount(ctx, password)
+		require.NoError(t, err)
+	}
+	assert.Equal(t, fmt.Sprintf("%d", numAccounts-1), accountName)
+
+	// Initialize the secret keys cache for the keymanager.
+	require.NoError(t, dr.initializeSecretKeysCache())
+	publicKeys, err := dr.FetchValidatingPublicKeys(ctx)
+	require.NoError(t, err)
+
+	// We prepare naive data to sign.
+	data := []byte("eth2data")
+	signRequest := &validatorpb.SignRequest{
+		PublicKey:   publicKeys[0][:],
+		SigningRoot: data,
+	}
+	sig, err := dr.Sign(ctx, signRequest)
+	require.NoError(t, err)
+	pubKey, err := bls.PublicKeyFromBytes(publicKeys[0][:])
+	require.NoError(t, err)
+	wrongPubKey, err := bls.PublicKeyFromBytes(publicKeys[1][:])
+	require.NoError(t, err)
+
+	// Check if the signature verifies.
+	assert.Equal(t, true, sig.Verify(pubKey, data))
+	// Check if the bad signature fails.
+	assert.Equal(t, false, sig.Verify(wrongPubKey, data))
+}
+
+func TestDerivedKeymanager_Sign_NoPublicKeySpecified(t *testing.T) {
+	req := &validatorpb.SignRequest{
+		PublicKey: nil,
+	}
+	dr := &Keymanager{}
+	_, err := dr.Sign(context.Background(), req)
+	assert.NotNil(t, err)
+	assert.Equal(t, strings.Contains(err.Error(), "nil public key"), true)
+}
+
+func TestDerivedKeymanager_Sign_NoPublicKeyInCache(t *testing.T) {
+	req := &validatorpb.SignRequest{
+		PublicKey: []byte("hello world"),
+	}
+	dr := &Keymanager{
+		keysCache: make(map[[48]byte]bls.SecretKey),
+	}
+	_, err := dr.Sign(context.Background(), req)
+	assert.NotNil(t, err)
+	assert.Equal(t, strings.Contains(err.Error(), "no signing key found"), true)
+}