[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor-browser-spec/master] Adding the FF60 nework audit

To: tor-commits@xxxxxxxxxxxxxxxxxxxx
Subject: [tor-commits] [tor-browser-spec/master] Adding the FF60 nework audit
From: gk@xxxxxxxxxxxxxx
Date: Thu, 11 Jul 2019 08:03:33 +0000 (UTC)
Delivered-to: archiver@xxxxxxxx
Delivery-date: Thu, 11 Jul 2019 04:03:45 -0400
List-archive: <http://lists.torproject.org/pipermail/tor-commits/>
List-help: <mailto:tor-commits-request@lists.torproject.org?subject=help>
List-id: "auto: code repository commits" <tor-commits.lists.torproject.org>
List-post: <mailto:tor-commits@lists.torproject.org>
List-subscribe: <https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits>, <mailto:tor-commits-request@lists.torproject.org?subject=subscribe>
List-unsubscribe: <https://lists.torproject.org/cgi-bin/mailman/options/tor-commits>, <mailto:tor-commits-request@lists.torproject.org?subject=unsubscribe>
Patch-author: Georg Koppen <gk@xxxxxxxxxxxxxx>
Sender: "tor-commits" <tor-commits-bounces@xxxxxxxxxxxxxxxxxxxx>

commit 2ac466c637335af083b7b747b490979fb33f6a1c
Author: Georg Koppen <gk@xxxxxxxxxxxxxx>
Date:   Thu Jul 11 08:03:02 2019 +0000

    Adding the FF60 nework audit
---
 audits/FF60_NETWORK_AUDIT | 196 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 196 insertions(+)

