brave / browser-laptop

[DEPRECATED] Please see https://github.com/brave/brave-browser for the current version of Brave
https://www.brave.com
Other
7.95k stars 975 forks source link

disable webusb API #13374

Closed diracdeltas closed 6 years ago

diracdeltas commented 6 years ago

Test plan

See https://github.com/brave/browser-laptop/pull/13375

Original issue description

webusb can be used to bypass yubikey phishing protection at the moment: https://www.wired.com/story/chrome-yubikey-phishing-webusb/ -> we should disable navigator.usb

@flamsmark also suggested disabling navigator.bluetooth

maybe also https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon ?

diracdeltas commented 6 years ago

we actually don't support bluetooth yet AFAIK. https://googlechrome.github.io/samples/web-bluetooth/device-info.html?allDevices=true fails without showing a prompt in Brave.

karelbilek commented 6 years ago

Hello

I am a developer of Trezor - a cryptocurrency hardware wallet - and in next iteration of our products, we will support WebUSB.

I have tested WebUSB version of Trezor, and WebUSB doesn't actualy work in Brave. (Tested on Linux.)

navigator.usb is defined, but navigator.usb.getDevices() does not show the device picker, so the web never gets the permissions.

Our web wallet detects whether webusb is defined via navigator.usb, if it is not defined, we offer user to install a local server which browser then connects to. Anyway, this detection is broken in Brave - navigator.usb is defined, yet navigator.usb.getDevices() never shows anything.

So please, either disable it (by setting navigator.usb to null) or fix it. Right now, webUSB is enabled, but broken. Which is actually good security-wise (nobody can access any device anyway), but bad from web developer point of view.

(Btw - here is the public Chrome issue that deals with this - Google's solution is just blacklist the yubikey IDs. This is working to patch the bug, but not fixes the underlying issue, which would be fixable by at least optional descriptors, as I noted there and on the webusb github https://github.com/WICG/webusb/issues/127 )

karelbilek commented 6 years ago

I would personally prefer if the webusb was enabled, but the descriptor rules enforced :) which would fix this, as the yubikey devices definitely don't have the webusb descriptors.

But that would require changes probably beyond the scope of Brave

diracdeltas commented 6 years ago

@karel-3d as you can see in https://github.com/brave/browser-laptop/pull/13375 it's going to be disabled for now

karelbilek commented 6 years ago

Hm, this seems that navigator.usb is still defined, but the function returns rejecting promise. This is not ideal :/ as it requires special casing

diracdeltas commented 6 years ago

@karel-3d good point. i think i can set them to undefined

karelbilek commented 6 years ago

Thanks, this will help!

When Chrome recently (today) kill-switched webusb remotely, it set navigator.usb to undefined

diracdeltas commented 6 years ago

https://github.com/brave/browser-laptop/pull/13375 updated

LaurenWags commented 6 years ago

Verified on macOS 10.12.6 x64 using the following build:

Verified on Windows 7 x64

Verified on Ubuntu 17.10 x64 using the following build: