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

Expose chrome.hid to Crypto Wallets extension #16305

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

Expose chrome.hid to Crypto Wallets extension #16305

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

Comments

@bbondy
Copy link
Member

bbondy commented Jun 9, 2021

Crypto Wallets extension used to use U2F via a metamask.io page.
In Chromium 91 this changed to add a security check which we reverted temporarily in #16204

We'd like to revert the C91 patch after this issue is released which is tracked here:
#16205

Before we revert the C91 patch we need this to be on Release channel so that people that use Crypto Wallets with U2F won't break.

Code for using the new HID API is here:
brave/ethereum-remote-client#243
and here: brave/eth-ledger-bridge-keyring@6db82c1

This issue is to track just allow the extension to use chrome.hid though.

Test plan

  • launch brave via --use-dev-goupdater-url and pull Crypto Wallets 1.0.30
  • Load brave://wallet and create a wallet
  • Go to chrome://extensions and enable dev mode, then click on inspect background page for the wallet extension
  • type chrome.hid and make sure it's not undefined

Release notes title can be something like: Support Ledger devices in Crypto Wallets via HID transport.

@kjozwiak
Copy link
Member

kjozwiak commented Jun 11, 2021

@bbondy I don't think this is working. I tried it on macOS using the following build and got undefined. Example:

Brave | 1.25.72 Chromium: 91.0.4472.101 (Official Build) (x86_64)
--- | ---
Revision | af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS | macOS Version 11.3 (Build 20E232)

Screen Shot 2021-06-11 at 1 14 08 AM

Same results on Ubuntu 20.04 x64 using the following build:

Brave | 1.25.72 Chromium: 91.0.4472.101 (Official Build) (64-bit)
--- | ---
Revision | af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS | Linux

image

Once we get the Win x64 executable, I can try on Win but so far it doesn't seem like it worked. Believe you mentioned this can be left in the release even if it doesn't work and release-notes/exclude.

@srirambv srirambv added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jun 11, 2021
@srirambv
Copy link
Contributor

Same on Windows as well. Tried connecting to a Dapp and checked shows undefined,
image

@kjozwiak
Copy link
Member

kjozwiak commented Jun 11, 2021

Verification PASSED on Win 10 x64 using the following build:

Brave | 1.25.72 Chromium: 91.0.4472.101 (Official Build) (64-bit)
--- | ---
Revision | af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS | Windows 10 OS Version 2009 (Build 19042.1023)

image


Verification PASSED on macOS 11.3 x64 using the following build:

Brave | 1.25.72 Chromium: 91.0.4472.101 (Official Build) (x86_64)
--- | ---
Revision | af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS | macOS Version 11.3 (Build 20E232)

Screen Shot 2021-06-11 at 4 33 33 PM


Verification PASSED on Ubuntu 20.04 x64 using the following build:

Brave | 1.25.72 Chromium: 91.0.4472.101 (Official Build) (64-bit)
--- | ---
Revision | af52a90bf87030dd1523486a1cd3ae25c5d76c9b-refs/branch-heads/4472@{#1462}
OS | Linux

image

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