search

getdnsapi / getdns

A modern asynchronous DNS API https://getdnsapi.net/
Other
461 stars 127 forks source link

Truncated DNS/UDP responses when using getdns #495

Open amialkow opened 3 years ago

amialkow commented 3 years ago

Hi, I experienced issue with Amazon Smart Plug not being able to obtain server address after enabling stubby 0.3.0/getdns 1.6.0 on my OpenWrt 19.07.4. Later I found that same issue exist with OpenWRT nslookup (below). In case of use getdns legacy dns UDP packet size limit is exceeded because because of no support for compacting repeating names in answers. Below is my very basic implementation of compacting scheme working for my case, it use only query name as reference. Please let me know if there are other interesting cases, I'm not DNS expert by any mean. Notice also I blindly changed all wire formatting functions not really understanding purpose. I can spend little more time and provide proper patch. Regards, Andy

Similar issue (as far as I see different closure): https://github.com/getdnsapi/getdns/issues/430#issuecomment-700593315_

Query over stubby (truncated)

Server:     127.0.0.1
Address:    127.0.0.1#5453

*** Can't find a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com: No answer
*** Can't find a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com: No answer

Direct query:

Server:     1.1.1.1
Address:    1.1.1.1#53

Name:      a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com
Address 1: 54.152.172.109
Address 2: 52.20.87.96
Address 3: 52.7.87.83
Address 4: 54.156.167.119
Address 5: 52.3.195.129
Address 6: 34.202.122.173
Address 7: 3.221.54.248
Address 8: 34.233.73.54
Address 9: 2406:da00:ff00::22c2:f93
Address 10: 2406:da00:ff00::22ea:eb05
Address 11: 2406:da00:ff00::3417:5af2
Address 12: 2406:da00:ff00::3d1:ecd1
Address 13: 2406:da00:ff00::3e3:55e9
Address 14: 2406:da00:ff00::22c0:4eda
Address 15: 2406:da00:ff00::3436:5a5a
Address 16: 2406:da00:ff00::36a7:92bc

Binary dump of original server response, please notice C0 0C relative name references cause packet well under 512 byte limit.

0000   c0 56 27 10 27 18 00 01 5c 63 12 46 08 00 45 20   .V'.'...\c.F..E 
0010   00 d8 bc 51 40 00 38 11 c0 0d 01 01 01 01 47 c5   ...Q@.8.......G.
0020   7b cf 00 35 c3 52 00 c4 94 f3 65 4b 81 80 00 01   {..5.R....eK....
0030   00 08 00 00 00 00 0e 61 31 67 37 77 6d 78 77 6f   .......a1g7wmxwo
0040   6f 76 78 61 7a 03 69 6f 74 09 75 73 2d 65 61 73   ovxaz.iot.us-eas
0050   74 2d 31 09 61 6d 61 7a 6f 6e 61 77 73 03 63 6f   t-1.amazonaws.co
0060   6d 00 00 01 00 01 c0 0c 00 01 00 01 00 00 00 18   m...............
0070   00 04 34 cd ae a5 c0 0c 00 01 00 01 00 00 00 18   ..4.............
0080   00 04 34 16 67 0f c0 0c 00 01 00 01 00 00 00 18   ..4.g...........
0090   00 04 34 16 ce e9 c0 0c 00 01 00 01 00 00 00 18   ..4.............
00a0   00 04 34 04 9d a9 c0 0c 00 01 00 01 00 00 00 18   ..4.............
00b0   00 04 03 d7 c0 b3 c0 0c 00 01 00 01 00 00 00 18   ................
00c0   00 04 22 c6 2a a4 c0 0c 00 01 00 01 00 00 00 18   ..".*...........
00d0   00 04 03 d0 dd d3 c0 0c 00 01 00 01 00 00 00 18   ................
00e0   00 04 22 cd 72 3c                                 ..".r<

Please this is old version of the patch, I post newer version with more general approach down below.

diff --git a/src/convert.c b/src/convert.c
index 8dc44ddf..b027e530 100644
--- a/src/convert.c
+++ b/src/convert.c
@@ -257,7 +257,7 @@ getdns_rr_dict2wire_scan(

    gldns_buffer_init_vfixed_frm_data(&gbuf, *wire, *wire_sz);
-   if ((r = _getdns_rr_dict2wire(rr_dict, &gbuf)))
+   if ((r = _getdns_rr_dict2wire(rr_dict, &gbuf, NULL, 0)))
        return r;

    if (gldns_buffer_position(&gbuf) == 0)
@@ -407,14 +407,14 @@ getdns_rr_dict2str_scan(
        return GETDNS_RETURN_INVALID_PARAMETER;

    gldns_buffer_init_vfixed_frm_data(&gbuf, buf, sizeof(buf_spc));
-   r = _getdns_rr_dict2wire(rr_dict, &gbuf);
+   r = _getdns_rr_dict2wire(rr_dict, &gbuf, NULL, 0);
    if (gldns_buffer_position(&gbuf) > sizeof(buf_spc)) {
        if (!(buf = GETDNS_XMALLOC(
            rr_dict->mf, uint8_t, (sz = gldns_buffer_position(&gbuf))))) {
            return GETDNS_RETURN_MEMORY_ERROR;
        }
        gldns_buffer_init_frm_data(&gbuf, buf, sz);
-       r = _getdns_rr_dict2wire(rr_dict, &gbuf);
+       r = _getdns_rr_dict2wire(rr_dict, &gbuf, NULL, 0);
    }
    if (r) {
        if (buf != buf_spc)
@@ -805,7 +805,7 @@ _getdns_reply_dict2wire_hdr(
    if (!getdns_dict_get_list(reply, "additional", &section)) {
        for ( n = 0, i = 0
            ; !getdns_list_get_dict(section, i, &rr_dict); i++) {
-            if (!_getdns_rr_dict2wire(rr_dict, gbuf))
+           if (!_getdns_rr_dict2wire(rr_dict, gbuf, NULL, 0))
                 n++;
        }
        gldns_buffer_write_u16_at(gbuf, pkt_start+GLDNS_ARCOUNT_OFF, n);
@@ -823,6 +823,7 @@ _getdns_reply_dict2wire(
    getdns_list *section;
    getdns_dict *rr_dict;
    getdns_bindata *qname;
+   getdns_bindata *qname_ref = NULL;
    int remove_dnssec;

    pkt_start = gldns_buffer_position(buf);
@@ -853,6 +854,7 @@ _getdns_reply_dict2wire(
        !getdns_dict_get_int(reply, "/question/qtype", &qtype)) {
        (void)getdns_dict_get_int(reply, "/question/qclass", &qclass);
        gldns_buffer_write(buf, qname->data, qname->size);
+       qname_ref = qname;
        gldns_buffer_write_u16(buf, (uint16_t)qtype);
        gldns_buffer_write_u16(buf, (uint16_t)qclass);
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_QDCOUNT_OFF, 1);
@@ -875,7 +877,7 @@ _getdns_reply_dict2wire(
                !getdns_dict_get_int(rr_dict, "type", &rr_type) &&
                rr_type == GETDNS_RRTYPE_RRSIG)
                continue;
-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire(rr_dict, buf, qname_ref, GLDNS_HEADER_SIZE))
                 n++;
        }
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_ANCOUNT_OFF, n);
@@ -892,7 +894,7 @@ _getdns_reply_dict2wire(
                || rr_type == GETDNS_RRTYPE_DS
                ))
                continue;
-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire(rr_dict, buf, NULL, 0))
                 n++;
        }
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_NSCOUNT_OFF, n);
@@ -905,7 +907,7 @@ _getdns_reply_dict2wire(
                !getdns_dict_get_int(rr_dict, "type", &rr_type) &&
                rr_type == GETDNS_RRTYPE_RRSIG)
                continue;
-            if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire(rr_dict, buf, NULL, 0))
                 n++;
        }
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_ARCOUNT_OFF, n);
diff --git a/src/rr-dict.c b/src/rr-dict.c
index fc675830..da57cb35 100644
--- a/src/rr-dict.c
+++ b/src/rr-dict.c
@@ -1294,7 +1294,8 @@ write_rdata_field(gldns_buffer *buf, uint8_t *rdata_start,
 }

 getdns_return_t
-_getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
+_getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf,
+            getdns_bindata *qname, unsigned qname_offset)
 {
    getdns_return_t r = GETDNS_RETURN_GOOD;
    getdns_bindata root = { 1, (void *)"" };
@@ -1325,7 +1326,13 @@ _getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
        } else
            return r;
    }
-   gldns_buffer_write(buf, name->data, name->size);
+   if((qname != NULL) && (qname_offset < 0xc000) &&
+      (qname->size == name->size) &&
+      !memcmp(qname->data, name->data, qname->size))
+       gldns_buffer_write_u16(buf, (uint16_t)(0xc000 | qname_offset));
+   else
+       gldns_buffer_write(buf, name->data, name->size);
+
    gldns_buffer_write_u16(buf, (uint16_t)rr_type);

    (void) getdns_dict_get_int(rr_dict, "class", &rr_class);
diff --git a/src/rr-dict.h b/src/rr-dict.h
index e19e0387..6fd0ebc0 100644
--- a/src/rr-dict.h
+++ b/src/rr-dict.h
@@ -144,7 +144,7 @@ typedef struct _getdns_rr_def {
 const _getdns_rr_def *_getdns_rr_def_lookup(uint16_t rr_type);

 getdns_return_t _getdns_rr_dict2wire(
-    const getdns_dict *rr_dict, gldns_buffer *buf);
+    const getdns_dict *rr_dict, gldns_buffer *buf, getdns_bindata *qname, unsigned qname_offset);

 const char *_getdns_rr_type_name(int rr_type);

