[tbb-bugs] #13998 [Tor Browser]: Tor Browser needs to handle changes in NoScript 2.6.9.8+

[Tor Bug Tracker & Wiki]

#13998: Tor Browser needs to handle changes in NoScript 2.6.9.8+
-------------------------+-------------------------------------------------
     Reporter:           |      Owner:  tbb-team
  mikeperry              |     Status:  closed
         Type:  defect   |  Milestone:
     Priority:  normal   |    Version:
    Component:  Tor      |   Keywords:  TorBrowserTeam201501R,
  Browser                |  tbb-4.5-alpha-3, MikePerry201501R
   Resolution:  fixed    |  Parent ID:
Actual Points:           |
       Points:           |
-------------------------+-------------------------------------------------
Changes (by mikeperry):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Replying to [comment:3 gk]:
 > Replying to [ticket:13998 mikeperry]:
 > > In NoScript 2.6.9.8, Giorgio landed several fixes for us that will
 result in changes to how Tor Browser interacts with NoScript. Here are the
 ones I'm aware of:
 > >
 > > 1. We no longer need to add https: to the whitelist for the "Medium-
 High" security slider position due to fixes to
 noscript.globalHttpsWhitelist.
 >
 > Should be fixed in my bug_13998 in my public Torbutton repo. Looking at
 NoScript code I found a more serious bug that is fixed now as well: we
 need to disable NoScript in the Medium/High mode. Otherwise any JavaScript
 resource is loaded as the HTTPS checks are only considered if JavaScript
 is disabled. See: `isJSEnabled()` in noscriptService.js.

 Ah yes, I mentioned this in my comment on the security slider ticket:
 https://trac.torproject.org/projects/tor/ticket/9387#comment:70

 I also think I found the "custom settings" flapping bug I mentioned there.
 I think we actually need to inspect all of the prefs upon update, to see
 if they actually match the expected values before setting (or unsetting)
 custom. I noted this in a comment, and also removed the pref observer for
 the NoScript whitelist pref, so that simply touching that pref doesn't
 cause custom settings. I pushed this in an extra commit on top of your
 work back to origin/master.

 > > 1. Temporary permissions are no longer stored in the noscript.temp
 pref (which gets written to disk). Instead, they are stored in a new
 memory-only data structure. This means New Identity needs to change how it
 clears NoScript permissions.
 >
 > What do you mean by "new memory-only data structure"? As far as I can
 see there is no such thing. `tempSites` and `gTempSites` are already
 available in 2.6.9.6 and cleared if we call `eraseTemp`. (The same happens
 still with 2.6.9.10 which is why we don't have to change anything wrt
 temporary permissions, I think)

 Ah, I wasn't sure the noscript function properly dealt with this new
 mechanism. Sounds good.

 > > 1. The pref noscript.volatilePrivatePermissions (which governs if
 temporary permissions are used for Private Browsing Mode) is false by
 default. We probably want to set it to true if disk records are disabled,
 but false if they are enabled. We will also need to ensure "New Identity"
 properly clears the permissions in both cases.
 >
 > Looking at the code this pref is true by default beginning from 2.6.9.7,
 if I see that correctly. Grepping gives me something like:
 >
 > {{{
 >
 ./defaults/preferences/noscript.js:pref("noscript.volatilePrivatePermissions",
 true);
 > }}}
 >
 > And updating a freshly downloaded 4.5-alpha-2 to 2.6.9.10 shows `true`
 as well. Did you have something else here in mind?

 Giorgio told me he was going to change this default in 2.6.9.8. I guess he
 changed his mind? Or delayed the change?

 > `volatilePrivatePermissions` should now be bound to no-disk/disk but as
 in the previous point you mentioned fixed there is no change in handling
 temporary permissions. This pref is more used to hide non-temporary
 menuitems and not to tamper with the nature of the permissions itself. I
 tested it a bit though and think we don't need to implement changes here
 unless we want to erase the permanent permissions as well on New Identity.

 This looks good. As I said, I've merged your work, but you should perhaps
 look at my latest commit and comment. We don't need to rush that fix for
 the release, but we should fix that eventually.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13998#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online