LLMNR has issues in dev/IPv6_integration

[evpopov]

Description

• When an LLMNR AAAA query comes in through a IPv4 end-point, the responding code in DNS_ParseDNSReply() will include xEndPoint.ipv6_settings.xIPAddress in the reply. This being an IPv4 end-point though, means that the IPv6 settings are inaccurate. This is a weird one.... xApplicationDNSQueryHook_Multi() simply said: "Yes, I have this hostname" but it had no information on the type of query ( A or AAAA ) and it cannot control which resulting end-point gets used for generating an answer. I don't know the best approach here, so I'm reporting the issue and hope that smarter heads will chime in :-)
• A similar to the above issue exists with multiple IP addresses. Consider the very simple scenario from the attached Wireshark capture. My PC has a few IP addresses so does the device that I'm trying to ping. The FreeRTOS+TCP device responds to one of the LLMNR requests but the response only has one answer. My PC has no idea of the other IP addresses that the device has and therefor no communication can be established. I may be wrong here, but I believe a response to an "A" query should contain all IPv4 endpoints and a response to an "AAAA" query should contain all IPv6 endpoints.
• The code in DNS_ParseDNSReply() uses pxDuplicateNetworkBufferWithDescriptor and I believe it should use pxResizeNetworkBufferWithDescriptor. I believe @htibosch fixed a similar issue just a few days ago in DNS_TreatNBNS() sha:afd6ffc4252f0fca7e0d3dc91e2ca92a5878a91c

LLMNR_SingleAnswer.zip

[htibosch]

About xApplicationDNSQueryHook_Multi(pxEndPoint, pcName) :

The endpoint parameter tells you what type of name is looked for: pxEndPoint->usDNSType, which has the value dnsTYPE_A_HOST or dnsTYPE_AAAA_HOST. The member usDNSType should be treated as a const value.
The LLMNR packet is received on an endpoint, and if there is a name match, the answer will be sent through the same endpoint.
I tested all 4 combinations: receive LLMNR on an IPv4 or on an IPv6 endpoint, while treating A and AAAA records. I created LLMNR names like "xxx4" and "xxx6", so I could test like this:

    ping -4 xxx4
    ping -6 xxx4
    ping -4 xxx6
    ping -6 xxx6

My application hook xApplicationDNSQueryHook_Multi() calls a function setEndPoint(), it fills in one of these fields:

    pxEndPoint->ipv4_settings.ulIPAddress  // when usDNSType == dnsTYPE_A_HOST
    pxEndPoint->ipv6_settings.xIPAddress   // when usDNSType == dnsTYPE_AAAA_HOST

You are right that this method does not allow to reply multiple addresses.

Often we, developers, are looking for a balance between a luxurious user interface that can do everything, and an implementation that is simple and has a small memory footprint. When designing xApplicationDNSQueryHook(), I chose for simplicity and a small footprint.

Isn't it possible that you give different names to different endpoints? "mydevice.1" up to "mydevice.4"? Or can you give rotating answers on repeated LLMNR/mDNS requests?

The code in DNS_ParseDNSReply() uses pxDuplicateNetworkBufferWithDescriptor and I believe it should use pxResizeNetworkBufferWithDescriptor

It can be replaced with pxResizeNetworkBufferWithDescriptor(), but it is correct as it is now. The duplicate of the network buffer is large enough, and I don't see any memory or network buffer leaks.
In DNS_TreatNBNS() there was a different problem.

[evpopov]

Thanks for the elaborate answer @htibosch
Wow, I would have never even thought of looking for a field like usDNSType in the end-point.
I also find it strange that the TCP stack expects the application hook to fill-in an IP address for the response. I thought long and hard over the weekend and I believe the TCP stack has or should have all the correct answers already and does not need to ask
the application to do extra work to figure out the correct answer. Seems like wasted CPU. Let me try and give an example and I'll use IPv4 because IPv6 still feels a bit foreign to me.
Imagine 2 devices, both with multiple end-points:

Device-A:
end-point-1: 192.168.0.123/24
end-point-2: 10.10.1.123/24
end-point-3: 10.10.2.123/24

