aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFelix Lange <fjl@twurst.com>2015-07-22 19:00:09 +0800
committerFelix Lange <fjl@twurst.com>2015-07-22 19:00:09 +0800
commitd1d45aa8390731ad9d0422e6bbf2d451d11dab4d (patch)
treed562f038e25ce5f0fa0554db03f8b596bf4704a3
parented1d2f858e009477a67689dc31d3fd41921b261e (diff)
parent06d5898d6a0f1f99505e55454688dd67237a98f9 (diff)
downloadgo-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar.gz
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar.bz2
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar.lz
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar.xz
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.tar.zst
go-tangerine-d1d45aa8390731ad9d0422e6bbf2d451d11dab4d.zip
Merge pull request #1503 from fjl/fix-accounts-race
accounts: fix data race when key is locked after the unlock timeout
-rw-r--r--accounts/account_manager.go13
-rw-r--r--accounts/accounts_test.go52
-rw-r--r--crypto/key_store_passphrase.go1
3 files changed, 45 insertions, 21 deletions
diff --git a/accounts/account_manager.go b/accounts/account_manager.go
index 2a9d9b0ae..c6c2787dc 100644
--- a/accounts/account_manager.go
+++ b/accounts/account_manager.go
@@ -78,8 +78,8 @@ func (am *Manager) DeleteAccount(address common.Address, auth string) error {
func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error) {
am.mutex.RLock()
+ defer am.mutex.RUnlock()
unlockedKey, found := am.unlocked[a.Address]
- am.mutex.RUnlock()
if !found {
return nil, ErrLocked
}
@@ -87,14 +87,17 @@ func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error)
return signature, err
}
-// unlock indefinitely
+// Unlock unlocks the given account indefinitely.
func (am *Manager) Unlock(addr common.Address, keyAuth string) error {
return am.TimedUnlock(addr, keyAuth, 0)
}
-// Unlock unlocks the account with the given address. The account
-// stays unlocked for the duration of timeout
-// it timeout is 0 the account is unlocked for the entire session
+// TimedUnlock unlocks the account with the given address. The account
+// stays unlocked for the duration of timeout. A timeout of 0 unlocks the account
+// until the program exits.
+//
+// If the accout is already unlocked, TimedUnlock extends or shortens
+// the active unlock timeout.
func (am *Manager) TimedUnlock(addr common.Address, keyAuth string, timeout time.Duration) error {
key, err := am.keyStore.GetKey(addr, keyAuth)
if err != nil {
diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go
index 9d09156af..0be8c50bd 100644
--- a/accounts/accounts_test.go
+++ b/accounts/accounts_test.go
@@ -23,9 +23,10 @@ import (
"time"
"github.com/ethereum/go-ethereum/crypto"
- "github.com/ethereum/go-ethereum/crypto/randentropy"
)
+var testSigData = make([]byte, 32)
+
func TestSign(t *testing.T) {
dir, ks := tmpKeyStore(t, crypto.NewKeyStorePlain)
defer os.RemoveAll(dir)
@@ -33,26 +34,24 @@ func TestSign(t *testing.T) {
am := NewManager(ks)
pass := "" // not used but required by API
a1, err := am.NewAccount(pass)
- toSign := randentropy.GetEntropyCSPRNG(32)
am.Unlock(a1.Address, "")
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != nil {
t.Fatal(err)
}
}
func TestTimedUnlock(t *testing.T) {
- dir, ks := tmpKeyStore(t, crypto.NewKeyStorePassphrase)
+ dir, ks := tmpKeyStore(t, crypto.NewKeyStorePlain)
defer os.RemoveAll(dir)
am := NewManager(ks)
pass := "foo"
a1, err := am.NewAccount(pass)
- toSign := randentropy.GetEntropyCSPRNG(32)
// Signing without passphrase fails because account is locked
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != ErrLocked {
t.Fatal("Signing should've failed with ErrLocked before unlocking, got ", err)
}
@@ -63,28 +62,26 @@ func TestTimedUnlock(t *testing.T) {
}
// Signing without passphrase works because account is temp unlocked
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != nil {
t.Fatal("Signing shouldn't return an error after unlocking, got ", err)
}
// Signing fails again after automatic locking
time.Sleep(150 * time.Millisecond)
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != ErrLocked {
t.Fatal("Signing should've failed with ErrLocked timeout expired, got ", err)
}
-
}
func TestOverrideUnlock(t *testing.T) {
- dir, ks := tmpKeyStore(t, crypto.NewKeyStorePassphrase)
+ dir, ks := tmpKeyStore(t, crypto.NewKeyStorePlain)
defer os.RemoveAll(dir)
am := NewManager(ks)
pass := "foo"
a1, err := am.NewAccount(pass)
- toSign := randentropy.GetEntropyCSPRNG(32)
// Unlock indefinitely
if err = am.Unlock(a1.Address, pass); err != nil {
@@ -92,7 +89,7 @@ func TestOverrideUnlock(t *testing.T) {
}
// Signing without passphrase works because account is temp unlocked
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != nil {
t.Fatal("Signing shouldn't return an error after unlocking, got ", err)
}
@@ -103,20 +100,45 @@ func TestOverrideUnlock(t *testing.T) {
}
// Signing without passphrase still works because account is temp unlocked
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != nil {
t.Fatal("Signing shouldn't return an error after unlocking, got ", err)
}
// Signing fails again after automatic locking
time.Sleep(150 * time.Millisecond)
- _, err = am.Sign(a1, toSign)
+ _, err = am.Sign(a1, testSigData)
if err != ErrLocked {
t.Fatal("Signing should've failed with ErrLocked timeout expired, got ", err)
}
}
-//
+// This test should fail under -race if signing races the expiration goroutine.
+func TestSignRace(t *testing.T) {
+ dir, ks := tmpKeyStore(t, crypto.NewKeyStorePlain)
+ defer os.RemoveAll(dir)
+
+ // Create a test account.
+ am := NewManager(ks)
+ a1, err := am.NewAccount("")
+ if err != nil {
+ t.Fatal("could not create the test account", err)
+ }
+
+ if err := am.TimedUnlock(a1.Address, "", 15*time.Millisecond); err != nil {
+ t.Fatalf("could not unlock the test account", err)
+ }
+ end := time.Now().Add(80 * time.Millisecond)
+ for time.Now().Before(end) {
+ if _, err := am.Sign(a1, testSigData); err == ErrLocked {
+ return
+ } else if err != nil {
+ t.Errorf("Sign error: %v", err)
+ return
+ }
+ }
+ t.Errorf("Account did not lock within the timeout")
+}
func tmpKeyStore(t *testing.T, new func(string) crypto.KeyStore) (string, crypto.KeyStore) {
d, err := ioutil.TempDir("", "eth-keystore-test")
diff --git a/crypto/key_store_passphrase.go b/crypto/key_store_passphrase.go
index 86b0b33c2..9700adce1 100644
--- a/crypto/key_store_passphrase.go
+++ b/crypto/key_store_passphrase.go
@@ -147,7 +147,6 @@ func (ks keyStorePassphrase) DeleteKey(keyAddr common.Address, auth string) (err
}
func decryptKeyFromFile(keysDirPath string, keyAddr common.Address, auth string) (keyBytes []byte, keyId []byte, err error) {
- fmt.Printf("%v\n", keyAddr.Hex())
m := make(map[string]interface{})
err = getKey(keysDirPath, keyAddr, &m)
if err != nil {