Closed Bug 1756280 Opened 2 years ago Closed 2 years ago

RFP + navigator.pdfViewerEnabled

Categories

(Core Graveyard :: Plug-ins, defect, P1)

Product:
Core Graveyard
Component:
Plug-ins
Version:
Firefox 99
Type:
defect

Tracking

(firefox100 fixed)

Status:
VERIFIED FIXED
Milestone:
100 Branch
Tracking Flags:
Tracking Status
firefox100 --- fixed

People

(Reporter: simon.mainey, Assigned: handyman)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

https://arkenfox.github.io/TZP/tzp.html#devices
14.32 KB, image/png
Details
Bug 1756280: Force navigator.pdfViewerEnabled, etc. if privacy.resistFingerprinting is set r=peterv!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1756280: Test combinations of privacy.resistFingerprinting and pdfjs.disabled in navigator r=#preferences-reviewers!
48 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

Following on from Bug 1720353

When RFP is enabled, enumeration of mimeTypes and plugins will return an "empty" object

However, regardless of RFP, the value of pdfjs.disabled is reflected in navigator.pdfViewerEnabled, thus completely bypassing the purpose of hiding the enumeration of the hardcoded items in mimeTypes and plugins

Actual results:

STR

TEST ONE: RFP off

  • make sure pdfjs.disabled = false
    • console.log (navigator.pdfViewerEnabled) // true
  • flip pdfjs.disabled = true
    • console.log (navigator.pdfViewerEnabled) // false

TEST TWO: repeat but with RFP on

  • results are the same

Expected results:

FIX

When RFP is enabled, navigator.pdfViewerEnabled should return false

Component: Untriaged → Plug-ins
Product: Firefox → Core

pinging David

Flags: needinfo?(davidp99)

RFP off (top), RFP off (bottom)

This adds entropy, as RFP users could be true or false

This makes sense to me but comes with a bit of a penalty. We can easily make navigator.pdfViewerEnabled depend on privacy.resistFingerprinting but that would also be expected to mess with some (future) pages' use of PDFs. navigator.pdfViewerEnabled use is not yet widespread but it was introduced because of experimentation that suggested the feature was necessary to avoid breaking some sites. OTOH, under RFP, we are already blocking the plugins and mimeTypes that were also included to aid consistency with pdf use in current sites. In other words, under RFP, I assume some current sites to have an issue with the plugins and mimeTypes because they are using them in an invalid way (it's bad to parse them). I would expect future sites to have an issue with pdfViewerEnabled because they are using it the right way -- and we accept that. NI to confirm I'm thinking about this the right way.

Flags: needinfo?(davidp99) → needinfo?(simon.mainey)

RFP breaks web standards all the time. For Tor Project, they recommend users not change prefs, and ship to a captive audience, so entropy will be mostly be moot (they could lock the pref). We could just as easily have NOT forced the objects to be empty.

Doing one (objects) and not the other (navigator) renders the protection useless. My preferred suggestion is to NOT have RFP do anything with mimeTypes and plugins since they are static (if that static changes, then that's equivalency of FF version - but it becomes dynamic, then it is a problem)

So tom (and gk, sysrqb) should decide: do both (tighter) or do none (better compat and I see no entropy for Tor Browser users): it's pointless to not cover all methods

Flags: needinfo?(simon.mainey) → needinfo?(sysrqb)

FYI, the RFP behavior on plugins and mimeTypes was added because a test checked that they were empty with it on. The entries in those arrays are hard-coded for HTML and should only change, statically, by the spec (unlikely). So all proposed options are reasonable and we can go with whatever is preferred.

Yup, thanks David. The only entropy I see is if users change the setting (in about:config or via the UI in General > Applications > Portable Document Format (PDF)). I actually don't see a compat issue. As long as navigator returns a boolean, then it is what it is. Excuse my ignorance, but what is the use-case for websites to know if internal pdf rendering is enabled and how would that break anything: if they're checking for the value then it won't break - and if it returns false then it can be handled elegantly by the website? Totes struggling with this compat angle :)

So, after further reflection, I'm all for returning false on navigator and maintaining the empty objects - it's much tighter (given the UI allows users to change the value - including tor browser users)

just to clarify, if we do, we should return false for the navigator key but not interfere with pdfjs.disabled (because if that is treated as true, that can?/does?/may? break "state separation" of browser content by forcing an external app to open pdfs (I think there is a default open with dialog, but only the first time). IDK if that's feasible, or if it would cause, or could be used to cause, external apps to open pdfs.

pdfjs.disabled is default false in TB (and FF)

Per [this comment]:

Okay, I've asked internally, and I got a reply that we had to list the PDF plugin or else multiple websites would ask the user to download Adobe Acrobat Reader so there is some compatibility risk for this change.

And other comments in this thread; the behavior I think we would want (for both RFP and Tor Browser):

  • Enable pdfjs - on the logic that as far as we know, pdfjs is more secure than anything the user may installed
  • Report true for navigator.pdfViewerEnabled (no matter what) - on the logic that even if the browser can't open the pdf (because for some reason pdfjs got disabled despite our intentions?) - we don't want websites to encourage users to download and use external software

Posing to Richard, as he's taking over TB team

Flags: needinfo?(sysrqb) → needinfo?(richard)

tjr: Sounds good to me :)

Flags: needinfo?(richard)

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(jmathies)
Severity: -- → S3
Flags: needinfo?(jmathies)
Assignee: nobody → davidp99
Priority: -- → P1

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #8)

  • Enable pdfjs - on the logic that as far as we know, pdfjs is more secure than anything the user may installed
  • Report true for navigator.pdfViewerEnabled (no matter what) - on the logic that even if the browser can't open the pdf (because for some reason pdfjs got disabled despite our intentions?) - we don't want websites to encourage users to download and use external software

Checking to see if I'm following this correctly. So, under RFP, the stuff in navigator should always indicate PDF support, regardless of pdfjs.disabled. As for actually showing/using pdfjs, it sounds like we also want to override pdfjs.disabled for that as well, and always use pdfjs? Or do we want the launch of pdfjs to fail? If it's the "fail" option, I take it the behavior should be to just download the pdf file? This would not give the site a chance to detect missing pdf support and, as a result, decide to do something we don't want.

Flags: needinfo?(tom)

You can leave pdfjs.disabled alone - Tor Browser should set it; but FF doesn't need to have any special behavior for RFP mode. In RFP is enabled; navigator.pdfViewerEnabled should report true unconditionally.

Flags: needinfo?(tom)

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #12)

but FF doesn't need to have any special behavior for RFP mode.

I take this to also mean plugins and mimeTypes are not spoofed as empty

You can leave pdfjs.disabled alone - Tor Browser should set it [snip] In RFP is enabled; navigator.pdfViewerEnabled should report true unconditionally.

Am I missing something here. I don't see any reason why the pref needs to be set by Tor Browser (even if it needed to be flipped which it doesn't) - it should be pref tampering-proof

