search

ungoogled-software / ungoogled-chromium

Google Chromium, sans integration with Google
BSD 3-Clause "New" or "Revised" License
20.77k stars 842 forks source link

[Proposal/Discussion] Fingerprinting mitigations from Bromite #429

Closed csagan5 closed 6 years ago

csagan5 commented 6 years ago

I am opening this as a "ping" and to offer a better alternative to what was described in https://github.com/gcarq/inox-patchset/issues/52 (embedding addons is a bad idea).

Please see these patches that I developed for Bromite:

For these you probably would want to add a flag; I am not sure about a general flag for anti-fingerprinting or one for each aspect, I would be inclined for a general one, but I leave that aspect to you.

Eloston commented 6 years ago

Thanks for letting me know about these features.

For these you probably would want to add a flag; I am not sure about a general flag for anti-fingerprinting or one for each aspect, I would be inclined for a general one, but I leave that aspect to you.

While creating one flag for all fingerprinting deception would be less implementation work, I still want to give users more flexibility when it comes to features that change the browser's behavior.

you already have an older version of this patch here

I don't see any new changes in the Bromite patch you linked me. Can you clarify?

https://github.com/bromite/bromite/blob/master/patches/BRM049_Battery-API-return-nothing.patch

What purpose does this patch serve when the other battery API patch is also included?

csagan5 commented 6 years ago

I don't see any new changes in the Bromite patch you linked me. Can you clarify?

Right, I can see only the percentage was downscaled from 5% to 3%; I did this because I suspect some websites have a "flickering" effect as they try to move objects while you scroll (suspicion not verified though). But it's pretty arbitrary so I am not suggesting you should change that too.

What purpose does this patch serve when the other battery API patch is also included?

Now that you mention it, I am not sure. I will merge both.

HotelBellaMuerte commented 6 years ago

at canvas effect you need add an extra patch stupid clang rules

https://p.teknik.io/Jdcaf

im just importing it AS IS ..

csagan5 commented 6 years ago

@Eloston I forgot to mention: in the DNS-over-HTTPS patch I have done some other changes than just allowing a provider selection via chrome://flags: I set the URL request priority to high (it seems like a reasonable thing to do) and I reduce the headers to minimum with a new load mode (see https://github.com/bromite/bromite/issues/70).

Eloston commented 6 years ago

After looking through the code just now, I'm going to include these two patches behind flags:

@csagan5 Do you have any other patches that are worth considering? EDIT: This wasn't intended to be offensive. There are a lot of patches, and the effects aren't always obvious just by reading the patch.

csagan5 commented 6 years ago

@Eloston considering the scope of ungoogled-chromium, I would additionally suggest only:

On the same topic of anti-fingerprinting mitigations, but decisively more invasive (since it breaks the functionality):

Not so important, but since Firefox has this configurable option you could also consider:

I can also tell you that I checked the Brave implementation for fingerprinting mitigation and I concluded that Bromite's patches (so basically those you mentioned) are not that bad, probably better (I can't say as I am biased).

Please note that the URLs you are using point to master and not the specific commit and unfortunately sometimes I reorganise the patches and the BRMxx prefix would change so the links would return 404 eventually (it would still be possible to find them by their subject as that does not change)

Eloston commented 6 years ago

https://github.com/bromite/bromite/blob/master/patches/BRM056_Add-a-flag-for-DNS-over-HTTPS.patch (should work fine on desktop browsers too)

I was debating about including this since I personally believe DNS over HTTPS is silly (almost defeating the purpose of having different networking protocols on different ports, and abuse of HTTP), but it doesn't look like it would be degrading performance or anything, so I'll probably include it.

https://github.com/bromite/bromite/blob/master/patches/BRM003_Battery-API-return-nothing.patch (the patch you already have is not sufficient, for some reason)

I tried the Battery API with the latest tag of ungoogled-chromium (68.0.3440.106-1), and it doesn't seem to be returning real data. Can you elaborate on what is being leaked? Maybe this is an Android-specific problem?

https://github.com/bromite/bromite/blob/master/patches/BRM051_AudioBuffer-AnalyserNode-fingerprinting-mitigations-via-IDL.patch (I am planning to redo this patch to provide always randomised results as the other patches do, eventually)

I might wait for the rewrite then ;)

https://github.com/bromite/bromite/blob/master/patches/BRM058_Add-flag-to-configure-maximum-connections-per-host.patch

This is a nice patch, but could you explain what you mean by this?: this can however be detrimental to devices with limited CPU/memory resources and it is disabled by default. How exactly can this be detrimental?

I can also tell you that I checked the Brave implementation for fingerprinting mitigation and I concluded that Bromite's patches (so basically those you mentioned) are not that bad, probably better (I can't say as I am biased).

I see, I'll take a look there too. Thanks for the heads up.

Please note that the URLs you are using point to master and not the specific commit and unfortunately sometimes I reorganise the patches and the BRMxx prefix would change so the links would return 404 eventually (it would still be possible to find them by their subject as that does not change)

No problem. I've already dealt with this once before and I'm not much better myself...

csagan5 commented 6 years ago

This is a nice patch, but could you explain what you mean by this?: this can however be detrimental to devices with limited CPU/memory resources and it is disabled by default. How exactly can this be detrimental?

I am reporting almost 1:1 the comment you can find here: https://github.com/chromium/chromium/blob/master/net/socket/client_socket_pool_manager.cc#L37

However I do not "buy" it: Firefox has been using 15 for ages, I have done some experimentation and there is no difference, without mentioning that a cap to 6 connections per host is yet another way to detect that a Chrome-based browser is running on the other end of the wire (I know, super-tinfoil-hat idea, but it would not be difficult at all to detect browsers capped at 6 connections...).

I tried the Battery API with the latest tag of ungoogled-chromium (68.0.3440.106-1), and it doesn't seem to be returning real data. Can you elaborate on what is being leaked? Maybe this is an Android-specific problem?

It could be. Perhaps on Android it has some other hook to update state? It has been a while since I last touched that part of the code; it had been reported to me in this issue and I verified that indeed without the patch battery data was returned.

I was debating about including this since I personally believe DNS over HTTPS is silly (almost defeating the purpose of having different networking protocols on different ports, and abuse of HTTP)

I still believe it is of great advantage (and the least of all evils, a reasonable compromise) in current situation on Android; see also my write up here where I articulate why between your ISP and Google, Cloudflare might be a better choice as they should have less datapoints about the user/user's IP: https://github.com/bromite/bromite/issues/98#issuecomment-408224699 It is technically an abuse of the protocol, but I prefer it to the complete lack of customisation of the DNS choice on Android; it is not enabled by default on Bromite but these days I advise users to do so as explained in the wiki page: https://github.com/bromite/bromite/wiki/Enabling-DNS-over-HTTPS

Eloston commented 6 years ago

I am reporting almost 1:1 the comment you can find here: https://github.com/chromium/chromium/blob/master/net/socket/client_socket_pool_manager.cc#L37

I see. A lot of the original discussion seems to be around HTTP 1.0 connection persistence and also large and/or full duplex data transfers (where Web Sockets were brought up multiple times by Chromium devs). I don't think this is as much of an issue in 2018, but we still have other ones with intermediate networking nodes imposing their own restrictions and a niche of users having specific requirements. So I see no issue with including the patch.

It could be. Perhaps on Android it has some other hook to update state? It has been a while since I last touched that part of the code; it had been reported to me in this issue and I verified that indeed without the patch battery data was returned.

In that case, I'll see if I can find a cleaner solution by disable the monitoring service or system calls for the info. If not, I'll use what you have.

It is technically an abuse of the protocol, but I prefer it to the complete lack of customisation of the DNS choice on Android

Hm, I didn't consider the lack of good DNS configuration on Android. It's quite a limited system in some ways...

The current options don't bother me, but I suppose it will be some work to allow the users customize this freely since the chrome://flags infrastructure only supports finite states. I'll just use what you have.

csagan5 commented 6 years ago

In that case, I'll see if I can find a cleaner solution by disable the monitoring service or system calls for the info. If not, I'll use what you have.

