refactoring: read_name and name compression - #492
Conversation
|
Hi, I did a quick test rebuilding netmuxd against this branch, and it seems the issue is still present:
I'll make a proper and clean tcpdump and report back with it! :) |
from your log, it seems that the test was not using this PR branch? The following log line no longer exists in this branch:
Just a note, you can update the dependency using the following :
|
|
Oops, sorry, thanks for pointing out. I had updated the Cargo.toml but forgot to run a cargo update prior to build. Upon rebuild, these are the typical log lines that I get: Thanks! |
|
Thanks for rebuilding with the PR branch. I decoded the packet from the Bad name dump: Line 43..61 is clean (c0 0c is a valid backward pointer). The failure is that a later name contains a pointer to offset 80, which is in the middle of the "Denon" label at the last line — a pointer that doesn't land on a label boundary. The 169 at offset 125 / 150 at offset 148 errors are the other kind: pointers that point forward, which RFC 1035 section 4.1.4 forbids ("a pointer to a prior occurrence"). Note those offsets are identical to what the old build reported (169 ... 125), so it's the same reproducible packet. I suspect it might be the reflector that re-constructs a new packet that coalesces records, and might not enforce RFC's backward only pointers. To find out more, I have updated the PR diff to log the full packet when this error happens. Could you please do And, If it's easy, a raw capture on the IoT VLAN as a cross-check, using Thanks again for the persistence here; the captures have been really useful. |
|
Hey! Many thanks for looking into it! I have captured the following errors: I missed the tcpdump for these, but I have the following one with tcpdump attached: and tcpdump: |
|
@Phyks thanks for your test data! It's very helpful. It shows the NSEC records in the packet was malformed (bad compression pointer). To handle such packets, I've updated the diff to skip malformed records while still keep the rest of the packet decoded. Can you help try out the updated diff and see if that works for you? Thanks! |
|
Thanks a lot! I've successfully rebuilt and deployed. The issue was happening no matter the actual use case (netmuxd, iPhone backup) as it was caused by the relay of mdns packets. I can notice now that a lot of packets are dropped, typically: It's slightly unclear to me whether ALL packets are dropped (therefore, not really enabling mdns features) or whether only a fraction of them are blocked. Also, I won't be able to test the real use case involving netmuxd and an iPhone before next week. I'll report back ASAP. Also, another question, if this is stemming from avahi not conforming to the RFC, would it make sense to report it somewhere upstream? (I must confess I know very little about the mdns RFC and specs, and am quite surprised that Avahi would not honor it) |
Not whole packets. The drop (skip) is per record (only NSEC record in this case). Other records (PTR/SRV/A/AAAA records) in the same packet should be processed and it shouldn't stop the mDNS from working. Let me know if that works in a real use case when you get chance. The main fix is making mdns-sd resilient to malformed records, and not to drop whole packets.
I don't think it's a RFC conforming issue. As NSEC is not very commonly used in mDNS, I'm not surprised if there is a bug in handling (copying?) NSEC RDATA that has name compression, especially acting as a repeater. I think reporting to avahi is reasonable, and to make it airtight, the definitive evidence is a before/after capture: the same device's NSEC announcement on its own VLAN (these devices are on 10.42.2.x, e.g. the Dreame at 10.42.2.50) with a valid next-domain pointer, vs. the reflected copy on the mdns-sd VLAN (10.42.4.x) with the broken one. That proves avahi mangles it rather than the device shipping it broken. Without that, avahi could reasonably say "the device sent it that way." |
|
Hi, So I got a chance to test with real devices and it is not working so far. I'll make a proper tcpdump capture to further audit it. I get this kind of errors: Meanwhile, is there an easy way to have the positive dump of devices, that is the list of devices properly parsed by mdns-sd? The log in debug level would only show the negative one (ie devices that have incorrect fields and resulting in errors). Thanks! |
2 participants
Re-implemented
read_namethat hopefully help resolve this comment: #468 (comment)