stdin redirection does not work when recovering accounts

[helder-moreira]

Summary of Bug

Redirecting mnemonic and keyring passphrase to stdin stopped working when recovering an account.

Version

>= v0.42.5

Steps to Reproduce

Add the following to a test.txt file:

sibling pride cross win vacuum exhaust oxygen social pioneer buffalo enlist world tube high boil profit tobacco valley awake sort model hour rebel will
12345678
12345678

Compile simapp from v0.42.4 and then try to redirect to stdin from test.txt file:

$ simd keys add test --recover < test.txt

- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

Compile simapp from v0.42.5 and then retry:

$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts
Usage:
  simd keys add <name> [flags]

Flags:
      --account uint32           Account number for HD derivation
      --algo string              Key signing algorithm to generate keys for (default "secp256k1")
      --coin-type uint32         coin type number for HD derivation (default 118)
      --dry-run                  Perform action, but don't add key to local keystore
      --hd-path string           Manual HD Path derivation (overrides BIP44 config)
  -h, --help                     help for add
      --index uint32             Address index number for HD derivation
  -i, --interactive              Interactively prompt user for BIP39 passphrase and mnemonic
      --ledger                   Store a local reference to a private key on a Ledger device
      --multisig strings         Construct and store a multisig public key (implies --pubkey)
      --multisig-threshold int   K out of N required signatures. For use in conjunction with --multisig (default 1)
      --no-backup                Don't print out seed phrase (if others are watching the terminal)
      --nosort                   Keys passed to --multisig are taken in the order they're supplied
      --pubkey string            Parse a public key in bech32 format and save it to disk
      --recover                  Provide seed phrase to recover existing key instead of creating

Global Flags:
      --home string              The application home directory (default "/home/ubuntu/.simapp")
      --keyring-backend string   Select keyring's backend (os|file|test) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --log_format string        The logging format (json|plain) (default "plain")
      --log_level string         The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --output string            Output format (text|json) (default "text")
      --trace                    print out full stack trace on errors

Also affects v0.42.6.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[amaury1093]

Thanks @helder-moreira! Did you try and debug to see where in the code this comes from (e.g. which PR)?

[helder-moreira]

Thanks @helder-moreira! Did you try and debug to see where in the code this comes from (e.g. which PR)?

@AmauryM No, I did try but not extensively.

[toschdev]

I've now tried to reproduce this behavior on various versions and two machines.
Here is the output that I got.

MacOS Catalina version 10.15.7 (works fine)

> go version
go version go1.16 darwin/amd64
> simd version
0.43.0-beta1-109-g1c7776671
> simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A8pi/kYASd2ZREdJ2v08mxyiJV3C1vydAVpqnmHBCD/r"}'
  mnemonic: ""

> simd version
0.42.6
> simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

> simd version
0.42.5
> simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

Ubuntu 20.04 (LTS) x64 (here is the issue)

user@simd-test:~$ go version
go version go1.16.5 linux/amd64
user@simd-test:~$ simd version
0.43.0-beta1-109-g1c7776671
user@simd-test:~$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts

user@simd-test:~$ simd version
0.42.6
user@simd-test:~$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts

user@simd-test:~$ simd version
0.42.5
user@simd-test:~$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts

Worked fine with version 0.42.4

user@simd-test:~$ simd version
0.42.4
user@simd-test:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

[likhita-809]

Worked fine with Ubuntu 20.04.1 (LTS)

user@Vostro-3491:~$ go version
go version go1.15.6 linux/amd64
user@Vostro-3491:~$ simd version
0.42.5
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491~$ simd version
0.42.6
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491:~$ simd version
0.43.0-beta1-138-gc91f3b265
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

Worked fine with Ubuntu 18.04.5 (LTS) also

user@Vostro-3491:~$ go version
go version go1.15.2 linux/amd64
user@Vostro-3491:~$ simd version
0.42.5
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491~$ simd version
0.42.6
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491:~$ simd version
0.43.0-beta1-138-gc91f3b265
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

[likhita-809]

Everything seems to be working fine. Maybe we don't need to work actively. Wdyt @helder-moreira ?

[helder-moreira]

@likhita-809 maybe its with go 1.16? Can you try on your end? @toschdev was also using go 1.16. Just checked on my linux setup and its go 1.16 too:

$ go version
go version go1.16.5 linux/amd64

[likhita-809]

Working fine with Ubuntu 20.04.1 (LTS), go 1.16 version

user@Vostro-3491:~$ go version
go version go1.16.5 linux/amd64
user@Vostro-3491:~$ simd version
0.42.5
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491~$ simd version
0.42.6
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

user@Vostro-3491:~$ simd version
0.43.0-beta1-138-gc91f3b265
user@Vostro-3491:~$ simd keys add test --recover < test.txt
- name: test
  type: local
  address: cosmos1rmtrsh8y4nmx864h4rw2hryy6yjk30lajx5xzl
  pubkey: cosmospub1addwnpepq09x9ljxqpyamx2ygaya4lfunvw2yf2actt0e8gptf4fucwppql7kyjeff9
  mnemonic: ""
  threshold: 0
  pubkeys: []

Seems like everything is good with go 1.16 too. Can you confirm if we can close this issue @helder-moreira ?

[helder-moreira]

I keep getting the same error. To be sure, I have just launched a fresh VM on digital ocean and here is the result:

$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.04
DISTRIB_CODENAME=focal
DISTRIB_DESCRIPTION="Ubuntu 20.04.2 LTS"

$ cat test.txt
sibling pride cross win vacuum exhaust oxygen social pioneer buffalo enlist world tube high boil profit tobacco valley awake sort model hour rebel will
12345678
12345678

