tendermint / light-client

DEPRECATED: A light client for tendermint, supporting signatures, proofs, and validation (see github.com/tendermint/tendermint/lite)
Other
17 stars 9 forks source link

Machine writeability #23

Closed rigelrozanski closed 7 years ago

rigelrozanski commented 7 years ago

It would be nice to be able to send requests with the password as a flag to allow for more scripting solutions. def a hassle having to use spawn/expect commands (https://github.com/tendermint/trackomatron/blob/master/test/lightcli.sh) Not sure if this sacrifices security though

ethanfrey commented 7 years ago

Totally insecure as cli is visible by other users in the system.

Env variables are a bit more hidden, but visible by any process running as the same user. But maybe a choice.

Maybe you can research a secure and usable method?

ethanfrey commented 7 years ago

Some relavent web pages. Please read and let's think about a best practice to integrate here.

https://unix.stackexchange.com/questions/29111/safe-way-to-pass-password-for-1-programs-in-bash?rq=1

https://stackoverflow.com/questions/6607675/shell-script-password-security-of-command-line-parameters

It seems we can read stdin from a non-standard file descriptor. Will see how hard that is to integrate with the scripts or if we could just modify them a bit.

Let's talk tomorrow if this idea seems secure and easy to automate

rigelrozanski commented 7 years ago

Just looked at the links. I agree that it looks not difficult to implement, wondering how much more secure/susinct it is rather than just using spawn/expect as I've already done... I get that the flag system is insecure, but were you also implying that spawn/expect is also insecure? I guess it doesn't matter for functionality testing with dummy accounts, but if we need this as a part of any production scripts then yeah it's an issue

ethanfrey commented 7 years ago

No, spawn/expect is secure. Flags (and to a lesser extent env) is insecure.

I think this is secure and possibly easier to test that spawn/expect. But I'll let you be the judge there.

ethanfrey commented 7 years ago

I think this is not such the hard problem actually... looking at your code:

initLightCli(){
    expect <<- DONE
      spawn trackocli init --chainid=test_chain_id --node=tcp://localhost:46657
      expect "Is this valid (y/n)?" 
      send -- "y\r"
      expect eof
      catch wait result
DONE
}

I can emulate this with:

echo y | tmcli --chainid ball-n-chain --node tcp://146.185.166.215:46657 init --force-reset

The only issue I come across is you cannot test success or failure without parsing the output. We should return an error code to the shell if we output an error...

ethanfrey commented 7 years ago

Okay, made a fix to tmlibs, and updated vendor deps in 081a60983b093eb6b2ae896ca9fe04eaf7bba8b6

Now, we return an error code if the command failed. So you can handle this much easier from the command line....

# echo n | tmcli --chainid ball-n-chain --node tcp://146.185.166.215:46657 init --force-reset > /dev/null
# echo $?
1

# echo y | tmcli --chainid ball-n-chain --node tcp://146.185.166.215:46657 init --force-reset > /dev/null
# echo $?
0

You can use set -e on the top to exit as soon as anything fails. Or you can just use && to trigger actions.

# (echo n | tmcli --chainid ball-n-chain --node tcp://146.185.166.215:46657 init --force-reset > /dev/null) && echo cool

# (echo y | tmcli --chainid ball-n-chain --node tcp://146.185.166.215:46657 init --force-reset > /dev/null) && echo cool
cool

Note that echo cool is only executed when the command succeeds. You can use cmd || handleError if you want to execute handleError only when cmd fails. This is just standard bash stuff...

Main point is you can just redirect stdin and check error codes to handle the tests

ethanfrey commented 7 years ago

This also works with keys, even if it does spit out some warnings....

# echo '1234567890\n1234567890' | tmcli keys new foo
Enter a passphrase:stty: stdin isn't a terminal
stty: stdin isn't a terminal

Repeat the passphrase:stty: stdin isn't a terminal
stty: stdin isn't a terminal

foo     5162936A76C3C66111E85A3D393B845438D7BF8D

Better may be this:

# echo '1234567890\n1234567890' | tmcli keys new foo 2>/dev/null
Enter a passphrase:
Repeat the passphrase:
ERROR: Writing data: open /Users/ethan/.tmcli/keys/foo.pub: file exists
# echo $?
1
# echo '1234567890\n1234567890' | tmcli keys new bar 2>/dev/null
Enter a passphrase:
Repeat the passphrase:
bar     D025E627C20D6BBBE101B7D72C05057AEB373610
# echo $?
0
ethanfrey commented 7 years ago

I still wouldn't recommend using these echo commands on production system, but they work well for test scripts without opening any security issues, and let us use standard shell tooling better.

Please let me know if this addresses your issue and close it if so.

rigelrozanski commented 7 years ago

Awesome, I'm going to implement these changes within trackotron.

ethanfrey commented 7 years ago

Added first cli tests in 9f51bb636c28d41f96bdd9a128fbfcdcb86c4b4e

Seems to work nice with the error code. Take a look at this for an example: https://github.com/tendermint/light-client/blob/develop/test/keys.sh#L14-L19

Closing now, please re-open if there are some issues.