diff --git a/src/util-internal.c b/src/util-internal.c
index 7432191c..f210cae2 100644
--- a/src/util-internal.c
+++ b/src/util-internal.c
@@ -423,7 +423,7 @@ _getdns_rr_iter2rr_dict_canonical(
            return rr_dict;

        gldns_buffer_init_frm_data(&gbuf, data, owner_len+10+rdata_sz);
-       if (_getdns_rr_dict2wire(rr_dict, &gbuf) ||
+       if (_getdns_rr_dict2wire(rr_dict, &gbuf, NULL, 0) ||
            gldns_buffer_position(&gbuf) != owner_len + 10 + rdata_sz ||
            !(bindata = GETDNS_MALLOC(*mf, struct getdns_bindata))) {
            GETDNS_FREE(*mf, data);
@@ -1540,7 +1540,7 @@ static void _getdns_reply2wire_buf(gldns_buffer *buf, const getdns_dict *reply)
    uint16_t ancount, nscount;
    uint32_t qtype, qclass = GETDNS_RRCLASS_IN, rcode = GETDNS_RCODE_NOERROR;
    getdns_bindata *qname;
-
+   getdns_bindata *qname_ref = NULL;

    pkt_start = gldns_buffer_position(buf);
    /* Empty header */**
@@ -1551,8 +1551,8 @@ static void _getdns_reply2wire_buf(gldns_buffer *buf, const getdns_dict *reply)
    if (   !getdns_dict_get_dict(reply, "question", &q_dict)
        && !getdns_dict_get_int(q_dict, "qtype", &qtype)
        && !getdns_dict_get_bindata(q_dict, "qname", &qname)) {
-
        gldns_buffer_write(buf, qname->data, qname->size);
+       qname_ref = qname;
        gldns_buffer_write_u16(buf, (uint16_t)qtype);
        gldns_buffer_write_u16(buf, (uint16_t)qclass);
        gldns_buffer_write_u16_at(**
@@ -1569,7 +1569,7 @@ static void _getdns_reply2wire_buf(gldns_buffer *buf, const getdns_dict *reply)
            ; !getdns_list_get_dict(section, i, &rr_dict)
            ; i++ ) {

-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire(rr_dict, buf, qname_ref, GLDNS_HEADER_SIZE))
                ancount++;
        }
        gldns_buffer_write_u16_at(
@@ -1580,7 +1580,7 @@ static void _getdns_reply2wire_buf(gldns_buffer *buf, const getdns_dict *reply)
            ; !getdns_list_get_dict(section, i, &rr_dict)
            ; i++ ) {

-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire(rr_dict, buf, NULL, 0))
                nscount++;
        }
        gldns_buffer_write_u16_at(
@@ -1595,6 +1595,7 @@ static void _getdns_list2wire_buf(gldns_buffer *buf, const getdns_list *l)
    uint16_t ancount;
    uint32_t qtype, qclass = GETDNS_RRCLASS_IN;
    getdns_bindata *qname;
+   getdns_bindata *qname_ref = NULL;

    pkt_start = gldns_buffer_position(buf);
    /* Empty header */
@@ -1611,6 +1612,7 @@ static void _getdns_list2wire_buf(gldns_buffer *buf, const getdns_list *l)
            continue;
        (void) getdns_dict_get_int(rr_dict, "qclass", &qclass);
        gldns_buffer_write(buf, qname->data, qname->size);
+       qname_ref = qname;
        gldns_buffer_write_u16(buf, (uint16_t)qtype);
        gldns_buffer_write_u16(buf, (uint16_t)qclass);
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_QDCOUNT_OFF, 1);
@@ -1620,7 +1622,7 @@ static void _getdns_list2wire_buf(gldns_buffer *buf, const getdns_list *l)
        ; !getdns_list_get_dict(l, i, &rr_dict)
        ; i++ ) {

-       if (!_getdns_rr_dict2wire(rr_dict, buf))
+       if (!_getdns_rr_dict2wire(rr_dict, buf, qname_ref, GLDNS_HEADER_SIZE))
            ancount++;
    }
    gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_ANCOUNT_OFF, ancount);
jonathanunderwood commented 3 years ago

@amialkow the formatting here has seriously screwed up your patch making it practically unreadable. Can you place the patch into a code block please?

hanvinke commented 3 years ago

Hi. I am also using OpenWrt 19.07.6 with stubby/getdns/dnsmasq on my router. With dig (on Arch linux within bind package) I see no problems. I do not use nslookup because it is deprecated. Unfortunately i do not have an Amazon Smart Plug to test with.

ARCH-PC% dig a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com http://a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453

; <<>> DiG 9.16.13 <<>> a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453 ;; global options: +cmd ;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 11094 ;; flags: qr rd ra; QUERY: 1, ANSWER: 8, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags: do; udp: 1232 ;; QUESTION SECTION: ;a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. IN A

;; ANSWER SECTION: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 34.232.233.94 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.221.81.122 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.224.51.158 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 52.204.38.169 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 52.45.85.193 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.227.79.178 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 54.159.58.62 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.225.102.27

;; Query time: 323 msec ;; SERVER: 192.168.1.1#53(192.168.1.1) ;; WHEN: zo apr 04 20:11:02 CEST 2021 ;; MSG SIZE rcvd: 535

;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 61707 ;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 4, ADDITIONAL: 1

;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags: do; udp: 1232 ;; QUESTION SECTION: ;127.0.0.1#5453. IN A

;; AUTHORITY SECTION: . 86400 IN SOA a.root-servers.net. nstld.verisign-grs.com. 2021040401 1800 900 604800 86400 . 86400 IN RRSIG SOA 8 0 86400 20210417170000 20210404160000 14631 . am6Wwo2pbSVSyiFv3lckzMdYb2QTZdR1FZ43GsqLgrK+L7WlbFGNpi4P +pZ5BGdOiYOYeKk8GdzjqNb6NTE7Kf7Fsxxr7jz8zL+RMGJ3tVawRAKg 6hY5MIH9i/o0FI2wwUO7eMHUK96K36c9O2jBIdb5KcAtHXUZDctqUNxp khWFkFYbtni35lPjXXDj6cVLRKrUMe0tQmZIVyID6w0hVWOs8pnwYokx 1FrrOpsXj3g11nT+SaGdZSZxuoQBHHWORUgMUckFT7i7gjG8bP0egmiZ N8CQ8mpyn77Wpma+eNKpKqL3+LP5imAhx0hUVduMg4fi4120EZqCo0VY hdI5VQ== . 86400 IN NSEC aaa. NS SOA RRSIG NSEC DNSKEY . 86400 IN RRSIG NSEC 8 0 86400 20210417170000 20210404160000 14631 . ZyW+HnMRRV3Rq2aT87aZzOwpqVDlvUzEUh66EKsSavah/eJsvjQUa84b TdXMIBgmUQgoxhzDJHR+spL7TLTE9M4zFh/51i4Pdt5PbdegdgyBybTB X9JRCeJr3CDUBeTviBJq7qw3hFYqxZ+3vagjaVcoUZ5eCm62jw7Sp4Ya inLhDya04NbNZF0uWraBzVW6SMf5spWzpuuJcksUtIM9ADB7mOShAcjE RuHLGXQPgUdLSw/5wRvCgiR116rCci6oVCkHLiGsvoSTMvqK/bGtT8ao CMGFimi2OAO7IU2UpSJINkZQSQ6tKDIdwLKL1dlh5PgHk7iGRY0gnYNS W74Xkw==

;; Query time: 83 msec ;; SERVER: 192.168.1.1#53(192.168.1.1) ;; WHEN: zo apr 04 20:11:02 CEST 2021 ;; MSG SIZE rcvd: 715

Op zo 4 apr. 2021 om 13:20 schreef jonathanunderwood < @.***>:

@amialkow https://github.com/amialkow the formatting here has seriously screwed up your patch making it practically unreadable. Can you place the patch into a code block please?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/getdnsapi/getdns/issues/495#issuecomment-813016003, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGDNGSKPL7OIDM7BEREQAZ3THBDQBANCNFSM4T247BOA .

hanvinke commented 3 years ago

Directly from the router with nslookup a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com: ;; Truncated, retrying in TCP mode. Server: 127.0.0.1 Address: 127.0.0.1#53

Non-authoritative answer: Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 3.208.180.40 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 34.205.151.123 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 3.211.26.133 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 52.0.23.67 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 52.0.132.142 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 3.215.2.150 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 52.6.207.226 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 54.146.80.152 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::22c8:ae37 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::22c3:67e4 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::22c2:b471 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::3d1:5e12 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::3de:c532 Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::3437:920c Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::23ac:4caf Name: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com Address: 2406:da00:ff00::23ac:ae8b

Op zo 4 apr. 2021 om 20:22 schreef Han Vinke @.***>:

Hi. I am also using OpenWrt 19.07.6 with stubby/getdns/dnsmasq on my router. With dig (on Arch linux within bind package) I see no problems. I do not use nslookup because it is deprecated. Unfortunately i do not have an Amazon Smart Plug to test with.

ARCH-PC% dig a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com http://a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453

; <<>> DiG 9.16.13 <<>> a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453 ;; global options: +cmd ;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 11094 ;; flags: qr rd ra; QUERY: 1, ANSWER: 8, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags: do; udp: 1232 ;; QUESTION SECTION: ;a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. IN A

;; ANSWER SECTION: a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 34.232.233.94 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.221.81.122 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.224.51.158 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 52.204.38.169 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 52.45.85.193 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.227.79.178 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 54.159.58.62 a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com. 60 IN A 3.225.102.27

;; Query time: 323 msec ;; SERVER: 192.168.1.1#53(192.168.1.1) ;; WHEN: zo apr 04 20:11:02 CEST 2021 ;; MSG SIZE rcvd: 535

;; Got answer: ;; ->>HEADER<<- opcode: QUERY, status: NXDOMAIN, id: 61707 ;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 4, ADDITIONAL: 1

;; OPT PSEUDOSECTION: ; EDNS: version: 0, flags: do; udp: 1232 ;; QUESTION SECTION: ;127.0.0.1#5453. IN A

