Send AAAA records (over IPv4)

[fzs]

This is the second step towards IPv6 (#10), with the mdnsd sending AAAA records for interfaces that have a IPv6 address assigned.

It is currently broken (unfinished), because of how the current code creates A records. The A record is created unconditionally assuming that there will always be an IPv4 address. The address is set later in the record because it is not known at the time the record is created. This poses a problem when now a AAAA record is added the same way. Because now both records are created unconditionally without knowing an IPv4/6 address. But one cannot rely on the address being added later. Because maybe there is only a IPv4 address, or maybe there is only an IPv6 address.

What adds to the problem is how the message is put together. Resource records are added without the RDATA, and then the RDATA is copied in independently. This process has no idea if the RDATA is actually present and valid, which in the end leads to malformed mDNS packages that have resource records without the expected RDATA in there if an interface has only an IPv4 address but no IPv6 address assigned.

I see multiple approaches to tackle this, asking for comments.

One way would be to leave the creation of the AAAA record as it is, unconditionally in case we suddenly get an IPv6 address, and change the part how the message is put together. It needs to check if the record is valid before adding any resource record data to the message.

The second way would be to leave the message creation as is, relying on records being valid. Which means that the creation of the record needs to be changed. One option would be to change the order of reading the configuration and setting the IP addresses on mdns, and then only creating the record when an address is present. The other option would be to remove the creation of A and AAAA records from conf.c and move the logic into the mdnsd_set_address method. This would mean that when an address is set, the method is responsible to make sure that an updated A/AAAA record exists, either by updating it or by creating one. Should the address be invalid (i.e. 0), then an existing record needs to be removed.

One has to keep in mind that an interface can now get an address of a family assigned that it didn't have before, or vice versa. One cannot rely on an address always being there anymore.

[fzs]

@troglobit I am a bit surprised that the discover test doesn't fail. Well, I have to admit that I didn't try manually with mquery, I just ran mdnsd and traced traffic with Wireshark and saw the malformed packets.

[troglobit]

Yeah there's definite room for improvement across the board. I just whipped up a very basic first test 😅

[fzs]

Ya, it might have to do with how the test is set up. I may have to do some more manual testing. I also got buggy results on my PC with multiple interfaces.

Do you have any idea or comment on how to approach the problem with the incomplete records?

[troglobit]

You can add more treats if you like. I planned to use tshark to get pcap files for analysis. (tcpdump can't capture in non-privileged mode in an unshare)

I'll have to get back to you later today. Unfortunately don't have time during the workday to analyse the code changes in the PR.

[fzs]

Yeah, no worries. I just wasn't sure if you had seen the PR already. I'll have to do some more testing.

[troglobit]

Hi @fzs is this PR still in draft state, or do you want me to merge it?

[fzs]

HI, I was on vacation last week, so I didn't work on it any further. It is still in draft state, as it is not yet in a working state.

[troglobit]

OK, thanks for the update :-)

[fzs]

I ended up solving the problem with the A record creation in a rather quick and dirty way. It is not my preferred solution, but it works and moves this forward. The creation of an A and AAAA record in conf.c is now conditional and only done when an IP address is already set on the mdnsd. This means that the address(es) has to be set before conf_init is called.

Ideally, the mdnsd_set_address function would create a new record when none exists. The problem here is that creating a record requires information that the library doesn't have at this point, namely hostname and conflict callback for this record. So these would also have to be passed into the mdnsd_set_address method which does not look very clean. The current solution has the problem that implicit knowledge is needed, i.e. set an address or no A record will be created. Hopefully a better solution can be found in the future.

This PR leaves one issue open: deleting A/AAAA records when an IPv4/IPv6 address is removed from an interface. This needs to be done in a separate PR.

[troglobit]

lgtm, let's merge this! :)