search

troglobit / mdnsd

Jeremie Miller's original mdnsd
BSD 3-Clause "New" or "Revised" License
56 stars 35 forks source link

mquery sends malformed packets when known-answer suppression kicks in #79

Open evverx opened 1 month ago

evverx commented 1 month ago

I noticed log messages like

Received mdns UDP packet of size 128, ifindex=13, ttl=1, fragsize=0, sender=10.88.0.4, destination=224.0.0.251
Got mDNS query packet for id 0
Failed to extract resource records from incoming packet: Bad message
mDNS query processing failed: Bad message
server.c: Packet too short or invalid while reading known answer record. (Maybe a UTF-8 problem?)
server.c: Packet too short or invalid while reading known answer record. (Maybe a UTF-8 problem?)

on various machines when I experimented with mdnsd in https://github.com/avahi/avahi/issues/614.

mquery shows

mquery -linfo
mc_socket(): Bound to *:5353 on iface eth0
Querying for _services._dns-sd._udp.local. type 12 ... press Ctrl-C to stop
mdnsd_in(): Query for _services._dns-sd._udp.local. of type 12 ...
mdnsd_in(): Got Answer: Name: _services._dns-sd._udp.local., Type: 12
+ _qotd._tcp.local. (10.88.0.1)
mdnsd_in(): Query for _qotd._tcp.local. of type 12 ...
mdnsd_in(): Got Answer: Name: _qotd._tcp.local., Type: 12
+ H._qotd._tcp.local. (10.88.0.1)
mdnsd_out(): Add known answer: Name: _qotd._tcp.local., Type: 12
mdnsd_out(): Add known answer: Name: _services._dns-sd._udp.local., Type: 12
^C

Below are the packets it sent and received:

IP 10.88.0.4.mdns > mdns.mcast.net.mdns: 0 PTR (QM)? _services._dns-sd._udp.local. (46)
IP 10.88.0.1.mdns > mdns.mcast.net.mdns: 0*- [0q] 1/0/0 PTR _qotd._tcp.local. (65)
IP 10.88.0.4.mdns > mdns.mcast.net.mdns: 0 PTR (QM)? _qotd._tcp.local. (34)
IP 10.88.0.1.mdns > mdns.mcast.net.mdns: 0*- [0q] 1/0/0 PTR H._qotd._tcp.local. (44)
IP 10.88.0.4.mdns > mdns.mcast.net.mdns: 0 [2a] [2q] PTR (QM)? _qotd._tcp.local. PTR (QM)? _services._dns-sd._udp.local. (128)
IP 10.88.0.4.mdns > mdns.mcast.net.mdns: 0 [2a] [2q] PTR (QM)? _qotd._tcp.local. PTR (QM)? _services._dns-sd._udp.local. (128)

The queries where the known answers were added are malformed:

    Queries
        _qotd._tcp.local: type PTR, class IN, "QM" question
            Name: _qotd._tcp.local
            [Name Length: 16]
            [Label Count: 3]
            Type: PTR (12) (domain name PoinTeR)
            .000 0000 0000 0001 = Class: IN (0x0001)
            0... .... .... .... = "QU" question: False
        _services._dns-sd._udp.local: type PTR, class IN, "QM" question
            Name: _services._dns-sd._udp.local
            [Name Length: 28]
            [Label Count: 4]
            Type: PTR (12) (domain name PoinTeR)
            .000 0000 0000 0001 = Class: IN (0x0001)
            0... .... .... .... = "QU" question: False
    Answers
        _qotd._tcp.local: type PTR, class IN, <Unknown extended label>
            Name: _qotd._tcp.local
            Type: PTR (12) (domain name PoinTeR)
            .000 0000 0000 0001 = Class: IN (0x0001)
            0... .... .... .... = Cache flush: False
            Time to live: 67 (1 minute, 7 seconds)
            Data length: 4
            Domain Name: <Unknown extended label>
[Malformed Packet: mDNS]
    [Expert Info (Error/Malformed): Malformed Packet (Exception occurred)]
        [Malformed Packet (Exception occurred)]
        [Severity level: Error]
        [Group: Malformed]
>>> hexdump(pkts[4])
0000  01 00 5E 00 00 FB 06 E1 80 48 42 1A 08 00 45 00  ..^......HB...E.
0010  00 9C BA 62 40 00 01 11 D3 97 0A 58 00 04 E0 00  ...b@......X....
0020  00 FB 14 E9 14 E9 00 88 EB F0 00 00 00 00 00 02  ................
0030  00 02 00 00 00 00 05 5F 71 6F 74 64 04 5F 74 63  ......._qotd._tc
0040  70 05 6C 6F 63 61 6C 00 00 0C 00 01 09 5F 73 65  p.local......_se
0050  72 76 69 63 65 73 07 5F 64 6E 73 2D 73 64 04 5F  rvices._dns-sd._
0060  75 64 70 C0 17 00 0C 00 01 C0 0C 00 0C 00 01 00  udp.............
0070  00 00 43 00 04 0A 58 00 01 00 04 01 48 C0 0C 09  ..C...X.....H...
0080  5F 73 65 72 76 69 63 65 73 07 5F 64 6E 73 2D 73  _services._dns-s
0090  64 04 5F 75 64 70 C0 17 00 0C 00 01 00 00 00 43  d._udp.........C
00a0  00 04 0A 58 00 01 00 02 C0 0C                    ...X......

It's reproducible with 5346604fcdeccd99f9943ac5ef887c3880480338.

MonkeyBreaker commented 1 month ago

Hey @evverx ,

I also encountered that one, I did not yet do the PR, because I was trying to track what was the issue in on the ticket I created in avahi repository.

About this issue, what I found out, is that it seems the A record is always appended here, when creating an answer.

Right now, what I am testing is the following patch:

/* Copy the data bits only */
static void _a_copy(struct message *m, mdns_answer_t *a)
{
    if (a->rdata) {
        message_rdata_raw(m, a->rdata, a->rdlen);
        return;
    }

-   if (a->type == QTYPE_A && a->ip.s_addr)
+       if (a->ip.s_addr)
        message_rdata_ipv4(m, a->ip);
-   else if (!IN6_IS_ADDR_UNSPECIFIED(&(a->ip6)))
+       else if (a->type == QTYPE_AAAA && && !IN6_IS_ADDR_UNSPECIFIED(&(a->ip6)))
        message_rdata_ipv6(m, a->ip6);
    if (a->type == QTYPE_SRV)
        message_rdata_srv(m, a->srv.priority, a->srv.weight, a->srv.port, a->rdname);
    else if (a->rdname)
        message_rdata_name(m, a->rdname);
}

Right now, it seems to have fixed the issues you also encountered on your side. I am not yet done with the testing, therefore cannot say if this is a final fix. If you have some time and can test this, would help getting confidence here.

Best, Julián

evverx commented 1 month ago

@MonkeyBreaker I'm not familiar with the codebase but I applied the following patch (based on your diff with the second && removed)

diff --git a/libmdnsd/mdnsd.c b/libmdnsd/mdnsd.c
index 8f2d61a..4047a76 100644
--- a/libmdnsd/mdnsd.c
+++ b/libmdnsd/mdnsd.c
@@ -619,9 +619,9 @@ static void _a_copy(struct message *m, mdns_answer_t *a)
                return;
        }

-       if (a->ip.s_addr)
+       if (a->type == QTYPE_A && a->ip.s_addr)
                message_rdata_ipv4(m, a->ip);
-       else if (!IN6_IS_ADDR_UNSPECIFIED(&(a->ip6)))
+       else if (a->type == QTYPE_AAAA && !IN6_IS_ADDR_UNSPECIFIED(&(a->ip6)))
                message_rdata_ipv6(m, a->ip6);
        if (a->type == QTYPE_SRV)
                message_rdata_srv(m, a->srv.priority, a->srv.weight, a->srv.port, a->rdname);

and I can't see the malformed packets any more.

(Just to clarify I noticed it accidentally. I was just experimenting with mdnsd to figure out if it can be integrated into the avahi testsuite since it allows sending things like those one-label CNAMEs that can cover code paths that aren't covered currently there. With mdnsd those test should be shorter than with, say, scapy).