Skip to content

Commit

Permalink
feat: get keyring backend (#11484)
Browse files Browse the repository at this point in the history
## 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 99designs/keyring#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
  • Loading branch information
fedekunze committed Mar 29, 2022
1 parent 354faa8 commit 2083bc8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
29 changes: 21 additions & 8 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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},
Expand All @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 2083bc8

Please sign in to comment.