Import private key from keystore.json files

sembrestels commented 4 years ago

🚀 Feature

Include keystore files as an option to import the private key in aragon-cli.

Have you read the Contributing Guidelines on issues?

Yes

Motivation

The two options to set up a private key to be used by the aragon-cli are a private key and a mnemonic (see guides faq), and both expose the secret in plaintext to the filesystem. The only option to avoid it is using frame, with the --use frame flag, but it can be cumbersome to set it up just to interact with the aragon-cli in a secure way.

Pitch

If a keystore.json file is inside the ~/.aragon directory, the aragon-cli should take it into account as well as how it takes the <network>_key.json and mnemonic.json. For each required signature, aragon-cli should show a prompt asking for the password to decrypt the keystore.json.

welcome[bot] commented 4 years ago

Thanks for opening your first issue in aragonCLI! Someone will circle back soon ⚡

kernelwhisperer commented 4 years ago

How would the encrypt/decrypt feature work? Where is the encryption key stored?

kernelwhisperer commented 4 years ago

I think storing the mnemonic/private key securely is the single most important feature security-wise. (compared to #953 which is like a band-aid on an open wound :sweat_smile: ).

In my research I've concluded we should use the native os keychain mechanism: https://github.com/atom/node-keytar as well as extracting the sign transaction functionality in a standalone cli that has 0 dependencies, a kernel if you will. A kernel that can be used by other CLIs/plugins/etc.

Perhaps the frame folks could create such binary since they should be using this system to begin with.

kernelwhisperer commented 4 years ago

To recap, security should be approached as follows (IMO):

1. (most important) Protect against other programs installed on the machine.

Problem: a malicious program e.g.: aragon-mal could read private keys from ~/.aragon because it knows that's where we store secrets.

Solution: instead of storing the private keys in a plaintext file, they should be encrypted using the native MacOS/Windows/Linux capabilities: atom/node-keytar.

The way node-keytar works, it allows only our binary to read/write secrets, without having to manage an encryption key. It checks what is the path to the binary which should be something like ~/npm-global/node_modules/aragon/dist/index.js and it whitelists it.

Note: another similar approach is to use some other encryption mechanism (as proposed by @sembrestels, although this will require maintainers to control an encryption key, having a bigger attack surface.)

Important note for implementing node-keytar: the CLI will not be safe if it's executed by node index.js because the owner of the secrets will be considered to be node and other node processes can access this info. The CLI should be executed in production like it is today, without node prepended, just by calling the binary directly: aragon or ~/npm-global/node_modules/aragon which has the appropriate shebang. Read more here.

2. Protect against malicious dependencies.

Problem: An attacker can get access to a tiny dependency and push an update which reads the user's private key and sends them to a remote location. This happened before to wallets that relied on the popular event-stream package.

NodeJS is a memory safe language, therefore functions imported from other packages cannot read from outside their scope. Therefore, if private keys are handled in memory they are safe. However, if private keys are read from files or the native keytar they are not safe.

Solution: A small safe wallet should be developed that has no dependencies and has only one feature: to sign transactions. The aragonCLI or any other program can use this to send a tx and receive back a signed tx.

Note: This can be a CLI, an http api, etc.. 🤷‍♂. It can be considered as re-inventing the wheel though, so perhaps bundling an existing wallet like Frame makes more sense, if they are secure.

Note: Removing tiny dependencies from the project as suggested in #953 does improve security but only slightly and it comes at the cost of developers re-implementing and maintaining extra code. We currently have more than 40k dependencies so trying reduce this number is kinda pointless.

Takeaways:

The most secure way to use the CLI is to never disclose your private key and instead use Frame.
Perhaps the single most important thing to do right now is to drop support from private key files and move to in-memory private keys. However, the CLI is still a dev tool and it is common to see plaintext mnemonics and private keys configured in files for convenience. The best of both worlds would be to disable private key files for mainnet only. (When it comes to publishing your app to mainnet you pretty much want a secure deployment even it if costs convenience)

yeqbfgxjiq commented 4 years ago

@0x6431346e Thanks for writing this up. That's very helpful to better understand the security model of the Aragon CLI. The key warning in the Set private key section of hack.aragon is good, but could use more specificity. It would be great if the best practices described here were also incorporated (or linked to) in the hack.aragon docs.

sohkai commented 4 years ago

The best of both worlds would be to disable private key files for mainnet only. (When it comes to publishing your app to mainnet you pretty much want a secure deployment even it if costs convenience)

I don't know about dropping complete support for it, but at the very least, we should also warn and direct the user to more detailed documentation about how to set up more secure signers.

Perhaps we can prompt for a private key during run time for mainnet environments?

aragon / aragon-cli