diff --git a/CHANGELOG.md b/CHANGELOG.md index da4abf38ce..c98332bea0 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/Makefile b/Makefile index f02d60c893..8d947640f7 100644 --- a/Makefile +++ b/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 diff --git a/client/alias.go b/client/alias.go index e2a1caf167..eed043c544 100644 --- a/client/alias.go +++ b/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 diff --git a/client/config.go b/client/config.go index 61258d96fe..226d056fea 100644 --- a/client/config.go +++ b/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": diff --git a/client/flags/flags.go b/client/flags/flags.go index 62f2a8d012..9451500bfc 100644 --- a/client/flags/flags.go +++ b/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) } diff --git a/client/keys/export.go b/client/keys/export.go index 393785501b..dc6be244be 100644 --- a/client/keys/export.go +++ b/client/keys/export.go @@ -9,14 +9,13 @@ import ( ) func exportKeyCommand() *cobra.Command { - cmd := &cobra.Command{ + return &cobra.Command{ Use: "export ", 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 { diff --git a/client/keys/import.go b/client/keys/import.go index 127479ce99..1e754b7852 100644 --- a/client/keys/import.go +++ b/client/keys/import.go @@ -10,14 +10,13 @@ import ( ) func importKeyCommand() *cobra.Command { - cmd := &cobra.Command{ + return &cobra.Command{ Use: "import ", 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 { diff --git a/client/keys/root.go b/client/keys/root.go index b95740a2c0..5e54a5263e 100644 --- a/client/keys/root.go +++ b/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 } diff --git a/client/keys/root_test.go b/client/keys/root_test.go index 6794d725e6..49681b17e9 100644 --- a/client/keys/root_test.go +++ b/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()) +} diff --git a/client/keys/utils.go b/client/keys/utils.go index 2b2ca06f0e..1096ae6e07 100644 --- a/client/keys/utils.go +++ b/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) { diff --git a/client/keys/utils_test.go b/client/keys/utils_test.go new file mode 100644 index 0000000000..85fb7b63a1 --- /dev/null +++ b/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) +} diff --git a/crypto/keys/README.md b/crypto/keys/README.md new file mode 100644 index 0000000000..df690636bc --- /dev/null +++ b/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. diff --git a/crypto/keys/keyring.go b/crypto/keys/keyring.go index 839e15912f..28d9aaec8f 100644 --- a/crypto/keys/keyring.go +++ b/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) { diff --git a/crypto/keys/keyring_test.go b/crypto/keys/keyring_test.go index 0916e0f46e..84cfe3816f 100644 --- a/crypto/keys/keyring_test.go +++ b/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) +} diff --git a/x/genutil/client/cli/gentx.go b/x/genutil/client/cli/gentx.go index f6a2a334c7..462969e52e 100644 --- a/x/genutil/client/cli/gentx.go +++ b/x/genutil/client/cli/gentx.go @@ -23,6 +23,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/codec" kbkeys "github.com/cosmos/cosmos-sdk/crypto/keys" "github.com/cosmos/cosmos-sdk/server" @@ -194,6 +195,8 @@ func GenTxCmd(ctx *server.Context, cdc *codec.Codec, mbm module.BasicManager, sm cmd.Flags().String(client.FlagOutputDocument, "", "write the genesis transaction JSON document to the given file instead of the default location") cmd.Flags().AddFlagSet(fsCreateValidator) + cmd.Flags().String(client.FlagKeyringBackend, client.DefaultKeyringBackend, "Select keyring's backend (os|file|test)") + viper.BindPFlag(flags.FlagKeyringBackend, cmd.Flags().Lookup(flags.FlagKeyringBackend)) cmd.MarkFlagRequired(client.FlagName) return cmd