Closed Bug 1554976 Opened 6 months ago Closed 3 months ago

Generate mDNS ICE candidates in webrtc signaling

Categories

(Core :: WebRTC: Networking, enhancement, P2)

69 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: dminor, Assigned: dminor)

References

(Blocks 1 open bug)

Details

Attachments

(14 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

Bug 1548841 added support for handling incoming mDNS ICE candidates. The next step is to generate some of our own in Firefox.

Summary: Gemerate mDNS ICE candidates in webrtc signaling → Generate mDNS ICE candidates in webrtc signaling

This removes these functions: bind, getaddrinfo, recvfrom, sendto, setsockopt,
socket from the check_networking test to allow for their use by the Rust mDNS
responder.

This adds a mdns_service to mtransport to handle responding to mDNS queries.
All hostnames will be generated from UUIDs, so the responder assumes that it
is the only responder for a hostname which is registered with it. Because of
this, the responder does not first make a DNS query itself to see if any other
responder is handling a hostname, and does not wait a random amount of time
before replying, both of which are required by the specification to avoid
collisions with other responders.

Depends on D38488

Depends on D38489

Depends on D38492

Depends on D38494

This only enables mDNS on OS X for now. Some versions of Windows lack mDNS
support, there are some oddities with resolving IPv6 addresses on Linux, and
Android has not yet been tested. All of these will be addressed in follow on
bugs.

Depends on D38495

The default RTP candidate is used to populate the c= line in SDP. Rather than
using the mDNS address, which sipcc can not parse, we use 0.0.0.0:9, which is
what Chromium has chosen as well [1].

For RTCP, the mDNS obfuscated address is used.

[1] https://webrtc.googlesource.com/src.git/+/3ae59d33a310280e2f21ed4c53849950171e48e8

Depends on D38497

Dan, part of the changes here are to add a mDNS responder implemented in Rust to handle mDNS queries to support WebRTC address obfuscation.

There is already an implementation of mDNS in tree (netwerk/dns/mdns/libmdns) but it does not support hostname queries, only service discovery, and it is not well maintained. For instance, it does not seem to be working properly on OS X, which is the only platform for which we have test coverage in automation at the moment. We discussed this with the necko team, and we decided it made sense to write something specific for the WebRTC rather than try to update the existing code.

The two main risks I see are errors / security problems with the dns parser and abuse of the responder to flood the local network with UDP packets. To help mitigate the first risk, I'm using a third party dns parser library and have spent some time fuzzing it with AFL, with no problems turning up so far. For the second risk, we're currently only responding to queries, so we're relying upon the browser to not generate too many queries. There is rate limiting for DNS queries in Firefox, I believe but have not checked, that the situation is the same with other browsers.

Once this code lands, my next objective is to extend the mDNS service to generate queries as well. The DNS code in Firefox relies upon the operating system to make queries, and older versions of Windows do not support mDNS. In addition, mDNS is not working properly in CI for Linux. It's possible that the avahi service is just not installed on our AWS images and using our own code to generate queries will work in that environment. I plan to add rate limiting to the mDNS service as part of implementing queries.

Please let me know what I should be doing from a security point of view to get this code landed safely and especially if I need to formally request a security review. Thanks!

Flags: needinfo?(dveditz)

[Damn, thought I submitted this days ago and just found this comment still "live"]

Fuzzing is great but not always straightforward. You should get a quick review of your approach by the fuzzing team (if you haven't already) to make sure there's adequate coverage (is it hitting enough of the code, build options like ASAN, and number of iterations). They might even want to adopt your fuzzer into their harness as something to run every release or so to make sure regressions don't creep in over time.

Fuzzing has the best shot at finding memory corruption (crashes are obvious failures, and ASAN turns minor corruptions into crashes), which isn't going to happen much in rust unless it's built on unsafe libraries. Are there other DNS logic failures that would be worth asserting (at least in test builds) so fuzzing can detect them?

When you do start generating queries, what information in them comes from input supplied by web content? In addition to the DOS potential you mention (make sure there are tests for the rate-limiting) I'd also worry about whether this can be used to probe or map out someone's internal network.

Flags: needinfo?(dveditz)

With the move to the socket process, the UUID service is no longer available
in nricectx. This adds a pair of helper functions to mdns_service to generate
UUIDs and uses them to generate hostnames inside nricectx.

Depends on D38498

The current code causes one mDNS service to be created for each PeerConnection.
Due to Bug 1569311, the services persist until shutdown, which can lead to a
lot of mDNS threads running on sites which use WebRTC for fingerprinting. This
change makes it so we start at most one mDNS service.

I've filed Bug 1569955 to look at shutting down the mDNS service after the
last hostname is unregistered. As an alternative, if we fix Bug 1569311, we
could also use refcounting and stop the mDNS service after the last
StunAddrsRequestParent is freed.

Depends on D42150

Checking this assertion outside of the if statement can result in
a use-after-free in debug builds.

Depends on D42151

The Rust get_if_addrs library previously used does not build on Android with
our build system. Since we're already using nICEr to determine the local
interface addresses, rather than fix the Rust library, we can use those
addresses to set the interface on which mdns_service listens.

Depends on D42152

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0fc6a1d4332
Allow network functions needed by Rust mDNS responder; r=tjr
https://hg.mozilla.org/integration/autoland/rev/513026415092
Add rust mdns library to mtransport; r=ng,dragana
https://hg.mozilla.org/integration/autoland/rev/a506bb40047e
Run mach vendor rust; r=ng
https://hg.mozilla.org/integration/autoland/rev/0e34595c2680
Add methods to register/unregister mDNS hostnames to StunAddrsRequestParent; r=mjf
https://hg.mozilla.org/integration/autoland/rev/a099e69241a0
Generate mDNS addresses in nricectx; r=mjf
https://hg.mozilla.org/integration/autoland/rev/24728144c263
Copy actual address to CandidateInfo in MediaTransportHandler; r=mjf
https://hg.mozilla.org/integration/autoland/rev/e2c837d1e0a0
Register mDNS hostname if required; r=mjf
https://hg.mozilla.org/integration/autoland/rev/b49b4326dcfd
Add plumbing to enable/disable host address obfuscation; r=mjf
https://hg.mozilla.org/integration/autoland/rev/24f146b86cc4
Disable host address obfuscation for simulated NAT mochitests; r=mjf
https://hg.mozilla.org/integration/autoland/rev/77c4e76c8130
Obfuscate default rtp and rtcp candidates if required; r=ng
https://hg.mozilla.org/integration/autoland/rev/6a404fca61dc
Use mdns_service to generate UUIDs; r=mjf
https://hg.mozilla.org/integration/autoland/rev/3e16c10bb966
Make mDNS service a singleton; r=mjf
https://hg.mozilla.org/integration/autoland/rev/27b4dddf9597
Move thread assertion inside if statement in OnLookupComplete; r=mjf
https://hg.mozilla.org/integration/autoland/rev/a9b209d9d880
Use StunAddrs to set interface for mdns_service; r=mjf

Backed out 14 changesets (bug 1554976) for Windows build bustages on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/5b1d527eacfc0ef034297435297e890f342de51f

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=a9b209d9d880443578f19dfc1de4bf1a92b37ba4&selectedJob=263863190

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=263863190&repo=autoland&lineNumber=37788

Log snippet:

[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - [style 0.0.1] cargo:rerun-if-changed=z:/build/build/src/obj-firefox/dist\include\nsCycleCollectionParticipant.h
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - [style 0.0.1] cargo:rerun-if-changed=z:/build/build/src/obj-firefox/dist\include\mozilla/dom/HTMLDocumentBinding.h
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - [style 0.0.1] cargo:rerun-if-changed=z:/build/build/src/obj-firefox/dist\include\js/RealmOptions.h
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - [style 0.0.1] cargo:rerun-if-changed=z:/build/build/src/obj-firefox/dist\include\mozilla/dom/ElementBinding.h
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - Running set CARGO_PKG_VERSION_MINOR=0&& set OUT_DIR='z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\build\style-f3e16605d34e9cb6\out'&& set CARGO_PKG_VERSION_PATCH=1&& set CARGO_PKG_HOMEPAGE=&& set CARGO_PKG_VERSION_MAJOR=0&& set CARGO='\\?\Z:\task_1566995606\fetches\rustc\bin\cargo.exe'&& set CARGO_MANIFEST_DIR='Z:\build\build\src\servo\components\style'&& set CARGO_PKG_NAME=style&& set CARGO_PKG_DESCRIPTION=&& set CARGO_PKG_VERSION_PRE=&& set CARGO_PKG_VERSION=0.0.1&& set PATH='z:/build/build/src/obj-firefox\release\deps;Z:\task_1566995606\fetches\rustc\bin;z:\build\fetches\clang\bin;z:\build\fetches\clang\bin;c:\Program Files\Mercurial;c:\mozilla-build\bin;c:\mozilla-build\kdiff3;c:\mozilla-build\moztools-x64\bin;c:\mozilla-build\mozmake;C:\mozilla-build\msys\bin;C:\mozilla-build\msys\local\bin;c:\mozilla-build\nsis-3.01;c:\mozilla-build\python;c:\mozilla-build\python\Scripts;c:\mozilla-build\python3;c:\Windows\system32;c:\Windows;c:\Windows\System32\Wbem;c:\Windows\System32\WindowsPowerShell\v1.0\;c:\Program Files\Amazon\cfn-bootstrap\;c:\Program Files (x86)\GNU\GnuPG\pub;c:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit\;c:\mozilla-build\python\lib\site-packages\pywin32_system32;c:\mozilla-build\python\lib\site-packages\pywin32_system32;c:\mozilla-build\python\lib\site-packages\pywin32_system32;z:/build/build/src/vs2017_15.9.6/VC/bin/HostX64/arm64;C:\Users\task_1566995606\.cargo\bin;z:\build\.mozbuild\clang\bin;z:\build\.mozbuild\cbindgen;z:\build\.mozbuild\nasm;z:/build/build/src/vs2017_15.9.6/VC/bin/HostX64/x64'&& set CARGO_PKG_AUTHORS='The Servo Project Developers'&& set CARGO_PKG_REPOSITORY=&& z:/build/fetches/sccache/sccache.exe 'z:/build/fetches/rustc/bin/rustc.exe' --crate-name style 'servo\components\style\lib.rs' --color never --crate-type lib --emit=dep-info,metadata,link -C opt-level=2 -C panic=abort -C codegen-units=1 --cfg 'feature="bindgen"' --cfg 'feature="gecko"' --cfg 'feature="gecko_profiler"' --cfg 'feature="nsstring"' --cfg 'feature="regex"' --cfg 'feature="toml"' -C metadata=9bba6f0060d97307 -C extra-filename=-9bba6f0060d97307 --out-dir 'z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps' --target aarch64-pc-windows-msvc -C 'linker=z:/build/build/src/build/cargo-linker.bat' -L 'dependency=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps' -L 'dependency=z:/build/build/src/obj-firefox\release\deps' --extern 'app_units=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libapp_units-32835028171bf3f0.rlib' --extern 'arrayvec=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libarrayvec-a73c93f55f0fb22e.rlib' --extern 'atomic_refcell=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libatomic_refcell-018f15a7a61a2e10.rlib' --extern 'bitflags=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libbitflags-8bb04a2b4bfe6635.rlib' --extern 'byteorder=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libbyteorder-d646576f18cdc26f.rlib' --extern 'cssparser=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libcssparser-c020dc424a1d9e76.rlib' --extern 'derive_more=z:/build/build/src/obj-firefox\release\deps\derive_more-4560f407ccaa32f7.dll' --extern 'euclid=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libeuclid-cf06a4d11bce4f0a.rlib' --extern 'fallible=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libfallible-becf996133cb9f12.rlib' --extern 'fxhash=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libfxhash-65176deee248a106.rlib' --extern 'hashglobe=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libhashglobe-84b427be0a312fac.rlib' --extern 'indexmap=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libindexmap-ea730ec81449bbc0.rlib' --extern 'itertools=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libitertools-2231fdad07ad24e2.rlib' --extern 'itoa=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libitoa-c4919469ce1719a7.rlib' --extern 'lazy_static=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\liblazy_static-6ff2286f2142b226.rlib' --extern 'log=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\liblog-966a88658bfae66d.rlib' --extern 'malloc_size_of=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libmalloc_size_of-b75582a9b0cb2f30.rlib' --extern 'malloc_size_of_derive=z:/build/build/src/obj-firefox\release\deps\malloc_size_of_derive-befac20185a3a05e.dll' --extern 'matches=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libmatches-6f4b2704bc17bc06.rlib' --extern 'debug_unreachable=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libdebug_unreachable-2c566383d789c89a.rlib' --extern 'nsstring=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libnsstring-ae3f7d485786e954.rlib' --extern 'num_derive=z:/build/build/src/obj-firefox\release\deps\num_derive-c7d29693c3d1724f.dll' --extern 'num_integer=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libnum_integer-0f5a9d8516314127.rlib' --extern 'num_traits=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libnum_traits-876f8f69f745eb30.rlib' --extern 'num_cpus=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libnum_cpus-58d7fa0332c0e480.rlib' --extern 'ordered_float=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libordered_float-9c1781e3aa20a1b4.rlib' --extern 'owning_ref=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libowning_ref-bea3c7a1a9a87181.rlib' --extern 'parking_lot=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libparking_lot-951840c59dc96aff.rlib' --extern 'precomputed_hash=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libprecomputed_hash-9355f86c426a69cd.rlib' --extern 'rayon=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\librayon-866acf174e53c493.rlib' --extern 'selectors=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libselectors-bac9caa3e4f6272d.rlib' --extern 'servo_arc=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libservo_arc-0edf3b83ffe3bb2d.rlib' --extern 'smallbitvec=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libsmallbitvec-93036f1b8664e729.rlib' --extern 'smallvec=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libsmallvec-7878b0cf28876b6b.rlib' --extern 'static_prefs=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libstatic_prefs-05f56188f3e781f5.rlib' --extern 'style_derive=z:/build/build/src/obj-firefox\release\deps\style_derive-49c91b424ea21112.dll' --extern 'style_traits=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libstyle_traits-beffdc73d59c7e42.rlib' --extern 'thin_slice=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libthin_slice-f2630ea075337b3a.rlib' --extern 'time=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libtime-29e716f3879b92ff.rlib' --extern 'to_shmem=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libto_shmem-d7c4e8e7ac9fb842.rlib' --extern 'to_shmem_derive=z:/build/build/src/obj-firefox\release\deps\to_shmem_derive-6146ea3591f6f0ee.dll' --extern 'uluru=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libuluru-f48bb75c95b6d10f.rlib' --extern 'unicode_bidi=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libunicode_bidi-860940e46641583f.rlib' --extern 'unicode_segmentation=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libunicode_segmentation-826b4f75f6e6291a.rlib' --extern 'void=z:/build/build/src/obj-firefox\aarch64-pc-windows-msvc\release\deps\libvoid-38f61bd00916ae62.rlib' -C opt-level=2 -C debuginfo=2 -C force-frame-pointers=yes -Dwarnings -Clinker-plugin-lto
[task 2019-08-28T14:09:31.975Z] 14:09:31 ERROR - error[E0412]: cannot find type ULONG_PTR in this scope
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - --> Z:\build\build\src\third_party\rust\winapi-0.2.8\src\basetsd.rs:86:19
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - |
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - 86 | pub type SIZE_T = ULONG_PTR;
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - | ^^^^^^^^^ not found in this scope
[task 2019-08-28T14:09:31.975Z] 14:09:31 ERROR - error[E0412]: cannot find type ULONG_PTR in this scope
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - --> Z:\build\build\src\third_party\rust\winapi-0.2.8\src\basetsd.rs:87:25
[task 2019-08-28T14:09:31.975Z] 14:09:31 INFO - |
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - 87 | pub type PSIZE_T = *mut ULONG_PTR;
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - | ^^^^^^^^^ not found in this scope
[task 2019-08-28T14:09:31.976Z] 14:09:31 ERROR - error[E0412]: cannot find type LONG_PTR in this scope
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - --> Z:\build\build\src\third_party\rust\winapi-0.2.8\src\basetsd.rs:88:20
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - |
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - 88 | pub type SSIZE_T = LONG_PTR;
[task 2019-08-28T14:09:31.976Z] 14:09:31 INFO - | ^^^^^^^^ not found in this scope

Flags: needinfo?(dminor)

An unused import (mio) was pulling in winapi-0.2.8 which does not build on aarch64 windows. Looks good with this removed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=682700ad01446bfcf8c9ca4736b0682f565500f7.

Flags: needinfo?(dminor)
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e36be9c42ef1
Allow network functions needed by Rust mDNS responder; r=tjr
https://hg.mozilla.org/integration/autoland/rev/b1fb21e0d6c0
Add rust mdns library to mtransport; r=ng,dragana
https://hg.mozilla.org/integration/autoland/rev/e70a875ead24
Run mach vendor rust; r=ng
https://hg.mozilla.org/integration/autoland/rev/95f34f8de2c4
Add methods to register/unregister mDNS hostnames to StunAddrsRequestParent; r=mjf
https://hg.mozilla.org/integration/autoland/rev/3356f4e4828e
Generate mDNS addresses in nricectx; r=mjf
https://hg.mozilla.org/integration/autoland/rev/fd061900910b
Copy actual address to CandidateInfo in MediaTransportHandler; r=mjf
https://hg.mozilla.org/integration/autoland/rev/841a7ca50a14
Register mDNS hostname if required; r=mjf
https://hg.mozilla.org/integration/autoland/rev/e86aa4a5ee2c
Add plumbing to enable/disable host address obfuscation; r=mjf
https://hg.mozilla.org/integration/autoland/rev/73493c9b111a
Disable host address obfuscation for simulated NAT mochitests; r=mjf
https://hg.mozilla.org/integration/autoland/rev/3d3731561f2d
Obfuscate default rtp and rtcp candidates if required; r=ng
https://hg.mozilla.org/integration/autoland/rev/691cab7deca8
Use mdns_service to generate UUIDs; r=mjf
https://hg.mozilla.org/integration/autoland/rev/fa71a90e3ed0
Make mDNS service a singleton; r=mjf
https://hg.mozilla.org/integration/autoland/rev/1383a83336f2
Move thread assertion inside if statement in OnLookupComplete; r=mjf
https://hg.mozilla.org/integration/autoland/rev/542dfee3630a
Use StunAddrs to set interface for mdns_service; r=mjf
You need to log in before you can comment on or make changes to this bug.