Skip to content

Commit

Permalink
Merge PR cosmos#2555: Don't acquire lock on read-only keybase
Browse files Browse the repository at this point in the history
  • Loading branch information
cwgoes authored Oct 23, 2018
2 parents 701a00c + 450873d commit a231bd8
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 19 deletions.
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ IMPROVEMENTS
* Gaia CLI (`gaiacli`)
* [cli] #2060 removed `--select` from `block` command
* [cli] #2128 fixed segfault when exporting directly after `gaiad init`
* [cli] [\#1255](https://github.com/cosmos/cosmos-sdk/issues/1255) open KeyBase in read-only mode
for query-purpose CLI commands

* Gaia
* [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check.
Expand Down
6 changes: 3 additions & 3 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func runAddCmd(cmd *cobra.Command, args []string) error {
return errMissingName()
}
name = args[0]
kb, err = GetKeyBase()
kb, err = GetKeyBaseWithWritePerm()
if err != nil {
return err
}
Expand Down Expand Up @@ -174,7 +174,7 @@ func AddNewKeyRequestHandler(indent bool) http.HandlerFunc {
var kb keys.Keybase
var m NewKeyBody

kb, err := GetKeyBase()
kb, err := GetKeyBaseWithWritePerm()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(err.Error()))
Expand Down Expand Up @@ -311,7 +311,7 @@ func RecoverRequestHandler(indent bool) http.HandlerFunc {
return
}

kb, err := GetKeyBase()
kb, err := GetKeyBaseWithWritePerm()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(err.Error()))
Expand Down
4 changes: 2 additions & 2 deletions client/keys/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func deleteKeyCommand() *cobra.Command {
func runDeleteCmd(cmd *cobra.Command, args []string) error {
name := args[0]

kb, err := GetKeyBase()
kb, err := GetKeyBaseWithWritePerm()
if err != nil {
return err
}
Expand Down Expand Up @@ -73,7 +73,7 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
return
}

kb, err = GetKeyBase()
kb, err = GetKeyBaseWithWritePerm()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(err.Error()))
Expand Down
4 changes: 2 additions & 2 deletions client/keys/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func runUpdateCmd(cmd *cobra.Command, args []string) error {
name := args[0]

buf := client.BufferStdin()
kb, err := GetKeyBase()
kb, err := GetKeyBaseWithWritePerm()
if err != nil {
return err
}
Expand Down Expand Up @@ -74,7 +74,7 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
return
}

kb, err = GetKeyBase()
kb, err = GetKeyBaseWithWritePerm()
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
w.Write([]byte(err.Error()))
Expand Down
36 changes: 26 additions & 10 deletions client/keys/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keys

import (
"fmt"
"github.com/syndtr/goleveldb/leveldb/opt"
"path/filepath"

"github.com/spf13/viper"
Expand All @@ -25,14 +26,6 @@ var keybase keys.Keybase

type bechKeyOutFn func(keyInfo keys.Info) (KeyOutput, error)

// TODO make keybase take a database not load from the directory

// initialize a keybase based on the configuration
func GetKeyBase() (keys.Keybase, error) {
rootDir := viper.GetString(cli.HomeFlag)
return GetKeyBaseFromDir(rootDir)
}

// GetKeyInfo returns key info for a given name. An error is returned if the
// keybase cannot be retrieved or getting the info fails.
func GetKeyInfo(name string) (keys.Info, error) {
Expand Down Expand Up @@ -82,10 +75,33 @@ func ReadPassphraseFromStdin(name string) (string, error) {
return passphrase, nil
}

// initialize a keybase based on the configuration
// TODO make keybase take a database not load from the directory

// GetKeyBase initializes a read-only KeyBase based on the configuration.
func GetKeyBase() (keys.Keybase, error) {
rootDir := viper.GetString(cli.HomeFlag)
return GetKeyBaseFromDir(rootDir)
}

// GetKeyBaseWithWritePerm initialize a keybase based on the configuration with write permissions.
func GetKeyBaseWithWritePerm() (keys.Keybase, error) {
rootDir := viper.GetString(cli.HomeFlag)
return GetKeyBaseFromDirWithWritePerm(rootDir)
}

// GetKeyBaseFromDirWithWritePerm initializes a keybase at a particular dir with write permissions.
func GetKeyBaseFromDirWithWritePerm(rootDir string) (keys.Keybase, error) {
return getKeyBaseFromDirWithOpts(rootDir, nil)
}

// GetKeyBaseFromDir initializes a read-only keybase at a particular dir.
func GetKeyBaseFromDir(rootDir string) (keys.Keybase, error) {
return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: true})
}

func getKeyBaseFromDirWithOpts(rootDir string, o *opt.Options) (keys.Keybase, error) {
if keybase == nil {
db, err := dbm.NewGoLevelDB(KeyDBName, filepath.Join(rootDir, "keys"))
db, err := dbm.NewGoLevelDBWithOpts(KeyDBName, filepath.Join(rootDir, "keys"), o)
if err != nil {
return nil, err
}
Expand Down
39 changes: 39 additions & 0 deletions client/keys/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package keys

import (
"github.com/cosmos/cosmos-sdk/crypto/keys"
"github.com/stretchr/testify/require"
"io/ioutil"
"os"
"testing"
)

func TestGetKeyBaseLocks(t *testing.T) {
dir, err := ioutil.TempDir("", "cosmos-sdk-keys")
require.Nil(t, err)
defer os.RemoveAll(dir)

// Acquire db
kb, err := GetKeyBaseFromDirWithWritePerm(dir)
require.Nil(t, err)
_, _, err = kb.CreateMnemonic("foo", keys.English, "12345678", keys.Secp256k1)
require.Nil(t, err)
// Reset global variable
keybase = nil
// Try to acquire another keybase from the same storage
_, err = GetKeyBaseFromDirWithWritePerm(dir)
require.NotNil(t, err)
_, err = GetKeyBaseFromDirWithWritePerm(dir)
require.NotNil(t, err)

// Close the db and try to acquire the lock
kb.CloseDB()
kb, err = GetKeyBaseFromDirWithWritePerm(dir)
require.Nil(t, err)

// Try to acquire another read-only keybase from the same storage
_, err = GetKeyBaseFromDir(dir)
require.Nil(t, err)

kb.CloseDB()
}
2 changes: 1 addition & 1 deletion client/lcd/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func GetKeyBase(t *testing.T) crkeys.Keybase {

viper.Set(cli.HomeFlag, dir)

keybase, err := keys.GetKeyBase()
keybase, err := keys.GetKeyBaseWithWritePerm()
require.NoError(t, err)

return keybase
Expand Down
5 changes: 5 additions & 0 deletions crypto/keys/keybase.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,11 @@ func (kb dbKeybase) Update(name, oldpass string, getNewpass func() (string, erro
}
}

// CloseDB releases the lock and closes the storage backend.
func (kb dbKeybase) CloseDB() {
kb.db.Close()
}

func (kb dbKeybase) writeLocalKey(priv tmcrypto.PrivKey, name, passphrase string) Info {
// encrypt private key using passphrase
privArmor := mintkey.EncryptArmorPrivKey(priv, passphrase)
Expand Down
3 changes: 3 additions & 0 deletions crypto/keys/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ type Keybase interface {

// *only* works on locally-stored keys. Temporary method until we redo the exporting API
ExportPrivateKeyObject(name string, passphrase string) (crypto.PrivKey, error)

// Close closes the database.
CloseDB()
}

// KeyType reflects a human-readable type for key listing.
Expand Down
2 changes: 1 addition & 1 deletion server/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func GenerateCoinKey() (sdk.AccAddress, string, error) {
func GenerateSaveCoinKey(clientRoot, keyName, keyPass string, overwrite bool) (sdk.AccAddress, string, error) {

// get the keystore from the client
keybase, err := clkeys.GetKeyBaseFromDir(clientRoot)
keybase, err := clkeys.GetKeyBaseFromDirWithWritePerm(clientRoot)
if err != nil {
return sdk.AccAddress([]byte{}), "", err
}
Expand Down

0 comments on commit a231bd8

Please sign in to comment.