[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #22176 [Applications/Tor Browser]: Review networking code for Firefox 60
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs