Should the daemon send the preset IPs to the browser

[fufesou]

Suppose we have some lans like the following:

B send a query. If A's response contains 172.21.5.234, it will be confusing.
In 'A''s response, the IP may be better set to the binding socket.

And I also think response may be better to send back to the interface, witch received the query. No need to send outging responses to all interfaces.

[keepsimple1]

And I also think response may be better to send back to the interface, witch received the query. No need to send outging responses to all interfaces.

I agree. It was a trade-off that we moved to use socket2 current release that does not have a good safe recv_from API. Let me think about a bit.

Also feel free to share your thoughts regarding how we can fix or work around this.

[keepsimple1]

@fufesou To figure out what we should do for a host that resides in more than one LAN:

1. Query: do you want to send out the query of service foo in all connected LANs?

2. If the answer of 1 is yes, what happens if there are responses from two LANs with the same service instance names, but different IP address? In such case, do you want to merge them into the one ServiceInfo back to the client, or have two separate ServiceInfo ? (I'm still trying to find related info in RFC 6762).

3. Response: if the connect LANs can route packets between them, do you want hosts in LAN A be aware of service instances in LAN B?

4. If the answer of 3 is no, do you think it's OK to check the DNS SRV record address to figure out what outgoing interface to send? For example, if the SRV record has address 192.168.1.2, we will only send the response out to the interface that in the LAN of 192.168.1.0/24. (If we do this, we can avoid dependency of recv_from API in socket2, which is unsafe in its current release).

also cc @lu-zero @indietyp as I am interested in their thoughts too, and if they have seen related use cases.

[fufesou]

My mistake, I want to show this picture.

1. Answer of 1 is yes. I think sending two separate ServiceInfo is good. Merging into one ServiceInfo may cause confusing.
2. Answer of 3 is no. I'm not sure about DNS SRV, but your idea seems work. And how about sending answers seperated by interfaces, it's also a trade-off.

[lu-zero]

What you suggest is an approximation of knowing the routes available per interface and filter accordingly, either at the source or the destination or both ways. I have no idea if there is a portable abstraction we could rely on.

What is the apple impl doing in this regard?

[indietyp]

I haven't yet had a use case where that would be the case, so cannot really comment on it. Sorry.

[keepsimple1]

@fufesou I've read the RFC 6762 again and here is what I think.

1. In section 6.2, it says:

When a Multicast DNS responder sends a Multicast DNS response message
containing its own address records, it MUST include all addresses
that are valid on the interface on which it is sending the message,
and MUST NOT include addresses that are not valid on that interface
(such as addresses that may be configured on the host's other
interfaces).

It means in your example, mDNS respond message should not include 172.21.0.0/16 address records when sending out the interface in 192.168.0.0/16 network segment. We could achieve that by filtering the addresses like you also mentioned above, but some details are not clear, for example: do we filter using /16 network mask or /24? or maybe using some other more reliable ways.

2. In section 14, it says:

A multihomed host with connections
to two different links may be able to communicate with two different
hosts that are validly using the same name. While this kind of name
duplication should be rare, it means that a host that wants to fully
support this case needs network programming APIs that allow
applications to specify on what interface to perform a link-local
Multicast DNS query, and to discover on what interface a Multicast
DNS response was received.

this is also related to question 1 we have earlier:

Answer of 1 is yes. I think sending two separate ServiceInfo is good. Merging into one ServiceInfo may cause confusing.

Currently our API of this crate does not support specifying which interface to use for the daemon. If the host is multi homed, the daemon binds to 0.0.0.0 and sends query on all interfaces. If two hosts on different LANs sends response for the same name, currently the 2nd response will be lost because the name was resolved by the 1st response. We only support multi-addresses in a single response.

To fix this part, we probably need to know the incoming interface, and track responses using interface and the service instance name. To do that, we might want to move back to use socket2 0.3 branch for its safe recv_from API.

In summary, I think your original problem is the item 1 (response address filter) in this comment, and we can probably implement that as the first step. What do you think?

@lu-zero

What is the apple impl doing in this regard?

Did you mean Apple's Bonjour in macOS? (btw I just notice that RFC 6762 was written by Apple engineers).

[lu-zero]

I consider their software (dns-ds & friends) a reference implementation because of that :) What is the problem in socket2 0.4 ?

[keepsimple1]

What is the problem in socket2 0.4 ?

socket2 0.4 changes its signature of recv_from, and makes it impossible to call in safe code using &mut [u8].

[fufesou]

@keepsimple1 Sorry for late.

I'm not very sure about the item 1(response address filter).
Two ways? And which is better?

1. Filter by DNS SRV record address.

do we filter using /16 network mask or /24

Netmask is provided in Ifv4Addr and Ifv6Addr, which are part of Interface

2. Send all interfaces with their own ips.

[keepsimple1]

yes I also noticed the netmask available in Interface struct. I think it's a good idea to use it.

I think we can :

1. Use the interface netmask to filter the addresses when sending outgoing messages.
2. We only send out response messages via the interface where the query was received. To support this, I'm OK with changing to use socket2 0.3 branch and its recv_from.
3. For announcing, we still send out messages via all valid interfaces.

I will give it a try locally.

[keepsimple1]

@fufesou based on what we discussed, I've drafted a diff in this gist. It builds OK and seems to be working for the basic local test.

Since I don't have a lab setup to test multi-homed mDNS, are you interested in working on an actual PR to fix this issue? My gist is for you only as a reference, feel free to make changes / improvements.

(I'm also OK if you would rather let me to do the fix, but then I probably would need your help for testing).

[fufesou]

Hi, my thoughts as following:

1. Incorrect ip check
   in file service_daemon.rs

        for (i, _) in self.respond_sockets.iter() {
            // if i.ip == src_ip {
            if (u32::from(i.ip) & u32::from(i.netmask))
                == (u32::from(src_ip) & u32::from(i.netmask))
            {
                intf_opt = Some(i);
                break;
            }
        }

2. If ip binding to current interface is not in addresses. Do not send back response.
3. If ip in addresses is not binding to current interface. Do not add answer. For subnet isolation.

[keepsimple1]

thanks @fufesou for your comment. Yes the ip check was not correct.

I refactored the approach and avoid to use socket2 recv_from. Now we have a IntfSock struct to link the interface with the socket. Please see the PR I created. Would you please help testing if that works for you?

[fufesou]

@keepsimple1 Looks good. But there is still one thing to confirm.

For announcing, we still send out messages via all valid interfaces.

If IPs on one interface is not set to serve the service. And service_info dose not contains the IPs bounded to that interface.

If 192.168.100.103 is not in adresses. Should the register process send to the bounded interface. I saw the packege has answers without ip 192.168.100.103.
And I saw it response to the browser without ip. The browser cann't resolve the service info. It seems an useless packet.

[keepsimple1]

Thanks @fufesou for checking. Indeed I noticed that later. I updated the diff to fix that, and added a new test case. Please take a look & check if works for your use case. You can also comment directly on the PR.