Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Truncated DNS/UDP responses when using getdns #495

Open
amialkow opened this issue Nov 19, 2020 · 8 comments
Open

Truncated DNS/UDP responses when using getdns #495

amialkow opened this issue Nov 19, 2020 · 8 comments

Comments

@amialkow
Copy link
Contributor

amialkow commented Nov 19, 2020 •
edited
Loading

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):
#430 (comment)_

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   [email protected].
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
Copy link

@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
Copy link

hanvinke commented Apr 4, 2021 via email

@hanvinke
Copy link

hanvinke commented Apr 4, 2021 via email

@hanvinke
Copy link

hanvinke commented Apr 5, 2021 via email •
edited
Loading

@amialkow
Copy link
Contributor Author

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:

  • Implementing circular buffer to handle overflows. As far as I see references are to previous name, this is simple change.
  • Use cache for CNAMEs. Not sure if this is allowed, need to check. There is also a little bit complexity because CNAME copying is buried in generic payload copy routines.
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);

@hanvinke hanvinke mentioned this issue Apr 20, 2021
Open
@amialkow
Copy link
Contributor Author

amialkow commented May 3, 2021

Hi,
Patch below contains two improvements:

  • Cache is now circular, it matches against last 4 unique names put in the packet.
  • Includes CNAME names in compression/reuse. This may solve teams
    Diff not really likes fact I increased indentation for one block in _getdns_rr_dict2wire, for better readability I put this section w/o white space changes below.
    I'm testing modified library on OpenWRT 19.07.7, three cases I was using for development are below. First case is original issue I encountered, second names in answer are different than in query and third is using CNAMEs that are later used in answers.
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
Copy link

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

@amialkow
Copy link
Contributor Author

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

@amialkow amialkow mentioned this issue May 15, 2021
Closed
@amialkow amialkow mentioned this issue May 26, 2021
Merged
@jamesmacwhite jamesmacwhite mentioned this issue Aug 13, 2021
Closed
@saradickinson saradickinson mentioned this issue Apr 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jonathanunderwood @hanvinke @amialkow