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

Can recover account with seedphrase created with keycard #9062

Closed
Serhy opened this issue Sep 27, 2019 · 21 comments
Closed

Can recover account with seedphrase created with keycard #9062

Serhy opened this issue Sep 27, 2019 · 21 comments

Comments

@Serhy
Copy link
Contributor

Serhy commented Sep 27, 2019

Type: Bug
Account created with keycard can be recovered in Status as normal account.

Summary: Account created with keycard can be recovered in Status as normal account.

Expected behavior

Image for expected case!
Screenshot 2019-09-27 at 17 06 17

Actual behavior

Account recovered as normal account

Reproduction

Precondition

Seedphrase of an account created with keycard exists (e.g. pepper save spread chaos unable narrow couple little clog monkey voice weapon)

Reproduction:

  • Open Status
  • Enter seedphrase from precondition step

Additional Information

  • Status version: develop (2019092702)
  • Operating System: Android and iOS
@Serhy Serhy added bug labels Sep 27, 2019
@Serhy
Copy link
Contributor Author

Serhy commented Sep 27, 2019

@guylouis the expected result of this behavior based on relatively old requirements. Would there might be another expected result?

@guylouis
Copy link
Contributor

guylouis commented Sep 30, 2019

@Serhy
do you mean that

  • user has a multiaccount on his phone created with seed1
  • user decides to create a new multiaccount, which : is protected by keycard, uses the same seed1

and in this case

  • there are two multiaccounts on the phone with the same seed1, one being a keycard multiaccount, and the other one a non-keycard ?

If yes, that's an issue since there should never be two multiaccounts with the same seed
as described here #8750

@churik
Copy link
Member

churik commented Oct 3, 2019

@guylouis the question here is that case:

  1. User create keycard multiaccount on device1 (seed1)
  2. User tries to restore ordinary account with seed1 on device2

On our test cases (written by Nastya) - it is not possible, error is
Screenshot 2019-09-27 at 17 06 17

I want to find out what is expected result in this case and in case if we plan to restore account from seed phrase in MetaMask for example.

@dmitryn
Copy link
Contributor

dmitryn commented Oct 3, 2019

It's not the case when 2 identical account are created.
For same seed phrase status-go multiAccountImportMnemonic and keycard generateAndLoadKey derive different keys. For instance, curtain mosquito taste relief topple valid initial plug fall there talk core derive path m/43'/60'/1581'/0'/0:

  • status-go 0x04a0a4d09cdafdca75b131d82df8a1a593efe0293573694a74ff1d99be65e39899d331b0f21201a044893e34c718276c9563803a66cb600e07559d5a5d15f2361b
  • keycard
    0x047079b55c3a9e1bc37e45253150a515e4f1001357d957b9873940c79dd5cc005f4039dc37422dd0ae46086fc121e5a071a30010c043cef08206a13602edfa3431

@dmitryn
Copy link
Contributor

dmitryn commented Oct 3, 2019

I'd like to ask someone from @status-im/go to check if multiAccountImportMnemonic works as expected.

@guylouis
Copy link
Contributor

guylouis commented Oct 3, 2019

cc @gravityblast

@guylouis
Copy link
Contributor

guylouis commented Oct 16, 2019

tested with today's nightly StatusIm-191016-025900-8e9010-nightly-universal.apk
and I confirm that on the same phone if I create

  • a multiaccount with seed1 with no keycard
  • a multiaccount with seed1 with one keycard

Then: I get two multiaccounts on the phone with different

  • 3 names
  • wallet account

Whereas the intended behaviour is that

  • these two multiaccounts should be identical (same seed)
  • the UX should prevent to create a new multiaccount with same seed, because it would totally defeat keycard security scheme to have the same multiaccount with no keycard on the same phone :) see Prevent 2 multiaccounts on the same phone with same seed #8750

@gravityblast see the discussion above, can the issue be in go ?

@gravityblast
Copy link
Member

I'm trying with yesterday's and today's nightly but even when I select keycard it asks for a password and the keycard is never used.
Does it work for you?

@guylouis
Copy link
Contributor

guylouis commented Oct 17, 2019

no it doesn't
opened an issue #9231

@gravityblast
Copy link
Member

gravityblast commented Oct 17, 2019

User create keycard multiaccount on device1 (seed1)
User tries to restore ordinary account with seed1 on device2
On our test cases (written by Nastya) - it is not possible, error is

@churik are you only trying to recover a normal mnemonic phrase in a different device?
Maybe I'm missing something but device 2 shouldn't know that the mnemonic has been used for a keycard. or does device 2 already have the same account with keycard as well?

@gravityblast
Copy link
Member

@dmitryn
We tried to import your mnemonic to both keycard and status-go manually (outside status) and deriving the chat key.
They both derive the same public key 0x04a0a4d09cdafdca75b131d82df8a1a593efe0293573694a74ff1d99be65e39899d331b0f21201a044893e34c718276c9563803a66cb600e07559d5a5d15f2361b.

So I guess maybe we are deriving something else from within status?
I can't really test it because the nightly build doesn't work with the keycard.
Is there anyone else that can try importing that specific mnemonic to a keycard with a nightly build?

@churik
Copy link
Member

churik commented Oct 17, 2019

@gravityblast
nightly build is working with keycard, I suppose you download it from https://status.im/nightly/ which is not updated ~for 1 month.
Please use https://ci.status.im/job/status-react/job/nightly/ to get latest develop builds.