$ go version
go version go1.16.5 linux/amd64
$ simd version
0.42.5
$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts
Usage:
  simd keys add <name> [flags]

Flags:
      --account uint32           Account number for HD derivation
      --algo string              Key signing algorithm to generate keys for (default "secp256k1")
      --coin-type uint32         coin type number for HD derivation (default 118)
      --dry-run                  Perform action, but don't add key to local keystore
      --hd-path string           Manual HD Path derivation (overrides BIP44 config)
  -h, --help                     help for add
      --index uint32             Address index number for HD derivation
  -i, --interactive              Interactively prompt user for BIP39 passphrase and mnemonic
      --ledger                   Store a local reference to a private key on a Ledger device
      --multisig strings         Construct and store a multisig public key (implies --pubkey)
      --multisig-threshold int   K out of N required signatures. For use in conjunction with --multisig (default 1)
      --no-backup                Don't print out seed phrase (if others are watching the terminal)
      --nosort                   Keys passed to --multisig are taken in the order they're supplied
      --pubkey string            Parse a public key in bech32 format and save it to disk
      --recover                  Provide seed phrase to recover existing key instead of creating

Global Flags:
      --home string              The application home directory (default "/home/helder/.simapp")
      --keyring-backend string   Select keyring's backend (os|file|test) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --log_format string        The logging format (json|plain) (default "plain")
      --log_level string         The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --output string            Output format (text|json) (default "text")
      --trace                    print out full stack trace on errors

$ simd version
0.42.6
$ simd keys add test --recover < test.txt
EOF
EOF
EOF
Error: too many failed passphrase attempts
Usage:
  simd keys add <name> [flags]

Flags:
      --account uint32           Account number for HD derivation
      --algo string              Key signing algorithm to generate keys for (default "secp256k1")
      --coin-type uint32         coin type number for HD derivation (default 118)
      --dry-run                  Perform action, but don't add key to local keystore
      --hd-path string           Manual HD Path derivation (overrides BIP44 config)
  -h, --help                     help for add
      --index uint32             Address index number for HD derivation
  -i, --interactive              Interactively prompt user for BIP39 passphrase and mnemonic
      --ledger                   Store a local reference to a private key on a Ledger device
      --multisig strings         Construct and store a multisig public key (implies --pubkey)
      --multisig-threshold int   K out of N required signatures. For use in conjunction with --multisig (default 1)
      --no-backup                Don't print out seed phrase (if others are watching the terminal)
      --nosort                   Keys passed to --multisig are taken in the order they're supplied
      --pubkey string            Parse a public key in bech32 format and save it to disk
      --recover                  Provide seed phrase to recover existing key instead of creating

Global Flags:
      --home string              The application home directory (default "/home/helder/.simapp")
      --keyring-backend string   Select keyring's backend (os|file|test) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --log_format string        The logging format (json|plain) (default "plain")
      --log_level string         The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --output string            Output format (text|json) (default "text")
      --trace                    print out full stack trace on errors

[amaury1093]

It seems like the regression was introduced in 9fe61a7

[daeMOn63]

Hi, I was hitting the same issue and managed to understand what's going on. I've only hit the issue using "file" backend, "os" works fine.

• Over https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/client/context.go#L339, the keyring is created and set on the context using ctx.Input as a reading source. It's a simple io.Reader.

• On the keys add command, a new bufio.Reader is created to read the mnemonic from: https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/client/keys/add.go#L86, which will hold a reference on the same io.Reader held by the context described above.

• Then, the mnemonic is read: https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/client/keys/add.go#L217. inBuf being a bufio.Reader, it will first fill its internal buffer which will "consume" 4096 bytes (the bufio.Reader default buffer size) from its underlying reader (here stdin / ctx.Input)

So at this point, we have:

inBuf:
- current read pointer @ mnemonic size
- internal cache containing mnemonic + keyring pass 1 + keyring pass 2 (assuming they size up to less than 4096 bytes)
stdin: current read pointer @ EOF

• Next the keyring is unlocked to write the key at https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/client/keys/add.go#L271, which will try to read the keyring password from its reader (ctx.Input, remember ?). But this buffer has already been fully consumed by the bufio.Reader when reading the mnemonic -> EOF error.

Now I'll need some feedback for the fix, the only option I've had so far is exploiting a "feature" of bufio.NewReader(r io.Reader) which returns a casted version of r if r is already a bufio.Reader (https://cs.opensource.google/go/go/+/refs/tags/go1.16.6:src/bufio/bufio.go;l=48-52). With that in mind, simply setting a bufio.Reader on the clientContext will allow every bufio.NewReader to actually share the same reader rather than creating new ones.

ie, from simapp/simd/cmd/root.go:

func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
	encodingConfig := simapp.MakeTestEncodingConfig()
	initClientCtx := client.Context{}.
		WithJSONMarshaler(encodingConfig.Marshaler).
		WithInterfaceRegistry(encodingConfig.InterfaceRegistry).
		WithTxConfig(encodingConfig.TxConfig).
		WithLegacyAmino(encodingConfig.Amino).
-		WithInput(os.Stdin).
+		WithInput(bufio.NewReader(os.Stdin)). 
		WithAccountRetriever(types.AccountRetriever{}).
		WithHomeDir(simapp.DefaultNodeHome).
		WithViper("") // In simapp, we don't use any prefix for env variables.

And also making sure the client/keys/add.go command buf is created with an initialized context:

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)
}

With those changes, the key recovery from piped file works again. I still don't really like my solution as it requires the client app to provide the right kind of buffer and I'm pretty sure we can do better. If anyone could give me some pointers I'd happily provide PR and tests.