Add fingerprinting resistance for WebGL (Tor 16005)

RESOLVED FIXED in Firefox 57

Status

()

defect
P1
normal
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: arthur, Assigned: cfu)

Tracking

(Blocks 2 bugs)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [tor][tor-standalone][fingerprinting][fp:m3])

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
In Tor Browser, a patch was introduced to slightly relax the restrictions on WebGL when webgl.min_capability_mode is active. The idea is to give more usable WebGL in minimal mode while at the same time keeping fingerprinting risk to a minimum.

Here's a part of the original motivation (https://trac.torproject.org/16005):

> We set webgl.min_capability_mode in an effort to resist fingerprinting.
> Unfortunately, it seems minimal mode might be a little too minimal. It
> completely breaks the HexGL racing game (at ​http://hexgl.bkcore.com/play).
> We should try to figure out what exactly about minimal mode is breaking
> this game, and make a guess as to if it is really a fingerprinting issue.

A patch was found and applied to Tor Browser -- we'd like to upstream it to Firefox, if possible.

Here is a link that tracks the latest version of the relevant Tor Browser patches: https://torpat.ch/16005.
(Reporter)

Updated

4 years ago
Whiteboard: [tor]
(Reporter)

Comment 1

4 years ago
Here is Tor Browser's patch, rebased to mozilla-central.
Attachment #8677763 - Flags: review?(jgilbert)
Comment on attachment 8677763 [details] [diff] [review]
0001-Bug-1217290-Relax-WebGL-minimal-mode-to-make-it-more.patch

Review of attachment 8677763 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLContext.h
@@ +66,4 @@
>  #define MINVALUE_GL_MAX_VARYING_VECTORS               8     // Page 164
>  #define MINVALUE_GL_MAX_TEXTURE_IMAGE_UNITS           8     // Page 164
> +#define MINVALUE_GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS    8     // Page 164
> +#define MINVALUE_GL_MAX_RENDERBUFFER_SIZE             2048  // Different from the spec, which sets it to 1 on page 164

Aww, but I love my 1x1 renderbuffers! :)

::: dom/canvas/WebGLContextValidate.cpp
@@ -1672,5 @@
>      mCanLoseContextInForeground = gfxPrefs::WebGLCanLoseContextInForeground();
>      mRestoreWhenVisible = gfxPrefs::WebGLRestoreWhenVisible();
>  
> -    if (MinCapabilityMode())
> -        mDisableFragHighP = true;

We definitely want to keep this. (r-)
Attachment #8677763 - Flags: review?(jgilbert) → review-
(Reporter)

Comment 3

4 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #2)
> Comment on attachment 8677763 [details] [diff] [review]
> 0001-Bug-1217290-Relax-WebGL-minimal-mode-to-make-it-more.patch
> 
> Review of attachment 8677763 [details] [diff] [review]:
> -----------------------------------------------------------------
[snip]
> ::: dom/canvas/WebGLContextValidate.cpp
> @@ -1672,5 @@
> >      mCanLoseContextInForeground = gfxPrefs::WebGLCanLoseContextInForeground();
> >      mRestoreWhenVisible = gfxPrefs::WebGLRestoreWhenVisible();
> >  
> > -    if (MinCapabilityMode())
> > -        mDisableFragHighP = true;
>
> We definitely want to keep this. (r-)

On a couple of desktop machines my colleagues found that disabling high precision caused flickering:
https://trac.torproject.org/projects/tor/ticket/16005#comment:17

Would it make sense to add another pref to allow us to set mDisableFragHighP = false under minimal mode? Or is there any other way to handle this problem?
Flags: needinfo?(jgilbert)

Comment 4