Let me know, I might use your version as well.

Hm, I didn't consider the lack of good DNS configuration on Android.

This as other patches of Bromite are mainly Android-focused, and very much following Bromite's mission of practical privacy for Android users. To be precise about DNS on Android, this is a summary of the situation:

(If I am wrong on this summary, please somebody correct me)

Hence the possibility of using DNS-over-HTTPS configurable from the browser becomes more practical than any of the available options, which 99% of the userbase will likely not pursue.

I suppose it will be some work to allow the users customize this freely since the chrome://flags infrastructure only supports finite states.

I noticed this and I plan to eventually add a chrome:// page where one could also customize proxy settings for the browser, however I am no good at UI changes so this is taking a while, nor I have had much time to work on it. Even if/when I would develop it, it would be mostly Android-focused for the reasons detailed above.

If you are building for Android you will see that this and other patches make more sense than for the desktop use-case sometimes.

Eloston commented 6 years ago

Let me know, I might use your version as well.

I'll close this issue after I've merged in these patches, so I'll be sure to let you know.

If you are building for Android you will see that this and other patches make more sense than for the desktop use-case sometimes.

I'm starting to see that piece-by-piece. Thanks for the insight into the Android world :+1:

csagan5 commented 6 years ago

FYI I updated the max connections patch yesterday: https://github.com/bromite/bromite/blob/master/patches/BRM058_Add-flag-to-configure-maximum-connections-per-host.patch

I fixed the logged warning and reduced the options to 6 and 15 since the default was already 6.

Eloston commented 6 years ago

The patch in 2db96d939088199d75a6e6c47c41fe85c8b96c3d should be able to replace the two battery API patches you have, which are:

csagan5 commented 6 years ago

@Eloston thanks, will give this a try. For the bulk of the deleted code in the .cc file, wouldn't the #if 0 approach be more cherry-pick friendly?

Eloston commented 6 years ago

It certainly would be, but then you don't get to easily see the code that changes within the conditional macro. This is problematic when combined with automatic patch refreshing; it runs the risk of excluding new code that you may not want to be excluded.

IMO, a good patch is one that explicitly shows its changes, not one that is easier to automatically refresh. It's generally easier to see the code's intent this way, and can help catch localized bugs (e.g. forgetting to update some state while stubbing out a function).

I also purposely changed more code than necessary to reduce the risk of upstream changes causing an implicit breakage. It can make things harder/longer to read, but I place more emphasis on safety in order to reduce complicated issues like #362.

csagan5 commented 6 years ago

I understand, you're right; not sure about the correlation with #362 though. You mean that more specific changes help in troubleshooting?

Eloston commented 6 years ago

You mean that more specific changes help in troubleshooting?

Sorry, I wasn't clear enough. Doing more extensive and "cleaner" stubbing can reduce the risk of breaking assumptions made in involved components. Memory leaking can be a symptom of assumptions being broken.

