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

Confusing error message when migration via keys list on macos #10220

Closed
4 tasks
Tracked by #12665 ...
amaury1093 opened this issue Sep 23, 2021 · 12 comments · Fixed by #12952
Closed
4 tasks
Tracked by #12665 ...

Confusing error message when migration via keys list on macos #10220

amaury1093 opened this issue Sep 23, 2021 · 12 comments · Fixed by #12952
Labels
C:Keys Keybase, KMS and HSMs S:zondax Squad: Zondax T:Bug

Comments

@amaury1093
Copy link
Contributor

Summary of Bug

When running simd keys list, the command runs a migration if needed. If we deny keychain access, the error message could be improved

Version

6e70d25, though it's probably caused by #9695

Steps to Reproduce

  • Create some keys on macos using a v042 node, using the "os" (keychain) backend
  • On a master node, run simd keys list
  • Deny keychain access

Expected: some error like "keychain access is denied, skipping key" or even no message like on 042.

Actual
ezgif com-gif-maker
:


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@amaury1093 amaury1093 added T:Bug C:Keys Keybase, KMS and HSMs labels Sep 23, 2021
@MarinX
Copy link
Contributor

MarinX commented Oct 25, 2021

this just a note:
Step into my debugger 😄
When we query an item, it fails here cosmos/keyring.
This is reported by keybase/go-keychain.

The go-keychain does not catch the correct error code, which is -128 (screenshot) and defaults to a generic error.
screenshot
The cosmos/keyring always return not found if no keys are returned or error code equals to no key.

Proposed steps:

  • Extend the keybase/go-keychain with additional error code -128
  • Catch the correct error in cosmos/keyring and return
  • Catch the error from cosmos/keyring and maybe rename the wrapKeyNotFound to wrapError to do a switch case of a error type (not found or denied)

@mkudimov
Copy link

I would like to fix it, however it isn't clear for me how to produce the next point

create some keys on macos using a v042 node, using the "os" (keychain) backend

Could you please give more detailed description of this step?

@amaury1093
Copy link
Contributor Author

sure!

# checkout the 0.42 branch
git checkout release/v0.42.x

# build the binary
make build

# rename the binary to a more explicit name
mv ./build/simd ./build/v042node

# create some keys
./build/v042node keys add jack --keyring-backend os

@ainhoa-a ainhoa-a mentioned this issue Aug 1, 2022
24 tasks
@raynaudoe
Copy link
Contributor

Fix implemented here Zondax/keyring#1

@alexanderbez
Copy link
Contributor

@raynaudoe does that mean we can close this issue or do we need to bump a dependency version?

@raynaudoe
Copy link
Contributor

The fix will be complete if we change the dependency

github.com/99designs/keyring v1.2.1

to use the Zondax fork instead

@alexanderbez
Copy link
Contributor

@raynaudoe let's do it! Can you open a PR pls?

@raynaudoe
Copy link
Contributor

@raynaudoe let's do it! Can you open a PR pls?

ok!, here's the PR

@tac0turtle
Copy link
Member

I think we should reopen this or open a new pr to remove the replace tag. the fork isn't maintained and there have been merges on the upstream

@ainhoa-a
Copy link

@tac0turtle how should we proceed with this? the PR has been open since January :-( 99designs/keyring#119

@tac0turtle
Copy link
Member

we can close this and if it gets merged then we can update. If we dont remove it as a dep by then

@ainhoa-a
Copy link

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Keys Keybase, KMS and HSMs S:zondax Squad: Zondax T:Bug
Projects
None yet
8 participants