From 2083bc8646f1fbdc5474b7338601a09a3369c03b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Tue, 29 Mar 2022 15:14:59 +0200 Subject: [PATCH] feat: get keyring backend (#11484) ## Description This PR introduces a getter for the keyring backend type used in the keyring config. This is useful to disable endpoints whenever the keyring `test` backend is used. This is a workaround since the SDK keyring dependency doesn't support locking accounts. See https://github.com/99designs/keyring/issues/85 for context. Attack on ethereum that affects Ethermint chain validators/nodes using `keyring_backend=test`, making their funds remotely accessible via `eth_sendTransaction` https://blog.ethereum.org/2015/08/29/security-alert-insecurely-configured-geth-can-make-funds-remotely-accessible/ --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] reviewed "Files changed" and left comments if necessary - [x] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] confirmed that this PR does not change production code --- CHANGELOG.md | 1 + crypto/keyring/keyring.go | 29 +++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85e87e93be7e..c615c52f17df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [\#11484](https://github.com/cosmos/cosmos-sdk/pull/11484) Implement getter for keyring backend option. * [\#11449](https://github.com/cosmos/cosmos-sdk/pull/11449) Improved error messages when node isn't synced. * [\#11349](https://github.com/cosmos/cosmos-sdk/pull/11349) Add `RegisterAminoMsg` function that checks that a msg name is <40 chars (else this would break ledger nano signing) then registers the concrete msg type with amino, it should be used for registering `sdk.Msg`s with amino instead of `cdc.RegisterConcrete`. * [\#11089](https://github.com/cosmos/cosmos-sdk/pull/11089]) Now cosmos-sdk consumers can upgrade gRPC to its newest versions. diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index db4eb8fbb9ca..ef1cec74fd96 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -52,6 +52,8 @@ var ( // Keyring exposes operations over a backend supported by github.com/99designs/keyring. type Keyring interface { + // Get the backend type used in the keyring config: "file", "os", "kwallet", "pass", "test", "memory". + Backend() string // List all keys. List() ([]*Record, error) @@ -162,11 +164,11 @@ type Options struct { // purposes and on-the-fly key generation. // Keybase options can be applied when generating this new Keybase. func NewInMemory(cdc codec.Codec, opts ...Option) Keyring { - return newKeystore(keyring.NewArrayKeyring(nil), cdc, opts...) + return newKeystore(keyring.NewArrayKeyring(nil), cdc, BackendMemory, opts...) } // New creates a new instance of a keyring. -// Keyring ptions can be applied when generating the new instance. +// Keyring options can be applied when generating the new instance. // Available backends are "os", "file", "kwallet", "memory", "pass", "test". func New( appName, backend, rootDir string, userInput io.Reader, cdc codec.Codec, opts ...Option, @@ -197,17 +199,19 @@ func New( return nil, err } - return newKeystore(db, cdc, opts...), nil + return newKeystore(db, cdc, backend, opts...), nil } type keystore struct { db keyring.Keyring cdc codec.Codec + backend string options Options } -func newKeystore(kr keyring.Keyring, cdc codec.Codec, opts ...Option) keystore { - // Default options for keybase +func newKeystore(kr keyring.Keyring, cdc codec.Codec, backend string, opts ...Option) keystore { + // Default options for keybase, these can be overwritten using the + // Option function options := Options{ SupportedAlgos: SigningAlgoList{hd.Secp256k1}, SupportedAlgosLedger: SigningAlgoList{hd.Secp256k1}, @@ -217,7 +221,17 @@ func newKeystore(kr keyring.Keyring, cdc codec.Codec, opts ...Option) keystore { optionFn(&options) } - return keystore{kr, cdc, options} + return keystore{ + db: kr, + cdc: cdc, + backend: backend, + options: options, + } +} + +// Backend returns the keyring backend option used in the config +func (ks keystore) Backend() string { + return ks.backend } func (ks keystore) ExportPubKeyArmor(uid string) (string, error) { @@ -369,7 +383,6 @@ func (ks keystore) SignByAddress(address sdk.Address, msg []byte) ([]byte, types } func (ks keystore) SaveLedgerKey(uid string, algo SignatureAlgo, hrp string, coinType, account, index uint32) (*Record, error) { - if !ks.options.SupportedAlgosLedger.Contains(algo) { return nil, fmt.Errorf( "%w: signature algo %s is not defined in the keyring options", @@ -747,7 +760,7 @@ func newRealPrompt(dir string, buf io.Reader) func(string) (string, error) { continue } - if err := os.WriteFile(dir+"/keyhash", passwordHash, 0555); err != nil { + if err := os.WriteFile(dir+"/keyhash", passwordHash, 0o555); err != nil { return "", err }