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

Desktop :: Implement "Enabling Sites to Determine Brave" Spec #8216

Closed
anthonypkeane opened this issue Feb 12, 2020 · 26 comments · Fixed by brave/brave-core#4721
Closed

Desktop :: Implement "Enabling Sites to Determine Brave" Spec #8216

anthonypkeane opened this issue Feb 12, 2020 · 26 comments · Fixed by brave/brave-core#4721

Comments

@anthonypkeane
Copy link

anthonypkeane commented Feb 12, 2020

Description

Please implement the "Enabling Sites to Determine Brave" Spec

Search for "Enabling Sites to Determine Brave" in G-Drive for the Spec
(also described in below post)
cc @snyderp

Android Link: #8215
iOS Link: brave/brave-ios#2323

Test plan

  1. Visit https://jsbin.com/sexawix/edit?html,js,console
  2. Output from console should look like:
"Brave detected"
@FatBirdie
Copy link

Please set default to enabled.
Otherwise this feature is more or less useless.

@bsclifton
Copy link
Member

@FatBirdie we'll share more details as the spec (not public, it's on our G-Drive as mentioned) gets hardened

As it stands, this would be something always available 😄

@bsclifton
Copy link
Member

bsclifton commented Feb 24, 2020

navigator.brave.isBrave() is exposed and can be called. Per the spec, the IDL roughly looks like:

Promise<boolean> navigator.brave.isBrave()

(BELOW IS OUTDATED) no longer relevant, but here for posterity

Here's the implementation matching what our spec (private) captures:

navigator.getUserAgent().then((response) => {
  if (response.brand === "Brave") {
    // browser is Brave
  }
});

navigator.getUserAgent() is exposed when client hints is enabled. I've got a patch which exposes this method and ONLY populates the brand field. Since client hints is still a work-in-progress, it's possible that this interface might change

bsclifton added a commit to brave/brave-core that referenced this issue Feb 24, 2020
Patches some client hints code but does NOT enable client hints. No additional info is specified (arch, model, platform, etc)

Fixes brave/brave-browser#8216
@bsclifton bsclifton added this to the 1.7.x - Nightly milestone Feb 24, 2020
@FatBirdie
Copy link

Here's the implementation matching what our spec (private) captures:

navigator.getUserAgent().then((response) => {
  if (response.brand === "Brave") {
    // browser is Brave
  }
});

navigator.getUserAgent() is exposed when client hints is enabled. I've got a patch which exposes this method and ONLY populates the brand field. Since client hints is still a work-in-progress, it's possible that this interface might change

Is this 'hints' option default on or off?

@bsclifton
Copy link
Member

@FatBirdie client hints is still turned off. The patch I did effectively only allows this one method and nulls out the other fields (architecture, model, platform, etc)

@da2x
Copy link

da2x commented Feb 24, 2020

@bsclifton that doesn’t seem to match what I’m observing in the Sec-Ch-Ua request header. My server logs show Sec-Ch-Ua: Brave Browser 78 and Sec-Ch-Ua: "Brave Browser 79" headers. (Not sure what’s up with the inconsistent use of quotes. No quotes are expected.)

Isn’t the navigator.getUserAgent()['brand'] API expected to return the same brand value as the Sec-Ch-Ua request header? As I’m understanding the spec, navigator.getUserAgent()['brand'] should return Brave and the Sec-Ch-Ua header should be set to Brave;v=78.

Or does Brave intend to not ship support for the Sec-Ch-Ua header?

Is this 'hints' option default on or off?

It’s still a draft specification not in the standards track. The spec deprecates the User-Agent request headers in favor of a series of Sec-CH-UA[-*] headers. It’s only backed by Chromium with no support from Gecko or WebKit yet.
https://wicg.github.io/ua-client-hints/
https://www.chromestatus.com/feature/5995832180473856

@bsclifton
Copy link
Member

bsclifton commented Feb 24, 2020

@da2x I can't speak to the standards aspect of this, but the work that was done in my pull request is separate from the Sec-Ch-Ua header. Thanks for calling out though, as these not matching may be an issue

I wasn't aware we are supporting this header yet? Did you have to compile with kUserAgentClientHint added to the enabled features? (ex: in src/brave/app/brave_main_delegate.cc)

@da2x
Copy link

da2x commented Feb 24, 2020

Thanks for calling out though, as these not matching may be an issue.

I just re-read the relevant parts of the spec. These should definitely be the same value.

I wasn't aware we are supporting this header yet? Did you have to compile with kUserAgentClientHint added to the enabled features? (ex: in src/brave/app/brave_main_delegate.cc)

Just enable chrome://flags/#enable-experimental-web-platform-features and restart the browser.

@da2x
Copy link

da2x commented Feb 25, 2020

Brave on Android identifies as Chromium 79 and macOS/Linux identify as Brave Browser 79.

Correction: Brave Beta on Android also identifies as Brave Browser 80.

@da2x
Copy link

da2x commented Feb 25, 2020

@bsclifton, with that flag enabled on the current stable version navigator.getUserAgent().then(d=>console.log(d['brand'])) already returns Brave Browser. So your change to Brave in brave-core#4721 introduces a regression.

@bsclifton
Copy link
Member

@da2x thanks for the input- after discussion, we've made some changes 😄 And the PR I created is updated to implement that. Please check out the JS Bin for example usage:
https://jsbin.com/sexawix/edit?html,js,console

@da2x
Copy link

da2x commented Feb 28, 2020

@bsclifton so that took an unexpected direction. Is the navigator.brave interface a temporary solution while waiting for the ua-client-hints spec? or have you decided to not support ua-client-hints and do this as a replacement?

I don’t believe that adding the navigator.brave object should cause any interoperability problems. However, the isBrave() function seems unnecessary. It would be enough to check if the navigator.brave object is defined. It could even be an empty object. What value does the explicit detection function add?

@bsclifton
Copy link
Member

@da2x I believe the idea would be that we could extend this attribute and add more methods if necessary. But starting off with just the 1. And correct, clients could potentially just check for navigator.brave- but then the recommended way would be navigator.brave.isBrave()

@bridiver
Copy link
Contributor

bridiver commented Mar 2, 2020

I agree with @da2x that it seems silly to add isBrave and I think it's only going to lead to websites making inconsistent checks and ignoring the "recommended way". I think we should just keep it simple and let them check navigator.brave !== undefined

@pes10k
Copy link
Contributor

pes10k commented Mar 2, 2020

@bridiver the thinking here is that its very likely that we'll want to add more JS functionality in the future, and having a single place to hang them off makes sense. (otherwise the chances of collisions on navigator will just keep increasing)

@jonathanKingston
Copy link

the thinking here is that its very likely that we'll want to add more JS functionality in the future, and having a single place to hang them off makes sense.

Will those pieces of functionality not be expected to be somewhat on a standards track? The benefit to not exposing Brave specific functionality would reduce the ability to fingerprint the difference in Brave over any other Chromium based browser.

I'm interested why this also couldn't be agreed with other browsers also. For example the original API promise could resolve as "chrome" in tor/private browsing mode to reduce fingerprinting.

@bridiver
Copy link
Contributor

bridiver commented Mar 2, 2020

@pes10k I'm saying it should just an empty object for now. That doesn't prohibit us from adding stuff later

@pes10k
Copy link
Contributor

pes10k commented Mar 2, 2020

@jonathanKingston

We're not concerned about Brave being distinguishable from Chrome or other browsers. Folks can easily distinguish already, and will be able to more so going forward. For fingerprinting concerns, our only goal (and the only goal we can reasonably hope to achieve) is to keep Brave users from being distinguishable from each other.

Will those pieces of functionality not be expected to be somewhat on a standards track?

I doubt that we'll be proposing anything like this for standards track anytime soon. FWIW, Client Hints UA is not standards track either currently (its all WICG).

I'm saying it should just an empty object for now. That doesn't prohibit us from adding stuff later

@bridiver I have no strong feeling about this either way other than the precedent it would set. This was proposed weeks ago, and no one objected despite several last calls for comment, and continued debating over the shape of this extremely minor feature has meant this tiny thing has taken weeks to ship, and its needed on all platforms w/in two weeks or so.

@bridiver
Copy link
Contributor

bridiver commented Mar 2, 2020

along the lines of what @jonathanKingston is saying, why navigator.brave when chrome has window.chrome? I don't really care that much about isBrave, it just seems silly to me.

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Mar 9, 2020

Verification passed on

Brave 1.5.109 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS Windows 10 OS Version 1803 (Build 17134.1006)
  • Verified STR from the description

image

Verification passed on

Brave 1.5.109 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS Ubuntu 18.04 LTS

Verified the test plan from the description

image

Verified passed with

Brave 1.5.109 Chromium: 80.0.3987.132 (Official Build) beta (64-bit)
Revision fcea73228632975e052eb90fcf6cd1752d3b42b4-refs/branch-heads/3987@{#974}
OS macOS Version 10.14.6 (Build 18G3020)
  • Verified test plan from the description:

Screen Shot 2020-03-11 at 10 11 56 AM

Screen Shot 2020-03-11 at 10 11 18 AM

@2b
Copy link

2b commented Mar 19, 2020

So did I get this right, you've implemented this detection without any way to disable it?

@bsclifton
Copy link
Member

@2b that is correct

cc: @pes10k

@bridiver
Copy link
Contributor

@2b what is your specific concern? As @pes10k mentioned we are working on a lot of projects to make Brave users indistinguishable from other Brave users, but hiding the fact that you're using Brave is very difficult because we are intentionally blocking some features that Chrome provides, among other things.

@2b
Copy link

2b commented Mar 20, 2020

@bridiver correct me if I'm wrong but up until now, with brave-related stuff like shields and rewards disabled, it was possible to tell with certainty only that someone uses Chromium-based browser. I understand it's not brave's privacy threat model to keep brave indistinguishable from chrome, as @pes10k mentioned, but don't you think it would be harder to track someone across Chromium users rather than just Brave users, considering the amount of them? Especially in Tor-windows.

@pes10k
Copy link
Contributor

pes10k commented Mar 20, 2020

@2b thats not correct. While we sent the Chromium UA, its easy for folks to tell Brave from Chrome/Chromium/etc by (for example) checking to see if the FP apis we modify work as expected, or if the referrer header is as expected, or any of the other features mentioned here https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove). Because of how fingerprinting attacks are carried out, there is no privacy benefit from being different from Chromium in known ways, and then adding an additional explicit way of knowing its Brave. Put diff, theres no change in the anonymity sets.

Happy to share more if the thinking is still unclear, but fwiw, you can be confident that this change does not affect how identifiable you are when using Brave.

@da2x
Copy link

da2x commented Mar 20, 2020

You’ll waste less CPU cycles on websites trying to fingerprint your browser to work out if you’re using Brave or not.

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