;; AUTHORITY SECTION: . 86400 IN SOA a.root-servers.net. nstld.verisign-grs.com. 2021040401 1800 900 604800 86400 . 86400 IN RRSIG SOA 8 0 86400 20210417170000 20210404160000 14631 . am6Wwo2pbSVSyiFv3lckzMdYb2QTZdR1FZ43GsqLgrK+L7WlbFGNpi4P +pZ5BGdOiYOYeKk8GdzjqNb6NTE7Kf7Fsxxr7jz8zL+RMGJ3tVawRAKg 6hY5MIH9i/o0FI2wwUO7eMHUK96K36c9O2jBIdb5KcAtHXUZDctqUNxp khWFkFYbtni35lPjXXDj6cVLRKrUMe0tQmZIVyID6w0hVWOs8pnwYokx 1FrrOpsXj3g11nT+SaGdZSZxuoQBHHWORUgMUckFT7i7gjG8bP0egmiZ N8CQ8mpyn77Wpma+eNKpKqL3+LP5imAhx0hUVduMg4fi4120EZqCo0VY hdI5VQ== . 86400 IN NSEC aaa. NS SOA RRSIG NSEC DNSKEY . 86400 IN RRSIG NSEC 8 0 86400 20210417170000 20210404160000 14631 . ZyW+HnMRRV3Rq2aT87aZzOwpqVDlvUzEUh66EKsSavah/eJsvjQUa84b TdXMIBgmUQgoxhzDJHR+spL7TLTE9M4zFh/51i4Pdt5PbdegdgyBybTB X9JRCeJr3CDUBeTviBJq7qw3hFYqxZ+3vagjaVcoUZ5eCm62jw7Sp4Ya inLhDya04NbNZF0uWraBzVW6SMf5spWzpuuJcksUtIM9ADB7mOShAcjE RuHLGXQPgUdLSw/5wRvCgiR116rCci6oVCkHLiGsvoSTMvqK/bGtT8ao CMGFimi2OAO7IU2UpSJINkZQSQ6tKDIdwLKL1dlh5PgHk7iGRY0gnYNS W74Xkw==

;; Query time: 83 msec ;; SERVER: 192.168.1.1#53(192.168.1.1) ;; WHEN: zo apr 04 20:11:02 CEST 2021 ;; MSG SIZE rcvd: 715

Op zo 4 apr. 2021 om 13:20 schreef jonathanunderwood < @.***>:

@amialkow https://github.com/amialkow the formatting here has seriously screwed up your patch making it practically unreadable. Can you place the patch into a code block please?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/getdnsapi/getdns/issues/495#issuecomment-813016003, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGDNGSKPL7OIDM7BEREQAZ3THBDQBANCNFSM4T247BOA .

hanvinke commented 3 years ago

When using dig directly on the router by installing bind-dig I get different results:

First with nslookup:

/etc/config$ nslookup a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453 nslookup: couldn't get address for '127.0.0.1#5453': not found /etc/config$ nslookup a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453 nslookup: couldn't get address for '127.0.0.1#5453': not found /etc/config$ dig a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453

This nslookup fails completely. It says the address does not exist. And of course it does.

Now with dig. Dig shows no tr flag : full_text.txt

It seems the problem is with using nslookup instead of dig. On the internet I found a link with a good explanation why nslookups might fail altogether: see https://archive.nanog.org/meetings/nanog52/presentations/Sunday/Sinatra-DNS-DNSSEC_tutorial-reallyfinal.pdf

Analysis of https://dnsviz.net/

a1g7wmxwoovxaz iot us-east-1 amazonaws com-2021-04-06-08_41_12-UTC

amialkow commented 3 years ago

Hi, @hanvinke this is same problem I was facing. @jonathanunderwood fixed formatting, please note above is old patch. Original patch works well but only if name in query is the same as in response, as in reported cases. I made new version that implements small name cache and is easier to merge.

Root of the issue is that getdns library unpacks responses from DNS server but does not implement name "compression" so resulting frames are too large for UDP and payload is removed. OpenWRT version of nslookup uses basic version UDP protocol, this is same as my Amazon plug.

Code below implements simple domain name cache. Each time when name is stored in destination packet it puts name pointer, and it's offset into cache entry. During subsequent calls cache lookup is performed and in case of hit relative pointer name is stored. I'm considering two more improvements:

diff -Nru a/src/convert.c b/src/convert.c
--- a/src/convert.c 2020-02-28 06:39:53.000000000 -0800
+++ b/src/convert.c 2021-04-10 21:13:36.532084800 -0700
@@ -754,6 +754,7 @@
    getdns_list *section;
    getdns_dict *rr_dict;
    getdns_bindata *qname;
+   name_cache_t name_cache = {0};
    int remove_dnssec;

    pkt_start = gldns_buffer_position(buf);
@@ -783,7 +784,7 @@
    if (!getdns_dict_get_bindata(reply, "/question/qname", &qname) &&
        !getdns_dict_get_int(reply, "/question/qtype", &qtype)) {
        (void)getdns_dict_get_int(reply, "/question/qclass", &qclass);
-       gldns_buffer_write(buf, qname->data, qname->size);
+       _getdns_rr_buffer_write_cached_name(buf, qname, &name_cache);
        gldns_buffer_write_u16(buf, (uint16_t)qtype);
        gldns_buffer_write_u16(buf, (uint16_t)qclass);
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_QDCOUNT_OFF, 1);
@@ -806,7 +807,7 @@
                !getdns_dict_get_int(rr_dict, "type", &rr_type) &&
                rr_type == GETDNS_RRTYPE_RRSIG)
                continue;
-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire_cache(rr_dict, buf, &name_cache))
                 n++;
        }
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_ANCOUNT_OFF, n);
diff -Nru a/src/rr-dict.c b/src/rr-dict.c
--- a/src/rr-dict.c 2020-02-28 06:39:53.000000000 -0800
+++ b/src/rr-dict.c 2021-04-10 21:17:45.682994949 -0700
@@ -1293,8 +1293,39 @@
    return r != GETDNS_RETURN_NO_SUCH_LIST_ITEM ? r : GETDNS_RETURN_GOOD;
 }

+void
+_getdns_rr_buffer_write_cached_name(gldns_buffer *buf, getdns_bindata *name, name_cache_t *name_cache)
+{
+   size_t name_size = name->size;
+   uint8_t *name_data = name->data;
+   if((NULL != name_cache) && (name_size > 2)) {
+       name_cache_entry_t *table_free, *entry_ptr;
+       table_free = entry_ptr = &name_cache->entry[name_cache->count];
+       name_cache_entry_t *table_start = &name_cache->entry[0];
+       // Search backward if name is already in cache
+       while(entry_ptr-- > table_start) {
+           if((entry_ptr->name->size == name_size) &&
+              !memcmp(entry_ptr->name->data, name_data, name_size)) {
+               gldns_buffer_write_u16(buf, (uint16_t)(0xc000 | entry_ptr->name_offset));
+               return;
+           }
+       }
+       unsigned name_offset = gldns_buffer_position(buf);
+       if ((name_cache->count < NAME_CACHE_ENTRIES) &&
+           (name_offset < 0xc000)) {
+           // Cache name
+           table_free->name = name;
+           table_free->name_offset = name_offset;
+           name_cache->count++;
+       }
+   }
+   gldns_buffer_write(buf, name_data, name_size);
+   return;
+}
+
 getdns_return_t
-_getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
+_getdns_rr_dict2wire_cache(const getdns_dict *rr_dict, gldns_buffer *buf,
+            name_cache_t *name_cache)
 {
    getdns_return_t r = GETDNS_RETURN_GOOD;
    getdns_bindata root = { 1, (void *)"" };
@@ -1325,7 +1356,7 @@
        } else
            return r;
    }
-   gldns_buffer_write(buf, name->data, name->size);
+   _getdns_rr_buffer_write_cached_name(buf, name, name_cache);
    gldns_buffer_write_u16(buf, (uint16_t)rr_type);

    (void) getdns_dict_get_int(rr_dict, "class", &rr_class);
diff -Nru a/src/rr-dict.h b/src/rr-dict.h
--- a/src/rr-dict.h 2020-02-28 06:39:53.000000000 -0800
+++ b/src/rr-dict.h 2021-04-10 21:13:36.532084800 -0700
@@ -143,8 +143,25 @@

 const _getdns_rr_def *_getdns_rr_def_lookup(uint16_t rr_type);

-getdns_return_t _getdns_rr_dict2wire(
-    const getdns_dict *rr_dict, gldns_buffer *buf);
+#define NAME_CACHE_ENTRIES 4
+
+typedef struct __name_cache_entry {
+   getdns_bindata *name;
+   unsigned name_offset;
+} name_cache_entry_t;
+
+typedef struct __name_cache {
+   unsigned count;
+   name_cache_entry_t entry[NAME_CACHE_ENTRIES];
+} name_cache_t;
+
+void _getdns_rr_buffer_write_cached_name(
+   gldns_buffer *buf, getdns_bindata *name, name_cache_t *name_cache);
+
+getdns_return_t _getdns_rr_dict2wire_cache(
+    const getdns_dict *rr_dict, gldns_buffer *buf, name_cache_t *name_cache);
+
+#define _getdns_rr_dict2wire(d, b)  _getdns_rr_dict2wire_cache((d),(b), NULL)

 const char *_getdns_rr_type_name(int rr_type);
amialkow commented 3 years ago

Hi, Patch below contains two improvements:

nslookup a1g7wmxwoovxaz.iot.us-east-1.amazonaws.com 127.0.0.1#5453
nslookup events.split.io 127.0.0.1#5453
nslookup cl5.apple.com 127.0.0.1#5453
diff --git a/src/convert.c b/src/convert.c
index 8dc44ddf..6dbd429a 100644
--- a/src/convert.c
+++ b/src/convert.c
@@ -823,6 +823,7 @@ _getdns_reply_dict2wire(
    getdns_list *section;
    getdns_dict *rr_dict;
    getdns_bindata *qname;
+   name_cache_t name_cache = {0};
    int remove_dnssec;

    pkt_start = gldns_buffer_position(buf);
@@ -852,7 +853,7 @@ _getdns_reply_dict2wire(
    if (!getdns_dict_get_bindata(reply, "/question/qname", &qname) &&
        !getdns_dict_get_int(reply, "/question/qtype", &qtype)) {
        (void)getdns_dict_get_int(reply, "/question/qclass", &qclass);
-       gldns_buffer_write(buf, qname->data, qname->size);
+       _getdns_rr_buffer_write_cached_name(buf, qname, &name_cache);
        gldns_buffer_write_u16(buf, (uint16_t)qtype);
        gldns_buffer_write_u16(buf, (uint16_t)qclass);
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_QDCOUNT_OFF, 1);
@@ -875,7 +876,7 @@ _getdns_reply_dict2wire(
                !getdns_dict_get_int(rr_dict, "type", &rr_type) &&
                rr_type == GETDNS_RRTYPE_RRSIG)
                continue;
-           if (!_getdns_rr_dict2wire(rr_dict, buf))
+           if (!_getdns_rr_dict2wire_cache(rr_dict, buf, &name_cache))
                 n++;
        }
        gldns_buffer_write_u16_at(buf, pkt_start+GLDNS_ANCOUNT_OFF, n);
diff --git a/src/rr-dict.c b/src/rr-dict.c
index fc675830..16813603 100644
--- a/src/rr-dict.c
+++ b/src/rr-dict.c
@@ -1293,8 +1293,39 @@ write_rdata_field(gldns_buffer *buf, uint8_t *rdata_start,
    return r != GETDNS_RETURN_NO_SUCH_LIST_ITEM ? r : GETDNS_RETURN_GOOD;
 }

+void
+_getdns_rr_buffer_write_cached_name(gldns_buffer *buf, getdns_bindata *name, name_cache_t *name_cache)
+{
+   size_t name_size = name->size;
+   uint8_t *name_data = name->data;
+   if((NULL != name_cache) && (name_size > 2)) {
+       unsigned count = name_cache->count;
+       name_cache_entry_t *entry_ptr = &name_cache->entry[(count < NAME_CACHE_ENTRIES)?count:NAME_CACHE_ENTRIES];
+       name_cache_entry_t *table_start = &name_cache->entry[0];
+       /* Search backward if name is already in cache */
+       while(entry_ptr-- > table_start) {
+           if((entry_ptr->name->size == name_size) &&
+              !memcmp(entry_ptr->name->data, name_data, name_size)) {
+               gldns_buffer_write_u16(buf, (uint16_t)(0xc000 | entry_ptr->name_offset));
+               return;
+           }
+       }
+       unsigned name_offset = gldns_buffer_position(buf);
+       if (name_offset < 0xc000) {
+           /* Cache name */
+           entry_ptr = &name_cache->entry[count % NAME_CACHE_ENTRIES];
+           entry_ptr->name = name;
+           entry_ptr->name_offset = name_offset;
+           name_cache->count = count + 1;
+       }
+   }
+   gldns_buffer_write(buf, name_data, name_size);
+   return;
+}
+
 getdns_return_t
-_getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
+_getdns_rr_dict2wire_cache(const getdns_dict *rr_dict, gldns_buffer *buf,
+            name_cache_t *name_cache)
 {
    getdns_return_t r = GETDNS_RETURN_GOOD;
    getdns_bindata root = { 1, (void *)"" };
@@ -1325,7 +1356,7 @@ _getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
        } else
            return r;
    }
-   gldns_buffer_write(buf, name->data, name->size);
+   _getdns_rr_buffer_write_cached_name(buf, name, name_cache);
    gldns_buffer_write_u16(buf, (uint16_t)rr_type);

    (void) getdns_dict_get_int(rr_dict, "class", &rr_class);
@@ -1378,41 +1409,50 @@ _getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
        gldns_buffer_skip(buf, 2);
        rdata_start = gldns_buffer_current(buf);

