Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keyring's encrypted file backend integration #5355

Merged
merged 34 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
376fe1f
Start working on keyring's encrypted file backend integration
Dec 3, 2019
a97002e
Store keyring and test keyring files under ~/.appcli/{,test-}keyring …
Dec 4, 2019
32c374c
Keybase -> keyring migration is in fact an API and ABI breakage break…
Dec 4, 2019
79f691d
Merge branch 'master' into alessio/keyring-file-backend
Dec 4, 2019
7a11d4e
Merge branch 'master' into alessio/keyring-file-backend
fedekunze Dec 5, 2019
14c1252
Improve keyring docs
Dec 5, 2019
69bb4af
Merge branch 'master' into alessio/keyring-file-backend
Dec 5, 2019
411cb42
Replace COSMOS_SDK_TEST_KEYRING env var with a global --keybase-backe…
Dec 6, 2019
9f0c12e
Merge branch 'master' into alessio/keyring-file-backend
Dec 6, 2019
5e963a9
Merge branch 'master' into alessio/keyring-file-backend
Dec 6, 2019
ed89b9c
Add tests
Dec 6, 2019
aca7026
Cosmetic adjustments
Dec 6, 2019
fd2355e
Fix gentx
Dec 6, 2019
abc25e0
Bind flag
Dec 6, 2019
8eaa41b
Merge branch 'master' into alessio/keyring-file-backend
Dec 6, 2019
563e2f3
Simplify flag handling
Dec 7, 2019
6b925f3
Add test case
Dec 8, 2019
e72641c
Add test case
Dec 8, 2019
ed05225
Update CHANGELOG
Dec 8, 2019
fc190ab
Update
Dec 8, 2019
c4b3fa6
Merge branch 'master' into alessio/keyring-file-backend
Dec 9, 2019
dcd967e
Merge branch 'master' into alessio/keyring-file-backend
alexanderbez Dec 9, 2019
5f777bf
Create README.md
Dec 9, 2019
810083f
Typo
Dec 9, 2019
7fc84fb
Fixes
Dec 9, 2019
34ced6d
Update crypto/keys/README.md
Dec 9, 2019
c2b248f
Update crypto/keys/README.md
Dec 9, 2019
6b3b8f3
Fix NewInMemory description
Dec 9, 2019
afbb163
Add info on where keyring files are stored
Dec 9, 2019
d1cfa57
Merge branch 'master' into alessio/keyring-file-backend
alexanderbez Dec 10, 2019
1701751
Merge branch 'alessio/keyring-file-backend' of github.com:cosmos/cosm…
Dec 10, 2019
d79acf2
Merge branch 'master' into alessio/keyring-file-backend
Dec 10, 2019
81bf4fb
Merge branch 'master' into alessio/keyring-file-backend
alexanderbez Dec 11, 2019
24ba280
Merge branch 'master' into alessio/keyring-file-backend
Dec 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
alessio marked this conversation as resolved.
Show resolved Hide resolved

### Features

Expand All @@ -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`
Expand Down
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions client/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
const (
DefaultGasAdjustment = flags.DefaultGasAdjustment
DefaultGasLimit = flags.DefaultGasLimit
DefaultKeyringBackend = flags.DefaultKeyringBackend
GasFlagAuto = flags.GasFlagAuto
BroadcastBlock = flags.BroadcastBlock
BroadcastSync = flags.BroadcastSync
Expand All @@ -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
Expand Down
11 changes: 6 additions & 5 deletions client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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":
Expand Down
10 changes: 9 additions & 1 deletion client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 1 addition & 2 deletions client/keys/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions client/keys/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions client/keys/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keys

import (
"github.com/spf13/cobra"
"github.com/spf13/viper"

"github.com/cosmos/cosmos-sdk/client/flags"
)
Expand Down Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions client/keys/root_test.go
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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())
}
18 changes: 12 additions & 6 deletions client/keys/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keys
import (
"fmt"
"io"
"os"
"path/filepath"

"github.com/99designs/keyring"
Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 25 additions & 0 deletions client/keys/utils_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
47 changes: 47 additions & 0 deletions crypto/keys/README.md
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should state where NewKeyringFile stores key material on disk (i.e. the default path).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.
43 changes: 35 additions & 8 deletions crypto/keys/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 9 additions & 0 deletions crypto/keys/keyring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package keys

import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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)
}
Loading