[tor-bugs] #22176 [Applications/Tor Browser]: Review networking code for Firefox 60

[Tor Bug Tracker & Wiki]

#22176: Review networking code for Firefox 60
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
                                                 |  team
     Type:  task                                 |         Status:  new
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ff60-esr, GeorgKoppen201809,         |  Actual Points:
  TorBrowserTeam201809                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by gk):

 Okay, I followed Mike in doing just a 'git diff -U10 esr52 esr60' and
 focused primarily on the new lines, looking for various types of socket
 usage. (Things like the PR_ calls, UDPSockets, SOCK_, etc). My review
 notes will be posted in the repo.

 I think we are good with respect to proxy bypass issues, however I have
 opened #27616 to have someone looking with fresh eyes on the Rust
 situation as it is new and complex. I did look at the laundry list from
 ESR52 (see: comment:3:ticket:21625 and comment:6:ticket:21625) while I was
 reviewing:

 **Make sure disabling WebRTC still disables all of the
 ./media/mtransport/* stuff
 and SCTP

 if CONFIG['MOZ_WEBRTC'] and CONFIG['COMPILE_ENVIRONMENT']:
     DIRS += [
         '/media/webrtc',
         '/media/mtransport',
     ]
 in toolkit/toolkit.mozbuild

 SCTP is only enabled with WebRTC (see MOZ_SCTP in old-configure.in and
 MOZ_WEBRTC in ./dom/base/moz.build)

 **Verify our defense-in-depth patches to NSS/OCSP still apply (ditto for
 other proxy patches)

 Done during rebase review.

 **Verify that the TCPSocket and UDPSocket DOM APIs are still disabled by
 pref (esp if the moz prefix goes away).

 TCP socket is chrome-only via ShouldTCPSocketExist() check. There is even
 a test for that: `test_tcpsocket_not_exposed_to_content.html` and we still
 have `pref("dom.udpsocket.enabled", false);` for UDP socket.

 The Rust review was complicated. There are a bunch of crates included,
 like `mio`, that can do all sorts of networking stuff, but we can't rip
 that out and modifying the crates is tricky as they are needed as a build
 time requirement and their integrity checked by hash. So, I did that part
 of the audit three-fold:

 1) I looked as good as I could for things that could use/provide UDP
 sockets etc.
 2) I checked where dangerous libraries got included into the tree and why,
 checking whether the new APIs got used there. Two examples would be:

 https://bugzilla.mozilla.org/show_bug.cgi?id=1391523 which is the cubeb
 bug which
 imports mio, mio-uds, net2, ws2_32.
 Later in https://bugzilla.mozilla.org/show_bug.cgi?id=1428952
 cubeb/audioipc got updated to use tokio for async socket processing and
 imports

 among others tokio-*

 ---

 https://bugzilla.mozilla.org/show_bug.cgi?id=1437571 updated mozrunner:
 winapi/winapi-0.2.8 -> winreg (update to 0.5.0) -> geckodriver/mozrunner

 3) I started to look backwards from the actual things that got embedded
 into `gkrust` which can be found in `toolkit/library/rust/gkrust-
 features.mozbuild` checking the features for potential proxy bypasses and
 I took Manish's great comment:11:ticket:21862 into account.

 Not sure if those are good approaches. Happy to improve that process and
 if we think we should indeed hack the Rust crates to back out dangerous
 calls I am open for that heavy-handed approach...

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