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

[Tor Bug Tracker & Wiki]

#21625: Review networking code for Firefox 52
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:
                                                 |  mikeperry
     Type:  task                                 |         Status:
                                                 |  assigned
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Critical                             |     Resolution:
 Keywords:  ff52-esr, tbb-7.0-must-alpha,        |  Actual Points:
  TorBrowserTeam201705                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by gk):

 mcs/brade: I'd like to hear your opinion about the TCPSocket stuff (see
 below) as you had concerns about that the last time which resulted into
 filing #18866. (All the other pieces replied to in this comment are even
 less problematic I think.)

 Replying to [comment:6 mikeperry]:
 > Stuff to verify is still patched or disabled (part 2/3)
 >  * The DNS service was changed a bit for e10s. See
 ./netwerk/dns/ChildDNSService.cpp. Verify our DNS patch still actually
 disables non-SOCKS DNS with e10s.

 ChildDNSService.cpp has no own resolver capabilities. Sync resolve is not
 supported at all;  `AsyncResolveExtended` creates a DNSChildRequest and
 starts that request. It gets sent to the parent process
 (SendPDNSReqeustContstructor()). The corresponding
 `RecvPDNSRequestConstructor` method calls `DoAsyncResolve` provided by
 `DNSRequestParent` which calls `AsyncResolveExtended` which we have
 patched in nsDNSService2.cpp.

 >  * Make sure RTSP is still disabled for desktop and Android
 (netwerk/protocol/rtsp/*)

 RTSP is gone with

 https://bugzilla.mozilla.org/show_bug.cgi?id=1295885
 https://bugzilla.mozilla.org/show_bug.cgi?id=1291629

 . The hint in the `moz.build` file is just a leftover.


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

 We have
 {{{
 if CONFIG['MOZ_WEBRTC']:
     DIRS += [
         '/media/webrtc',
         '/media/mtransport',
     ]
 }}}
 in `toolkit.mozbuild` and we don't set `MOZ_WEBRTC` as we don't compile it
 in with the configure switch.

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

 They do and other patches still applied as well (see #20680 for what we
 did and for review comments).

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

 There is no pref anymore for `TCPSocket`, rather it is bound to
 `ShouldTCPSocketExist`:
 {{{
 -  [NewObject, Pref="dom.mozTCPSocket.enabled", CheckAnyPermissions="tcp-
 socket"]
 +  [NewObject, Func="mozilla::dom::TCPSocket::ShouldTCPSocketExist"]
 }}}
 which does
 {{{
 return
 nsContentUtils::IsSystemPrincipal(nsContentUtils::ObjectPrincipal(global));
 }}}
 . Thus only chrome code can use it. I think we are not worse off than we
 were with the pref in ESR45.

 There are no changes regarding the UDPSocket DOM API, so we are still
 good.

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