[tor-commits] [tor-browser-spec/master] Add Firefox 68 network audit notes
mikeperry at torproject.org
mikeperry at torproject.org
Fri Oct 11 00:05:48 UTC 2019
commit 8edeebec646e37161b78fabda6ab51f794df7fdb
Author: Mike Perry <mikeperry-git at torproject.org>
Date: Thu Oct 10 19:02:02 2019 -0500
Add Firefox 68 network audit notes
---
audits/FF68_NETWORK_AUDIT | 311 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 311 insertions(+)
diff --git a/audits/FF68_NETWORK_AUDIT b/audits/FF68_NETWORK_AUDIT
new file mode 100644
index 0000000..f8965cb
--- /dev/null
+++ b/audits/FF68_NETWORK_AUDIT
@@ -0,0 +1,311 @@
+Things to grep for in general when investigating changes:
+
+=============== Native DNS Portion =============
+
+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
+
+============ Misc Socket Portion ==============
+
+SOCK_
+SOCKET_
+_SOCKET
+UDPSocket
+TCPSocket
+ PR_NewTCPSocket
+ AsyncTCPSocket
+
+Misc PR_Socket
+
+=========== Misc XPCOM Portion ================
+
+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
+
+============ Rust Portion ================
+
+Rust
+ - XXX: What do we grep for here? Or do we rely on Ritter's compile-time tool?
+ - Check for new sendmsg and recvmsg usage
+
+============ Android Portion =============
+
+Android Java calls
+ - URLConnection
+ - XXX: getInputStream? other methods?
+ - HttpURLConnection
+ - UrlConnectionDownloader
+ - ch.boye.httpclientandroidlib.impl.client.* (look for execute() calls)
+ - grep -n openConnection\( mobile/android/thirdparty/
+ - java.net
+ - java.net.URL -- has SEVERAL proxy bypass URL fetching methods :/
+ - javax.net
+ - ch.boye.httpclientandroidlib.conn.* (esp ssl)
+ - ch.boye.httpclientandroidlib.impl.conn.* (esp ssl)
+ - Sudden appearance of thirdparty libs:
+ - OkHttp
+ - Retrofit
+ - Glide
+ - com.amitshekhar.android
+ - IntentHelper
+ - openUriExternal (can come from GeckoAppShell too)
+ - getHandlersForMimeType
+ - getHandlersForURL
+ - getHandlersForIntent
+ - android.content.Intent - too common; instead find launch methods:
+ - startActivity
+ - startActivities
+ - sendBroadcast
+ - sendOrderedBroadcast
+ - startService
+ - bindService
+ - android.app.PendingIntent
+ - android.app.DownloadManager
+ - ActivityHandlerHelper.startIntentAndCatch
+
+
+
+============ Regression/Prior Vuln Review =========
+
+Review proxy bypass bugs; check for new vectors to look for:
+ - https://trac.torproject.org/projects/tor/query?keywords=~tbb-proxy
+
+-------------------------------------------------------------------------------
+I followed the diff approach by doing `git diff esr60 esr68` and then
+going over all the changes containing the above mentioned potentially dangerous
+calls and features.
+
+Direct Paths to DNS resolution:
+ + PR_GetHostByName
+ + No new cals in diff
+PR_GetIPNodeByName
+ + No new calls in diff
+PR_GetAddrInfoByName
+ + Edited calls in diff, but no change
+PR_StringToNetAddr (itself is good as it passes AI_NUMERICHOST to getaddrinfo. No resolution.)
+ + One change in NSS unit tests ("127.0.0.1" to higher level call)
+
+MDNS:
+ + No substantive changes
+
+nsDNSService:
+ - nsDNSServiceDiscovery appears to support fallback JS mDNS option
+ + nsDNSServiceDiscovery itself is not new..
+ + nsDNSService has an mProxyType now
+ - nsDNSService::AsyncResolveInternal()?
+ + Patch blocks this (XXX: Maybe should move to PreprocessHostName)
+ + nsDNSAsyncRequest
+ - nHostResolver::ResolveHost
+ + not used outside dnsservice
+ + nsDNSService::ResolveInternal()
+ + ResolveHost() with type now...
+ + Patched
+ + AsyncResolve, AsyncResolveNative, AsyncResolveByType
+ + AsyncResolveInternal()
+ + Patched
+ + Resolve()
+ + ResolveNative
+ + ResolveInternal()
+ + Patched
+ + What about our dns service patch? where did that go?
+ + https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-68.1.0esr-9.0-2&id=97b3893a5710229a4efaf009d1b1b80d9a3918f0
+ + https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-60.8.0esr-8.5-1&id=edd3fbf5af01a812c1f5e8e04731fa1abc52da7f
+nsHostResolver:
+ + Only used by DNS service
+
+
+SOCK_
+ + libvpx has unit tests that use sockets
+ + many changes due to moved webrtc code..
+ + All disabled and not compiled in
+ + Lots of python unittests
+ + Sctp code has sockets.. Is this used?
+ + by webrtc, which we disable
+ - rust/cloudabi has some...
+ + recordreplay has some but they are AF_UNIX
+
+SOCKET_
+ + Again, lots due to moved WebRTC code
+ + Some android sockets in "LeanPlum" android lib
+ + leanplum is disabled
+ + Curl is in the tree now? lol
+ + Some sandboxing constants
+
+_SOCKET
+ - media/audioipc in rust?
+ - mio in rust?
+ + is media/mtransport/nr_socket_prsock.cpp off with webrtc?
+ + looks like yes
+ - IOActivityMonitor?
+ + Looks benign and also not new
+
+UDPSocket
+ - XXX: devtools debugger discovery still uses it
+ - ./devtools/shared/discovery/discovery.js
+ + webrtc moved code
+ + dom.udpsocket.enabled still false
+ - fallback multicastDNS uses it (not new)
+TCPSocket
+ + devtools adbsocket uses, but to localhost
+ + mtransport uses, but disabled for webrtc
+ + Tests use
+
+SocketTransport
+ + IOactivityMonitor, NetworkActivityMonitor? (old)
+ - XXX: presentation control service? I think this is old but moved
+ - dom/presentation/PresentationTCPSessionTransport.cpp
+ - App to app service..
+ - Dashboard
+ - netwerk/base/Dashboard.cpp
+ + Only examine opened sockets and cached DNS
+ - XXX: Roku control (old)
+ - ./toolkit/modules/secondscreen/RokuApp.jsm
+ - NetworkConnectivityService?? This looks new
+ - ./netwerk/base/NetworkConnectivityService.cpp
+ + Uses DNS service and nsIChannels.. Proxy safe
+ - TCPFastOpen (old, but flagged before)
+ - ./netwerk/base/TCPFastOpenLayer.cpp
+ - AttachTCPFastOpenIOLayer
+ + Appears to attach to nsSocketTransport such that it would only fast
+ open to the socks port..
+ - TCPFastOpenConnect
+ + Only used via nsSocketTransport
+
+Extra:
+ PR_Socket()
+ + None
+ PR_Connect()?
+ + Minor changes
+
+
+Misc XPCOM (including commands for pre-diff review approach)
+ + *SocketProvider
+ + Only minor changes
+ grep -R udp-socket .
+ + minor changes
+ grep -R tcp-socket .
+ + docstring only
+ grep for tcpsocket
+ + adb uses for 127.0.0.1 connection
+ + webrtc
+ grep -R "NS_" | grep SOCKET | grep "_C"
+ + indent and thread check changes; removed code
+ grep -R "@mozilla.org/network/" . | grep socket | grep -v udp-socket
+ + minor changes to spdy and unittests
+
+Rust
+ + Check for new sendmsg and recvmsg usage
+ - XXX: only in mio, audioipc..
+ - XXX: ask tjr to re-run tool?
+
+Android Java calls
+ - URLConnection (and HttpURLConnection)
+ + updater code moved; still looks ok
+ + BitmapConnection extends in mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+ + Some DRM thing
+ + Crashreporter
+ + leanplum (disabled)
+ + DefaultHttpDataSource (used only by exoplayer/HLS, which is disabled)
+ + tests
+ + grep -n openConnection\( mobile/android/thirdparty/
+ + leanplum (disabled)
+ + UrlConnectionDownloader in mobile/android/thirdparty/com/squareup/picasso/
+ + ch.boye.httpclientandroidlib.impl.client.* (look for execute() calls)
+ + BaseResource.java (is this proxied? Who calls this?)
+ + Lots of consumers..
+ - XXX: Patch going back in: https://bugs.torproject.org/31934
+ + leanplum (disabled)
+ + Adjust SDK uses execute directly
+ + https://bugs.torproject.org/25906 disabled
+ + java.net
+ + mobile/android/geckoview/src/thirdparty/java/com/google/android/exoplayer2/upstream/UdpDataSource.java
+ + Not used anywhere
+ + also: https://gitweb.torproject.org/tor-browser.git/commit/mobile/android/geckoview/src/thirdparty?h=tor-browser-68.1.0esr-9.0-2&id=662ebfc05416d2c0cd7769f864116581a1a78cad
+ + also https://gitweb.torproject.org/tor-browser.git/commit/mobile/android/moz.configure?h=tor-browser-68.1.0esr-9.0-2&id=ed1e6aecbd3c1ef4e4949e08cb023e9ef83cc142
+ - java.net.URL
+ - XXX: GeckoApplication.downloadImageForSetImage uses URL.openStream()
+ - XXX: GeckoActionProvider.downloadImageForIntent uses java.net.URL.openStream()
+ - XXX: GeckAppShell has many wrappers to create inputstreams from URLConnections
+ - Do these always have to be opened/connected first?
+ - XXX: GeckoMediaDrmBridgeV21.java - uses android.media.MediaDrm which seems to fetch stuff??
+ - XXX: BitmapUtils.decodeUrl uses openStream for non-jar urls
+ - XXX: GeckoJarReader - tons of use.. Can this be used on remote jars?
+ - XXX: AbstractCommunicator.openConnectionAndSetHeaders() - uses url.openConnection()
+ - XXX: AbstractCommunicator.sendData() - uses url.getOutputStream().. maybe ok?
+ + javax.net
+ + TLSSocketFactory from sync
+ + not used
+ + ch.boye.httpclientandroidlib.conn.* (esp ssl)
+ + Not used
+ + ch.boye.httpclientandroidlib.impl.conn.* (esp ssl)
+ + Not used
+ - IntentHelper
+ + openUriExternal prompts if private browsing mode enabled
+ - XXX: Do we enable this? Can users turn it off?
+ - XXX: ./mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/ActivityStreamContextMenu.java
+ - Uses intents to direct fetch URL
+ - XXX: Sets "show in private browsing mode" prompt to false
+ - XXX: ./mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+ - Also does telemetry via IntentHelper.openURIExternal
+ - XXX: Delegates to BrowserAppDelegate list in onNewIntent()... What's in this
+ list?
+ - XXX: ./mobile/android/base/java/org/mozilla/gecko/ChromeCastDisplay.java
+ - XXX: Looks like it might launch external service?
+ - XXX: ./mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java
+ - getHandlersFor*
+ - HelperApps.jsm
+ + browser.js uses prompt
+ + HelperAppDialog.js
+ - android.content.Intent? OMG everywhere :(
+ - startActivity
+ - XXX: ActivityHandlerHelper - Good candidate to patch for external
+ activities
+ - XXX: BrowserApp.onUrlOpenWithRefferer - Might be able to launch other
+ apps if OPEN_WITH_INTENT flag is set?
+ - XXX: CustomTabsActivity.java - Several methods
+ - XXX: WebAppActivity.onLoadRequest
+ - XXX: BasicGeckoViewPrompt.onFilePrompt()
+ - XXX: GeckoViewActivity.onExternalResponse()
+ + startActivities
+ + not used
+ + sendBroadcast
+ + Some wifi scanning/GPS stuff but is local broadcast..
+ + sendOrderedBroadcast
+ + not used
+ + startService
+ + all use seems targeted to sub-components
+ - bindService
+ - XXX: SurfaceAllocator - no idea what is happening here :/
+ - XXX: RemoteManager - no idea what is happening here :/
+- android.app.PendingIntent
+ - XXX: ChromeCastDisplay.java - probably want to make sure this is disabled?
+ - It uses RemotePresentationService
+ - XXX: CustomTabsActivity.performPendingIntent - again, hard to tell what is
+ happening here :/
+- android.app.DownloadManager
+ - XXX: BrowserApp.java uses it
+ - XXX: DownloadsIntegration.java uses it, but has a check for useSystemDownloadManager()
++ ActivityHandlerHelper.startIntentAndCatch
+ + Used only by other wrappers already covered
+
+
+
+TRR:
+ - assuming disabled by pref; deferring audit
+
+
More information about the tor-commits
mailing list