pdfjs.disabled controls navigator.pdfViewerEnabled (and plugins + mimeTypes). So effectively what you're saying to ignore the pref value and always return it as true = which auto fixes the navigator key (as always true) and also fixes plugins + mimeTypes (with hardcoded values). Or did you want it handled/tightened differently where each is used?

(In reply to Simon Mainey from comment #13)

(In reply to Tom Ritter [:tjr] (ni? for response to CVE/sec-approval/advisories/etc) from comment #12)

but FF doesn't need to have any special behavior for RFP mode.

I take this to also mean plugins and mimeTypes are not spoofed as empty

I didn't think we would change behavior for these at all.

You can leave pdfjs.disabled alone - Tor Browser should set it [snip] In RFP is enabled; navigator.pdfViewerEnabled should report true unconditionally.

Am I missing something here. I don't see any reason why the pref needs to be set by Tor Browser (even if it needed to be flipped which it doesn't) - it should be pref tampering-proof

pdfjs.disabled controls navigator.pdfViewerEnabled (and plugins + mimeTypes). So effectively what you're saying to ignore the pref value and always return it as true = which auto fixes the navigator key (as always true) and also fixes plugins + mimeTypes (with hardcoded values). Or did you want it handled/tightened differently where each is used?

Tor Browser as a general guide (I think) tries not to restrict you from changing how the browser operates (although it recommends against it.) So if a user wants to flip pdfjs.disabled, they probably should be allowed to. But no matter what you change, what TB exposes to the web (navigator.pdfViewerEnabled) should be constant.

I didn't think we would change behavior for these at all

Seems unnecessary - it'll be the only browser/config to have pdf true but mimetypes + plugins empty - unless you're concerned about future changes. There's no entropy if you allow the hardcoded results, and also no entropy for TB if you return none. Not that we're trying to hide RFP/TB (and ultimately can't), but I'm not a fan of giving something as simple as this

https://arkenfox.github.io/TZP/tzp.html#devices: FF99+

  • 2 lies bypassed

(In reply to Simon Mainey from comment #15)

I didn't think we would change behavior for these at all

Seems unnecessary - it'll be the only browser/config to have pdf true but mimetypes + plugins empty - unless you're concerned about future changes. There's no entropy if you allow the hardcoded results, and also no entropy for TB if you return none. Not that we're trying to hide RFP/TB (and ultimately can't), but I'm not a fan of giving something as simple as this

https://arkenfox.github.io/TZP/tzp.html#devices: FF99+

  • 2 lies bypassed

You're right; if those values are hardcoded by the spec we don't need to have RFP affect them, we can return those values.

navigator.{pdfViewerEnabled, mimeTypes, plugins} should ignore pdfjs.disabled when RFP is set.

RFP should make navigator ignore pdfjs.disabled and always report that a PDF viewer is supported. It should also return the hard-coded values for navigator.plugins and navigator.mimeTypes. Without RFP, navigator.pdfViewerSupported should be false and plugins and mimeTypes should be empty when pdfjs.disabled is set.

Depends on D140762

Without RFP, navigator.pdfViewerSupported should be false and plugins and mimeTypes should be empty when pdfjs.disabled is set

without RFP we honor the pref
i.e navigator.pdfViewerSupported == !pdf.disabled and plugins/mimeTypes = (pdf.disabled ? "" : hardcoded)

nvm, ... " when pdfjs.disabled is set" being at the end threw me

Pushed by daparks@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0d6793b3c6f
Force navigator.pdfViewerEnabled, etc. if privacy.resistFingerprinting is set r=peterv,tjr
Pushed by daparks@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a1e111031e92
Force navigator.pdfViewerEnabled, etc. if privacy.resistFingerprinting is set r=peterv,tjr
https://hg.mozilla.org/integration/autoland/rev/1fd694e26ee5
Test combinations of privacy.resistFingerprinting and pdfjs.disabled in navigator r=peterv

https://hg.mozilla.org/mozilla-central/rev/a1e111031e92
https://hg.mozilla.org/mozilla-central/rev/1fd694e26ee5

Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
status-firefox100: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: