Switch secret store to the keyring secret store

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## 0.34.8
+
+### Improvements
+
+#### SDK
+* Switching back-end to new secret store using library [Keyring](https://github.com/99designs/keyring)
+  * This is intended to provide stronger security guarantees by using the OS built-in secret store instead of the legacy key database which was designed to single user computers. 
+  * Security audit recommendation was to use the OS secret store. 
+  * Existing users are expected to migrate their keys using the migrate command:
+    * `gaiacli keys migrate`
+  * Support for legacay keystore is available through the secret store flag:
+    * `gaiacli keys add <key_name> --legacy-secret-store` 
+  * Signing transactions is allowed on the legacy store if you pass in the legacy secret store flag
+  * Running the tests locally may require entering your user password to access your keystore large amount of times (if anyone has a workaround, please leave a comment, thanks) 
+
+
 ## 0.34.7
 
 ### Bug Fixes

client/context/context.go

@@ -36,6 +36,7 @@ type CLIContext struct {
 	Client        rpcclient.Client
 	Keybase       cryptokeys.Keybase
 	Output        io.Writer
+	Input         io.Reader
 	OutputFormat  string
 	Height        int64
 	NodeURI       string
@@ -51,17 +52,18 @@ type CLIContext struct {
 	FromName      string
 	Indent        bool
 	SkipConfirm   bool
+	SecretStore   bool
 }
 
 // NewCLIContextWithFrom returns a new initialized CLIContext with parameters from the
 // command line using Viper. It takes a key name or address and populates the FromName and
 // FromAddress field accordingly.
-func NewCLIContextWithFrom(from string) CLIContext {
+func NewCLIContextWithFrom(from string, input io.Reader) CLIContext {
 	var nodeURI string
 	var rpc rpcclient.Client
 
 	genOnly := viper.GetBool(flags.FlagGenerateOnly)
-	fromAddress, fromName, err := GetFromFields(from, genOnly)
+	fromAddress, fromName, err := GetFromFields(from, genOnly, viper.GetBool((flags.FlagSecretStore)), input)
 	if err != nil {
 		fmt.Printf("failed to get from fields: %v", err)
 		os.Exit(1)
@@ -80,9 +82,10 @@ func NewCLIContextWithFrom(from string) CLIContext {
 		verifierHome = viper.GetString(flags.FlagHome)
 	}
 
-	return CLIContext{
+	return CLIContext{ //assign value to new boolean var based on parsing the flag
 		Client:        rpc,
 		Output:        os.Stdout,
+		Input:         input,
 		NodeURI:       nodeURI,
 		From:          viper.GetString(flags.FlagFrom),
 		OutputFormat:  viper.GetString(cli.OutputFlag),
@@ -97,12 +100,15 @@ func NewCLIContextWithFrom(from string) CLIContext {
 		FromName:      fromName,
 		Indent:        viper.GetBool(flags.FlagIndentResponse),
 		SkipConfirm:   viper.GetBool(flags.FlagSkipConfirmation),
+		SecretStore:   viper.GetBool(flags.FlagSecretStore),
 	}
 }
 
 // NewCLIContext returns a new initialized CLIContext with parameters from the
 // command line using Viper.
-func NewCLIContext() CLIContext { return NewCLIContextWithFrom(viper.GetString(flags.FlagFrom)) }
+func NewCLIContext(input io.Reader) CLIContext {
+	return NewCLIContextWithFrom(viper.GetString(flags.FlagFrom), input)
+}
 
 func createVerifier() tmlite.Verifier {
 	trustNodeDefined := viper.IsSet(flags.FlagTrustNode)
@@ -267,7 +273,7 @@ func (ctx CLIContext) PrintOutput(toPrint fmt.Stringer) (err error) {
 // GetFromFields returns a from account address and Keybase name given either
 // an address or key name. If genOnly is true, only a valid Bech32 cosmos
 // address is returned.
-func GetFromFields(from string, genOnly bool) (sdk.AccAddress, string, error) {
+func GetFromFields(from string, genOnly, legacySecretStore bool, input io.Reader) (sdk.AccAddress, string, error) {
 	if from == "" {
 		return nil, "", nil
 	}
@@ -281,12 +287,17 @@ func GetFromFields(from string, genOnly bool) (sdk.AccAddress, string, error) {
 		return addr, "", nil
 	}
 
-	keybase, err := keys.NewKeyBaseFromHomeFlag()
-	if err != nil {
-		return nil, "", err
+	var keybase cryptokeys.Keybase
+	if legacySecretStore {
+		var err error
+		keybase, err = keys.NewKeyBaseFromHomeFlag()
+		if err != nil {
+			return nil, "", err
+		}
+	} else {
+		keybase = keys.NewKeyringKeybase(input) //if flag is set, add flag to struct, then boolean variable.
 	}
-
-	var info cryptokeys.Info
+	var info cryptokeys.Info //chedck boolean var if true - do old keyring, else the new one
 	if addr, err := sdk.AccAddressFromBech32(from); err == nil {
 		info, err = keybase.GetByAddress(addr)
 		if err != nil {

client/flags/flags.go

@@ -54,6 +54,7 @@ const (
 	FlagRPCWriteTimeout    = "write-timeout"
 	FlagOutputDocument     = "output-document" // inspired by wget -O
 	FlagSkipConfirmation   = "yes"
+	FlagSecretStore        = "legacy-secret-store"
 )
 
 // LineBreak can be included in a command list to provide a blank line
@@ -99,6 +100,7 @@ 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().Bool(FlagSecretStore, false, "Use legacy secret store")
 
 		// --gas can accept integers and "simulate"
 		c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(

client/keys/add.go

@@ -72,6 +72,7 @@ the flag --nosort is set.
 	cmd.Flags().Uint32(flagAccount, 0, "Account number for HD derivation")
 	cmd.Flags().Uint32(flagIndex, 0, "Address index number for HD derivation")
 	cmd.Flags().Bool(flags.FlagIndentResponse, false, "Add indent to JSON response")
+	cmd.Flags().Bool(flags.FlagSecretStore, false, "Use legacy secret store")
 	return cmd
 }
 
@@ -101,16 +102,23 @@ func runAddCmd(cmd *cobra.Command, args []string) error {
 		kb = keys.NewInMemory()
 		encryptPassword = DefaultKeyPass
 	} else {
-		kb, err = NewKeyBaseFromHomeFlag()
-		if err != nil {
-			return err
+		// c.Flags().Bool(FlagSecretSore, false, "Use legacy secret store")
+		if viper.GetBool(flags.FlagSecretStore) {
+			fmt.Println("Using deprecated secret store. This will be removed in a future release.")
+			kb, err = NewKeyBaseFromHomeFlag()
+			if err != nil {
+				return err
+			}
+		} else {
+			kb = NewKeyringKeybase(inBuf)
 		}
-
 		_, err = kb.Get(name)
 		if err == nil {
 			// account exists, ask for user confirmation
 			response, err2 := input.GetConfirmation(fmt.Sprintf("override the existing name %s", name), inBuf)
+
 			if err2 != nil {
+				fmt.Println("testing error")
 				return err2
 			}
 			if !response {
@@ -152,13 +160,15 @@ func runAddCmd(cmd *cobra.Command, args []string) error {
 		}
 
 		// ask for a password when generating a local key
-		if viper.GetString(FlagPublicKey) == "" && !viper.GetBool(flags.FlagUseLedger) {
+		if viper.GetString(FlagPublicKey) == "" && !viper.GetBool(flags.FlagUseLedger) && viper.GetBool(flags.FlagSecretStore) {
 			encryptPassword, err = input.GetCheckPassword(
 				"Enter a passphrase to encrypt your key to disk:",
 				"Repeat the passphrase:", inBuf)
 			if err != nil {
 				return err
 			}
+		} else {
+			encryptPassword = DefaultKeyPass
 		}
 	}
 

client/keys/add_ledger_test.go

@@ -35,8 +35,11 @@ func Test_runAddCmdLedger(t *testing.T) {
 	assert.NoError(t, runAddCmd(cmd, []string{"keyname1"}))
 
 	// Now check that it has been stored properly
-	kb, err := NewKeyBaseFromHomeFlag()
-	assert.NoError(t, err)
+	kb := NewKeyringKeybase()
+	defer func() {
+		kb.Delete("keyname1", "", false)
+	}()
+
 	assert.NotNil(t, kb)
 	key1, err := kb.Get("keyname1")
 	assert.NoError(t, err)

client/keys/add_test.go

@@ -1,8 +1,10 @@
 package keys
 
 import (
+	"fmt"
 	"testing"
 
+	"github.com/99designs/keyring"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 
@@ -13,8 +15,17 @@ import (
 )
 
 func Test_runAddCmdBasic(t *testing.T) {
+
+	backends := keyring.AvailableBackends()
+
+	runningOnServer := false
+
+	if len(backends) == 2 && backends[1] == keyring.BackendType("file") {
+		runningOnServer = true
+	}
 	cmd := addKeyCommand()
 	assert.NotNil(t, cmd)
+
 	mockIn, _, _ := tests.ApplyMockIO(cmd)
 
 	kbHome, kbCleanUp := tests.NewTestCaseDir(t)
@@ -24,25 +35,52 @@ func Test_runAddCmdBasic(t *testing.T) {
 
 	viper.Set(cli.OutputFlag, OutputFormatText)
 
-	mockIn.Reset("test1234\ntest1234\n")
+	if runningOnServer {
+		mockIn.Reset("testpass1\ntestpass1\n")
+
+	} else {
+		mockIn.Reset("y\n")
+		kb := NewKeyringKeybase(mockIn)
+		defer func() {
+			kb.Delete("keyname1", "", false)
+			kb.Delete("keyname2", "", false)
+		}()
+	}
 	err := runAddCmd(cmd, []string{"keyname1"})
 	assert.NoError(t, err)
 
 	viper.Set(cli.OutputFlag, OutputFormatText)
 
-	mockIn.Reset("test1234\ntest1234\n")
+	if runningOnServer {
+		mockIn.Reset("testpass1\nN\n")
+
+	} else {
+		mockIn.Reset("N\n")
+	}
 	err = runAddCmd(cmd, []string{"keyname1"})
+	fmt.Println(err)
 	assert.Error(t, err)
 
 	viper.Set(cli.OutputFlag, OutputFormatText)
 
-	mockIn.Reset("y\ntest1234\ntest1234\n")
+	if runningOnServer {
+		mockIn.Reset("testpass1\ny\ntestpass1\n")
+
+	} else {
+		mockIn.Reset("y\n")
+	}
 	err = runAddCmd(cmd, []string{"keyname1"})
 	assert.NoError(t, err)
 
 	viper.Set(cli.OutputFlag, OutputFormatJSON)
 
-	mockIn.Reset("test1234\ntest1234\n")
+	if runningOnServer {
+		mockIn.Reset("testpass1\n")
+
+	} else {
+		mockIn.Reset("y\n")
+	}
 	err = runAddCmd(cmd, []string{"keyname2"})
 	assert.NoError(t, err)
+
 }

client/keys/delete.go

@@ -3,9 +3,11 @@ package keys
 import (
 	"bufio"
 	"errors"
+	"fmt"
 
 	"github.com/spf13/viper"
 
+	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/client/input"
 	"github.com/cosmos/cosmos-sdk/crypto/keys"
 
@@ -35,23 +37,31 @@ private keys stored in a ledger device cannot be deleted with the CLI.
 		"Skip confirmation prompt when deleting offline or ledger key references")
 	cmd.Flags().BoolP(flagForce, "f", false,
 		"Remove the key unconditionally without asking for the passphrase")
+	cmd.Flags().Bool(flags.FlagSecretStore, false, "Use legacy secret store")
 	return cmd
 }
 
 func runDeleteCmd(cmd *cobra.Command, args []string) error {
 	name := args[0]
+	var kb keys.Keybase
+	buf := bufio.NewReader(cmd.InOrStdin())
 
-	kb, err := NewKeyBaseFromHomeFlag()
-	if err != nil {
-		return err
+	if viper.GetBool(flags.FlagSecretStore) {
+		fmt.Println("Using deprecated secret store. This will be removed in a future release.")
+		var err error
+		kb, err = NewKeyBaseFromHomeFlag()
+		if err != nil {
+			return err
+		}
+	} else {
+		kb = NewKeyringKeybase(buf)
 	}
 
 	info, err := kb.Get(name)
 	if err != nil {
 		return err
 	}
 
-	buf := bufio.NewReader(cmd.InOrStdin())
 	if info.GetType() == keys.TypeLedger || info.GetType() == keys.TypeOffline {
 		if !viper.GetBool(flagYes) {
 			if err := confirmDeletion(buf); err != nil {
@@ -68,7 +78,7 @@ func runDeleteCmd(cmd *cobra.Command, args []string) error {
 	// skip passphrase check if run with --force
 	skipPass := viper.GetBool(flagForce)
 	var oldpass string
-	if !skipPass {
+	if !skipPass && viper.GetBool(flags.FlagSecretStore) {
 		if oldpass, err = input.GetPassword(
 			"DANGER - enter password to permanently delete key:", buf); err != nil {
 			return err