Merge branch 'nullbyte_lpe' into 'master'

Add check for \0 bytes in QNAME labels

See merge request knot/knot-dns!1726

15 files changed, 127 insertions, 42 deletions

diff --git a/distro/pkg/deb/libknot15.symbols b/distro/pkg/deb/libknot15.symbols
index cb7732929..682df260c 100644
--- a/distro/pkg/deb/libknot15.symbols
+++ b/distro/pkg/deb/libknot15.symbols
@@ -53,6 +53,7 @@ libknot.so.15 libknot15 #MINVER#
  knot_dname_to_wire@Base 3.4.0
  knot_dname_unpack@Base 3.4.0
  knot_dname_wire_check@Base 3.4.0
+ knot_dname_with_null@Base 3.4.3
  knot_dnssec_alg_names@Base 3.4.0
  knot_edns_add_option@Base 3.4.0
  knot_edns_alignment_size@Base 3.4.0
diff --git a/src/knot/nameserver/internet.c b/src/knot/nameserver/internet.c
index 034fd268c..949642050 100644
--- a/src/knot/nameserver/internet.c
+++ b/src/knot/nameserver/internet.c
@@ -461,7 +461,7 @@ static knotd_in_state_t solve_name(knotd_in_state_t state, knot_pkt_t *pkt,
 {
 	int ret = zone_contents_find_dname(qdata->extra->contents, qdata->name,
 	                                   &qdata->extra->node, &qdata->extra->encloser,
-	                                   &qdata->extra->previous);
+	                                   &qdata->extra->previous, qdata->query->flags & KNOT_PF_NULLBYTE);
 
 	switch (ret) {
 	case ZONE_NAME_FOUND:
diff --git a/src/knot/nameserver/nsec_proofs.c b/src/knot/nameserver/nsec_proofs.c
index b67566c8b..9e7494645 100644
--- a/src/knot/nameserver/nsec_proofs.c
+++ b/src/knot/nameserver/nsec_proofs.c
@@ -118,8 +118,9 @@ static const knot_dname_t *get_next_closer(const knot_dname_t *closest_encloser,
                                            const knot_dname_t *name)
 {
 	// make name only one label longer than closest_encloser
-	size_t ce_labels = knot_dname_labels(closest_encloser, NULL);
-	size_t qname_labels = knot_dname_labels(name, NULL);
+	ssize_t ce_labels = knot_dname_labels(closest_encloser, NULL);
+	ssize_t qname_labels = knot_dname_labels(name, NULL);
+	assert(qname_labels > ce_labels);
 	for (int i = 0; i < (qname_labels - ce_labels - 1); ++i) {
 		name = knot_dname_next_label(name);
 	}
@@ -189,7 +190,8 @@ static int put_covering_nsec(const zone_contents_t *zone,
 
 	const zone_node_t *proof = NULL;
 
-	int ret = zone_contents_find_dname(zone, name, &match, &closest, &prev);
+	int ret = zone_contents_find_dname(zone, name, &match, &closest, &prev,
+	                                   qdata->query->flags & KNOT_PF_NULLBYTE);
 	if (ret == ZONE_NAME_FOUND) {
 		proof = match;
 	} else if (ret == ZONE_NAME_NOT_FOUND) {
diff --git a/src/knot/zone/contents.c b/src/knot/zone/contents.c
index 262136b17..8a32cc87e 100644
--- a/src/knot/zone/contents.c
+++ b/src/knot/zone/contents.c
@@ -276,11 +276,28 @@ zone_node_t *zone_contents_find_node_for_rr(zone_contents_t *contents, const kno
 	               get_node(contents, rrset->owner);
 }
 
+static bool null_byte_mismatch(const zone_node_t *node, const knot_dname_t *name,
+                               bool name_nullbyte)
+{
+	/* WARNING: for the sake of efficiency, Knot does not handle \0 byte
+	 * in qname correctly, which can lead to disasters here and there.
+	 * The following check should cover most of the cases.
+	 */
+
+	bool node_nullbyte = (node->flags & NODE_FLAGS_NULLBYTE);
+	if (node_nullbyte != name_nullbyte ||
+	    (node_nullbyte && !knot_dname_is_equal(node->owner, name))) {
+		return true;
+	}
+	return false;
+}
+
 int zone_contents_find_dname(const zone_contents_t *zone,
                              const knot_dname_t *name,
                              const zone_node_t **match,
                              const zone_node_t **closest,
-                             const zone_node_t **previous)
+                             const zone_node_t **previous,
+                             bool name_nullbyte)
 {
 	if (name == NULL || match == NULL || closest == NULL) {
 		return KNOT_EINVAL;
@@ -299,44 +316,29 @@ int zone_contents_find_dname(const zone_contents_t *zone,
 
 	int found = zone_tree_get_less_or_equal(zone->nodes, name, &node, &prev);
 	if (found < 0) {
-		// error
 		return found;
-	} else if (found == 1 && previous != NULL) {
-		// exact match
-
-		assert(node && prev);
+	}
 
-		*match = node;
-		*closest = node;
+	if (previous != NULL) { // Null previous means zone not adjusted yet.
+		assert(prev);
 		*previous = prev;
+	}
 
-		return ZONE_NAME_FOUND;
-	} else if (found == 1 && previous == NULL) {
-		// exact match, zone not adjusted yet
-
+	if (found == 1 && !null_byte_mismatch(node, name, name_nullbyte)) {
 		assert(node);
 		*match = node;
 		*closest = node;
-
 		return ZONE_NAME_FOUND;
 	} else {
-		// closest match
-
-		assert(!node && prev);
-
+		assert(prev);
+		*match = NULL;
 		node = prev;
 		size_t matched_labels = knot_dname_matched_labels(node->owner, name);
 		while (matched_labels < knot_dname_labels(node->owner, NULL)) {
 			node = node_parent(node);
 			assert(node);
 		}
-
-		*match = NULL;
 		*closest = node;
-		if (previous != NULL) {
-			*previous = prev;
-		}
-
 		return ZONE_NAME_NOT_FOUND;
 	}
 }
@@ -436,7 +438,7 @@ bool zone_contents_find_node_or_wildcard(const zone_contents_t *contents,
                                          const zone_node_t **found)
 {
 	const zone_node_t *encloser = NULL;
-	zone_contents_find_dname(contents, find, found, &encloser, NULL);
+	zone_contents_find_dname(contents, find, found, &encloser, NULL, knot_dname_with_null(find));
 	if (*found == NULL && encloser != NULL && (encloser->flags & NODE_FLAGS_WILDCARD_CHILD)) {
 		*found = zone_contents_find_wildcard_child(contents, encloser);
 		assert(*found != NULL);
diff --git a/src/knot/zone/contents.h b/src/knot/zone/contents.h
index e92bf61d4..0acde8319 100644
--- a/src/knot/zone/contents.h
+++ b/src/knot/zone/contents.h
@@ -119,6 +119,7 @@ zone_node_t *zone_contents_find_node_for_rr(zone_contents_t *contents, const kno
  *                       May match \a match if found exactly.
  * \param[out] previous  Previous domain name in canonical order.
  *                       Always previous, won't match \a match.
+ * \param[in] name_nullbyte The \a name parameter contains \0 byte.
  *
  * \note The encloser and previous mustn't be used directly for DNSSEC proofs.
  *       These nodes may be empty non-terminals or not authoritative.
@@ -133,7 +134,8 @@ int zone_contents_find_dname(const zone_contents_t *contents,
                              const knot_dname_t *name,
                              const zone_node_t **match,
                              const zone_node_t **closest,
-                             const zone_node_t **previous);
+                             const zone_node_t **previous,
+                             bool name_nullbyte);
 
 /*!
  * \brief Tries to find a node with the specified name among the NSEC3 nodes
diff --git a/src/knot/zone/node.c b/src/knot/zone/node.c
index 291454bd7..82d930d8f 100644
--- a/src/knot/zone/node.c
+++ b/src/knot/zone/node.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2021 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2024 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -124,6 +124,10 @@ zone_node_t *node_new(const knot_dname_t *owner, bool binode, bool second, knot_
 	// Node is authoritative by default.
 	ret->flags = NODE_FLAGS_AUTH;
 
+	if (knot_dname_with_null(owner)) {
+		ret->flags |= NODE_FLAGS_NULLBYTE;
+	}
+
 	if (binode) {
 		ret->flags |= NODE_FLAGS_BINODE;
 		if (second) {
diff --git a/src/knot/zone/node.h b/src/knot/zone/node.h
index d30cc6e1c..cab0604f9 100644
--- a/src/knot/zone/node.h
+++ b/src/knot/zone/node.h
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2021 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2024 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -104,6 +104,8 @@ enum node_flags {
 	NODE_FLAGS_SUBTREE_AUTH =    1 << 11,
 	/*! \brief The node or some node in subtree has any data in it, possibly just insec deleg. */
 	NODE_FLAGS_SUBTREE_DATA =    1 << 12,
+	/*! \brief Node owner name contains \0 byte in some label. */
+	NODE_FLAGS_NULLBYTE =        1 << 13,
 };
 
 typedef void (*node_addrem_cb)(zone_node_t *, void *);
diff --git a/src/knot/zone/semantic-check.c b/src/knot/zone/semantic-check.c
index 3d085d875..d449c5f77 100644
--- a/src/knot/zone/semantic-check.c
+++ b/src/knot/zone/semantic-check.c
@@ -173,7 +173,8 @@ static int check_delegation(const zone_node_t *node, semchecks_data_t *data)
 		const knot_dname_t *ns_dname = knot_ns_name(ns_rr);
 		const zone_node_t *glue_node = NULL, *glue_encloser = NULL;
 		int ret = zone_contents_find_dname(data->zone, ns_dname, &glue_node,
-		                                   &glue_encloser, NULL);
+		                                   &glue_encloser, NULL,
+		                                   knot_dname_with_null(ns_dname));
 		switch (ret) {
 		case KNOT_EOUTOFZONE:
 			continue; // NS is out of bailiwick
diff --git a/src/knot/zone/zone-tree.h b/src/knot/zone/zone-tree.h
index 384e87efd..cc6d348cb 100644
--- a/src/knot/zone/zone-tree.h
+++ b/src/knot/zone/zone-tree.h
@@ -155,18 +155,18 @@ zone_node_t *zone_tree_get(zone_tree_t *tree, const knot_dname_t *owner);
  * \brief Tries to find the given domain name in the zone tree and returns the
  *        associated node and previous node in canonical order.
  *
- * \param tree Zone to search in.
- * \param owner Owner of the node to find.
- * \param found Found node.
+ * \param tree     Zone to search in.
+ * \param owner    Owner of the node to find.
+ * \param found    Found node.
  * \param previous Previous node in canonical order (i.e. the one directly
  *                 preceding \a owner in canonical order, regardless if the name
  *                 is in the zone or not).
  *
- * \retval > 0 if the domain name was found. In such case \a found holds the
- *             zone node with \a owner as its owner.
- *             \a previous is set properly.
- * \retval 0 if the domain name was not found. \a found may hold any (or none)
- *           node. \a previous is set properly.
+ * \retval 1 if the domain name was found. In such case \a found holds the
+ *           zone node with \a owner as its owner.
+ *           \a previous is set properly.
+ * \retval 0 if the domain name was not found. \a found holds nothing.
+ *           \a previous is set properly.
  * \retval KNOT_EINVAL
  * \retval KNOT_ENOMEM
  */
diff --git a/src/libknot/dname.c b/src/libknot/dname.c
index d166f8d39..3cfc5a557 100644
--- a/src/libknot/dname.c
+++ b/src/libknot/dname.c
@@ -707,6 +707,17 @@ bool knot_dname_is_case_equal(const knot_dname_t *d1, const knot_dname_t *d2)
 }
 
 _public_
+bool knot_dname_with_null(const knot_dname_t *name)
+{
+	if (name == NULL) {
+		return false;
+	}
+
+	size_t size = knot_dname_size(name);
+	return (size != strnlen((const char *)name, size) + 1);
+}
+
+_public_
 size_t knot_dname_prefixlen(const uint8_t *name, unsigned nlabels)
 {
 	if (name == NULL) {
diff --git a/src/libknot/dname.h b/src/libknot/dname.h
index 6d72f901b..47e514c40 100644
--- a/src/libknot/dname.h
+++ b/src/libknot/dname.h
@@ -291,6 +291,17 @@ _pure_
 bool knot_dname_is_case_equal(const knot_dname_t *d1, const knot_dname_t *d2);
 
 /*!
+ * \brief Checks the domain name if it contains '\0' byte within a label.
+ *
+ * \param name  Domain name.
+ *
+ * \retval true if a zero byte is included
+ * \retval false if a zero byte isn't included
+ */
+_pure_
+bool knot_dname_with_null(const knot_dname_t *name);
+
+/*!
  * \brief Count length of the N first labels.
  *
  * \param name     Domain name.
diff --git a/src/libknot/packet/pkt.c b/src/libknot/packet/pkt.c
index 728bb3ee2..94bb310db 100644
--- a/src/libknot/packet/pkt.c
+++ b/src/libknot/packet/pkt.c
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2018 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2024 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -616,6 +616,11 @@ int knot_pkt_parse_question(knot_pkt_t *pkt)
 	/* Copy QNAME and canonicalize to lowercase. */
 	knot_dname_copy_lower(pkt->lower_qname, pkt->wire + KNOT_WIRE_HEADER_SIZE);
 
+	size_t str_len = strnlen((char *)(pkt->wire + KNOT_WIRE_HEADER_SIZE), pkt->qname_size) + 1;
+	if (pkt->qname_size != str_len) {
+		pkt->flags |= KNOT_PF_NULLBYTE;
+	}
+
 	return KNOT_EOK;
 }
 
diff --git a/src/libknot/packet/pkt.h b/src/libknot/packet/pkt.h
index da69c8cfc..ac7ac1851 100644
--- a/src/libknot/packet/pkt.h
+++ b/src/libknot/packet/pkt.h
@@ -1,4 +1,4 @@
-/*  Copyright (C) 2020 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
+/*  Copyright (C) 2024 CZ.NIC, z.s.p.o. <knot-dns@labs.nic.cz>
 
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -52,6 +52,7 @@ enum {
 	KNOT_PF_NOCANON   = 1 << 5, /*!< Don't canonicalize rrsets during parsing. */
 	KNOT_PF_ORIGTTL   = 1 << 6, /*!< Write RRSIGs with their original TTL. */
 	KNOT_PF_SOAMINTTL = 1 << 7, /*!< Write SOA with its minimum-ttl as TTL. */
+	KNOT_PF_NULLBYTE  = 1 << 8, /*!< At lest one \0 byte is present in some qname label. */
 };
 
 typedef struct knot_pkt knot_pkt_t;
diff --git a/tests-extra/tests/basic/zerobyte/data/test.zone b/tests-extra/tests/basic/zerobyte/data/test.zone
new file mode 100644
index 000000000..01c5592dc
--- /dev/null
+++ b/tests-extra/tests/basic/zerobyte/data/test.zone
@@ -0,0 +1,9 @@
+$ORIGIN test.
+
+@	SOA	dns hostmaster 2010111201 10800 3600 1209600 7200
+	NS	dns
+dns	A	192.0.2.1
+
+psy	TXT	text
+cho.psy	NS	example.com.
+exis\000ing A	1.2.3.4
diff --git a/tests-extra/tests/basic/zerobyte/test.py b/tests-extra/tests/basic/zerobyte/test.py
new file mode 100644
index 000000000..9dc736178
--- /dev/null
+++ b/tests-extra/tests/basic/zerobyte/test.py
@@ -0,0 +1,34 @@
+#!/usr/bin/env python3
+
+'''Test for zero byte in a QNAME label.'''
+
+from dnstest.test import Test
+
+t = Test()
+
+master = t.server("knot")
+zone = t.zone("test.", storage=".")
+
+t.link(zone, master)
+
+master.dnssec(zone).enable = True
+master.dnssec(zone).nsec3 = True
+master.dnssec(zone).nsec3_opt_out = True
+
+t.start()
+
+master.zone_wait(zone)
+
+resp = master.dig("psy\\000cho.test.", "A", dnssec=True)
+resp.check(rcode="NXDOMAIN")
+
+resp = master.dig("psy\\000cho\\000nxd.test.", "A", dnssec=True)
+resp.check(rcode="NXDOMAIN")
+
+resp = master.dig("exis\\000ing.test.", "A", dnssec=True)
+resp.check(rcode="NOERROR", rdata="1.2.3.4")
+
+resp = master.dig("ing.exis.test.", "A", dnssec=True)
+resp.check(rcode="NXDOMAIN", nordata="1.2.3.4")
+
+t.end()