dhcpv6: Properly separate domain names in search list

To prepare the DHCPv6 domain search list option, we go over the
flattened list of domains, and replace both dots and zero bytes with a
counter of bytes in the next label, implementing the encoding
specified by section 3.1 of RFC 1035.

If there are multiple domains in the list, however, zero bytes serve
as markers for the end of a domain name, and we'll replace them with
the length of the first label of the next domain, plus one. This is
wrong. We should only convert the dots before the labels.

To distinguish between label separators and domain names separators,
for simplicity, introduce a dot before the first label of every
domain we copy to form the list. All dots are then replaced by label
lengths, and separators (zero bytes) remain as they are.

As we do this, we need to make sure we don't replace the trailing
dot, if present: that's already a separator. Skip copying it, and
just add separators as needed.

Now that we don't copy those, though, we might end up with
zero-length domains: skip them, as they're meaningless anyway.

And as we might skip domains, we can't use the index 'i' to check if
we're at the beginning of the option -- use 'srch' instead.

This is very similar to how we prepare the list for NDP option 31,
except that we don't need padding (RFC 8106, 5.2) here, and we should
refactor this into common functions, but it probably makes sense to
rework the NDP responder (https://bugs.passt.top/show_bug.cgi?id=21)
first.

Reported-by: Sebastian Mitterle <smitterl@redhat.com>
Link: https://bugs.passt.top/show_bug.cgi?id=75
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

1 files changed, 17 insertions, 7 deletions

diff --git a/dhcpv6.c b/dhcpv6.c
index fc42a84..58171bb 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -376,24 +376,34 @@ search:
 		return offset;
 
 	for (i = 0; *c->dns_search[i].n; i++) {
-		if (!i) {
+		size_t name_len = strlen(c->dns_search[i].n);
+
+		/* We already append separators, don't duplicate if present */
+		if (c->dns_search[i].n[name_len - 1] == '.')
+			name_len--;
+
+		/* Skip root-only search domains */
+		if (!name_len)
+			continue;
+
+		if (!srch) {
 			srch = (struct opt_dns_search *)(buf + offset);
 			offset += sizeof(struct opt_hdr);
 			srch->hdr.t = OPT_DNS_SEARCH;
 			srch->hdr.l = 0;
 			p = srch->list;
-			*p = 0;
 		}
 
-		p = stpcpy(p + 1, c->dns_search[i].n);
-		*(p++) = 0;
-		srch->hdr.l += strlen(c->dns_search[i].n) + 2;
-		offset += strlen(c->dns_search[i].n) + 2;
+		*p = '.';
+		p = stpncpy(p + 1, c->dns_search[i].n, name_len);
+		p++;
+		srch->hdr.l += name_len + 2;
+		offset += name_len + 2;
 	}
 
 	if (srch) {
 		for (i = 0; i < srch->hdr.l; i++) {
-			if (srch->list[i] == '.' || !srch->list[i]) {
+			if (srch->list[i] == '.') {
 				srch->list[i] = strcspn(srch->list + i + 1,
 							".");
 			}