keyring's encrypted file backend integration (cosmos#5355)

Client commands accept a new `--keyring-backend` option through which users can specify which backend should be used by the new key store:
- os: use OS default credentials storage (default).
- file: use encrypted file-based store.
- test: use password-less key store (highly insecure).

CHANGELOG.md

@@ -84,6 +84,7 @@ if the provided arguments are invalid.
 * (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) The gas required to pass the `AnteHandler` has
 increased significantly due to modular `AnteHandler` support. Increase GasLimit accordingly.
 * (rest) [\#5336](https://github.com/cosmos/cosmos-sdk/issues/5336) `MsgEditValidator` uses `description` instead of `Description` as a JSON key.
+* (keys) [\#5097](https://github.com/cosmos/cosmos-sdk/pull/5097) Due to the keybase -> keyring transition, keys need to be migrated. See `keys migrate` command for more info.
 
 ### Features
 
@@ -104,6 +105,11 @@ the following [issue](https://github.com/keybase/go-keychain/issues/47) with the
 you encounter this issue, you must upgrade your xcode command line tools to version >= `10.2`. You can
 upgrade via: `sudo rm -rf /Library/Developer/CommandLineTools; xcode-select --install`. Verify the
 correct version via: `pkgutil --pkg-info=com.apple.pkg.CLTools_Executables`.
+* [\#5355](https://github.com/cosmos/cosmos-sdk/pull/5355) Client commands accept a new `--keyring-backend` option through which users can specify which backend should be used
+by the new key store:
+  - `os`: use OS default credentials storage (default).
+  - `file`: use encrypted file-based store.
+  - `test`: use password-less key store. *For testing purposes only. Use it at your own risk.*
 * (keys) [\#5097](https://github.com/cosmos/cosmos-sdk/pull/5097) New `keys migrate` command to assist users migrate their keys
 to the new keyring.
 * (keys) [\#5366](https://github.com/cosmos/cosmos-sdk/pull/5366) `keys list` now accepts a `--list-names` option to list key names only, whilst the `keys delete`

Makefile

@@ -10,7 +10,6 @@ SIMAPP = ./simapp
 MOCKS_DIR = $(CURDIR)/tests/mocks
 
 export GO111MODULE = on
-export COSMOS_SDK_TEST_KEYRING = y
 
 all: tools build lint test
 

client/alias.go

@@ -24,6 +24,7 @@ import (
 const (
 	DefaultGasAdjustment   = flags.DefaultGasAdjustment
 	DefaultGasLimit        = flags.DefaultGasLimit
+	DefaultKeyringBackend  = flags.DefaultKeyringBackend
 	GasFlagAuto            = flags.GasFlagAuto
 	BroadcastBlock         = flags.BroadcastBlock
 	BroadcastSync          = flags.BroadcastSync
@@ -46,12 +47,16 @@ const (
 	FlagDryRun             = flags.FlagDryRun
 	FlagGenerateOnly       = flags.FlagGenerateOnly
 	FlagIndentResponse     = flags.FlagIndentResponse
+	FlagKeyringBackend     = flags.FlagKeyringBackend
 	FlagListenAddr         = flags.FlagListenAddr
 	FlagMaxOpenConnections = flags.FlagMaxOpenConnections
 	FlagRPCReadTimeout     = flags.FlagRPCReadTimeout
 	FlagRPCWriteTimeout    = flags.FlagRPCWriteTimeout
 	FlagOutputDocument     = flags.FlagOutputDocument
 	FlagSkipConfirmation   = flags.FlagSkipConfirmation
+	KeyringBackendFile     = flags.KeyringBackendFile
+	KeyringBackendOS       = flags.KeyringBackendOS
+	KeyringBackendTest     = flags.KeyringBackendTest
 	DefaultKeyPass         = keys.DefaultKeyPass
 	FlagAddress            = keys.FlagAddress
 	FlagPublicKey          = keys.FlagPublicKey

client/config.go

@@ -20,10 +20,11 @@ const (
 )
 
 var configDefaults = map[string]string{
-	"chain-id":       "",
-	"output":         "text",
-	"node":           "tcp://localhost:26657",
-	"broadcast-mode": "sync",
+	"chain-id":        "",
+	"keyring-backend": "os",
+	"output":          "text",
+	"node":            "tcp://localhost:26657",
+	"broadcast-mode":  "sync",
 }
 
 // ConfigCmd returns a CLI command to interactively create an application CLI
@@ -98,7 +99,7 @@ func runConfigCmd(cmd *cobra.Command, args []string) error {
 
 	// set config value for a given key
 	switch key {
-	case "chain-id", "output", "node", "broadcast-mode":
+	case "chain-id", "output", "node", "broadcast-mode", "keyring-backend":
 		tree.Set(key, value)
 
 	case "trace", "trust-node", "indent":

client/flags/flags.go

@@ -20,6 +20,12 @@ const (
 	DefaultGasLimit      = 200000
 	GasFlagAuto          = "auto"
 
+	// DefaultKeyringBackend
+	DefaultKeyringBackend = KeyringBackendOS
+	KeyringBackendFile    = "file"
+	KeyringBackendOS      = "os"
+	KeyringBackendTest    = "test"
+
 	// BroadcastBlock defines a tx broadcasting mode where the client waits for
 	// the tx to be committed in a block.
 	BroadcastBlock = "block"
@@ -54,6 +60,7 @@ const (
 	FlagRPCWriteTimeout    = "write-timeout"
 	FlagOutputDocument     = "output-document" // inspired by wget -O
 	FlagSkipConfirmation   = "yes"
+	FlagKeyringBackend     = "keyring-backend"
 )
 
 // LineBreak can be included in a command list to provide a blank line
@@ -99,16 +106,17 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 		c.Flags().Bool(FlagDryRun, false, "ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it")
 		c.Flags().Bool(FlagGenerateOnly, false, "Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible and the node operates offline)")
 		c.Flags().BoolP(FlagSkipConfirmation, "y", false, "Skip tx broadcasting prompt confirmation")
+		c.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|test)")
 
 		// --gas can accept integers and "simulate"
 		c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(
 			"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)",
 			GasFlagAuto, DefaultGasLimit,
 		))
-
 		viper.BindPFlag(FlagTrustNode, c.Flags().Lookup(FlagTrustNode))
 		viper.BindPFlag(FlagUseLedger, c.Flags().Lookup(FlagUseLedger))
 		viper.BindPFlag(FlagNode, c.Flags().Lookup(FlagNode))
+		viper.BindPFlag(FlagKeyringBackend, c.Flags().Lookup(FlagKeyringBackend))
 
 		c.MarkFlagRequired(FlagChainID)
 	}

client/keys/export.go

@@ -9,14 +9,13 @@ import (
 )
 
 func exportKeyCommand() *cobra.Command {
-	cmd := &cobra.Command{
+	return &cobra.Command{
 		Use:   "export <name>",
 		Short: "Export private keys",
 		Long:  `Export a private key from the local keybase in ASCII-armored encrypted format.`,
 		Args:  cobra.ExactArgs(1),
 		RunE:  runExportCmd,
 	}
-	return cmd
 }
 
 func runExportCmd(cmd *cobra.Command, args []string) error {

client/keys/import.go

@@ -10,14 +10,13 @@ import (
 )
 
 func importKeyCommand() *cobra.Command {
-	cmd := &cobra.Command{
+	return &cobra.Command{
 		Use:   "import <name> <keyfile>",
 		Short: "Import private keys into the local keybase",
 		Long:  "Import a ASCII armored private key into the local keybase.",
 		Args:  cobra.ExactArgs(2),
 		RunE:  runImportCmd,
 	}
-	return cmd
 }
 
 func runImportCmd(cmd *cobra.Command, args []string) error {

client/keys/root.go

@@ -2,6 +2,7 @@ package keys
 
 import (
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
 
 	"github.com/cosmos/cosmos-sdk/client/flags"
 )
@@ -31,5 +32,7 @@ func Commands() *cobra.Command {
 		parseKeyStringCommand(),
 		migrateCommand(),
 	)
+	cmd.PersistentFlags().String(flags.FlagKeyringBackend, flags.DefaultKeyringBackend, "Select keyring's backend (os|file|test)")
+	viper.BindPFlag(flags.FlagKeyringBackend, cmd.Flags().Lookup(flags.FlagKeyringBackend))
 	return cmd
 }

client/keys/root_test.go

@@ -1,9 +1,13 @@
 package keys
 
 import (
+	"os"
 	"testing"
 
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
+
+	"github.com/cosmos/cosmos-sdk/client/flags"
 )
 
 func TestCommands(t *testing.T) {
@@ -13,3 +17,8 @@ func TestCommands(t *testing.T) {
 	// Commands are registered
 	assert.Equal(t, 11, len(rootCommands.Commands()))
 }
+
+func TestMain(m *testing.M) {
+	viper.Set(flags.FlagKeyringBackend, flags.KeyringBackendTest)
+	os.Exit(m.Run())
+}

client/keys/utils.go

@@ -3,7 +3,6 @@ package keys
 import (
 	"fmt"
 	"io"
-	"os"
 	"path/filepath"
 
 	"github.com/99designs/keyring"
@@ -46,14 +45,21 @@ func NewKeyringFromHomeFlag(input io.Reader) (keys.Keybase, error) {
 	return NewKeyringFromDir(viper.GetString(flags.FlagHome), input)
 }
 
-// NewKeyBaseFromDir initializes a keyring at a particular dir.
-// If the COSMOS_SDK_TEST_KEYRING environment variable is set and not empty it will
-// return an on-disk, password-less keyring that could be used for testing purposes.
+// NewKeyBaseFromDir initializes a keyring at the given directory.
+// If the viper flag flags.FlagKeyringBackend is set to file, it returns an on-disk keyring with
+// CLI prompt support only. If flags.FlagKeyringBackend is set to test it will return an on-disk,
+// password-less keyring that could be used for testing purposes.
 func NewKeyringFromDir(rootDir string, input io.Reader) (keys.Keybase, error) {
-	if os.Getenv("COSMOS_SDK_TEST_KEYRING") != "" {
+	keyringBackend := viper.GetString(flags.FlagKeyringBackend)
+	switch keyringBackend {
+	case flags.KeyringBackendTest:
 		return keys.NewTestKeyring(sdk.GetConfig().GetKeyringServiceName(), rootDir)
+	case flags.KeyringBackendFile:
+		return keys.NewKeyringFile(sdk.GetConfig().GetKeyringServiceName(), rootDir, input)
+	case flags.KeyringBackendOS:
+		return keys.NewKeyring(sdk.GetConfig().GetKeyringServiceName(), rootDir, input)
 	}
-	return keys.NewKeyring(sdk.GetConfig().GetKeyringServiceName(), rootDir, input)
+	return nil, fmt.Errorf("unknown keyring backend %q", keyringBackend)
 }
 
 func getLazyKeyBaseFromDir(rootDir string) (keys.Keybase, error) {

client/keys/utils_test.go

@@ -0,0 +1,25 @@
+package keys
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/tests"
+)
+
+func TestNewKeyringFromDir(t *testing.T) {
+	dir, cleanup := tests.NewTestCaseDir(t)
+	defer cleanup()
+	viper.Set(flags.FlagKeyringBackend, flags.KeyringBackendTest)
+	_, err := NewKeyringFromDir(filepath.Join(dir, "test"), nil)
+	require.NoError(t, err)
+	viper.Set(flags.FlagKeyringBackend, flags.KeyringBackendFile)
+	buf := strings.NewReader("password\npassword\n")
+	_, err = NewKeyringFromDir(filepath.Join(dir, "test"), buf)
+	require.NoError(t, err)
+}

crypto/keys/README.md

@@ -0,0 +1,47 @@
+# Keys API
+
+[![API Reference](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys?status.svg)](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys)
+
+
+## The Keybase interface
+
+The [Keybase](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#Keybase) interface defines
+the methods that a type needs to implement to be used as key storage backend. This package provides
+few implementations out-of-the-box.
+
+## Constructors
+
+### New
+
+The [New](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#New) constructor returns
+an on-disk implementation backed by LevelDB storage that has been the default implementation used by the SDK until v0.38.0.
+Due to [security concerns](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-006-secret-store-replacement.md), we recommend to drop
+it in favor of the `NewKeyring` or `NewKeyringFile` constructors. We strongly advise to migrate away from this function as **it may be removed in a future
+release**.
+
+### NewInMemory
+
+The [NewInMemory](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#NewInMemory) constructor returns
+an implementation backed by an in-memory, goroutine-safe map that we've historically used for testing purposes or on-the-fly
+key generation and we consider safe for the aforementioned use cases since the generated keys are discarded when the process
+terminates or the type instance is garbage collected.
+
+### NewKeyring
+
+The [NewKeyring](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#NewKeyring) constructor returns
+an implementation backed by the [Keyring](https://github.com/99designs/keyring) library, whose aim is to provide a common
+abstraction and uniform interface between secret stores available for Windows, macOS, and most GNU/Linux distributions.
+The instance returned by this constructor will use the operating system's default credentials store, which will then handle
+keys storage operations securely. 
+
+### NewKeyringFile, NewTestKeyring
+
+Both [NewKeyringFile](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#NewKeyringFile) and
+[NewTestKeyring](https://godoc.org/github.com/cosmos/cosmos-sdk/crypto/keys#NewTestKeyring) constructors return
+on-disk implementations backed by the [Keyring](https://github.com/99designs/keyring) `file` backend.
+Whilst `NewKeyringFile` returns a secure, encrypted file-based type that requires user's password in order to
+function correctly, the implementation returned by `NewTestKeyring` stores keys information in clear text and **must be used
+only for testing purposes**.
+
+`NewKeyringFile` and `NewTestKeyring` store key files in the client home directory's `keyring`
+and `keyring-test` subdirectories respectively.

crypto/keys/keyring.go

@@ -26,6 +26,11 @@ import (
 	"github.com/cosmos/cosmos-sdk/types"
 )
 
+const (
+	keyringDirName     = "keyring"
+	testKeyringDirName = "keyring-test"
+)
+
 var _ Keybase = keyringKeybase{}
 
 // keyringKeybase implements the Keybase interface by using the Keyring library
@@ -47,6 +52,16 @@ func NewKeyring(name string, dir string, userInput io.Reader) (Keybase, error) {
 	return newKeyringKeybase(db), nil
 }
 
+// NewKeyringFile creates a new instance of an encrypted file-backed keyring.
+func NewKeyringFile(name string, dir string, userInput io.Reader) (Keybase, error) {
+	db, err := keyring.Open(newFileBackendKeyringConfig(name, dir, userInput))
+	if err != nil {
+		return nil, err
+	}
+
+	return newKeyringKeybase(db), nil
+}
+
 // NewTestKeyring creates a new instance of an on-disk keyring for
 // testing purposes that does not prompt users for password.
 func NewTestKeyring(name string, dir string) (Keybase, error) {
@@ -458,12 +473,30 @@ func lkbToKeyringConfig(name, dir string, buf io.Reader, test bool) keyring.Conf
 		return keyring.Config{
 			AllowedBackends:  []keyring.BackendType{"file"},
 			ServiceName:      name,
-			FileDir:          dir,
+			FileDir:          filepath.Join(dir, testKeyringDirName),
 			FilePasswordFunc: fakePrompt,
 		}
 	}
 
-	realPrompt := func(prompt string) (string, error) {
+	return keyring.Config{
+		ServiceName:      name,
+		FileDir:          dir,
+		FilePasswordFunc: newRealPrompt(dir, buf),
+	}
+}
+
+func newFileBackendKeyringConfig(name, dir string, buf io.Reader) keyring.Config {
+	fileDir := filepath.Join(dir, keyringDirName)
+	return keyring.Config{
+		AllowedBackends:  []keyring.BackendType{"file"},
+		ServiceName:      name,
+		FileDir:          fileDir,
+		FilePasswordFunc: newRealPrompt(fileDir, buf),
+	}
+}
+
+func newRealPrompt(dir string, buf io.Reader) func(string) (string, error) {
+	return func(prompt string) (string, error) {
 		keyhashStored := false
 		keyhashFilePath := filepath.Join(dir, "keyhash")
 
@@ -532,12 +565,6 @@ func lkbToKeyringConfig(name, dir string, buf io.Reader, test bool) keyring.Conf
 			return pass, nil
 		}
 	}
-
-	return keyring.Config{
-		ServiceName:      name,
-		FileDir:          dir,
-		FilePasswordFunc: realPrompt,
-	}
 }
 
 func fakePrompt(prompt string) (string, error) {

crypto/keys/keyring_test.go

@@ -2,6 +2,7 @@
 package keys
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -323,3 +324,11 @@ func TestLazySeedPhraseKeyRing(t *testing.T) {
 	require.Equal(t, info.GetPubKey().Address(), newInfo.GetPubKey().Address())
 	require.Equal(t, info.GetPubKey(), newInfo.GetPubKey())
 }
+
+func TestNewKeyringFile(t *testing.T) {
+	dir, cleanup := tests.NewTestCaseDir(t)
+	defer cleanup()
+	buf := strings.NewReader("password\npassword\n")
+	_, err := NewKeyringFile("test", dir, buf)
+	require.NoError(t, err)
+}