Correct value of DNS_NAME_MAXLABELS

It should be floor(DNS_NAME_MAXWIRE / 2) + 1 == 128

The mistake was introduced in c6bf51492dbd because:

  * I was refactoring an existing `DNS_MAX_LABELS` defined as 127

  * There was a longstanding bug in `dns_name_isvalid()` which
    checked the number of labels against 127U instead of 128

  * I mistakenly thought `dns_name_isvalid()` was correct and
    `dns_name_countlabels()` was incorrect, but the reverse was true.

After this commit, occurrances of `DNS_NAME_MAXLABELS` with value
128 are consistent with the use of 127 or 128 before commit
c6bf51492dbd except for the mistake in `dns_name_isvalid()`.
This commit adds a test case that checks the MAXLABELS case
in `dns_name_fromtext()` and `dns_name_isvalid()`.

diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h
index f525c4349586737bdc430659b83e570e8913201e..51568b54bbf1fed237cca677a039cc26d95186a6 100644 (file)
--- a/lib/dns/include/dns/name.h
+++ b/lib/dns/include/dns/name.h
@@ -185,9 +185,11 @@ extern const dns_name_t *dns_wildcardname;
  * Standard sizes of a wire format name
  */
 #define DNS_NAME_MAXWIRE   255
-#define DNS_NAME_MAXLABELS 127
+#define DNS_NAME_MAXLABELS 128
 #define DNS_NAME_LABELLEN  63
 
+typedef unsigned char dns_offsets_t[DNS_NAME_MAXLABELS];
+
 /*
  * Text output filter procedure.
  * 'target' is the buffer to be converted.  The region to be converted

diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h
index 7109f759a934fd7c60e0e0b190b8ebd8c5362e30..96297e8f6afd574a45f1364b04f3879a6d4f1860 100644 (file)
--- a/lib/dns/include/dns/types.h
+++ b/lib/dns/include/dns/types.h
@@ -118,7 +118,6 @@ typedef struct dns_name                dns_name_t;
 typedef ISC_LIST(dns_name_t) dns_namelist_t;
 typedef struct dns_ntatable      dns_ntatable_t;
 typedef uint16_t                 dns_opcode_t;
-typedef unsigned char            dns_offsets_t[128];
 typedef struct dns_order         dns_order_t;
 typedef struct dns_peer                  dns_peer_t;
 typedef struct dns_peerlist      dns_peerlist_t;

diff --git a/lib/dns/name.c b/lib/dns/name.c
index 3a44c1dc901d240ab6eee6687991b1121964a2a8..fb6863231161b56bfeec3abc41ea8eaa7f8379de 100644 (file)
--- a/lib/dns/name.c
+++ b/lib/dns/name.c
@@ -951,7 +951,7 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source,
                                }
                                *label = count;
                                labels++;
-                               INSIST(labels <= DNS_NAME_MAXLABELS);
+                               INSIST(labels < DNS_NAME_MAXLABELS);
                                offsets[labels] = nused;
                                if (tlen == 0) {
                                        labels++;
@@ -1048,7 +1048,7 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source,
                        INSIST(label != NULL);
                        *label = count;
                        labels++;
-                       INSIST(labels <= DNS_NAME_MAXLABELS);
+                       INSIST(labels < DNS_NAME_MAXLABELS);
                        offsets[labels] = nused;
                }
                if (origin != NULL) {
@@ -1075,7 +1075,7 @@ dns_name_fromtext(dns_name_t *name, isc_buffer_t *source,
                                }
                                labels++;
                                if (n1 > 0) {
-                                       INSIST(labels <= DNS_NAME_MAXLABELS);
+                                       INSIST(labels < DNS_NAME_MAXLABELS);
                                        offsets[labels] = nused;
                                }
                        }
@@ -1469,7 +1469,7 @@ set_offsets(const dns_name_t *name, unsigned char *offsets,
        nlabels = 0;
        absolute = false;
        while (offset != length) {
-               INSIST(nlabels <= DNS_NAME_MAXLABELS);
+               INSIST(nlabels < DNS_NAME_MAXLABELS);
                offsets[nlabels++] = offset;
                count = *ndata;
                INSIST(count <= DNS_NAME_LABELLEN);

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 194caf6539478ebcd14bf2c2c288af76b5d8b78f..e472c6e78eadfc10312a97105ffb9d8d45d34b9e 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -4155,7 +4155,7 @@ resume_qmin(void *arg) {
        case ISC_R_FAILURE:
                if ((fctx->options & DNS_FETCHOPT_QMIN_STRICT) == 0) {
                        /* Disable minimization in relaxed mode */
-                       fctx->qmin_labels = DNS_NAME_MAXLABELS + 1;
+                       fctx->qmin_labels = DNS_NAME_MAXLABELS;
                        /*
                         * We store the result. If we succeed in the end
                         * we'll issue a warning that the server is
@@ -10167,7 +10167,7 @@ fctx_minimize_qname(fetchctx_t *fctx) {
                        fctx->qmin_labels = nlabels;
                }
        } else if (fctx->qmin_labels > DNS_QMIN_MAXLABELS) {
-               fctx->qmin_labels = DNS_NAME_MAXLABELS + 1;
+               fctx->qmin_labels = DNS_NAME_MAXLABELS;
        }
 
        if (fctx->qmin_labels < nlabels) {

diff --git a/tests/dns/name_test.c b/tests/dns/name_test.c
index fa923fb6ec685eba0b588502b5565016ae27a79e..0079fc126a01c3aede0347e5a374b56b8d271381 100644 (file)
--- a/tests/dns/name_test.c
+++ b/tests/dns/name_test.c
@@ -824,6 +824,29 @@ ISC_RUN_TEST_IMPL(getlabelsequence) {
        }
 }
 
+ISC_RUN_TEST_IMPL(maxlabels) {
+       isc_result_t result;
+       dns_fixedname_t fixed;
+       dns_name_t *name = NULL;
+
+       const char one_too_many[] =
+               "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y."
+               "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y."
+               "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y."
+               "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y."
+               "a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y."
+               "a.b.c.";
+
+       name = dns_fixedname_initname(&fixed);
+       result = dns_name_fromstring(name, one_too_many, 0, NULL);
+       assert_int_equal(result, ISC_R_NOSPACE);
+
+       name = dns_fixedname_initname(&fixed);
+       result = dns_name_fromstring(name, one_too_many + 2, 0, NULL);
+       assert_int_equal(result, ISC_R_SUCCESS);
+       assert_true(dns_name_isvalid(name));
+}
+
 #ifdef DNS_BENCHMARK_TESTS
 
 /*
@@ -919,6 +942,7 @@ ISC_TEST_ENTRY(issubdomain)
 ISC_TEST_ENTRY(countlabels)
 ISC_TEST_ENTRY(getlabel)
 ISC_TEST_ENTRY(getlabelsequence)
+ISC_TEST_ENTRY(maxlabels)
 #ifdef DNS_BENCHMARK_TESTS
 ISC_TEST_ENTRY(benchmark)
 #endif /* DNS_BENCHMARK_TESTS */