2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00

Correct value of DNS_NAME_MAXLABELS

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

The mistake was introduced in c6bf51492d 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
c6bf51492d 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()`.
This commit is contained in:
Tony Finch
2023-04-05 13:42:52 +01:00
parent 3948827c0b
commit e8ff0f0c08
5 changed files with 33 additions and 8 deletions

View File

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

View File

@@ -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;

View File

@@ -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);

View File

@@ -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) {

View File

@@ -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 */