4 years ago
To add to Arthur's questions: What is the reasoning behind keeping this? We tried to find old hardware to test setting |mDisableFragHighP| not to |true| and it basically worked everywhere. Is it on mobile where this would break?
(In reply to Georg Koppen from comment #4)
> To add to Arthur's questions: What is the reasoning behind keeping this? We
> tried to find old hardware to test setting |mDisableFragHighP| not to |true|
> and it basically worked everywhere. Is it on mobile where this would break?

Yes. It's very important that minimal mode captures the minimum capabilities of the vast majority of hardware. Flickering in the instance mentioned above is a symptom of assuming that highp is the default, and is mostly likely a bug with the webpage.
Flags: needinfo?(jgilbert)
(Reporter)

Comment 6

4 years ago
I discussed with Jeff on IRC, and he suggested that we make an independent pref for a reduced fingerprinting mode for WebGL, instead of altering the existing minimal mode (which is intended for compatibility testing). So I'm changing the title of this bug and I will try and create an anti-fingerprinting patch.
Summary: Relax WebGL minimal mode to make it work better while still offering fingerprinting protection → Add a "fingerprinting resistance" mode for WebGL, behind a pref
(Reporter)

Comment 7

4 years ago
Here's a new version of the patch that leaves minimal mode unchanged, but instead activates fingerprinting resistance settings when the "privacy.resistFingerprinting" pref is enabled.
Attachment #8677763 - Attachment is obsolete: true
Attachment #8682789 - Flags: review?(jgilbert)
Comment on attachment 8682789 [details] [diff] [review]
0001-Bug-1217290-Add-WebGL-fingerprinting-resistance-mode.patch

Review of attachment 8682789 [details] [diff] [review]:
-----------------------------------------------------------------

This is great. I didn't know about ResistFingerprinting().

I'll formalize this in a way we want it. As such, procedurally I'm going to r- this, but I'll get us a patch that does this the way we want.

::: dom/canvas/WebGLContextState.cpp
@@ +158,5 @@
> +            case LOCAL_GL_MAX_VARYING_VECTORS:
> +                return JS::Int32Value(COMMON_GL_MAX_VARYING_VECTORS);
> +
> +            case LOCAL_GL_MAX_TEXTURE_SIZE:
> +                return JS::Int32Value(COMMON_GL_MAX_TEXTURE_SIZE);

Setting these here is great in isolation, but it doesn't stop larger textures from being allocated. We should set the limits to members during our initialization. It's easiest if I just write this bit.
Attachment #8682789 - Flags: review?(jgilbert) → review-
(Reporter)

Comment 9

3 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #8)

> I'll formalize this in a way we want it. As such, procedurally I'm going to
> r- this, but I'll get us a patch that does this the way we want.

Thanks! Much appreciated.
Summary: Add a "fingerprinting resistance" mode for WebGL, behind a pref → Add fingerprinting resistance for WebGL
Assignee: nobody → jgilbert
Attachment #8682789 - Attachment is obsolete: true
Attachment #8700834 - Flags: review?(jmuizelaar)
Comment on attachment 8700834 [details] [diff] [review]
0001-Add-fingerprinting-resistance-to-WebGL.patch

Review of attachment 8700834 [details] [diff] [review]:
-----------------------------------------------------------------

This patch seems to have a bunch of changes. Any chance you can split it up to make it easier to review?
Flags: needinfo?(jgilbert)
FWIW, I don't think I actually finished all the changes I needed to make, so we don't need to start review yet.

I'll see if I can't timeslice some time for finishing this.
Flags: needinfo?(jgilbert)
Priority: -- → P2
Priority: P2 → P3
(Reporter)

Updated

3 years ago
Summary: Add fingerprinting resistance for WebGL → Add fingerprinting resistance for WebGL (Tor 16005)
Blocks: meta_tor
Whiteboard: [tor] → [tor][tor-standalone]
(Reporter)

Updated

2 years ago
Whiteboard: [tor][tor-standalone] → [tor][tor-standalone][fingerprinting]

Updated

2 years ago
Priority: P3 → P2
See Also: → 1041818
Priority: P2 → P1
Whiteboard: [tor][tor-standalone][fingerprinting] → [tor][tor-standalone][fingerprinting][fp:m1]
Whiteboard: [tor][tor-standalone][fingerprinting][fp:m1] → [tor][tor-standalone][fingerprinting][fp:m2]
(Reporter)

Comment 14

2 years ago
Please note that Tor Browser has now also disabled web extensions: `pref("webgl.disable-extensions", true);`.
(Reporter)

Comment 15

2 years ago
And I should also mention the following two prefs have been set to resist fingerprinting:

pref("webgl.disable-fail-if-major-performance-caveat", true);
pref("webgl.enable-webgl2", false);
Hi Jeff,

Do you have time to work on this bug now?
If not, could we take it over?  :)
Flags: needinfo?(jgilbert)
(In reply to Ethan Tseng [:ethan] from comment #16)
> Hi Jeff,
> Do you have time to work on this bug now?
> If not, could we take it over?  :)

