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.

Sign up for GitHub

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

Jump to bottom

Fix check for passphrase to seed derivation in BatClient::continueRecover #1636

Closed
jasonrsadler opened this issue Oct 17, 2018 · 5 comments · Fixed by brave/brave-core#656
Closed

Fix check for passphrase to seed derivation in BatClient::continueRecover #1636

jasonrsadler opened this issue Oct 17, 2018 · 5 comments · Fixed by brave/brave-core#656
Assignees
@jasonrsadler
Labels
feature/rewards QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Milestone
0.55.x - Release ...

Comments

@jasonrsadler
Copy link

Description

written needs to be dereferenced on https://github.com/brave-intl/bat-native-ledger/blob/f8bb01713bfb05b69aab1720f5cca460303343f3/src/bat_client.cc#L1002

Otherwise, the check is guaranteed to succeed and wallet recovery will continue regardless of whether the derivation of the wallet seed from the provided passphrase succeeded.
@jasonrsadler jasonrsadler added this to the FROZEN: 0.55.x - Release milestone Oct 17, 2018
@jasonrsadler jasonrsadler mentioned this issue Oct 17, 2018
Merged
18 tasks
@srirambv
Copy link
Contributor

@jasonrsadler any QA steps for this?

@jasonrsadler
Copy link
Author

@bbondy Can you verify no-QA?

@bbondy
Copy link
Member

bbondy commented Oct 18, 2018

can't we QA this with wrong seed phrase or something like that?

@jasonrsadler
Copy link
Author

Sure. @srirambv, use test plan from brave/brave-core#530

@bbondy bbondy added the QA/Yes label Oct 18, 2018
@srirambv
Copy link
Contributor

srirambv commented Oct 18, 2018 •
edited by LaurenWags
Loading

Verification Passed on

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Linux
  • Verified 16 word recovery key restored the wallet with balance
  • Verified 24 word recovery key restored the wallet with balance
  • Verified wrong key didn't restore wallet and throws error
  • Verified correct 16/24 recovery key with a leading space doesn't restore the wallet

Verification Passed on

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (32-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Windows
  • Verified 16 word recovery key restored the wallet with balance
  • Verified 24 word recovery key restored the wallet with balance
  • Verified wrong key didn't restore wallet and throws error
  • Verified correct 16/24 recovery key with a leading space doesn't restore the wallet

Verified passed with

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Mac OS X
  • Verified 16 word recovery key restored the wallet with balance
  • Verified 24 word recovery key restored the wallet with balance
  • Verified wrong key didn't restore wallet and throws error
  • Verified correct 16/24 recovery key with a leading space doesn't restore the wallet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jasonrsadler jasonrsadler

Labels
feature/rewards QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Projects
None yet
Milestone
0.55.x - Release (Release Channel)
Development

Successfully merging a pull request may close this issue.

Correctly check result of seed derivation from passphrase
6 participants
@garrettr @bbondy @srirambv @jasonrsadler @LaurenWags @GeetaSarvadnya