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

fix: file keyring fails to add/import/export keys when input is not stdin (fix #9566) (backport #9821) #9881

Merged
merged 1 commit into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 1 deletion client/context.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package client

import (
"bufio"
"encoding/json"
"io"
"os"
Expand Down Expand Up @@ -70,7 +71,10 @@ func (ctx Context) WithKeyringOptions(opts ...keyring.Option) Context {

// WithInput returns a copy of the context with an updated input.
func (ctx Context) WithInput(r io.Reader) Context {
ctx.Input = r
// convert to a bufio.Reader to have a shared buffer between the keyring and the
// the Commands, ensuring a read from one advance the read pointer for the other.
// see https://github.com/cosmos/cosmos-sdk/issues/9566.
ctx.Input = bufio.NewReader(r)
return ctx
}

Expand Down
2 changes: 1 addition & 1 deletion client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ Example:
}

func runAddCmdPrepare(cmd *cobra.Command, args []string) error {
buf := bufio.NewReader(cmd.InOrStdin())
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}

buf := bufio.NewReader(clientCtx.Input)
return runAddCmd(clientCtx, cmd, args, buf)
}

Expand Down
47 changes: 46 additions & 1 deletion client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
bip39 "github.com/cosmos/go-bip39"
)

func Test_runAddCmdBasic(t *testing.T) {
Expand All @@ -30,7 +31,7 @@ func Test_runAddCmdBasic(t *testing.T) {
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
require.NoError(t, err)

clientCtx := client.Context{}.WithKeyringDir(kbHome)
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

t.Cleanup(func() {
Expand Down Expand Up @@ -227,3 +228,47 @@ func Test_runAddCmdDryRun(t *testing.T) {
})
}
}

func TestAddRecoverFileBackend(t *testing.T) {
cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())

mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kbHome := t.TempDir()

clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

cmd.SetArgs([]string{
"keyname1",
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
fmt.Sprintf("--%s=%s", cli.OutputFlag, OutputFormatText),
fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendFile),
fmt.Sprintf("--%s", flagRecover),
})

keyringPassword := "12345678"

entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
require.NoError(t, err)

mnemonic, err := bip39.NewMnemonic(entropySeed)
require.NoError(t, err)

mockIn.Reset(fmt.Sprintf("%s\n%s\n%s\n", mnemonic, keyringPassword, keyringPassword))
require.NoError(t, cmd.ExecuteContext(ctx))

kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendFile, kbHome, mockIn)
require.NoError(t, err)

t.Cleanup(func() {
mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword))
_ = kb.Delete("keyname1")
})

mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword))
info, err := kb.Key("keyname1")
require.NoError(t, err)
require.Equal(t, "keyname1", info.GetName())
}
2 changes: 1 addition & 1 deletion client/keys/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ FULLY AWARE OF THE RISKS. If you are unsure, you may want to do some research
and export your keys in ASCII-armored encrypted format.`,
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
buf := bufio.NewReader(cmd.InOrStdin())
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
buf := bufio.NewReader(clientCtx.Input)
unarmored, _ := cmd.Flags().GetBool(flagUnarmoredHex)
unsafe, _ := cmd.Flags().GetBool(flagUnsafe)

Expand Down
139 changes: 90 additions & 49 deletions client/keys/export_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keys

import (
"bufio"
"context"
"fmt"
"testing"
Expand All @@ -17,55 +18,95 @@ import (
)

func Test_runExportCmd(t *testing.T) {
cmd := ExportKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)

// Now add a temporary keybase
kbHome := t.TempDir()

// create a key
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
require.NoError(t, err)
t.Cleanup(func() {
kb.Delete("keyname1") // nolint:errcheck
})

path := sdk.GetConfig().GetFullBIP44Path()
_, err = kb.NewAccount("keyname1", testutil.TestMnemonic, "", path, hd.Secp256k1)
require.NoError(t, err)

// Now enter password
args := []string{
"keyname1",
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
testCases := []struct {
name string
keyringBackend string
extraArgs []string
userInput string
mustFail bool
expectedOutput string
}{
{
name: "--unsafe only must fail",
keyringBackend: keyring.BackendTest,
extraArgs: []string{"--unsafe"},
mustFail: true,
},
{
name: "--unarmored-hex must fail",
keyringBackend: keyring.BackendTest,
extraArgs: []string{"--unarmored-hex"},
mustFail: true,
},
{
name: "--unsafe --unarmored-hex fail with no user confirmation",
keyringBackend: keyring.BackendTest,
extraArgs: []string{"--unsafe", "--unarmored-hex"},
userInput: "",
mustFail: true,
expectedOutput: "",
},
{
name: "--unsafe --unarmored-hex succeed",
keyringBackend: keyring.BackendTest,
extraArgs: []string{"--unsafe", "--unarmored-hex"},
userInput: "y\n",
mustFail: false,
expectedOutput: "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n",
},
{
name: "file keyring backend properly read password and user confirmation",
keyringBackend: keyring.BackendFile,
extraArgs: []string{"--unsafe", "--unarmored-hex"},
// first 2 pass for creating the key, then unsafe export confirmation, then unlock keyring pass
userInput: "12345678\n12345678\ny\n12345678\n",
mustFail: false,
expectedOutput: "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n",
},
}

mockIn.Reset("123456789\n123456789\n")
cmd.SetArgs(args)

clientCtx := client.Context{}.
WithKeyringDir(kbHome).
WithKeyring(kb)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

require.NoError(t, cmd.ExecuteContext(ctx))

argsUnsafeOnly := append(args, "--unsafe")
cmd.SetArgs(argsUnsafeOnly)
require.Error(t, cmd.ExecuteContext(ctx))

argsUnarmoredHexOnly := append(args, "--unarmored-hex")
cmd.SetArgs(argsUnarmoredHexOnly)
require.Error(t, cmd.ExecuteContext(ctx))

argsUnsafeUnarmoredHex := append(args, "--unsafe", "--unarmored-hex")
cmd.SetArgs(argsUnsafeUnarmoredHex)
require.Error(t, cmd.ExecuteContext(ctx))

mockIn, mockOut := testutil.ApplyMockIO(cmd)
mockIn.Reset("y\n")
require.NoError(t, cmd.ExecuteContext(ctx))
require.Equal(t, "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n", mockOut.String())
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
kbHome := t.TempDir()
defaultArgs := []string{
"keyname1",
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, tc.keyringBackend),
}

cmd := ExportKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())

cmd.SetArgs(append(defaultArgs, tc.extraArgs...))
mockIn, mockOut := testutil.ApplyMockIO(cmd)

mockIn.Reset(tc.userInput)
mockInBuf := bufio.NewReader(mockIn)

// create a key
kb, err := keyring.New(sdk.KeyringServiceName(), tc.keyringBackend, kbHome, bufio.NewReader(mockInBuf))
require.NoError(t, err)
t.Cleanup(func() {
kb.Delete("keyname1") // nolint:errcheck
})

path := sdk.GetConfig().GetFullBIP44Path()
_, err = kb.NewAccount("keyname1", testutil.TestMnemonic, "", path, hd.Secp256k1)
require.NoError(t, err)

clientCtx := client.Context{}.
WithKeyringDir(kbHome).
WithKeyring(kb).
WithInput(mockInBuf)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

err = cmd.ExecuteContext(ctx)
if tc.mustFail {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tc.expectedOutput, mockOut.String())
}
})
}
}
2 changes: 1 addition & 1 deletion client/keys/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ func ImportKeyCommand() *cobra.Command {
Long: "Import a ASCII armored private key into the local keybase.",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
buf := bufio.NewReader(cmd.InOrStdin())
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
buf := bufio.NewReader(clientCtx.Input)

bz, err := ioutil.ReadFile(args[1])
if err != nil {
Expand Down
Loading