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

Temporarily revert U2F sig check for Ledger U2F transport support #16204

Closed
bbondy opened this issue Jun 2, 2021 · 3 comments · Fixed by brave/brave-core#8995
Closed

Temporarily revert U2F sig check for Ledger U2F transport support #16204

bbondy opened this issue Jun 2, 2021 · 3 comments · Fixed by brave/brave-core#8995

Comments

@bbondy
Copy link
Member

bbondy commented Jun 2, 2021

chromium/chromium@07ef8d7 breaks the U2F support of Ledger.

This is a problem with the transport being used to send arbitrary data, our wallet and other wallets still use the Ledger U2F transport though. So this issue is to track a temporary revert as we work out how to communicate over webhid instead.

@bbondy bbondy added OS/Android Fixes related to Android browser functionality OS/Desktop labels Jun 2, 2021
@bbondy bbondy self-assigned this Jun 2, 2021
@bbondy bbondy added this to the 1.25.x - Release #3 milestone Jun 2, 2021
bbondy added a commit to brave/brave-core that referenced this issue Jun 2, 2021
Issue to revert the Chromium code here:
brave/brave-browser#16204

Issue to revert the revert here:
brave/brave-browser#16205

Issue to implement webhid for our wallet here:
brave/brave-browser#16206
@bbondy bbondy removed the OS/Android Fixes related to Android browser functionality label Jun 2, 2021
@bbondy
Copy link
Member Author

bbondy commented Jun 3, 2021

Re-opening until it's landed on 1.25.x

@kjozwiak
Copy link
Member

kjozwiak commented Jun 3, 2021

@bbondy assuming the following STR/Cases are sufficient enough?

  • ensure that you can create a new wallet via brave://wallet
  • ensure that you can connect a Nano Ledger via brave://wallet without any issues

Anything else that QA should run through?

@srirambv srirambv added the feature/web3/wallet Integrating Ethereum+ wallet support label Jun 3, 2021
@srirambv
Copy link
Contributor

srirambv commented Jun 3, 2021

Verification passed on

Brave 1.25.70 Chromium: 91.0.4472.77 (Official Build) (x86_64)
Revision 1cecd5c8a856bc2a5adda436e7b84d8d21b339b6-refs/branch-heads/4472@{#1246}
OS macOS Version 10.15.7(Build 19H114)
  • Verified able to create wallet on brave://wallet
  • Verified able to successfully connect Ledger Nano S via brave://wallet and unlock the address

Verification passed on

Brave 1.25.70 Chromium: 91.0.4472.77 (Official Build) (64 bit)
Revision 1cecd5c8a856bc2a5adda436e7b84d8d21b339b6-refs/branch-heads/4472@{#1246}
OS Windows 10 OS Version 2009 (Build 19042.985)
  • Verified able to create wallet on brave://wallet
  • Verified able to successfully connect Ledger Nano S via brave://wallet and unlock the wallet
    Note: Had an issue with the Windows connection popup going in loop when trying to connect the wallet. Dismissing the prompt did allow to connect the h/w and unlock the address

Verification partially passed on

Brave 1.25.70 Chromium: 91.0.4472.77 (Official Build) (64-bit)
Revision 1cecd5c8a856bc2a5adda436e7b84d8d21b339b6-refs/branch-heads/4472@{#1246}
OS Linux
  • Verified able to create wallet on brave://wallet
  • Unable to connect Ledger Nano due to TransportError: Failed to sign with Ledger device: U2F DEVICE_INELIGIBLE
    Linux Wallet

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