Confirmed with Jeff by email.
Assignee: jgilbert → cfu
Flags: needinfo?(jgilbert)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8700834 - Attachment is obsolete: true
Attachment #8700834 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Attachment #8891248 - Flags: review?(jmuizelaar)
Attachment #8891249 - Flags: review?(jmuizelaar)
Attachment #8891250 - Flags: review?(jmuizelaar)
Attachment #8891251 - Flags: review?(jmuizelaar)
Attachment #8891251 - Flags: review?(arthuredelstein)
Attachment #8891252 - Flags: review?(jmuizelaar)
(Assignee)

Comment 23

2 years ago
The patches are based on Jeff Gilbert's great work.
The concept is to set capability limits to WebGLContext members.
The major difference from Jeff Gilbert's patch is that in minimum capability mode or fingerprinting resisting mode, if there is any parameter whose value is lower than its related predefined constant, the initialization of WebGL context will fail.
However, I found the hardware capability is lower than the predefined constants on some machines.
For example, the MAX_CUBE_MAP_TEXTURE_SIZE value of OS X 10.10 on try server is 512, but the constant for fingerprinting resistance is 2048.
So I use the minimum of the original value and the related constant, and my test checks <= instead of ==.
For comparison, please refer to the "fnRestrict" function in Jeff's patch and the "RestrictCap" function in my "Store constant values in members" patch.
Hi Jeff,

Do you have time to review the patch?
If not, could you recommend another reviewer?  :)
Flags: needinfo?(jmuizelaar)
(In reply to Ethan Tseng [:ethan] from comment #24)
> Hi Jeff,
> 
> Do you have time to review the patch?
> If not, could you recommend another reviewer?  :)

daoshengmu has been reviewing a bunch of jgilbert's recent webgl stuff. Maybe he would be a good choice?
Flags: needinfo?(jmuizelaar)
(Reporter)

Comment 26

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review171938
Attachment #8891251 - Flags: review?(arthuredelstein) → review+
(Assignee)

Updated

2 years ago
Attachment #8891248 - Flags: review?(jmuizelaar) → review?(dmu)
Attachment #8891249 - Flags: review?(jmuizelaar) → review?(dmu)
Attachment #8891250 - Flags: review?(jmuizelaar) → review?(dmu)
Attachment #8891251 - Flags: review?(jmuizelaar) → review?(dmu)
Attachment #8891252 - Flags: review?(jmuizelaar) → review?(dmu)
(Assignee)

Comment 27

2 years ago
Hi Daosheng,

Thanks for your help!

You can compare the patches with attachment 8700834 [details] [diff] [review] for reference, which is the original patch by Jeff Gilbert.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #25)
> daoshengmu has been reviewing a bunch of jgilbert's recent webgl stuff.
> Maybe he would be a good choice?
Thanks, Jeff!

Comment 29

2 years ago
mozreview-review
Comment on attachment 8891248 [details]
Bug 1217290 - Remove WebGLContext::MinCapabilityMode

https://reviewboard.mozilla.org/r/162454/#review172030

LGTM, r+
Attachment #8891248 - Flags: review?(dmu) → review+

Comment 30

2 years ago
mozreview-review
Comment on attachment 8891249 [details]
Bug 1217290 - Remove WebGLContext impl members

https://reviewboard.mozilla.org/r/162456/#review172048

r=me after replace the gl->fGetIntegerv()