@churik are you only trying to recover a normal mnemonic phrase in a different device?
Maybe I'm missing something but device 2 shouldn't know that the mnemonic has been used for a keycard. or does device 2 already have the same account with keycard as well?

I don't know for sure. My only source of truth was testcase, created on 01/2019, where is clearly stated that should be error at attempt to recover keycard account with password.

keycard : can't recover keycard account with password - TestRail 2019-10-17 12-53-06

@guylouis
Copy link
Contributor

Full reproduction of the issue. I have to use a different mnemonic than the one above because it's not possible to recover a seed on a keycard now (issue #9231 )

FIRST STEP: SET UP A KEYCARD MULTIACCOUNT

  • download latest nightly, in my case: StatusIm-191017-025900-7b6dfa-nightly-universal
  • use a brand new keycard
  • generate a key
  • chose "Far Peppery Harpseal" ....5817
  • chose to store it on keycard
  • write down seed "nurse casual reform alter fun nuclear suffer chicken toward sibling strike quiz"
    -> multiaccount set up
    wallet adress : 0x023093588608C8Cb438A1e3f12A7bCC7BFd7894E
    chat key:0x043601006ec788418dc0ed1fe79d6148b5e0eb0641362d0e7bad0a5fe96ef4cfd935d7ea356ad58b15f261434fba2a481b49a7eb6507f81eff3bb9afc7fdee5817

SECOND STEP: SET UP A NON-KEYCARD MULTIACCOUNT WITH SAME SEED ON THE SAME PHONE

  • chose "access key"
  • chose "enter seed phrase"
  • type the same seed as above
  • get a three words"Some Scrawny Thrip" ..81e9
  • chose "re-encrypt your key"
  • chose "on this device"
  • create a password
    wallet adress: 0x67731BCE113b905c86A726BaA314c5a21f5cB8E4
    chat key: 0x04aa9c3d9a19f5c58a0cba22dee559f294578d732a99b96d9790da87cd4c38cfaee2da6ec30dc1629db1892f214fbe7e099a9b3e0fdf31f791fe92c88c3be981e9

CONCLUSION: same seed but everything is different: chat key, wallet address and three names

@gravityblast
Copy link
Member

@guylouis I think we display the wrong public key.
Try again setting up a Keycard, then re-install status and pair the card with your pairing password.
The public key is different, and in this case we aren't even using the mnemonic phrase.

@gravityblast
Copy link
Member

@churik I used the build you say but trying importing a mnemonic to a keycard instead of setting up a new account from scratch and it wasn't working

@guylouis
Copy link
Contributor

@gravityblast yes this is #9231

with regards to the public key issue, who can check where is the bug, is it @dmitryn ? Can you share here elements that leads to think issue is on react and not go ? Thanks

@gravityblast
Copy link
Member

@guylouis first of all there's something wrong with the mnemonic. if you try to import it in any other wallet like mycrypto, you will see it's invalid.
Then if I use the mnemonic with our new and old libraries in go, I see that the right address is the second one. so I guess there's something wrong when we call the keycard API maybe.
yes I htink @dmitryn knows more about it

@rasom
Copy link
Contributor

rasom commented Nov 1, 2019

just to clarify, issues title states:

Can recover account with seedphrase created with keycard

As far as I understand, there is no problem in ability to recover keycard's account as a regular account on device. Almost likely we want that account/phrase to be compatible with any other wallet/storage, so that shouldn't be a problem at all.

But we still have two problems with keycard's mnemonic:

  1. The phrase which we show during keycard setup can not be used for restoring account later. It is actually a mnemonic generated for pairing, at least how I understood. Probably it can be used for restoring keycards setup somehow, but you definitely can't use this passphrase at MEW or other wallet.
  2. We can have two accounts with the same root key on a single device, one stored on keycard and another on device. This issue is covered here Prevent 2 multiaccounts on the same phone with same seed #8750

So I would just close this issue as it is a bit misleading already and create a new one for 1. if it doesn't exist already.

@rasom rasom removed their assignment Nov 1, 2019
@flexsurfer
Copy link
Member

@rachelhamlin @guylouis wdyt ?

@guylouis
Copy link
Contributor

guylouis commented Nov 4, 2019

There were several problems yes !

#9231 should solve the issue of creating a keycard account based on a seed entered by the user. This was a typo issue in the code that you fixed. It seems there are device specific issues that prevent losing it now.

#8750 tackles the issue of preventing to have "two accounts with the same root key on a single device, one stored on keycard and another on device", because from a security perspective it makes no sense, and gives a false sense of security to the keycard account, since the same private key are actually on the phone in the other account. I added some comments in #8750 on the roadmap to make transitioning from on keycard account (KCA) to on-device account (ODA) KCA->ODA, ODA->KCA, KCA-> KCA, ODA->ODA.

As for the original problem from this issue, it now has several subparts caused by the same root cause or not. I'll close this one and open a new one (#9393) that shows the symptoms

  • when generating a new KCA, status provides a seed to the user that is not BIP39 compliant: it's not valid and can't be used in an other wallet
  • (might very well be a direct consequence of the previous one) if user generates a KCA and its seed, then tries to import an ODA with the same seed, it generates an account, but it's different than the KCA. Correct behaviour should be that it tries to generate an account, but fails because of security considerations (purpose of Prevent 2 multiaccounts on the same phone with same seed #8750).

@guylouis
Copy link
Contributor

guylouis commented Nov 4, 2019

closing this one, since tackled in the three issues listed above

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

7 participants