Add Lock/TimedUnlock for Hashicorp accts & expose in PrivateAccountAPI

* Vault keys can be retrieved and stored for a limited time, reducing the number of calls needed to the Vault server

* Existing timed unlocks can be overriden

* Unlocks can be indefinite, requiring an explicit call to lock

accounts/vault/vault_backend.go

@@ -4,9 +4,13 @@ import (
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/log"
+	"reflect"
 	"sort"
 )
 
+// BackendType is the reflect type of a vault backend.
+var BackendType = reflect.TypeOf(&VaultBackend{})
+
 type VaultBackend struct {
 	wallets []accounts.Wallet
 	updateScope event.SubscriptionScope

accounts/vault/vault_backend_test.go

@@ -78,8 +78,8 @@ func TestVaultBackend_Wallets_ReturnsWallets(t *testing.T) {
 			want []accounts.Wallet
 	}{
 		"empty": {in: []accounts.Wallet{}, want: []accounts.Wallet{}},
-		"single": {in: []accounts.Wallet{vaultWallet{}}, want: []accounts.Wallet{vaultWallet{}}},
-		"multiple": {in: []accounts.Wallet{vaultWallet{}, vaultWallet{}}, want: []accounts.Wallet{vaultWallet{}, vaultWallet{}}},
+		"single": {in: []accounts.Wallet{VaultWallet{}}, want: []accounts.Wallet{VaultWallet{}}},
+		"multiple": {in: []accounts.Wallet{VaultWallet{}, VaultWallet{}}, want: []accounts.Wallet{VaultWallet{}, VaultWallet{}}},
 	}
 
 	for name, tt := range tests {
@@ -98,13 +98,13 @@ func TestVaultBackend_Wallets_ReturnsWallets(t *testing.T) {
 func TestVaultBackend_Wallets_ReturnsCopy(t *testing.T) {
 	b := VaultBackend{
 		wallets: []accounts.Wallet{
-			vaultWallet{url: accounts.URL{Scheme: "http", Path: "url"}},
+			VaultWallet{url: accounts.URL{Scheme: "http", Path: "url"}},
 		},
 	}
 
 	got := b.Wallets()
 
-	got[0] = vaultWallet{url: accounts.URL{Scheme: "http", Path: "otherurl"}}
+	got[0] = VaultWallet{url: accounts.URL{Scheme: "http", Path: "otherurl"}}
 
 	if reflect.DeepEqual(b.wallets, got) {
 		t.Fatal("changes to returned slice should not affect slice in backend")
@@ -122,7 +122,7 @@ func TestVaultBackend_Subscribe_SubscriberReceivesEventsAddedToFeed(t *testing.T
 	}
 
 	// mock an event
-	e := accounts.WalletEvent{Wallet: vaultWallet{}, Kind: accounts.WalletOpened}
+	e := accounts.WalletEvent{Wallet: VaultWallet{}, Kind: accounts.WalletOpened}
 	b.updateFeed.Send(e)
 
 	if len(subscriber) != 1 {
@@ -144,7 +144,7 @@ func TestVaultBackend_Subscribe_SubscriberReceivesEventsAddedToFeedByHashicorpWa
 		t.Fatalf("incorrect number of wallets: want: %v, got: %v", 1, len(b.wallets))
 	}
 
-	w := b.wallets[0].(vaultWallet)
+	w := b.wallets[0].(VaultWallet)
 
 	subscriber := make(chan accounts.WalletEvent, 1)
 	b.Subscribe(subscriber)
@@ -154,7 +154,7 @@ func TestVaultBackend_Subscribe_SubscriberReceivesEventsAddedToFeedByHashicorpWa
 	}
 
 	// mock an event
-	e := accounts.WalletEvent{Wallet: vaultWallet{}, Kind: accounts.WalletOpened}
+	e := accounts.WalletEvent{Wallet: VaultWallet{}, Kind: accounts.WalletOpened}
 	w.updateFeed.Send(e)
 
 	if len(subscriber) != 1 {

accounts/vault/vault_wallet.go

@@ -21,7 +21,7 @@ import (
 	"time"
 )
 
-type vaultWallet struct {
+type VaultWallet struct {
 	url accounts.URL
 	vault vaultService
 	updateFeed *event.Feed
@@ -34,19 +34,21 @@ type vaultService interface {
 	close() error
 	accounts() []accounts.Account
 	getKey(acct accounts.Account) (key *ecdsa.PrivateKey, zeroFn func(), err error)
+	timedUnlock(acct accounts.Account, timeout time.Duration) error
+	lock(acct accounts.Account) error
 }
 
-func newHashicorpWallet(config HashicorpWalletConfig, updateFeed *event.Feed) (vaultWallet, error) {
+func newHashicorpWallet(config HashicorpWalletConfig, updateFeed *event.Feed) (VaultWallet, error) {
 	var url accounts.URL
 
 	//to parse a string url as an accounts.URL it must first be in json format
 	toParse := fmt.Sprintf("\"%v\"", config.Client.Url)
 
 	if err := url.UnmarshalJSON([]byte(toParse)); err != nil {
-		return vaultWallet{}, err
+		return VaultWallet{}, err
 	}
 
-	w := vaultWallet{
+	w := VaultWallet{
 		url: url,
 		vault: newHashicorpService(config),
 		updateFeed: updateFeed,
@@ -55,16 +57,16 @@ func newHashicorpWallet(config HashicorpWalletConfig, updateFeed *event.Feed) (v
 	return w, nil
 }
 
-func (w vaultWallet) URL() accounts.URL {
+func (w VaultWallet) URL() accounts.URL {
 	return w.url
 }
 
 // the vault service should return open and nil error if status is good
-func (w vaultWallet) Status() (string, error) {
+func (w VaultWallet) Status() (string, error) {
 	return w.vault.status()
 }
 
-func (w vaultWallet) Open(passphrase string) error {
+func (w VaultWallet) Open(passphrase string) error {
 	if err := w.vault.open(); err != nil {
 		return err
 	}
@@ -74,15 +76,15 @@ func (w vaultWallet) Open(passphrase string) error {
 	return nil
 }
 
-func (w vaultWallet) Close() error {
+func (w VaultWallet) Close() error {
 	return w.vault.close()
 }
 
-func (w vaultWallet) Accounts() []accounts.Account {
+func (w VaultWallet) Accounts() []accounts.Account {
 	return w.vault.accounts()
 }
 
-func (w vaultWallet) Contains(account accounts.Account) bool {
+func (w VaultWallet) Contains(account accounts.Account) bool {
 	equal := func(a, b accounts.Account) bool {
 		return a.Address == b.Address && (a.URL == b.URL || a.URL == accounts.URL{} || b.URL == accounts.URL{})
 	}
@@ -97,13 +99,13 @@ func (w vaultWallet) Contains(account accounts.Account) bool {
 	return false
 }
 
-func (w vaultWallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Account, error) {
+func (w VaultWallet) Derive(path accounts.DerivationPath, pin bool) (accounts.Account, error) {
 	return accounts.Account{}, accounts.ErrNotSupported
 }
 
-func (w vaultWallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {}
+func (w VaultWallet) SelfDerive(base accounts.DerivationPath, chain ethereum.ChainStateReader) {}
 
-func (w vaultWallet) SignHash(account accounts.Account, hash []byte) ([]byte, error) {
+func (w VaultWallet) SignHash(account accounts.Account, hash []byte) ([]byte, error) {
 	if !w.Contains(account) {
 		return nil, accounts.ErrUnknownAccount
 	}
@@ -119,7 +121,7 @@ func (w vaultWallet) SignHash(account accounts.Account, hash []byte) ([]byte, er
 	return crypto.Sign(hash, key)
 }
 
-func (w vaultWallet) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
+func (w VaultWallet) SignTx(account accounts.Account, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
 	if !w.Contains(account) {
 		return nil, accounts.ErrUnknownAccount
 	}
@@ -145,21 +147,37 @@ func (w vaultWallet) SignTx(account accounts.Account, tx *types.Transaction, cha
 	return types.SignTx(tx, types.HomesteadSigner{}, key)
 }
 
-func (w vaultWallet) SignHashWithPassphrase(account accounts.Account, passphrase string, hash []byte) ([]byte, error) {
+func (w VaultWallet) SignHashWithPassphrase(account accounts.Account, passphrase string, hash []byte) ([]byte, error) {
 	return w.SignHash(account, hash)
 }
 
-func (w vaultWallet) SignTxWithPassphrase(account accounts.Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
+func (w VaultWallet) SignTxWithPassphrase(account accounts.Account, passphrase string, tx *types.Transaction, chainID *big.Int) (*types.Transaction, error) {
 	return w.SignTx(account, tx, chainID)
 }
 
+func (w VaultWallet) TimedUnlock(account accounts.Account, timeout time.Duration) error {
+	if !w.Contains(account) {
+		return accounts.ErrUnknownAccount
+	}
+
+	return w.vault.timedUnlock(account, timeout)
+}
+
+func (w VaultWallet) Lock(account accounts.Account) error {
+	if !w.Contains(account) {
+		return accounts.ErrUnknownAccount
+	}
+
+	return w.vault.lock(account)
+}
+
 type hashicorpService struct {
 	client      *api.Client
 	config      hashicorpClientConfig
 	secrets     []hashicorpSecretConfig
 	mutex       sync.RWMutex
 	accts       []accounts.Account
-	keyHandlers map[common.Address]map[accounts.URL]hashicorpKeyHandler
+	keyHandlers map[common.Address]map[accounts.URL]*hashicorpKeyHandler
 
 }
 
@@ -167,7 +185,7 @@ func newHashicorpService(config HashicorpWalletConfig) *hashicorpService {
 	s := &hashicorpService{
 		config:      config.Client,
 		secrets:     config.Secrets,
-		keyHandlers: make(map[common.Address]map[accounts.URL]hashicorpKeyHandler),
+		keyHandlers: make(map[common.Address]map[accounts.URL]*hashicorpKeyHandler),
 	}
 
 	return s
@@ -175,7 +193,9 @@ func newHashicorpService(config HashicorpWalletConfig) *hashicorpService {
 
 type hashicorpKeyHandler struct {
 	secret hashicorpSecretConfig
+	mutex       sync.RWMutex
 	key *ecdsa.PrivateKey
+	cancel chan struct{}
 }
 
 const (
@@ -344,7 +364,7 @@ func (h *hashicorpService) accountRetrievalLoop(ticker *time.Ticker) {
 			h.mutex.Lock()
 
 			if _, ok := h.keyHandlers[acct.Address]; !ok {
-				h.keyHandlers[acct.Address] = make(map[accounts.URL]hashicorpKeyHandler)
+				h.keyHandlers[acct.Address] = make(map[accounts.URL]*hashicorpKeyHandler)
 			}
 
 			keyHandlersByUrl := h.keyHandlers[acct.Address]
@@ -355,7 +375,7 @@ func (h *hashicorpService) accountRetrievalLoop(ticker *time.Ticker) {
 				continue
 			}
 
-			keyHandlersByUrl[acct.URL] = hashicorpKeyHandler{secret: s}
+			keyHandlersByUrl[acct.URL] = &hashicorpKeyHandler{secret: s}
 			h.accts = append(h.accts, acct)
 
 			h.mutex.Unlock()
@@ -373,7 +393,7 @@ func (h *hashicorpService) getAddressFromVault(s hashicorpSecretConfig) (common.
 	return common.HexToAddress(hexAddr), nil
 }
 
-func countRetrievedKeys(keyHandlers map[common.Address]map[accounts.URL]hashicorpKeyHandler) int {
+func countRetrievedKeys(keyHandlers map[common.Address]map[accounts.URL]*hashicorpKeyHandler) int {
 	var n int
 
 	for _, khByUrl := range keyHandlers {
@@ -411,9 +431,7 @@ func (h *hashicorpService) privateKeyRetrievalLoop(ticker *time.Ticker) {
 				}
 
 				h.mutex.Lock()
-				existing := h.keyHandlers[addr][u]
-				updated := hashicorpKeyHandler{secret: existing.secret, key: key}
-				h.keyHandlers[addr][u] = updated
+				h.keyHandlers[addr][u].key = key
 				h.mutex.Unlock()
 			}
 		}
@@ -497,10 +515,20 @@ func (s accountsByURL) Less(i, j int) bool { return s[i].URL.Cmp(s[j].URL) < 0 }
 func (s accountsByURL) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
 
 func (h *hashicorpService) getKey(acct accounts.Account) (*ecdsa.PrivateKey, func(), error) {
+	keyHandler, err := h.getKeyHandler(acct)
+
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return h.getKeyFromHandler(*keyHandler)
+}
+
+func (h *hashicorpService) getKeyHandler(acct accounts.Account) (*hashicorpKeyHandler, error) {
 	keyHandlersByUrl, ok := h.keyHandlers[acct.Address]
 
 	if !ok {
-		return nil, nil, accounts.ErrUnknownAccount
+		return nil, accounts.ErrUnknownAccount
 	}
 
 	if (acct.URL == accounts.URL{}) && len(keyHandlersByUrl) > 1 {
@@ -517,27 +545,28 @@ func (h *hashicorpService) getKey(acct accounts.Account) (*ecdsa.PrivateKey, fun
 			Matches: ambiguousAccounts,
 		}
 
-		return nil, nil, err
+		return nil, err
 	}
 
 	// return the only key for this address
 	if (acct.URL == accounts.URL{}) && len(keyHandlersByUrl) == 1 {
-		var keyHandler hashicorpKeyHandler
+		var keyHandler *hashicorpKeyHandler
 
 		for _, kh := range keyHandlersByUrl {
 			keyHandler = kh
+			break
 		}
 
-		return h.getKeyFromHandler(keyHandler)
+		return keyHandler, nil
 	}
 
 	keyHandler, ok := keyHandlersByUrl[acct.URL]
 
 	if !ok {
-		return nil, nil, accounts.ErrUnknownAccount
+		return nil, accounts.ErrUnknownAccount
 	}
 
-	return h.getKeyFromHandler(keyHandler)
+	return keyHandler, nil
 }
 
 func (h *hashicorpService) getKeyFromHandler(handler hashicorpKeyHandler) (*ecdsa.PrivateKey, func(), error) {
@@ -562,3 +591,102 @@ func (h *hashicorpService) getKeyFromHandler(handler hashicorpKeyHandler) (*ecds
 
 	return key, zeroFn, nil
 }
+
+func (h *hashicorpService) timedUnlock(acct accounts.Account, duration time.Duration) error {
+	keyHandler, err := h.getKeyHandler(acct)
+
+	if err != nil {
+		return err
+	}
+
+	alreadyExisted, err := h.updateKeyHandler(keyHandler)
+
+	if err != nil {
+		return err
+	}
+
+	if alreadyExisted {
+		if keyHandler.cancel == nil {
+			return nil
+		}
+
+		close(keyHandler.cancel)
+		keyHandler.cancel = nil
+	}
+
+	if duration == 0 {
+		keyHandler.cancel = nil
+	} else if duration > 0 {
+		go keyHandler.timedLock(duration)
+	}
+
+	return nil
+}
+
+func (h *hashicorpService) updateKeyHandler(handler *hashicorpKeyHandler) (bool, error) {
+	var (
+		key *ecdsa.PrivateKey
+		alreadyExisted bool
+	)
+
+	if k := handler.key; k != nil && k.D.Int64() != 0 {
+		key = k
+		alreadyExisted = true
+	} else {
+		var err error
+		key, err = h.getKeyFromVault(handler.secret)
+
+		if err != nil {
+			return false, err
+		}
+
+		handler.key = key
+	}
+
+	return alreadyExisted, nil
+}
+
+func (h *hashicorpService) lock(acct accounts.Account) error {
+	keyHandler, err := h.getKeyHandler(acct)
+
+	if err != nil {
+		return err
+	}
+
+	// cancel any existing timed lock
+	if keyHandler.cancel != nil {
+		close(keyHandler.cancel)
+		keyHandler.cancel = nil
+	}
+
+	// zero the key if it is present
+	if keyHandler.key != nil {
+		b := keyHandler.key.D.Bits()
+		for i := range b {
+			b[i] = 0
+		}
+		keyHandler.key = nil
+	}
+
+	return nil
+}
+
+func (h *hashicorpKeyHandler) timedLock(duration time.Duration) {
+	h.mutex.Lock()
+	defer h.mutex.Unlock()
+
+	t := time.NewTimer(duration)
+	defer t.Stop()
+	h.cancel = make(chan struct{})
+
+	select {
+	case <-t.C:
+		b := h.key.D.Bits()
+		for i := range b {
+			b[i] = 0
+		}
+		h.key = nil
+	case <-h.cancel:
+		//do nothing
+	}
+}

console/bridge.go

@@ -127,6 +127,8 @@ func (b *bridge) OpenWallet(call otto.FunctionCall) (response otto.Value) {
 	return val
 }
 
+// TODO Implement bridge methods for LockVaultAccount and UnlockVaultAccount
+
 // UnlockAccount is a wrapper around the personal.unlockAccount RPC method that
 // uses a non-echoing password prompt to acquire the passphrase and executes the
 // original RPC method (saved in jeth.unlockAccount) with it to actually execute

internal/ethapi/api.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/ethereum/go-ethereum/accounts/vault"
 	"math/big"
 	"strings"
 	"time"
@@ -327,6 +328,7 @@ func (s *PrivateAccountAPI) ImportRawKey(privkey string, password string) (commo
 // UnlockAccount will unlock the account associated with the given address with
 // the given password for duration seconds. If duration is nil it will use a
 // default of 300 seconds. It returns an indication if the account was unlocked.
+// TODO it does not return an indication if the account was unlocked, need to make upstream change and also apply to UnlockVaultAccount
 func (s *PrivateAccountAPI) UnlockAccount(addr common.Address, password string, duration *uint64) (bool, error) {
 	const max = uint64(time.Duration(math.MaxInt64) / time.Second)
 	var d time.Duration
@@ -349,6 +351,65 @@ func (s *PrivateAccountAPI) LockAccount(addr common.Address) bool {
 	return fetchKeystore(s.am).Lock(addr) == nil
 }
 
+// fetchVaultBackends retrieves the backends for all supported vaults from the account manager.
+func fetchVaultWallet(am *accounts.Manager, acct accounts.Account) (*vault.VaultWallet, error) {
+
+	for _, b := range am.Backends(vault.BackendType) {
+		for _, w := range b.Wallets() {
+			if w.Contains(acct) {
+				return w.(*vault.VaultWallet), nil
+			}
+		}
+	}
+
+	return nil, fmt.Errorf("no vault wallet found that contains account %v", acct)
+}
+
+// UnlockVaultAccount will unlock the account associated with the given address for duration seconds  if the account is stored in a vault. If duration is nil it will use a default of 300 seconds.
+func (s *PrivateAccountAPI) UnlockVaultAccount(addr common.Address, duration *uint64) (bool, error) {
+	const max = uint64(time.Duration(math.MaxInt64) / time.Second)
+	var d time.Duration
+	if duration == nil {
+		d = 300 * time.Second
+	} else if *duration > max {
+		return false, errors.New("unlock duration too large")
+	} else {
+		d = time.Duration(*duration) * time.Second
+	}
+
+	acct := accounts.Account{Address: addr}
+
+	w, err := fetchVaultWallet(s.am, acct)
+
+	if err != nil {
+		return false, err
+	}
+
+	err = w.TimedUnlock(accounts.Account{Address: addr}, d)
+
+	return err == nil, err
+}
+
+// LockVaultAccount will lock the account associated with the given address if the account is stored in a vault.  Returns true if successful, false if an error was encountered.  The error message is logged.
+// This exists as a separate API endpoint to the existing LockAccount method to prevent coupling it to Quorum-related changes
+func (s *PrivateAccountAPI) LockVaultAccount(addr common.Address) bool {
+	acct := accounts.Account{Address: addr}
+
+	w, err := fetchVaultWallet(s.am, acct)
+
+	if err != nil {
+		log.Warn("unable to lock account", "err", err)
+		return false
+	}
+
+	if err = w.Lock(acct); err != nil {
+		log.Warn("unable to lock account", "err", err)
+		return false
+	}
+
+	return true
+}
+
 // signTransactions sets defaults and signs the given transaction
 // NOTE: the caller needs to ensure that the nonceLock is held, if applicable,
 // and release it after the transaction has been submitted to the tx pool