::: dom/canvas/WebGLContext.cpp:2435
(Diff revision 1)
> +WebGLContext::UpdateDrawBuffers()
> +{
> +    gl::GLContext* gl = GL();
> +    gl->fGetIntegerv(LOCAL_GL_MAX_COLOR_ATTACHMENTS, (GLint*)&mGLMaxColorAttachments);
> +    gl->fGetIntegerv(LOCAL_GL_MAX_DRAW_BUFFERS, (GLint*)&mGLMaxDrawBuffers);
> +

Please replace gl->fGetIntegerv with gl->GetPotentialInteger.
Attachment #8891249 - Flags: review?(dmu) → review+

Comment 31

2 years ago
mozreview-review
Comment on attachment 8891250 [details]
Bug 1217290 - Refine WebGLContext members

https://reviewboard.mozilla.org/r/162458/#review172084

Please modify the auto& and send r? again.

::: dom/canvas/WebGL2ContextSamplers.cpp:32
(Diff revision 1)
>  WebGL2Context::DeleteSampler(WebGLSampler* sampler)
>  {
>      if (!ValidateDeleteObject("deleteSampler", sampler))
>          return;
>  
> -    for (int n = 0; n < mGLMaxTextureUnits; n++) {
> +    for (uint32_t n = 0; n < mGLMaxTextureUnits; n++) {

using auto& to access the items here would be better.
Attachment #8891250 - Flags: review?(dmu)

Comment 32

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review172102

LGTM
Attachment #8891251 - Flags: review?(dmu) → review+

Comment 33

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review172104

::: commit-message-0a606:3
(Diff revision 1)
> +Bug 1217290 - Store constant values in members
> +
> +MozReview-Commit-ID: 5fxOdV8euJ0

This commit message is meaningless. I suggest to modify them with "Refactor WebGL max & min attribute constants for WebGL fingerprinting."

Comment 34

2 years ago
mozreview-review
Comment on attachment 8891252 [details]
Bug 1217290 - Add test cases for WebGL fingerprinting resistance

https://reviewboard.mozilla.org/r/162462/#review172106

r=me. Please modify the description of the test.

::: commit-message-912c6:3
(Diff revision 1)
> +Bug 1217290 - Add test cases
> +
> +MozReview-Commit-ID: LeEJ4V7iYto

We need to add some information to mention this test is for webgl resist fingerprinting

::: browser/components/resistfingerprinting/test/mochitest/mochitest.ini:15
(Diff revision 1)
>  scheme = https
>  [test_reduce_time_precision.html]
>  [test_hide_gamepad_info.html]
>  support-files = test_hide_gamepad_info_iframe.html
>  [test_speech_synthesis.html]
> +[test_bug1217290_webgl.html]

I suggest add more information about this test is related with resistfingerprinting instead of using the bug id. test_resist_webgl.html or something else would be better.
Attachment #8891252 - Flags: review?(dmu) → review+
Please push to try server for running webgl tests before land.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 40

2 years ago
mozreview-review
Comment on attachment 8891250 [details]
Bug 1217290 - Refine WebGLContext members

https://reviewboard.mozilla.org/r/162458/#review172154
Attachment #8891250 - Flags: review?(dmu) → review+
This needs my sign-off before landing.

Comment 42

2 years ago
mozreview-review
Comment on attachment 8891248 [details]
Bug 1217290 - Remove WebGLContext::MinCapabilityMode

https://reviewboard.mozilla.org/r/162454/#review172540

::: dom/canvas/WebGLExtensionDrawBuffers.cpp:52
(Diff revision 1)
>  WebGLExtensionDrawBuffers::IsSupported(const WebGLContext* webgl)
>  {
>      gl::GLContext* gl = webgl->GL();
>  
> -    if (!gl->IsExtensionSupported(gl::GLContext::ARB_draw_buffers) &&
> -        !gl->IsExtensionSupported(gl::GLContext::EXT_draw_buffers))
> +    return gl->IsExtensionSupported(gl::GLContext::ARB_draw_buffers) ||
> +        gl->IsExtensionSupported(gl::GLContext::EXT_draw_buffers);

Align gl-> with gl->.

Comment 43

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review172542

::: dom/canvas/WebGLContextState.cpp:507
(Diff revision 2)
> -                driverPName = LOCAL_GL_POINT_SIZE_RANGE;
> -            }
> -
>              GLfloat fv[2] = { 0 };
> -            gl->fGetFloatv(driverPName, fv);
> +            switch (pname) {
> +                case LOCAL_GL_ALIASED_POINT_SIZE_RANGE:

case line should have the same indentation as its switch.

::: dom/canvas/WebGLContextValidate.cpp:84
(Diff revision 2)
> +const float kCommonAliasedPointSizeRangeMax = 63;
> +const float kCommonAliasedLineWidthRangeMin =  1;
> +const float kCommonAliasedLineWidthRangeMax =  5;
> +
> +template<class Type>
> +bool RestrictCap(Type* const var, const Type cap)

```c++
template<typename T>
static bool
RestrictCap(T* const cap, const T restrictedVal)
{
    if (*cap < restrictedVal)
        return false;
      
    *cap = restrictedVal;
    return true;
}
```

::: dom/canvas/WebGLContextValidate.cpp:597
(Diff revision 2)
> +    mGLAliasedLineWidthRangeMin = maxLineWidth[0];
> +    mGLAliasedLineWidthRangeMax = maxLineWidth[1];

I would probably use
float mGLAliasedLineWidthRange[2];

::: dom/canvas/WebGLContextValidate.cpp:606
(Diff revision 2)
> +    GLenum driverPName = gl->IsCoreProfile() ?
> +                         LOCAL_GL_POINT_SIZE_RANGE :
> +                         LOCAL_GL_ALIASED_POINT_SIZE_RANGE;
> +    gl->fGetFloatv(driverPName, maxPointSize);
> +    mGLAliasedPointSizeRangeMin = maxPointSize[0];
> +    mGLAliasedPointSizeRangeMax = maxPointSize[1];

I would probably use
float mGLAliasedPointSizeRange[2];
Attachment #8891251 - Flags: review+

Comment 44

2 years ago
mozreview-review
Comment on attachment 8891249 [details]
Bug 1217290 - Remove WebGLContext impl members

https://reviewboard.mozilla.org/r/162456/#review172544

::: dom/canvas/WebGLContext.cpp:2430
(Diff revision 2)
>  
>  ////////////////////////////////////////////////////////////////////////////////
>  // XPCOM goop
>  
>  void
> +WebGLContext::UpdateDrawBuffers()

s/UpdateDrawBuffers/UpdateMaxDrawBuffers/

::: dom/canvas/WebGLContext.cpp:2432
(Diff revision 2)
>  // XPCOM goop
>  
>  void
> +WebGLContext::UpdateDrawBuffers()
> +{
> +    gl::GLContext* gl = GL();

gl is a member of WebGLContext. Use it directly.

::: dom/canvas/WebGLContext.cpp:2434
(Diff revision 2)
>  void
> +WebGLContext::UpdateDrawBuffers()
> +{
> +    gl::GLContext* gl = GL();
> +    gl->GetPotentialInteger(LOCAL_GL_MAX_COLOR_ATTACHMENTS, (GLint*)&mGLMaxColorAttachments);
> +    gl->GetPotentialInteger(LOCAL_GL_MAX_DRAW_BUFFERS, (GLint*)&mGLMaxDrawBuffers);

Change these to fGetInteger, since we shouldn't be calling them if they're not supported now.
Attachment #8891249 - Flags: review+

Comment 45

2 years ago
mozreview-review
Comment on attachment 8891250 [details]
Bug 1217290 - Refine WebGLContext members

https://reviewboard.mozilla.org/r/162458/#review172548

::: dom/canvas/WebGLContextValidate.cpp:458
(Diff revision 2)
>      // Note: GL_MAX_TEXTURE_UNITS is fixed at 4 for most desktop hardware,
>      // even though the hardware supports much more.  The
>      // GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS value is the accurate value.
> -    gl->fGetIntegerv(LOCAL_GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, &mGLMaxTextureUnits);
> +    gl->fGetIntegerv(LOCAL_GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS, (GLint*)&mGLMaxTextureUnits);
>  
> -    if (mGLMaxTextureUnits < 8) {
> +    if (mGLMaxTextureUnits < 8U) {

I don't believe this U is necessary. If you are actually getting a warning here, use a lowercase u.

::: dom/canvas/WebGLContextValidate.cpp:499
(Diff revision 2)
>      }
>  
>      ////////////////
>  
>      if (gl->IsSupported(gl::GLFeature::ES2_compatibility)) {
> -        gl->fGetIntegerv(LOCAL_GL_MAX_FRAGMENT_UNIFORM_VECTORS, &mGLMaxFragmentUniformVectors);
> +        gl->fGetIntegerv(LOCAL_GL_MAX_FRAGMENT_UNIFORM_VECTORS, (GLint*)&mGLMaxFragmentUniformVectors);

You can also use GetUInteger() or GetIntAs<uint32_t>()
Attachment #8891250 - Flags: review+

Comment 46

2 years ago
mozreview-review
Comment on attachment 8891252 [details]
Bug 1217290 - Add test cases for WebGL fingerprinting resistance

https://reviewboard.mozilla.org/r/162462/#review172550

::: browser/components/resistfingerprinting/test/mochitest/mochitest.ini:15
(Diff revision 2)
>  scheme = https
>  [test_reduce_time_precision.html]
>  [test_hide_gamepad_info.html]
>  support-files = test_hide_gamepad_info_iframe.html
>  [test_speech_synthesis.html]
> +[test_webgl_parameters.html]

This actually needs to go in dom/canvas/test/webgl-mochitest, since we don't necessarily have webgl on all our test machines.

::: browser/components/resistfingerprinting/test/mochitest/test_webgl_parameters.html:2
(Diff revision 2)
> +<!DOCTYPE html>
> +<meta charset="utf8">

'utf-8' not 'utf8'

::: browser/components/resistfingerprinting/test/mochitest/test_webgl_parameters.html:14
(Diff revision 2)
> +    set: [
> +      ["privacy.resistFingerprinting", true]
> +    ]
> +  });
> +
> +  let canvas = document.getElementById("webgl-test");

Generally just use `document.createElement('canvas')`.

::: browser/components/resistfingerprinting/test/mochitest/test_webgl_parameters.html:18
(Diff revision 2)
> +
> +  let canvas = document.getElementById("webgl-test");
> +  SimpleTest.ok(canvas, "Get canvas");
> +  let gl = canvas.getContext("webgl");
> +  SimpleTest.ok(gl, "Get WebGL context");
> +  SimpleTest.ok(gl.getParameter(gl.MAX_TEXTURE_SIZE) <= 2048, "MAX_TEXTURE_SIZE <= 2048");

These should all be ==, not <=.
If one of these params were < the expected value, that would add fingerprinting bits.
Attachment #8891252 - Flags: review-

Comment 47

2 years ago
mozreview-review
Comment on attachment 8891248 [details]
Bug 1217290 - Remove WebGLContext::MinCapabilityMode

https://reviewboard.mozilla.org/r/162454/#review172552
Attachment #8891248 - Flags: review+

Comment 48

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review172556

RestrictCap doesn't work as written.
Attachment #8891251 - Flags: review+ → review-
(Assignee)

Comment 49

2 years ago
(In reply to Jeff Gilbert [:jgilbert] from comment #48)
> Comment on attachment 8891251 [details]
> Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL
> fingerprinting
> 
> https://reviewboard.mozilla.org/r/162460/#review172556
> 
> RestrictCap doesn't work as written.

I want to ask about this.

As I mentioned in comment 23, we spoof MAX_CUBE_MAP_TEXTURE_SIZE by 2048 for fingerprinting resistance, but I found the hardware capability of OS X 10.10 on try server is 512.

So if RestrictCap works like

> ```c++
> template<typename T>
> static bool
> RestrictCap(T* const cap, const T restrictedVal)
> {
>     if (*cap < restrictedVal)
>         return false;
> 
>     *cap = restrictedVal;
>     return true;
> }
> ```

WebGL will not be able to be initialized (cannot get webgl context from canvas).

What should we do in this case?
Flags: needinfo?(jgilbert)
Mark the test as failing on mac. This is a restriction due to a bug in Mac+Intel's cubemap handling before 10.12. Since our try machines are still running 10.10, they are restricted to 512.
Flags: needinfo?(jgilbert)
Flags: needinfo?(cfu)
(Assignee)

Comment 51

2 years ago
Thanks for the instruction.

Should I use SimpleTest.todo or something in the test if it fails to get webgl context?

Or should I file a bug for this case?
Flags: needinfo?(cfu) → needinfo?(jgilbert)
Whiteboard: [tor][tor-standalone][fingerprinting][fp:m2] → [tor][tor-standalone][fingerprinting][fp:m3]
Add a failure annotation to that test, like the other annotations in dom/canvas/test/webgl-mochitest/mochitest.ini:
> fail-if = (os == 'mac')
Flags: needinfo?(jgilbert) → needinfo?(cfu)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8891251 - Flags: review?(jgilbert)
(Assignee)

Updated

2 years ago
Attachment #8891252 - Flags: review?(jgilbert)
(Assignee)

Comment 59

2 years ago
Thank you very much.

Try looks good but it seems we also have to mark the test failing on win.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2549edab0d3a188a3bf22e8b76dab9da26029f7

LOCAL_GL_ALIASED_LINE_WIDTH_RANGE returns [1, 1] on win but it is expected to be greater than [1, 5].
https://treeherder.mozilla.org/logviewer.html#?job_id=123487942&repo=try&lineNumber=5855
Flags: needinfo?(cfu)

Comment 60

2 years ago
mozreview-review
Comment on attachment 8891251 [details]
Bug 1217290 - Refactor WebGL max & min attribute constants for WebGL fingerprinting

https://reviewboard.mozilla.org/r/162460/#review176680

::: dom/canvas/WebGLContextValidate.cpp:83
(Diff revision 3)
> +template<class Type>
> +bool RestrictCap(Type* const cap, const Type restrictedVal)

template<typename T>
static bool
RestrictCap(...)

::: dom/canvas/WebGLContextValidate.cpp:599
(Diff revision 3)
> +    GLenum driverPName = gl->IsCoreProfile() ?
> +                         LOCAL_GL_POINT_SIZE_RANGE :
> +                         LOCAL_GL_ALIASED_POINT_SIZE_RANGE;

There's plenty of room here to do:
= foo ? A
      : B;
      
Also use const when it's easy to do so, like here.
Attachment #8891251 - Flags: review?(jgilbert) → review+

Comment 61

2 years ago
mozreview-review
Comment on attachment 8891252 [details]
Bug 1217290 - Add test cases for WebGL fingerprinting resistance

https://reviewboard.mozilla.org/r/162462/#review176682
Attachment #8891252 - Flags: review?(jgilbert) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 69

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a4b4d3922386
Remove WebGLContext::MinCapabilityMode r=daoshengmu,jgilbert
https://hg.mozilla.org/integration/autoland/rev/715cea82455c
Remove WebGLContext impl members r=daoshengmu,jgilbert
https://hg.mozilla.org/integration/autoland/rev/3fc40b0c5e0a
Refine WebGLContext members r=daoshengmu,jgilbert
https://hg.mozilla.org/integration/autoland/rev/61d074cfdeaf
Refactor WebGL max & min attribute constants for WebGL fingerprinting r=arthuredelstein,daoshengmu,jgilbert
https://hg.mozilla.org/integration/autoland/rev/49462cfe24ec
Add test cases for WebGL fingerprinting resistance r=daoshengmu,jgilbert
Keywords: checkin-needed

Comment 71

2 years ago
FYI, enabling privacy.resistFingerprinting on OSX 10.13 Beta (17B35a) leads to WebGL context creation errors, for instance here:

https://threejs.org/examples/#webgl_animation_cloth

With errors on the JS console:

Error: WebGL warning: Unable to restrict WebGL limits in order to resist fingerprinting
Detector.js:13:99
Error: WebGL warning: Failed to create WebGL context: WebGL creation failed: 
* 
* Exhausted GL driver options.

I see the same in my own demos.

Should I open a separate ticket, or are these known issues? Is this feature supposed to be enabled by default some time in the future? What does this mean for WebGL capabilities (like max texture size, max uniforms, etc...) will those be capped to common values that work across all hardware? (or basically the lowest guaranteed values?)
(Assignee)

Comment 72

2 years ago
(In reply to Andre Weissflog from comment #71)
> FYI, enabling privacy.resistFingerprinting on OSX 10.13 Beta (17B35a) leads
> to WebGL context creation errors, for instance here:
> 
> https://threejs.org/examples/#webgl_animation_cloth
> 
> With errors on the JS console:
> 
> Error: WebGL warning: Unable to restrict WebGL limits in order to resist
> fingerprinting
> Detector.js:13:99
> Error: WebGL warning: Failed to create WebGL context: WebGL creation failed: 
> * 
> * Exhausted GL driver options.
> 
> I see the same in my own demos.
> 
> Should I open a separate ticket, or are these known issues? Is this feature
> supposed to be enabled by default some time in the future? What does this
> mean for WebGL capabilities (like max texture size, max uniforms, etc...)
> will those be capped to common values that work across all hardware? (or
> basically the lowest guaranteed values?)

Thank you for reporting this.

It is because the hardware capability can't meet the spoofed value set of parameters for fingerprinting resistance.  The values are determined to run most WebGL applications smoothly, so they are not that low.  I tried with my machine (macOS 10.12.6) and got [1, 1] from ALIASED_LINE_WIDTH_RANGE, where its corresponding spoofed value is [1, 5], so WebGL cannot be initialized in this case.  It is a known issue, but I remember when I was landing these patches, the same machine could get higher values from ALIASED_LINE_WIDTH_RANGE, and now it can't.  I guess there is something different in macOS graphics driver.

It seems better to me to file another bug in which we find a more proper spoofed value set that works across most hardware.

Comment 73

2 years ago
(In reply to Chung-Sheng Fu [:cfu] from comment #72)
> (In reply to Andre Weissflog from comment #71)
> > FYI, enabling privacy.resistFingerprinting on OSX 10.13 Beta (17B35a) leads
> > to WebGL context creation errors, for instance here:
> > 
> > https://threejs.org/examples/#webgl_animation_cloth
> > 
> > With errors on the JS console:
> > 
> > Error: WebGL warning: Unable to restrict WebGL limits in order to resist
> > fingerprinting
> > Detector.js:13:99
> > Error: WebGL warning: Failed to create WebGL context: WebGL creation failed: 
> > * 
> > * Exhausted GL driver options.
> > 
> > I see the same in my own demos.
> > 
> > Should I open a separate ticket, or are these known issues? Is this feature
> > supposed to be enabled by default some time in the future? What does this
> > mean for WebGL capabilities (like max texture size, max uniforms, etc...)
> > will those be capped to common values that work across all hardware? (or
> > basically the lowest guaranteed values?)
> 
> Thank you for reporting this.
> 
> It is because the hardware capability can't meet the spoofed value set of
> parameters for fingerprinting resistance.  The values are determined to run
> most WebGL applications smoothly, so they are not that low.  I tried with my
> machine (macOS 10.12.6) and got [1, 1] from ALIASED_LINE_WIDTH_RANGE, where
> its corresponding spoofed value is [1, 5], so WebGL cannot be initialized in
> this case.  It is a known issue, but I remember when I was landing these
> patches, the same machine could get higher values from
> ALIASED_LINE_WIDTH_RANGE, and now it can't.  I guess there is something
> different in macOS graphics driver.
> 
> It seems better to me to file another bug in which we find a more proper
> spoofed value set that works across most hardware.

Thanks for the quick reply. I just tested on a Win7 desktop machine with a Geforce GTX760, and the same problems happens there as well, I'll write a separate ticket.

Comment 74

2 years ago
New isolated ticket for the WebGL context initialization failure: https://bugzilla.mozilla.org/show_bug.cgi?id=1409677

Comment 75

2 years ago
Hmm, just an odd thought when reading this - wouldn't this kind of "resist fingerprinting" mode in WebGL be best to make all the caps mimic the current most popular WebGL hardware (some Intel HD 4000-like caps, or current most popular caps on Steam hardware survey), rather than latching the reported values to such hard minimums that will in turn expose that an anti-fingerprinting activity is taking place?

It looks like the bug Andre uncovered has caps set to values that never exist in the wild, effectively allowing one to fingerprint an anti-fingerprinting mode being active(?) and identify Tor? Or is that a non-concern? (I admit I have never touched that world in detail)
(In reply to Jukka Jylänki from comment #75)
> Hmm, just an odd thought when reading this - wouldn't this kind of "resist
> fingerprinting" mode in WebGL be best to make all the caps mimic the current
> most popular WebGL hardware (some Intel HD 4000-like caps, or current most
> popular caps on Steam hardware survey), rather than latching the reported
> values to such hard minimums that will in turn expose that an
> anti-fingerprinting activity is taking place?
> 
> It looks like the bug Andre uncovered has caps set to values that never
> exist in the wild, effectively allowing one to fingerprint an
> anti-fingerprinting mode being active(?) and identify Tor? Or is that a
> non-concern? (I admit I have never touched that world in detail)

I do not think a goal of turning on Anti-Fingerprinting is to hide the fact that you are preventing fingerprinting. We look sufficiently different from any 'normal' browsing mode that (I think) trying to hide the mode would be impossible.

And hiding the fact that one is using Tor is also effectively impossible, as your IP address will correlate to an exit node.
See Also: → 1409677
See Also: → 1453209
You need to log in before you can comment on or make changes to this bug.