Device-B:
end-point-1: 10.10.7.99/24
end-point-2: 192.168.0.99/24
end-point-3: 172.17.17.17/16

Device-A wants to talk to Device-B and sends an LLMNR "A" query for "Device-B"
In my experience with Windows, the source address of an LLMNR query is not very predictable when multiple addresses are present, so let's assume Device-A picks it's end-point-1 as the source of the query:
192.168.0.123 -> 224.0.0.252 : query A "Device-B"
When Device-B receives that frame, we hit our first snag. pxEasyFit() does not make use of the source address and it also doesn't work with multiple IP version matches and no exact IP match so it returns NULL. I will ignore that for the sake of this discussion and will assume that Device-B assigns its end-point-2 to the incoming LLMNR frame.
The points that I'm trying to make with this example are as follows:

• The receiving host has no foolproof way of knowing which of it's end-points' address will result in Device-A being able to connect to it. The reason is that Device-B has no knowledge of Device-A's network mask for the end-point Device-A used when sending the query. Device-B also has no knowledge of other end-points that Device-A may have. In my mind, Device-B has only two viable options:
  • Ideally, Device-A sends a list of all it's IPv4 addresses and lets Device-A figure out if a direct connection is possible. With this option, the TCP stack has the entire correct answer. All it needs from the application is a true/false answer to the question "Is Device-B our name?" The situation would be made better if the stack sending the LLMNR query sends one query per source IP that it has and different OS-es/stacks may act differently... I only have Windows experience with this case.
  • A simpler although not perfect option is for Device-B to include a single "best guess" answer in the query response. In this example, this would be it's end-point 2 based on source address match of the original frame. In this case again, the TCP stack has the correct answer. It simply should respond with the address of the end-point that was assigned to the LLMNR query frame. No searching, the end-point is already linked to the frame. This of course assumes that pxEasyFit() is modified to find the closest matching end-point. All the TCP stack needs from the application is a true/false answer to the question "Is Device-B our name?"
  • I'm purposefully ignoring the option of a rotating answer because that as far as I can see Windows only sends 1 query and imagine what would happen if in the future, OS-es decide to cache results and not resend queries for a few minutes. Let alone the complication of multiple devices trying to contact Device-B and Device-B rotating it's answers. It has no idea who's asking, so it just rotates..... again, I personally don't think this is a solution at all.
• I may be missing it, but I don't see a way for xApplicationDNSQueryHook_Multi() to know the source IP of the query, therefor it cannot make a good decision on what the IP in the answer should be.
• I also disagree that a hostname should be based on IP version. One could make the point that it is useful to have a different hostname for every interface, but per end-point? Is there really a need for this or is it just a case of "because we can"?

To summarize all the above, I believe the application hook should NOT be responsible for the IP in the answer, it should only provide a true/false result based on the hostname being queried and maybe the interface. As a side effect, the struct xNetworkEndPoint * pxEndPoint parameter could be made const and there wouldn't be a need to copy the end-point before calling xApplicationDNSQueryHook_Multi(). Perhaps even change the callback to only get a pointer to the interface and name.... both const. It's early enough in the release cycle.

I'm painfully aware of the compromises that developers have to make all the time. I respect your decision greatly @htibosch , but I just don't agree with it in this instance. I believe it is of great importance that the stack include all of it's IPs ( per version ) in the response because this is the only true correct thing to do. I also believe the stack shouldn't tempt the application designed to overly complicate things and "claim" different hostnames per IP type and end-point.

p.s. I am super excited about dev/IPv6_integration. Thanks for all the great work that everyone has put in. The above is my effort to improve the stack, not to criticize it.

p.p.s Yes, I understand that assigning an end-point to an incoming frame is the responsibility of whoever is writing the network driver and they are not required to use the combination of FreeRTOS_MatchingEndpoint() and consequently pxEasyFit(). Those two functions however are only used by the network driver, so I think they were meant as a "primer" on how an end-point should be matched to an incoming frame. I understand that I have the freedom to define my own "better" ;-) version of those functions.

[evpopov]

I may be able to help with that someday once I'm done with the multicast PR.
p.s. As a note for future me and/or others: RFC6762 section 6.2 discusses what records should/must be included in mDNS responses.