From 68d4d20d66562b69499711fd513edb11e999905f Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Sun, 7 May 2017 00:53:18 +0300 Subject: [PATCH] cmd/statusd, geth: VerifyAccountPassword method exposed, closes #151 --- cmd/statusd/library.go | 35 +++---------- cmd/statusd/utils.go | 35 +++++++++++++ geth/accounts.go | 24 +++++++++ geth/accounts_test.go | 74 +++++++++++++++++++++++++++ static/config/linter_exclude_list.txt | 1 + 5 files changed, 142 insertions(+), 27 deletions(-) diff --git a/cmd/statusd/library.go b/cmd/statusd/library.go index 76b007e35..c4c9afa8c 100644 --- a/cmd/statusd/library.go +++ b/cmd/statusd/library.go @@ -77,44 +77,25 @@ func RecoverAccount(password, mnemonic *C.char) *C.char { return C.CString(string(outBytes)) } +//export VerifyAccountPassword +func VerifyAccountPassword(keyPath, address, password *C.char) *C.char { + _, err := geth.VerifyAccountPassword(C.GoString(keyPath), C.GoString(address), C.GoString(password)) + return makeJSONErrorResponse(err) +} + //export Login func Login(address, password *C.char) *C.char { // loads a key file (for a given address), tries to decrypt it using the password, to verify ownership // if verified, purges all the previous identities from Whisper, and injects verified key as shh identity err := geth.SelectAccount(C.GoString(address), C.GoString(password)) - - errString := "" - if err != nil { - fmt.Fprintln(os.Stderr, err) - errString = err.Error() - } - - out := geth.JSONError{ - Error: errString, - } - outBytes, _ := json.Marshal(&out) - - return C.CString(string(outBytes)) + return makeJSONErrorResponse(err) } //export Logout func Logout() *C.char { - // This is equivalent to clearing whisper identities err := geth.Logout() - - errString := "" - if err != nil { - fmt.Fprintln(os.Stderr, err) - errString = err.Error() - } - - out := geth.JSONError{ - Error: errString, - } - outBytes, _ := json.Marshal(&out) - - return C.CString(string(outBytes)) + return makeJSONErrorResponse(err) } //export CompleteTransaction diff --git a/cmd/statusd/utils.go b/cmd/statusd/utils.go index f70d4b32e..35282d1ef 100644 --- a/cmd/statusd/utils.go +++ b/cmd/statusd/utils.go @@ -3,6 +3,7 @@ package main import "C" import ( "encoding/json" + "io/ioutil" "math/big" "os" "path/filepath" @@ -58,6 +59,10 @@ func testExportedAPI(t *testing.T, done chan struct{}) { "create main and child accounts", testCreateChildAccount, }, + { + "verify account password", + testVerifyAccountPassword, + }, { "recover account", testRecoverAccount, @@ -105,6 +110,36 @@ func testExportedAPI(t *testing.T, done chan struct{}) { done <- struct{}{} } +func testVerifyAccountPassword(t *testing.T) bool { + tmpDir, err := ioutil.TempDir(os.TempDir(), "accounts") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) // nolint: errcheck + + if err = geth.ImportTestAccount(tmpDir, "test-account1.pk"); err != nil { + t.Fatal(err) + } + + accountFilePath := filepath.Join(tmpDir, "test-account1.pk") + response := geth.JSONError{} + rawResponse := VerifyAccountPassword( + C.CString(accountFilePath), + C.CString(testConfig.Account1.Address), + C.CString(testConfig.Account1.Password)) + + if err := json.Unmarshal([]byte(C.GoString(rawResponse)), &response); err != nil { + t.Errorf("cannot decode response (%s): %v", C.GoString(rawResponse), err) + return false + } + if response.Error != "" { + t.Errorf("unexpected error: %s", response.Error) + return false + } + + return true +} + func testGetDefaultConfig(t *testing.T) bool { // test Mainnet config nodeConfig := params.NodeConfig{} diff --git a/geth/accounts.go b/geth/accounts.go index 1da85ee5d..36429ce47 100644 --- a/geth/accounts.go +++ b/geth/accounts.go @@ -3,8 +3,10 @@ package geth import ( "errors" "fmt" + "io/ioutil" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" "github.com/status-im/status-go/extkeys" @@ -125,6 +127,28 @@ func RecoverAccount(password, mnemonic string) (address, pubKey string, err erro return address, pubKey, nil } +// VerifyAccountPassword tries to decrypt a given account key file, with a provided password. +// If no error is returned, then account is considered verified. +func VerifyAccountPassword(keyPath, address, password string) (*keystore.Key, error) { + keyJSON, err := ioutil.ReadFile(keyPath) + if err != nil { + return nil, fmt.Errorf("invalid account key file: %v", err) + } + + key, err := keystore.DecryptKey(keyJSON, password) + if err != nil { + return nil, err + } + + // avoid swap attack + addr := common.BytesToAddress(common.FromHex(address)) + if key.Address != addr { + return nil, fmt.Errorf("account mismatch: have %x, want %x", key.Address, addr) + } + + return key, nil +} + // SelectAccount selects current account, by verifying that address has corresponding account which can be decrypted // using provided password. Once verification is done, decrypted key is injected into Whisper (as a single identity, // all previous identities are removed). diff --git a/geth/accounts_test.go b/geth/accounts_test.go index 874069c08..963a5993b 100644 --- a/geth/accounts_test.go +++ b/geth/accounts_test.go @@ -2,12 +2,86 @@ package geth_test import ( "errors" + "fmt" + "io/ioutil" + "os" + "path/filepath" "reflect" "testing" + "github.com/ethereum/go-ethereum/common" "github.com/status-im/status-go/geth" ) +func TestVerifyAccountPassword(t *testing.T) { + tmpDir, err := ioutil.TempDir(os.TempDir(), "accounts") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) // nolint: errcheck + + if err = geth.ImportTestAccount(tmpDir, "test-account1.pk"); err != nil { + t.Fatal(err) + } + + accountFilePath := filepath.Join(tmpDir, "test-account1.pk") + account1Address := common.BytesToAddress(common.FromHex(testConfig.Account1.Address)) + account2Address := common.BytesToAddress(common.FromHex(testConfig.Account2.Address)) + + testCases := []struct { + name string + keyPath string + address string + password string + expectedError error + }{ + { + "correct address, correct password (decrypt should succeed)", + accountFilePath, + testConfig.Account1.Address, + testConfig.Account1.Password, + nil, + }, + { + "correct address, correct password, invalid key file", + filepath.Join(tmpDir, "non-existent-file.pk"), + testConfig.Account1.Address, + testConfig.Account1.Password, + fmt.Errorf("invalid account key file: open %s/non-existent-file.pk: no such file or directory", tmpDir), + }, + { + "wrong address, correct password", + accountFilePath, + testConfig.Account2.Address, // wrong address (swap attack) + testConfig.Account1.Password, + fmt.Errorf("account mismatch: have %x, want %x", account1Address, account2Address), + }, + { + "correct address, wrong password", + accountFilePath, + testConfig.Account1.Address, + "wrong password", // wrong password + errors.New("could not decrypt key with given passphrase"), + }, + } + for _, testCase := range testCases { + t.Log(testCase.name) + accountKey, err := geth.VerifyAccountPassword(testCase.keyPath, testCase.address, testCase.password) + if !reflect.DeepEqual(err, testCase.expectedError) { + t.Errorf("unexpected error: expected \n'%v', got \n'%v'", testCase.expectedError, err) + } + if err == nil { + if accountKey == nil { + t.Error("no error reported, but account key is missing") + } + accountAddress := common.BytesToAddress(common.FromHex(testCase.address)) + if accountKey.Address != accountAddress { + t.Errorf("account mismatch: have %x, want %x", accountKey.Address, accountAddress) + } + } + } +} + func TestAccountsList(t *testing.T) { err := geth.PrepareTestNode() if err != nil { diff --git a/static/config/linter_exclude_list.txt b/static/config/linter_exclude_list.txt index 7bfc49b87..113642d97 100644 --- a/static/config/linter_exclude_list.txt +++ b/static/config/linter_exclude_list.txt @@ -21,3 +21,4 @@ comment on exported function PopulateStaticPeers should be of the form "Populate comment on exported function AddPeer should be of the form "AddPeer ..." (golint) comment on exported function NotifyNode should be of the form "NotifyNode ..." (golint) comment on exported function TriggerTestSignal should be of the form "TriggerTestSignal ..." (golint) +comment on exported function VerifyAccountPassword should be of the form "VerifyAccountPassword ..." (golint)