-       for ( rd_def = rr_def->rdata
-           , n_rdata_fields = rr_def->n_rdata_fields
-           ; n_rdata_fields ; n_rdata_fields-- , rd_def++ ) {
+       /* Special case CNAME payload */
+       if((rr_type == GETDNS_RRTYPE_CNAME) && (n_rdata_fields == 1) &&
+          (rd_def->type & GETDNS_RDF_BINDATA) && !(rd_def->type & GETDNS_RDF_REPEAT) &&
+          (GETDNS_RETURN_GOOD == (r = getdns_dict_get_bindata(rdata, rd_def->name, &rdata_raw)))) {
+       
+           _getdns_rr_buffer_write_cached_name(buf, rdata_raw, name_cache);
+       } else {

-           if (rd_def->type == GETDNS_RDF_REPEAT)
-               break;
+           for ( rd_def = rr_def->rdata
+               , n_rdata_fields = rr_def->n_rdata_fields
+               ; n_rdata_fields ; n_rdata_fields-- , rd_def++ ) {

-           if ((r = write_rdata_field(buf,
-               rdata_start, rd_def, rdata)))
-               break;
-       }
-       if (n_rdata_fields == 0 || r) { 
-           /* pass */;
+               if (rd_def->type == GETDNS_RDF_REPEAT)
+                   break;

-       } else if ((r = getdns_dict_get_list(
-           rdata, rd_def->name, &list))) {
-           /* pass */;
+               if ((r = write_rdata_field(buf,
+                   rdata_start, rd_def, rdata)))
+                   break;
+           }
+           if (n_rdata_fields == 0 || r) {
+               /* pass */;

-       } else for ( i = 0
-                  ; r == GETDNS_RETURN_GOOD
-                  ; i++) {
+           } else if ((r = getdns_dict_get_list(
+               rdata, rd_def->name, &list))) {
+               /* pass */;

-           if ((r = getdns_list_get_dict(list, i, &rdata))) {
-               if (r == GETDNS_RETURN_NO_SUCH_LIST_ITEM)
-                   r = GETDNS_RETURN_GOOD;
-               break;
-           }
-           for ( rep_rd_def = rd_def + 1
-               , rep_n_rdata_fields = n_rdata_fields - 1
-               ; rep_n_rdata_fields
-               ; rep_n_rdata_fields--, rep_rd_def++ ) {
+           } else for ( i = 0
+                      ; r == GETDNS_RETURN_GOOD
+                      ; i++) {

-               if ((r = write_rdata_field(buf,
-                   rdata_start, rep_rd_def, rdata)))
+               if ((r = getdns_list_get_dict(list, i, &rdata))) {
+                   if (r == GETDNS_RETURN_NO_SUCH_LIST_ITEM)
+                       r = GETDNS_RETURN_GOOD;
                    break;
+               }
+               for ( rep_rd_def = rd_def + 1
+                   , rep_n_rdata_fields = n_rdata_fields - 1
+                   ; rep_n_rdata_fields
+                   ; rep_n_rdata_fields--, rep_rd_def++ ) {
+
+                   if ((r = write_rdata_field(buf,
+                       rdata_start, rep_rd_def, rdata)))
+                       break;
+               }
            }
        }
        gldns_buffer_write_u16_at(buf, rdata_size_mark,
diff --git a/src/rr-dict.h b/src/rr-dict.h
index e19e0387..f0788021 100644
--- a/src/rr-dict.h
+++ b/src/rr-dict.h
@@ -143,8 +143,25 @@ typedef struct _getdns_rr_def {

 const _getdns_rr_def *_getdns_rr_def_lookup(uint16_t rr_type);

-getdns_return_t _getdns_rr_dict2wire(
-    const getdns_dict *rr_dict, gldns_buffer *buf);
+#define NAME_CACHE_ENTRIES 4
+
+typedef struct __name_cache_entry {
+   getdns_bindata *name;
+   unsigned name_offset;
+} name_cache_entry_t;
+
+typedef struct __name_cache {
+   unsigned count;
+   name_cache_entry_t entry[NAME_CACHE_ENTRIES];
+} name_cache_t;
+
+void _getdns_rr_buffer_write_cached_name(
+    gldns_buffer *buf, getdns_bindata *name, name_cache_t *name_cache);
+
+getdns_return_t _getdns_rr_dict2wire_cache(
+    const getdns_dict *rr_dict, gldns_buffer *buf, name_cache_t *name_cache);
+
+#define _getdns_rr_dict2wire(d, b)  _getdns_rr_dict2wire_cache((d),(b), NULL)

 const char *_getdns_rr_type_name(int rr_type);

Change in _getdns_rr_dict2wire with --ignore-all-space. Original code is put in else statement, diff just gets confused.

@@ -1378,6 +1409,14 @@ _getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
        gldns_buffer_skip(buf, 2);
        rdata_start = gldns_buffer_current(buf);

+       /* Special case CNAME payload */
+       if((rr_type == GETDNS_RRTYPE_CNAME) && (n_rdata_fields == 1) &&
+          (rd_def->type & GETDNS_RDF_BINDATA) && !(rd_def->type & GETDNS_RDF_REPEAT) &&
+          (GETDNS_RETURN_GOOD == (r = getdns_dict_get_bindata(rdata, rd_def->name, &rdata_raw)))) {
+       
+           _getdns_rr_buffer_write_cached_name(buf, rdata_raw, name_cache);
+       } else {
+
            for ( rd_def = rr_def->rdata
                , n_rdata_fields = rr_def->n_rdata_fields
                ; n_rdata_fields ; n_rdata_fields-- , rd_def++ ) {
@@ -1415,6 +1454,7 @@ _getdns_rr_dict2wire(const getdns_dict *rr_dict, gldns_buffer *buf)
                        break;
                }
            }
+       }
        gldns_buffer_write_u16_at(buf, rdata_size_mark,
            (uint16_t)(gldns_buffer_position(buf)-rdata_size_mark-2));
    }
jonathanunderwood commented 3 years ago

Looks like some great progress here - why not submit the patch as a pull request?

amialkow commented 3 years ago

@jonathanunderwood It looks tree is broken for long time, opened https://github.com/getdnsapi/getdns/issues/504. I have few fixes in totally wrong order stashed together in https://github.com/getdnsapi/getdns/pull/503. Failure of https://github.com/getdnsapi/getdns/issues/505 is triggered by my changes. Apparently this is not the end. Slowly trying to cleanup this.