diff --git a/audits/FF60_NETWORK_AUDIT b/audits/FF60_NETWORK_AUDIT
new file mode 100644
index 0000000..282df0a
--- /dev/null
+++ b/audits/FF60_NETWORK_AUDIT
@@ -0,0 +1,196 @@
+Things to grep for in general when investigating changes:
+
+PR_GetHostByName
+PR_GetIPNodeByName
+PR_GetAddrInfoByName
+PR_StringToNetAddr (itself is good as it passes AI_NUMERICHOST to getaddrinfo. No resolution.)
+
+MDNS
+TRR (DNS Trusted Recursive Resolver)
+
+Direct Paths to DNS resolution:
+nsDNSService::Resolve
+nsDNSService::AsyncResolve
+nsHostResolver::ResolveHost
+
+SOCK_
+SOCKET_
+_SOCKET
+UDPSocket
+PR_NewTCPSocket
+AsyncTCPSocket
+TCPSocket
+
+Rust
+
+Misc PR_Socket
+
+Misc XPCOM (including commands for pre-diff review approach)
+ *SocketProvider
+ grep -R udp-socket .
+ grep -R tcp-socket .
+ grep for tcpsocket
+ grep -R "NS_" | grep SOCKET | grep "_C"
+ grep -R "@mozilla.org/network/" . | grep socket | grep -v udp-socket
+
+Android Java calls
+
+-------------------------------------------------------------------------------
+I followed the diff approach by doing `git diff -U10 esr52 esr60` and then
+going over all the changes containing the above mentioned potentially dangerous
+calls and features. For additional details see: comment:15:ticket:22176.
+
+Direct Paths to DNS resolution:
++ changes mainly to take originAttributes into account
++ nsHostResolver::ResolveHost
+  + Only used by nsDNSService
++ nsDNSService::Resolve
+  - Patched for safety (XXX: Verify application)
++ nsDNSService::AsyncResolve
+  - Patched for safety (XXX: Verify application)
++ ChildDNSService::AsyncResolve and ChildDNSService::Resolve
+   + ./netwerk/dns/ChildDNSService.cpp
+     + Resolve is not implemented and AsyncResolve goes over NeckoParent and
+       DNSRequestParent to nsDNSService::AsyncResolve
+       nsDNSService.
+   + ./netwerk/ipc/NeckoParent.cpp
+     + Calls into DNS service via DNSRequestParent::DoAsyncResolve()
+   + ./netwerk/ipc/NeckoChild.cpp
+
+- TRR (DNS Trusted Recursive Resolver)
+  - off for now (`network.trr.mode` is `0`), should be unproblematic, though, as
+  we make sure to resolve DNS remotely anyway
+    - XXX verify when it gets enabled
+
++ FlyWeb
+  + removed in esr 60, bug 1374574
+
++ Mortar
+  + not compiled in as we are not compiling with --enable-mortar
+
+Strange things doing DNS:
+  - media/webrtc/trunk/webrtc/voice_engine/test/channel_transport/udp_transport_impl.cc
+    + (just a test; is using getaddrinfo)
+  - a bunch of gtest.cc inclusions
+    + (just a test; using getaddrinfo (via
+      StreamingListener::SocketWriter::MakeConnection() or
+      StreamingListener::MakeConnection))
+  + third_party/python/requests/requests/packages/urllib3/util/connection.py
+    (getaddrinfo)
+  - mtransport
+    + Not compiled in due to us using --disable-webrtc
+
+Rust
+  + Auditing Rust for proxy bypasses is tricky.
+    https://bugzilla.mozilla.org/show_bug.cgi?id=1376621
+    (https://hg.mozilla.org/mozilla-central/rev/435183826647) got some way to
+    enforce proxy safety for Linux at least (bug 1524408 is the bug for
+    macOS/Windows). I went backwards from the enabled features in
+    toolkit/library/rust/gkrust-features.mozbuild and think we are good here.
+    For a double-check of that approach and a different one, see: #27616.
+    See as well comment:11:ticket:21862.
+
+SOCK_
+  + media/webrtc/trunk/webrtc/base/network.cc,
+    media/webrtc/trunk/webrtc/base/physicalsocketserver.cc and other WebRTC
+    code
+    + disabled via WebRTC
+  - netwerk/srtp/src/test/rtpw.c (just a test)
+  - nsprpub/pr/src/md/windows/w95sock.c XXX part of TCP Fast Open implementation
+    (bug 1188435; disabled by pref right now)
+  - in a bunch of .py files in psutil update (which is used for unit tests)
+  - third_party/python/pyasn1-modules/tools/snmpget.py in pyasn1-modules update
+    (which is used for unit tests)
+
+UDPSocket
+  + Mortar (not compiled in as we are not compiling with --enable-mortar)
+
+PR_NewTCPSocket
+  - security/nss/fuzz/tls_server_target.cc (just used for fuzzing)
+
+AsyncTCPSocket
+  + /media/webrtc/trunk/webrtc/base/nat_unittest.cc (just test and we compile
+    WebRTC out)
+
+TCPSocket (test in bug 1329245 that ensures TCPSocket is not exposed to content)
+  + Mortar (not compiled in as we are not compiling with --enable-mortar)
+
+udp-socket
+  + netwerk/test/unit/test_udpsocket.js (test-only)
+  + netwerk/test/unit/test_udpsocket_offline.js (test-only)
+
+tcpsocket
+  + not exposed to content; test_tcpsocket_not_exposed_to_content.html checks
+    that (see bug 1329245)
+
+NS_*SOCKET*_C
+  - mtransport (media/mtransport/ipc/StunAddrsRequestParent.cpp)
+    + not compiled in via MOZ_WEBRTC not specified (nor MOZ_SCTP, which relies on
+      the former being available)
+
+@mozilla.org/network/*socket
+  + browser/base/content/test/urlbar/browser_urlbar_search_no_speculative_connect_with_client_cert.js
+
+Android
+  - HttpURLConnection
+  + ch.boye.httpclientandroidlib.impl.client.* seems good
+  - ch.boye.httpclientandroidlib.impl.conn XXX: needs hard-coding proxy settings
+
+Android Java calls
+  + Uses ch.boye.httpclientandroidlib.impl.client.*:
+    + mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/oauth/FxAccountAbstractClient.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/push/autopush/AutopushClient.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/AbstractBearerTokenAuthHeaderProvider.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/AuthHeaderProvider.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResource.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResourceDelegate.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BasicAuthHeaderProvider.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/HMACAuthHeaderProvider.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/HawkAuthHeaderProvider.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/ResourceDelegate.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/SyncStorageCollectionRequest.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/SyncStorageRequest.java
+    + mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java
+    + mobile/android/services/src/test/java/org/mozilla/android/sync/test/helpers/MockResourceDelegate.java
+    + mobile/android/services/src/test/java/org/mozilla/gecko/sync/net/test/TestHawkAuthHeaderProvider.java
+    + mobile/android/services/src/test/java/org/mozilla/gecko/sync/net/test/TestLiveHawkAuth.java
+
+  - Uses ch.boye.httpclientandroidlib.impl.conn.*
+    + mobile/android/base/java/org/mozilla/gecko/util/URIUtils.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResource.java
+      - XXX: needs hard-coding proxy settings
+    + mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/TLSSocketFactory.java
+
+  - Proxy bypass by mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/BaseResource.java|Resource.java
+    - mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/FxAccountClient20.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/oauth/FxAccountOAuthClient10.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/background/fxa/profile/FxAccountProfileClient10.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/browserid/verifier/BrowserIDRemoteVerifierClient10.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/browserid/verifier/BrowserIDRemoteVerifierClient20.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/push/autopush/AutopushClient.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/SyncStorageRecordRequest.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/SyncStorageRequest.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/uploaders/RecordUploadRunnable.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/EnsureCrypto5KeysStage.java
+    - mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java
+
+  - Misc other issues
+    - mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java (fixed upstream in bug 1459420)
+    - mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/UdpDataSource.java
+      - XXX: patch out
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/MetaGlobal.java
+      - XXX likely proxy bypass
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/net/TLSSocketFactory.java
+      - XXX possible bypass via ch.boye.httpclientandroidlib.conn.ssl.SSLSocketFactory.createSocket()
+    - mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/FetchInfoCollectionsStage.java
+      - XXX: likely proxy bypass
+    - mobile/android/stumbler/java/org/mozilla/mozstumbler/service/utils/AbstractCommunicator.java (URL.openConnection())
+    - mobile/android/thirdparty/com/leanplum/internal/WebSocketClient.java
+    - mobile/android/thirdparty/com/squareup/picasso
+      - XXX needs a patch (done in #27016)
+
+  - DNS leaks in Android core's library before Android O
+    - XXX: patched out in #28125 with duct tape

_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits

Prev by Author: [tor-commits] [tor-browser-build/maint-8.5] Pick up mobile default bridge changes
Next by Author: [tor-commits] [tor-browser/tor-browser-60.7.0esr-9.0-1] Bug 1549833, r=valentin
Previous by thread: [tor-commits] [translation/torbirdy] Update translations for torbirdy
Next by thread: [tor-commits] [tor-browser-spec/master] Rename the FF52 feature audit
Index(es):
- Author
- Thread