mjansson / mdns

Public domain mDNS/DNS-SD library in C
The Unlicense
459 stars 120 forks source link

Multicast client answer should not be discarded when doing one-shot discovery #60

Open belotv opened 2 years ago

belotv commented 2 years ago

We noticed that this library detect less devices than avahi or gstreamer mdns library (libmicrodns). It appears that when doing one shot query, some devices respond with multicast even if unicast is requested in the query.

This is actually accepted by the RFC:

The most basic kind of Multicast DNS client may simply send standard
   DNS queries blindly to 224.0.0.251:5[3](https://www.rfc-editor.org/rfc/rfc6762.html#section-3)53, without necessarily even
   being aware of what a multicast address is.  This change can
   typically be implemented with just a few lines of code in an existing
   DNS resolver library.  If a name being queried falls within one of
   the reserved Multicast DNS domains (see Sections 3 and [4](https://www.rfc-editor.org/rfc/rfc6762.html#section-4)), then,
   rather than using the configured Unicast DNS server address, the
   query is instead sent to 224.0.0.251:5353 (or its IPv6 equivalent
   [FF02::FB]:5353).

However this is not supported by this library.

mjansson commented 2 years ago

Well, that portion of the RFC is related to the query itself. The section related to answers is

When receiving a question with the unicast-response bit set, a
responder SHOULD usually respond with a unicast packet directed back
to the querier.  However, if the responder has not multicast that
record recently (within one quarter of its TTL), then the responder
SHOULD instead multicast the response so as to keep all the peer
caches up to date, and to permit passive conflict detection.  In the
case of answering a probe question (Section 8.1) with the unicast-
response bit set, the responder should always generate the requested
unicast response, but it may also send a multicast announcement if
the time since the last multicast announcement of that record is more
than a quarter of its TTL.

That said, the problem is that unicast bit is set when sending a query from a higher numbered ephemeral port, in which case the socket will not receive any multicast response (since that is delivered on port 5353) - so by definition the socket cannot read that data.

So yes, the library does support multicast response, but it requires you to have a second socket open and bound to multicast on port 5353. In which case you will probably send the query from that socket anyway.

belotv commented 2 years ago

Indeed, the section was related to the query. However, I noticed that if we used the --discover option with port set to MDNS_PORT, it fail to receive answers. After Wiresharking, client are actually answering back to the IP address of the computer running mdns rather than multicast address.

However, these packets are not being received by the socket (at least on macOS), which seems to only receive the multicast ones. Not sure where the packet goes (maybe catched by the native Bonjour ?).

Somehow, this is logical as the discovery query is QU (0x80). Changing the discovery query to QM (0x00) make the discovery working, but it means editing the library itself.

belotv commented 2 years ago

Ok, working with multiple sockets I discovered an issue in the library. According to the documentation "To bind the socket to a specific interface, pass in the appropriate socket address in saddr," However, it appears that multicast sockets are always listening to all interfaces.

I had to implement this in the socket_seup function to have a proper binding and avoid responses being received by the wrong socket (fixed for macOS, untested for Linux, did not touch the bind for Windows as I have no access to a test machine):

// Make sure that sockets are bound to specific interface, even when multicasting
#ifdef __APPLE__
    int interface_idx = if_nametoindex(interface);
    if (setsockopt(sock, IPPROTO_IP, IP_BOUND_IF, &interface_idx, sizeof(interface_idx)))
        return -1;
#elif defined _WIN32
    if (bind(sock, (struct sockaddr*)&sock_addr, sizeof(struct sockaddr_in)))
        return -1;
#else
    if (setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE, interface, strlen(interface)))
        return -1;
#endif
mjansson commented 2 years ago

Thank you for the additional info, I will certainly have to look into this in more detail