search

richardschneider / net-mdns

Simple multicast DNS
MIT License
232 stars 81 forks source link

Responding to address queries #25

Open richardschneider opened 6 years ago

richardschneider commented 6 years ago

From https://tools.ietf.org/html/rfc6762#section-6.2

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). For example, if an interface has both an IPv6 link- local and an IPv6 routable address, both should be included in the response message so that queriers receive both and can make their own choice about which to use. This allows a querier that only has an IPv6 link-local address to connect to the link-local address, and a different querier that has an IPv6 routable address to connect to the IPv6 routable address instead.

TillAlex commented 5 years ago

Is there any reason why #26 has not been merged? Would be great if this issue would be fixed as it's anoying to have to check address validity on the client side.

richardschneider commented 5 years ago

@TillAlex thank you for your interest in this package.

Since writing this I have the following issues and must admit I've simply moved this into the two hard bucket. Perhaps as a consumer of the package you can shed some light.

TillAlex commented 5 years ago

I did not find anything about loop back addresses in the documents so I don't know what behavior is expected. For me it would make sense if only replies on the loopback interface contain the loopback address.

In theory this change should not break other packages as the filtered addresses can not be expected to be reachable anyway. The current behavior is not standard-compliant and should not be relied on. However, there can be packages relying on this behavior...

jbrestan commented 5 years ago

Hi @richardschneider, I've also encountered this issue and would like to help resolve it, if needed. What's currently missing from #26?

richardschneider commented 5 years ago

@jbrestan This needs testing on a machine with multiple interfaces. Do you have this? If so, I'll rebase on master and then you can give it try.

jbrestan commented 5 years ago

Yes, having multiple interfaces was how we found out we may have this issue. I can test it on that setup.

richardschneider commented 5 years ago

I've rebased the PR, so you can test the latest bits.

jbrestan commented 5 years ago

Thank you. I was finally able to test it on this branch, and while #26 definitely filters out some unreachable combinations, I'm still seeing weird answers being sent out. I'll add those as comments to the PR. I'll try to dig deeper to find why I'm seeing those, but you most likely know more about this topic, so I wanted to run those comments by you first.

TillAlex commented 3 years ago

I just merged #26 into master and tested it using multiple interfaces. Everythings works as expected.

It would be great if this PR could be merged. If you are unsure if this change breaks other projects you could make it configurable and deactivate it by default.

The behaviour without this fix is very anoying. I guess a lot of people using this library will sooner or later spend some hours to find this problem...