An example of this is the disable-metrics patch that xsmile wrote a little while back, but it ended up causing the Chromium profile to experience unbounded disk growth. (It was decided that fixing the patch wasn't worth the effort, so it was removed.)

Eloston commented 6 years ago

By the way, is there any reason why DNS over HTTPS is available on Android only? Can it be enabled for all platforms?

Eloston commented 6 years ago

Forgot to reference eb5aa1a0431937aede292acd2f9481d3a26ab575 to this issue.

All the patches I wanted to include have been included now. After I get an answer to the earlier question, then I'll close this issue.

laamalif commented 6 years ago

By the way, is there any reason why DNS over HTTPS is available on Android only? Can it be enabled for all platforms?

You mean that more specific changes help in troubleshooting?

Sorry, I wasn't clear enough. Doing more extensive and "cleaner" stubbing can reduce the risk of breaking assumptions made in involved components. Memory leaking can be a symptom of assumptions being broken.

An example of this is the disable-metrics patch that xsmile wrote a little while back, but it ended up causing the Chromium profile to experience unbounded disk growth. (It was decided that fixing the patch wasn't worth the effort, so it was removed.)

Is possible to add custom servers? apart from Google and Cloudflare anyone can run there own DoH server.

https://github.com/jedisct1/rust-doh

csagan5 commented 6 years ago

By the way, is there any reason why DNS over HTTPS is available on Android only? Can it be enabled for all platforms?

More juicy marketing segments on Android, since you can correlate the data there better? No idea, just my speculation/guess. It will work just fine on all platforms; as you can see my implementation also tries to cover https://github.com/bromite/bromite/issues/70 (original thread: https://mailarchive.ietf.org/arch/msg/doh/vHjITrOMhWSdrozGFe4-eGNMEJc).

Since DoH is an opt-in setting, nothing wrong to allow it; Firefox even switches to it by default, which I actually think it's wrong because users which know how DNS work have the legit expectation that all DNS goes through the DNS that they configured.

The whole DoH-in-chromium feature seems an experimental/underdeveloped feature, I guess it's somebody's pet project and it is not yet clear where they want to go. Feels like they regret adding it and they do not move further? Correct me if I am wrong, but I have not seen anywhere published what are the upstream (Chromium) plans for this feature.

Edit: from what I remember there was no Android-specific code involved, it seemed to me as well to be arbitrarily limited to Android. Of course it is possible that there is some specific problem that makes it not working/unusable outside Android, but I guess you will have to find it out, since no comments mentioned anything of this sort in the relevant code.

Is possible to add custom servers? apart from Google and Cloudflare anyone can run there own DoH server.

In short, no. It is not related to DNS-over-HTTPS though, it is just that in Chromium adding a free-entry text field is a pain, flags cannot have it and thus you should implement the UI. UI means: Linux/Windows, iOS, Android. And the translations; that means some work, even if you cover only main OSes.

As usual, patches are welcome.

Forgot to reference eb5aa1a to this issue.

Yes, you have the correct (latest) version of this patch, I checked. Some previous version would spam the logs with a conversion error.

After I get an answer to the earlier question, then I'll close this issue.

Any idea for future similar collaborations? We do not really have forums or a similar place for OOB communication.

Eloston commented 6 years ago

Correct me if I am wrong, but I have not seen anywhere published what are the upstream (Chromium) plans for this feature.

I didn't even know it was being implemented in Chromium until you showed me your patch. I'm not well versed in DNS over HTTPS.

Edit: from what I remember there was no Android-specific code involved, it seemed to me as well to be arbitrarily limited to Android.

Alright. I'll just remove the restriction then.

In short, no. It is not related to DNS-over-HTTPS though, it is just that in Chromium adding a free-entry text field is a pain, flags cannot have it and thus you should implement the UI

At least on desktop, we could check the command-line arguments first before checking the feature flag setting. I don't think DNS over HTTPS will be that useful on desktop, though.

Any idea for future similar collaborations? We do not really have forums or a similar place for OOB communication.

I think the issue tracker works well enough as a forum substitute; I think it's perfectly fine to have issues purely for discussion. Otherwise, there are the Gitter and Matrix chat rooms.

csagan5 commented 6 years ago

The Android version even checks for command line arguments, but it's not possible to pass any there, so you see code reading from the process' command line but actually it's just a buffer for parameters inherited from elsewhere (never investigated deeper on this). It might be the case that with some special intent you can pass some information to an Android app, but I would not know how, nor if it is even possible.

Eloston commented 6 years ago

I decided not to include DoH: f05f73c9dcceef4a3059fc49961e16642685a98a

Everything else is still included, but I haven't tested them yet.

Eloston commented 6 years ago

Whoops, I forgot the Canvas fingerprinting patch...

csagan5 commented 6 years ago

I have seen your improvement to the randomisation in the Canvas patch and the "return nothing" non-breaking patch for the WebGL info; I will probably pick at least the latter and credit you in the patch commit message.

As for the randomisation improvement (e.g. reduction of calls to grab randomness), it should be fine as well but at the moment I am not quite sure if something similar was tried or not in terms of effectiveness.

Edit: I checked the returned WebGL information: unfortunately there is a lot more there, texture parameters etc. Hiding just the vendor and renderer name would not suffice to prevent unique identification of the GPU hardware.

Eloston commented 6 years ago

I have seen your improvement to the randomisation in the Canvas patch and the "return nothing" non-breaking patch for the WebGL info; I will probably pick at least the latter and credit you in the patch commit message.

:+1:

Edit: I checked the returned WebGL information: unfortunately there is a lot more there, texture parameters etc. Hiding just the vendor and renderer name would not suffice to prevent unique identification of the GPU hardware.

Interesting. How do you access this from JS?

csagan5 commented 6 years ago

@Eloston I think it's all the info below "WebGL Image" here: https://browserleaks.com/webgl

Also look at the two reports here: https://webglreport.com/?v=1 https://webglreport.com/?v=2

I suspect that this information can allow identifying the specific GPU hardware regardless of the vendor string (which should not be unmasked anyways)? Or, in "bits of uniqueness" parlance, it provides the same amount of bits.

Edit: now that I think of it, not reporting the WebGL information/capabilities might be "security by obscurity": what if it is perfectly possible to reconstruct it via Javascript/WebGL by attempting to use each of them? In that case I would be against having any security by obscurity in place.

Eloston commented 6 years ago

NOTE: My knowledge of information theory and 3D graphics is lacking. Some of what I say here may not be completely correct.

As we both know, the purpose of fingerprinting deception is to eliminate high entropy sources that do not affect functionality of web applications in any significant way. The high precision of the floating point numbers is a good example; that kind of precision is useless to a web application, but its consistency and uniqueness increases its entropy (due to the video driver, display configuration, etc.) EDIT: I had the getClientRects and measureText metrics in mind when I referred to "the floating point numbers"

So, when we evaluate potential sources of fingerprinting, we need to consider the two questions:

  1. How much entropy is being conveyed? Basically, is there a significant amount of data that is able to be used down to narrow down to a small set of users?
  2. How useful is this information to web applications?

Applying these questions to our situation:

  1. Some of the data looks to be based on powers of two, so those values shouldn't be too surprising to anyone (low entropy). I can't speak for the other values; we would need to find samples from other configurations to find out the kinds of variations for these values (i.e. sampling the random variable to determine the probability distribution).
  2. A lot of these attributes seem to be maximum values for various graphics-related features. I can see this being useful for applications that want to automatically determine a "quality level" to keep consistent performance across various devices. In extreme cases, this can be used to prevent visual bugs due to exceeding the capabilities of said devices.
csagan5 commented 6 years ago

Yes I agree in theory (related: https://github.com/bromite/bromite/issues/131); my point was more like: given a specific GPU hardware, and the complete set of values excluding GL renderer and vendor, is that combination unique? If yes there is no point to hide them since somewhere it can exist a database where given the hash of all the rest, you can get back the text we are hiding. Or more: the amount of entropy is unchanged by the patch, it just takes trivial more work to extract it.

This and other concerns could be talked better with some data at hand, but none of the online services (https://browserleaks.com/, https://amiunique.org/) publishes it, although they all do harvest it (the page I setup at https://www.bromite.org/detect doesn't: it's just a static github page).

There might be somewhere a database of these, probably researchers of the field and/or online marketers, but I am not aware of the public research data (although one could expect these to be published, I did not search) and I am not friend with the latter.

Eloston commented 6 years ago

given a specific GPU hardware, and the complete set of values excluding GL renderer and vendor, is that combination unique? This and other concerns could be talked better with some data at hand

I see what you mean now. You're right. We can't conclude anything else without data at this point.

For the debug info, the worst case scenario is the blocking breaks webpages. Considering that these strings are similar to User Agent strings, this is a non-zero possibility. However, there are APIs to test features now, and the APIs still respond normally with this patch, so I don't think it will be likely.

The debug info patch is very small right now; I don't suspect it will become a significant maintenance burden unless the APIs or code structure keep changing. I'm going to keep it in for now. The only major change I forsee is the ability to spoof these values, for reasons not unlike that of User Agent strings.

For the other values, I don't think we should manipulate them unless we have a good reason to. At least, I don't think we should for ungoogled-chromium.

EDIT: Fixed logical mistakes and added more info.