From 078c053969ce1724352ac477119d0ac2f7b8e6fa Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Wed, 11 Dec 2019 09:45:26 +0000 Subject: [PATCH] keyring's encrypted file backend integration (#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 | 6 +++++ Makefile | 1 - client/alias.go | 5 ++++ client/config.go | 11 ++++---- client/flags/flags.go | 10 +++++++- client/keys/export.go | 3 +-- client/keys/import.go | 3 +-- client/keys/root.go | 3 +++ client/keys/root_test.go | 9 +++++++ client/keys/utils.go | 18 +++++++++----- client/keys/utils_test.go | 25 +++++++++++++++++++ crypto/keys/README.md | 47 +++++++++++++++++++++++++++++++++++ crypto/keys/keyring.go | 43 ++++++++++++++++++++++++++------ crypto/keys/keyring_test.go | 9 +++++++ x/genutil/client/cli/gentx.go | 3 +++ 15 files changed, 171 insertions(+), 25 deletions(-) create mode 100644 client/keys/utils_test.go create mode 100644 crypto/keys/README.md diff --git a/CHANGELOG.md b/CHANGELOG.md index da4abf38ce45..c98332bea017 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 f02d60c893f9..8d947640f75a 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 e2a1caf167a4..eed043c54401 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 61258d96fe97..226d056fea06 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 62f2a8d012f9..9451500bfc21 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 393785501b45..dc6be244beec 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 127479ce994c..1e754b7852f9 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 b95740a2c036..5e54a5263ed4 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 6794d725e6f0..49681b17e95f 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 2b2ca06f0e5b..1096ae6e0790 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 000000000000..85fb7b63a116 --- /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 000000000000..df690636bc2a --- /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 839e15912f2e..28d9aaec8ff1 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 0916e0f46e9e..84cfe3816f34 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 f6a2a334c71b..462969e52ee7 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