keys list with keyring-backend=os requires you to enter your password once per key :o

[ebuchman]

Summary of Bug

It appears that trying to run keys list with keyring-backend=os requires you to type in your computers master password once for every key you have.

I always felt the OS backend was a bit under developed. I get that it's a nice idea to use the built in keyring system but having to enter your master password to use the tooling by default seems like an anti-pattern, and having to enter it once per key just to list the keys is clearly a problem.

For what it's worth, I never use it, but I'm helping someone debug and was pretty appalled to learn they've just been putting up with entering their password 20 times to run keys list :(

Version

v0.44.3 (v6 of gaia)

Steps to Reproduce

Run gaia keys list --keyring-backend=os on a system with a bunch of keys

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[fedekunze]

I've experienced this issue as well. It's super annoying

[sunnya97]

Yes, this is a big issue for testing purposes especially

[tac0turtle]

This has come up a few times since this feature landed in the sdk. It has been a problem with the amount of times you enter the password.

[sunnya97]

Is anyone working on this?

[tac0turtle]

We are working on bringing on zondax to help with the keyring and help in a possible rewrite of keyring to something like keystone. At this time we don't have anyone working on this.

[fedekunze]

ok this is extremely annoying, can someone fix this lol

[fedekunze]

cc: @julienrbrt @jleni

[julienrbrt]

cc: @julienrbrt @jleni

If @jleni does not have time, I could have a look 🤔

[tac0turtle]

lets hold off on working on this for now. We are working with Zondax to decide what the future of keyring is.

[rllola]

I am not able to reproduce this. Is it specific to gaia ?

I tried with gaia too and I can't reproduce it. I am running Ubuntu 20.04.

[raynaudoe]

Hi, I was able to reproduce the issue on macos 12.5. The fix for me was to simply click on Always allow when keychain prompt asks for my password.

[tac0turtle]

we should avoid recommended always allow. It could lead to unforeseen issues.

[sunnya97]

@raynaudoe using Always Allow fixes it for the current binary, but for developers (who are probably the main users of CLI), everytime we update the binary, we have to reallow every key

[raynaudoe]

Is there really something that can be done in cosmos side ?
IMO since this is a backend issue, you either change your OS' config or type the passwords each time its prompt.
Maybe I'm missing some other way ?

[alexanderbez]

I'm not really sure what the solution here is. It seems like requiring a passphrase per keyring item (i.e. keypair) is the default and correct safe behavior, but people are finding it annoying.

So is the idea that we want a passphrase on the entire keyring, not per item? If so, is that a config or something we can set? If not, perhaps we have to refactor how items are stored in the OS keyring so that they're treated as one "blob".

[raynaudoe]

One idea, is to just avoid calling this two functions together:

cosmos-sdk/crypto/keyring/keyring.go

Line 524 in 184235e

keys, err := ks.db.Keys()

and

cosmos-sdk/crypto/keyring/keyring.go

Line 539 in 184235e

item, err := ks.db.Get(key)

I think that if we create a function GetAll() that uses MatchLimitAll like here

and since would be only one call, the user would only have to type its password once to get all the items, instead of typing it once for getting the item list and then one more time per item on the list.
Thoughts ?

[alexanderbez]

@raynaudoe let's give it a shot!

[raynaudoe]

Seems that is not possible, at least on macos:
mogol/flutter_secure_storage#70 (comment)
I'll keep looking for another alternative that doesn't involve too much of a refactor

[raynaudoe]

ok, I found an easy way to reduce to a half the times the password gets prompted. Since the MigrateAll function does fairly the same process regarding keychain queries as the List function:
If we make MigrateAll to return the records it finds:
MigrateAll() ([]*Record, error)
we have all the information there and List() would simply be:

func (ks keystore) List() ([]*Record, error) {
  return ks.MigrateAll()
}

thus avoiding doing again calls to Keys() and Get() for each key

[alexanderbez]

Yeah sounds good! Recall, we don't mind breaking APIs here if it means a better UX! Let's go for it 🚀

[raynaudoe]

Ok! I'll prepare a PR soon