Closed Bug 1567201 Opened 1 year ago Closed 4 months ago

Support mDNS queries in mdns_service

Categories

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

70 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: dminor, Assigned: dminor)

References

(Blocks 1 open bug)

Details

Attachments

(11 files, 1 obsolete file)

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 1554976 is adding a Rust mdns_service to respond to mDNS queries. We'll need to expand that service to also support generating queries as some versions of Windows do not support mDNS and the necko DNS code relies upon the operating system to handle queries.

Marking this P2 for now to match the meta bug.

Priority: -- → P2
Assignee: nobody → dminor

Another try run hitting a wider range of builds: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0571b42eb329ca12d8275bbe011ddca7a74ad9ab

The build fails on mingw opt builds. Part of this change is adding an extern "C" callback from Rust back to C++. The mingw builds are --disable-webrtc and on the opt builds, it is complaining that the callback is undefined. I'm guessing the callback implementation in C++ is being optimized away as the debug builds are working. It would be nice to not build mdns_service for --disable-webrtc builds, but I'm not sure if that is possible. Nathan, do you have any suggestions on how to handle this? Thanks!

Flags: needinfo?(nfroyd)

Only building mdns_service for --enable-webrtc builds should be pretty straightforward; you need to set up some feature hereabouts:

https://searchfox.org/mozilla-central/source/toolkit/library/rust/gkrust-features.mozbuild

something like:

if CONFIG['MOZ_WEBRTC']:
    gkrust_features += ['mdns']

(or webrtc, or whatever you want to call the feature). You'll need to add it to:

https://searchfox.org/mozilla-central/source/toolkit/library/rust/Cargo.toml
https://searchfox.org/mozilla-central/source/toolkit/library/gtest/rust/Cargo.toml

as well and propagate the feature down into:

https://searchfox.org/mozilla-central/source/toolkit/library/rust/shared/Cargo.toml

where mdns would enable the mdns_service dependency (mdns_service would also become optional at that point). You'd also need to modify:

https://searchfox.org/mozilla-central/source/toolkit/library/rust/shared/lib.rs#53

to be guarded with the appropriate #[cfg(feature = "mdns")]

And that should be all you need.

Flags: needinfo?(nfroyd)

This adds basic query support to the mdns_service. Support for limiting
the number of pending queries, timeouts and retries is added in
another commit in this series.

Depends on D46974

We don't want web content to use mDNS to map out the local network, so
we only support registering and querying hostnames that are valid
UUIDs (followed by .local).

Depends on D46977

This limits the number of pending mDNS queries to 50, adds support for
retrying queries if they timeout, and support for reporting failure if a query
times out twice in a row.

Depends on D46978

This adds Rust unit tests for some of the edge cases in handling queries
like limits and retries.

Depends on D46979

We need to join the multicast group on all network interfaces or we may end up
missing packets. There is a limit on the number of groups joinable by a single
socket (20 on my Linux system) but it doesn't seem worth worrying about
hitting that limit at the moment as it seems unlikely that many users would
have more than 20 network interfaces on a system on which they are running
Firefox.

Depends on D46980

According to https://tools.ietf.org/html/draft-ietf-rtcweb-mdns-ice-candidates-03#section-3.1.2.1
we need to hide the rel-addr and rel-port attributes for server reflex
candidates in order to avoid leaking local addresses.

Depends on D46981

If there are multiple pending queries, this allows up to five of them to be
sent in a single packet. This also adds a check to ensure we don't already have
a pending query for a hostname prior to sending a new query.

Depends on D46982

Because this missed the deadline for QA for Firefox 71, I'm going to land this preffed off in 71 and flip the pref early in 72.

Attachment #9094959 - Attachment is obsolete: true
Attachment #9094969 - Attachment description: Bug 1567201 - Don't build mdns_service on --disable-webrtc builds; r=froydnj! → Bug 1567201 - Don't build mdns_service on --disable-webrtc builds
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be237496f41e
Add query support to mdns_service; r=mjf
https://hg.mozilla.org/integration/autoland/rev/2afa8a8a0445
Add mDNS query support to StunAddrsRequest; r=mjf
https://hg.mozilla.org/integration/autoland/rev/b2f61006436d
Use mdns_service to handle mDNS queries; r=mjf
https://hg.mozilla.org/integration/autoland/rev/6f29bc4ed952
Validate hostname prior to registration and querying; r=ng
https://hg.mozilla.org/integration/autoland/rev/fc0a313fa8e2
Limit pending mDNS queries; r=ng
https://hg.mozilla.org/integration/autoland/rev/9f849d55a360
Add unit tests for mdns_service; r=ng
https://hg.mozilla.org/integration/autoland/rev/87443564bb2a
Join multicast group on all interfaces; r=mjf
https://hg.mozilla.org/integration/autoland/rev/ddc6931cddec
Obfuscate server reflex candidates if necessary; r=ng
https://hg.mozilla.org/integration/autoland/rev/c45a8001b0a1
Support multiple queries in a single packet; r=ng
https://hg.mozilla.org/integration/autoland/rev/570b2370189e
Don't build mdns_service on --disable-webrtc builds r=froydnj
https://hg.mozilla.org/integration/autoland/rev/d93156659b1b
Use Maybe rather than empty string for timedout queries; r=ng
Regressions: 1603790
You need to log in before you can comment on or make changes to this bug.