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

navigator.hardwareConcurrency is broken, breaking my library in the process #13854

Closed
austinksmith opened this issue Jan 31, 2021 · 8 comments
Closed
Labels
closed/wontfix OS/Android Fixes related to Android browser functionality OS/Desktop

Comments

@austinksmith
Copy link

austinksmith commented Jan 31, 2021

Description

It appears that Brave browser has broken navigator.hardwareConcurrency causing it to return invalid values. No other browser I have tested so far has exhibited this behavior and this bug is essentially forcing my library to run in an artificially limited mode, there is no valid reason that navigator.hardwareConcurrency should be like this.

My machine has 8 logical cores and this is properly detected on Firefox, Edge, and Chrome, only Brave browser is reporting incorrect values that do not match reality. Brave consistently is returning half of the true number of cores on desktop and on android on a phone with 4 cores is showing 3 logical cores detected.

My software requires this value to be correct and it is correct in every browser except Brave, and it used to work fine in Brave but something has changed recently and is going to result in loss of value for my library and Brave is essentially forcing artificial limitations on the entire web by changing how this standard feature works.

Steps to Reproduce

  1. Visit https://www.hamsters.io
  2. Scroll to the bottom of the page and look at the Client Summary section and look at the Maximum Threads value
  3. Compare the value reported and compare the number of logical threads your test machine has
  4. Also, you can simply open the development console and type navigator.hardwareConcurrency and see the same thing

Actual result:

navigator.hardwareConcurrency is returning a number that does not match the number of logical cores on the computer

Hamsters.js Screenshot

Expected result:

navigator.hardwareConcurrency returns the actual number of logical cores on the computer

Reproduces how often:

Easily reproduced

@austinksmith austinksmith added OS/Android Fixes related to Android browser functionality OS/Desktop labels Jan 31, 2021
@austinksmith
Copy link
Author

austinksmith commented Jan 31, 2021

I've tested this even further and the returned value seems to be a randomly generated number that's lower than the true count, anytime you open a new tab and type navigator.hardwareConcurrency in the console the number returned is different for every new tab. Why? This also happens even when all privacy shields are disabled.

@diracdeltas
Copy link
Member

@pes10k do you know if this is intentional?

@pes10k
Copy link
Contributor

pes10k commented Feb 2, 2021

The randomized hardwareConcurrency value is intentional, its one of the points we farble (see #10808). But it happening when shields is down is definitely unintentional. Let me see if i can reproduce. Might be that we forgot to guard the hardwareConcurrency method…

@pes10k
Copy link
Contributor

pes10k commented Feb 2, 2021

@austinksmith can you double check that you still see the randomization when shields are disabled? When i test with shields down i consistently get the expected true value. And the implementation looks correct too as far as i can tell (https://github.com/brave/brave-core/blob/612896e4b30cca7352dfe2aa1247e2995f97b580/chromium_src/third_party/blink/renderer/core/frame/navigator_concurrent_hardware.cc#L29)

If you're still seeing the randomization with shields down (or shields up but fingerprinting protections off), can you let me know what platform you're on, and exactly what steps you're taking so i can try to reproduce better?

Thanks!

@austinksmith
Copy link
Author

austinksmith commented Feb 2, 2021

The randomized hardwareConcurrency value is intentional, its one of the points we farble (see #10808). But it happening when shields is down is definitely unintentional. Let me see if i can reproduce. Might be that we forgot to guard the hardwareConcurrency method…

I feel this should be only enabled with strict finger printing enabled as it has the disclaimer it may break some sites, having this enabled by default doesn't make much sense to me as its a standard feature of HTML5.

I do see this happening even when shields are completely disabled on a new tab. See the attached screenshot..

shields down

I am running Windows 10 64-bit Home Edition Version #2004 with Brave Version 1.19.88 Chromium: 88.0.4324.96 (Official Build) (64-bit)

@austinksmith
Copy link
Author

austinksmith commented Feb 2, 2021

According to https://developer.mozilla.org/en-US/docs/Web/API/NavigatorConcurrentHardware/hardwareConcurrency

The way a browser should report this value is either to reflect the true value, or alternatively reflect the true value but not if the true value exceeds the per origin limit set and in that case it should report the per origin limit (ie. 20)

if(trueValue > perOriginLimit) {
  return perOriginLimit;
} else {
 return trueValue;
}

For example, Firefox enforces a per origin limit of 20 worker threads on a single page, however Firefox still reports the true value so if you have greater than 20 logical cores, for example on a machine with 32 logical cores navigator.hardwareConcurrency will report 32 in firefox. My library accounts for this to ensure any user of Firefox with greater than 20 logical cores gets a limit set of 20, but with this situation it makes it impossible for me to know the true number and spawn an optimal number of threads for the library to use.

Some parallel computational tasks do not lend themselves well to odd numbers of threads and having a randomly generated number returned is inevitably going to lead to headaches and its also going to limit performance of my library at the same time.

Ideally it should be in accordance of the HTML Standard, if you guys can move it to being randomized only when strict fingerprinting is enabled that would make the most sense as you have a disclaimer it may break some websites.

@pes10k
Copy link
Contributor

pes10k commented Feb 2, 2021

I do see this happening even when shields are completely disabled on a new tab. See the attached screenshot..

Using the dev console on the new tab page will use the global shields setting, because we don't store shield values for the brave:// pages. If you go to an example.org or whatever, you should see the randomization respect shields settings.

Ideally it should be in accordance of the HTML Standard, if you guys can move it to being randomized only when strict fingerprinting is enabled that would make the most sense as you have a disclaimer it may break some websites.

While Brave deviates from standards wherever we think the standards are not sufficiently privacy preserving, i'll just note that Brave's behavior here is valid under the standard. The spec specifically says:

User agents should err toward exposing the number of logical processors available, using lower values only in cases where there are user-agent specific limits in place (such as a limitation on the number of workers that can be created) or when the user agent desires to limit fingerprinting possibilities. (emphasis added)

Over all, while I appreciate the time you've taken to file this issue, im going to close it and mark it wontfix. The randomization provides umbrella protections against fingerprinting, and sites should still work correctly if the page just uses a respectful number of workers (our intervention doesn't effect how many workers are actually spun up). And, most importantly, we se the API used far more often for fingerprinting than we do for user serving purposes, so we think its best to randomize as we do (reporting valid not not necessarily the full number of logical cores).

I appreciate this is not the outcome you wanted, but I believe its what best serves most of our users, in the vast majority of cases.

@austinksmith
Copy link
Author

@pes10k As you know I disagree with this decision and unfortunately as much as I like using the brave browser I'll now be forced to find an alternative that respects privacy without deviating from the specs in this manner. I understand respecting privacy however I think there are more options that can be explored here such as making this only the default behavior when using strict finger printing protection and alternatively also providing the ability to customize what finger printing protection is enabled, with this not enabled by default unless the user actually desires to limit this. A blanket all or nothing approach without the ability to change this is not a great move towards respecting the existing JavaScript ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed/wontfix OS/Android Fixes related to Android browser functionality OS/Desktop
Projects
None yet
Development

No branches or pull requests

3 participants