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

Key management improvements #252

Open
Anmol1696 opened this issue Oct 13, 2022 · 6 comments
Open

Key management improvements #252

Anmol1696 opened this issue Oct 13, 2022 · 6 comments

Comments

@Anmol1696
Copy link
Contributor

Overview

For making ts-relayer production ready, we need better key management, giving operators multiple ways to handle keys.
Again the goal is to make ts-relayer as a standalone relayer.

Currently

Mnemonics are passed a variables or stored in ~/.ibc-setup/app.yaml. This is only good for testing or debugging, and not for production nodes.

Proposal

Ability to handle multiple key-management systems from plain test, keyrings or additional settings.
Some of the work done in cosmology https://github.com/cosmology-tech/cosmology#mnemonics could be something easily ported here as well.

@ethanfrey
Copy link
Member

I agree.
The goal here was ease of use first (for testing environments) and add more security later.

I would be very happy for more secure key management apis that don't have negative impact on workflow

@Anmol1696
Copy link
Contributor Author

Cool. Will try to get this done as well. Thanks, might take a bit to understand the codebase.

@ethanfrey
Copy link
Member

Thank you. A quick intro:

src/lib has all the ibc code.

src/binary is all the cli tooling. you will just need to look in that directory. Particularly places like: https://github.com/confio/ts-relayer/blob/main/src/binary/ibc-setup/commands/keys-generate.ts

You can see how we load it currently:

You may want to first refactor all instances of this logic into one helper, then later extend it to load mnemonics other ways. It should work with ibc-setup and ibc-relayer commands

@ethanfrey
Copy link
Member

Note, we cannot use a ledger (which is interactive), but I believe there may be other hardware signers that don't need confirmation.

The --interactive prompt means, it is only entered in stdin, so it cannot be read from a file, the env or the command line args, and should be reasonable secure... just pushing the burden of security to whomever enters the text.

I guess an encrypted mnemonic with only a passphrase entered in stdin would be better. What is your design?

@Anmol1696
Copy link
Contributor Author

Anmol1696 commented Oct 15, 2022

Ledger support will be an issue, but I dont think most of the node operators use ledger since most of the nodes are remote anyways.

The way i am looking at this

  • Keychain: additional add-keys commands to use keychain via a mnemonic (via env variables as well as stdin). Since this is a one time thing, and later on the ibc-setup should read directly from keychain itself. We dont store the mnemonics anywhere, just the private keys in the keychain.
    ibc-setup keys add <name> --recover --keyring-backend <os|test|memory>
    # app.yaml also mentions the keyname and backend to read from (per chain)
    
  • Encrypted salt method, would require more brainstroming to make it useable from a node operator perspective, basically no interactive methods would be possible in that case.

We keep the current method of passing the mnemonic directly too, required for testing, but not recommed it for running on standalone nodes.

Also it would make sense to have seperate keys for seperate chains, node operators can use the same mnemonics as well, with differenent index.

@ethanfrey
Copy link
Member

Ah, I like the idea of using os-level keyring for the backend.

Something that can be unlocked once on startup is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants