Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Abstract the keystore #1752

Merged
merged 5 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ run:
concurrency: 4
deadline: 1m
issues-exit-code: 1
modules-download-mode: vendor
tests: true
skip-dirs:
- static
- vendor
skip-files:
- bindata.go
- .*_mock.go
- jail/doc.go
- contracts/

output:
Expand All @@ -26,6 +27,8 @@ linters-settings:
min-confidence: 0.8
gofmt:
simplify: true
goimports:
local-prefixes: github.com/ethereum/go-ethereum,github.com/status-im/status-go
gocyclo:
min-complexity: 16
maligned:
Expand All @@ -44,7 +47,7 @@ linters:
- gosec
- goconst
- gocyclo
- gofmt
- goimports
- golint
- govet
- ineffassign
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ prepare-release: clean-release
clean-release:
rm -rf $(RELEASE_DIR)

gofmt:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but there are many unrelated files changed because of this.

find . -name '*.go' -and -not -name 'bindata*' -and -not -name 'migrations.go' -and -not -wholename '*/vendor/*' -exec goimports -local 'github.com/ethereum/go-ethereum,github.com/status-im/status-go' -w {} \;
$(MAKE) vendor

check-existing-release:
@git ls-remote --exit-code origin "v$(RELEASE_TAG)" >/dev/null || exit 0; \
echo "$(YELLOW)Release tag already exists: v$(RELEASE_TAG)$(RESET)"; \
Expand Down
69 changes: 18 additions & 51 deletions account/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ import (
"path/filepath"
"sync"

"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/keystore"
"github.com/ethereum/go-ethereum/common"
"github.com/pborman/uuid"

"github.com/status-im/status-go/account/generator"
"github.com/status-im/status-go/eth-node/crypto"
"github.com/status-im/status-go/eth-node/keystore"
"github.com/status-im/status-go/eth-node/types"
"github.com/status-im/status-go/extkeys"
)
Expand All @@ -37,8 +35,7 @@ var zeroAddress = types.Address{}
// Manager represents account manager interface.
type Manager struct {
mu sync.RWMutex
keystore *keystore.KeyStore
manager *accounts.Manager
keystore types.KeyStore

accountsGenerator *generator.Generator
onboarding *Onboarding
Expand All @@ -48,46 +45,13 @@ type Manager struct {
watchAddresses []types.Address
}

// NewManager returns new node account manager.
func NewManager() *Manager {
m := &Manager{}
m.accountsGenerator = generator.New(m)
return m
}

// InitKeystore sets key manager and key store.
func (m *Manager) InitKeystore(keydir string) error {
m.mu.Lock()
defer m.mu.Unlock()
manager, err := makeAccountManager(keydir)
if err != nil {
return err
}
m.manager = manager
backends := manager.Backends(keystore.KeyStoreType)
if len(backends) == 0 {
return ErrAccountKeyStoreMissing
}
keyStore, ok := backends[0].(*keystore.KeyStore)
if !ok {
return ErrAccountKeyStoreMissing
}
m.keystore = keyStore
return nil
}

func (m *Manager) GetKeystore() *keystore.KeyStore {
// GetKeystore is only used in tests
func (m *Manager) GetKeystore() types.KeyStore {
m.mu.RLock()
defer m.mu.RUnlock()
return m.keystore
}

func (m *Manager) GetManager() *accounts.Manager {
m.mu.RLock()
defer m.mu.RUnlock()
return m.manager
}

// AccountsGenerator returns accountsGenerator.
func (m *Manager) AccountsGenerator() *generator.Generator {
return m.accountsGenerator
Expand Down Expand Up @@ -153,7 +117,7 @@ func (m *Manager) RecoverAccount(password, mnemonic string) (Info, error) {

// VerifyAccountPassword tries to decrypt a given account key file, with a provided password.
// If no error is returned, then account is considered verified.
func (m *Manager) VerifyAccountPassword(keyStoreDir, address, password string) (*keystore.Key, error) {
func (m *Manager) VerifyAccountPassword(keyStoreDir, address, password string) (*types.Key, error) {
var err error
var foundKeyFile []byte

Expand Down Expand Up @@ -202,7 +166,7 @@ func (m *Manager) VerifyAccountPassword(keyStoreDir, address, password string) (
}

// avoid swap attack
if types.Address(key.Address) != addressObj {
if key.Address != addressObj {
return nil, fmt.Errorf("account mismatch: have %s, want %s", key.Address.Hex(), addressObj.Hex())
}

Expand Down Expand Up @@ -240,9 +204,9 @@ func (m *Manager) SetChatAccount(privKey *ecdsa.PrivateKey) {

address := crypto.PubkeyToAddress(privKey.PublicKey)
id := uuid.NewRandom()
key := &keystore.Key{
key := &types.Key{
Id: id,
Address: common.Address(address),
Address: address,
PrivateKey: privKey,
}

Expand Down Expand Up @@ -302,9 +266,13 @@ func (m *Manager) ImportAccount(privateKey *ecdsa.PrivateKey, password string) (

account, err := m.keystore.ImportECDSA(privateKey, password)

return types.Address(account.Address), err
return account.Address, err
}

// ImportSingleExtendedKey imports an extended key setting it in both the PrivateKey and ExtendedKey fields
// of the Key struct.
// ImportExtendedKey is used in older version of Status where PrivateKey is set to be the BIP44 key at index 0,
// and ExtendedKey is the extended key of the BIP44 key at index 1.
func (m *Manager) ImportSingleExtendedKey(extKey *extkeys.ExtendedKey, password string) (address, pubKey string, err error) {
if m.keystore == nil {
return "", "", ErrAccountKeyStoreMissing
Expand Down Expand Up @@ -418,18 +386,17 @@ func (m *Manager) ImportOnboardingAccount(id string, password string) (Info, str
// AddressToDecryptedAccount tries to load decrypted key for a given account.
// The running node, has a keystore directory which is loaded on start. Key file
// for a given address is expected to be in that directory prior to node start.
func (m *Manager) AddressToDecryptedAccount(address, password string) (accounts.Account, *keystore.Key, error) {
func (m *Manager) AddressToDecryptedAccount(address, password string) (types.Account, *types.Key, error) {
if m.keystore == nil {
return accounts.Account{}, nil, ErrAccountKeyStoreMissing
return types.Account{}, nil, ErrAccountKeyStoreMissing
}

account, err := ParseAccountString(address)
if err != nil {
return accounts.Account{}, nil, ErrAddressToAccountMappingFailure
return types.Account{}, nil, ErrAddressToAccountMappingFailure
}

var key *keystore.Key
account, key, err = m.keystore.AccountDecryptedKey(account, password)
account, key, err := m.keystore.AccountDecryptedKey(account, password)
if err != nil {
err = fmt.Errorf("%s: %s", ErrAccountToKeyMappingFailure, err)
}
Expand All @@ -444,7 +411,7 @@ func (m *Manager) unlockExtendedKey(address, password string) (*SelectedExtKey,
}

selectedExtendedKey := &SelectedExtKey{
Address: types.Address(account.Address),
Address: account.Address,
AccountKey: accountKey,
}

Expand Down
43 changes: 43 additions & 0 deletions account/accounts_geth.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// +build !nimbus

package account

import (
"github.com/ethereum/go-ethereum/accounts"

"github.com/status-im/status-go/account/generator"
)

// GethManager represents account manager interface.
type GethManager struct {
*Manager

gethAccManager *accounts.Manager
}

// NewManager returns new node account manager.
func NewManager() *GethManager {
m := &GethManager{}
m.Manager = &Manager{accountsGenerator: generator.New(m)}
return m
}

// InitKeystore sets key manager and key store.
func (m *GethManager) InitKeystore(keydir string) error {
m.mu.Lock()
defer m.mu.Unlock()

var err error
m.gethAccManager, err = makeAccountManager(keydir)
if err != nil {
return err
}
m.keystore, err = makeKeyStore(m.gethAccManager)
return err
}

func (m *GethManager) GetManager() *accounts.Manager {
m.mu.RLock()
defer m.mu.RUnlock()
return m.gethAccManager
}
28 changes: 28 additions & 0 deletions account/accounts_nimbus.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// +build nimbus

package account

import (
"github.com/status-im/status-go/account/generator"
)

// NewManager returns new node account manager.
func NewManager() *Manager {
m := &Manager{}
m.accountsGenerator = generator.New(m)
return m
}

// InitKeystore sets key manager and key store.
func (m *Manager) InitKeystore(keydir string) error {
m.mu.Lock()
defer m.mu.Unlock()

// TODO: Wire with the Nimbus keystore
manager, err := makeAccountManager(keydir)
if err != nil {
return err
}
m.keystore, err = makeKeyStore(manager)
return err
}
4 changes: 2 additions & 2 deletions account/accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestVerifyAccountPassword(t *testing.T) {
require.Fail(t, "no error reported, but account key is missing")
}
accountAddress := types.BytesToAddress(types.FromHex(testCase.address))
if types.Address(accountKey.Address) != accountAddress {
if accountKey.Address != accountAddress {
require.Fail(t, "account mismatch: have %s, want %s", accountKey.Address.Hex(), accountAddress.Hex())
}
}
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestManagerTestSuite(t *testing.T) {
type ManagerTestSuite struct {
suite.Suite
testAccount
accManager *Manager
accManager *GethManager
keydir string
}

Expand Down
3 changes: 2 additions & 1 deletion account/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package account
import (
"testing"

"github.com/stretchr/testify/require"

"github.com/status-im/status-go/eth-node/crypto"
"github.com/status-im/status-go/eth-node/types"
"github.com/stretchr/testify/require"
)

func TestCreateAddress(t *testing.T) {
Expand Down
7 changes: 4 additions & 3 deletions account/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import (
"strings"
"sync"

"github.com/ethereum/go-ethereum/accounts"
"github.com/ethereum/go-ethereum/accounts/keystore"
"github.com/pborman/uuid"

"github.com/ethereum/go-ethereum/accounts/keystore"

"github.com/status-im/status-go/eth-node/crypto"
"github.com/status-im/status-go/eth-node/types"
"github.com/status-im/status-go/extkeys"
Expand All @@ -25,7 +26,7 @@ var (
)

type AccountManager interface {
AddressToDecryptedAccount(address, password string) (accounts.Account, *keystore.Key, error)
AddressToDecryptedAccount(address, password string) (types.Account, *types.Key, error)
ImportSingleExtendedKey(extKey *extkeys.ExtendedKey, password string) (address, pubKey string, err error)
ImportAccount(privateKey *ecdsa.PrivateKey, password string) (types.Address, error)
}
Expand Down
3 changes: 2 additions & 1 deletion account/generator/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"strings"
"testing"

"github.com/ethereum/go-ethereum/crypto"
"github.com/stretchr/testify/assert"

"github.com/ethereum/go-ethereum/crypto"
)

var testAccount = struct {
Expand Down
4 changes: 2 additions & 2 deletions account/generator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"bytes"
"errors"

"github.com/ethereum/go-ethereum/accounts/keystore"
"github.com/status-im/status-go/eth-node/crypto"
"github.com/status-im/status-go/eth-node/types"
"github.com/status-im/status-go/extkeys"
)

Expand All @@ -21,7 +21,7 @@ var (

// ValidateKeystoreExtendedKey validates the keystore keys, checking that
// ExtendedKey is the extended key of PrivateKey.
func ValidateKeystoreExtendedKey(key *keystore.Key) error {
func ValidateKeystoreExtendedKey(key *types.Key) error {
if key.ExtendedKey.IsZeroed() {
return nil
}
Expand Down
11 changes: 6 additions & 5 deletions account/generator/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package generator
import (
"testing"

"github.com/ethereum/go-ethereum/accounts/keystore"
"github.com/status-im/status-go/extkeys"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/status-im/status-go/eth-node/types"
"github.com/status-im/status-go/extkeys"
)

func generateTestKey(t *testing.T) *extkeys.ExtendedKey {
Expand All @@ -25,22 +26,22 @@ func TestValidateKeystoreExtendedKey(t *testing.T) {
extendedKey2 := generateTestKey(t)

// new keystore file format
key := &keystore.Key{
key := &types.Key{
PrivateKey: extendedKey1.ToECDSA(),
ExtendedKey: extendedKey1,
}
assert.NoError(t, ValidateKeystoreExtendedKey(key))

// old keystore file format where the extended key was
// from another derivation path and not the same of the private key
oldKey := &keystore.Key{
oldKey := &types.Key{
PrivateKey: extendedKey1.ToECDSA(),
ExtendedKey: extendedKey2,
}
assert.Error(t, ValidateKeystoreExtendedKey(oldKey))

// normal key where we don't have an extended key
normalKey := &keystore.Key{
normalKey := &types.Key{
PrivateKey: extendedKey1.ToECDSA(),
ExtendedKey: nil,
}
Expand Down
Loading