diff --git a/accounts/account_manager.go b/accounts/account_manager.go index 2a9d9b0ae4..c6c2787dcf 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 9d09156afc..0be8c50bd2 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")