From: Tony Finch Date: Thu, 23 Jun 2022 20:53:46 +0000 (+0100) Subject: Simplify and speed up DNS name compression X-Git-Tag: v9.19.7~59^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=45b2d8938bda00ef3d733501a74a510493c7377d;p=thirdparty%2Fbind9.git Simplify and speed up DNS name compression All we need for compression is a very small hash set of compression offsets, because most of the information we need (the previously added names) can be found in the message using the compression offsets. This change combines dns_compress_find() and dns_compress_add() into one function dns_compress_name() that both finds any existing suffix, and adds any new prefix to the table. The old split led to performance problems caused by duplicate names in the compression context. Compression contexts are now either small or large, which the caller chooses depending on the expected size of the message. There is no dynamic resizing. There is a behaviour change: compression now acts on all the labels in each name, instead of just the last few. A small benchmark suggests this is about 2x faster. --- diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 9d8cdc5b5ca..e4900dac356 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2451,8 +2451,7 @@ setup_lookup(dig_lookup_t *lookup) { lookup->sendspace = isc_mem_get(mctx, COMMSIZE); - result = dns_compress_init(&cctx, mctx); - check_result(result, "dns_compress_init"); + dns_compress_init(&cctx, mctx, 0); debug("starting to render the message"); isc_buffer_init(&lookup->renderbuf, lookup->sendspace, COMMSIZE); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 40aab3bc771..205a962eb4a 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -2548,7 +2548,7 @@ send_update(dns_name_t *zone, isc_sockaddr_t *primary) { isc_result_t result; dns_request_t *request = NULL; isc_sockaddr_t *srcaddr; - unsigned int options = DNS_REQUESTOPT_CASE; + unsigned int options = DNS_REQUESTOPT_CASE | DNS_REQUESTOPT_LARGE; dns_transport_t *req_transport = NULL; isc_tlsctx_cache_t *req_tls_ctx_cache = NULL; diff --git a/bin/tests/wire_test.c b/bin/tests/wire_test.c index d4224d68f67..fa6f7eae1dd 100644 --- a/bin/tests/wire_test.c +++ b/bin/tests/wire_test.c @@ -303,8 +303,7 @@ process_message(isc_buffer_t *source) { message->counts[i] = 0; /* Another hack XXX */ } - result = dns_compress_init(&cctx, mctx); - CHECKRESULT(result, "dns_compress_init() failed"); + dns_compress_init(&cctx, mctx, 0); result = dns_message_renderbegin(message, &cctx, &buffer); CHECKRESULT(result, "dns_message_renderbegin() failed"); diff --git a/fuzz/dns_message_parse.c b/fuzz/dns_message_parse.c index d0caa245ee6..ac0532a404c 100644 --- a/fuzz/dns_message_parse.c +++ b/fuzz/dns_message_parse.c @@ -112,10 +112,8 @@ render_message(dns_message_t **messagep) { message->counts[i] = 0; } - result = dns_compress_init(&cctx, mctx); - if (result != ISC_R_SUCCESS) { - return (result); - } + dns_compress_init(&cctx, mctx, 0); + CHECKRESULT(result, dns_message_renderbegin(message, &cctx, &buffer)); CHECKRESULT(result, dns_message_rendersection(message, diff --git a/fuzz/dns_rdata_fromwire_text.c b/fuzz/dns_rdata_fromwire_text.c index 5a970e3dfa7..d42dc543d4f 100644 --- a/fuzz/dns_rdata_fromwire_text.c +++ b/fuzz/dns_rdata_fromwire_text.c @@ -203,8 +203,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { /* * Convert rdata back to wire. */ - CHECK(dns_compress_init(&cctx, mctx)); - dns_compress_disable(&cctx); + dns_compress_init(&cctx, mctx, DNS_COMPRESS_DISABLED); isc_buffer_init(&target, towire, sizeof(towire)); result = dns_rdata_towire(&rdata1, &cctx, &target); dns_compress_invalidate(&cctx); diff --git a/lib/dns/compress.c b/lib/dns/compress.c index 37a89056935..d0cba5d38a2 100644 --- a/lib/dns/compress.c +++ b/lib/dns/compress.c @@ -11,444 +11,355 @@ * information regarding copyright ownership. */ -/*! \file */ - #define DNS_NAME_USEINLINE 1 -#include #include +#include +#include #include +#include +#include #include -#include -#include #include #include -#include -#include +#include + +#define HASH_INIT_DJB2 5381 #define CCTX_MAGIC ISC_MAGIC('C', 'C', 'T', 'X') -#define VALID_CCTX(x) ISC_MAGIC_VALID(x, CCTX_MAGIC) +#define CCTX_VALID(x) ISC_MAGIC_VALID(x, CCTX_MAGIC) + +void +dns_compress_init(dns_compress_t *cctx, isc_mem_t *mctx, + dns_compress_flags_t flags) { + dns_compress_slot_t *set = NULL; + uint16_t mask; -/* - * The tableindex array below is of size 256, one entry for each - * unsigned char value. The tableindex array elements are dependent on - * DNS_COMPRESS_TABLESIZE. The table was created using the following - * function. - * - * static void - * gentable(unsigned char *table) { - * unsigned int i; - * const unsigned int left = DNS_COMPRESS_TABLESIZE - 38; - * long r; - * - * for (i = 0; i < 26; i++) { - * table['A' + i] = i; - * table['a' + i] = i; - * } - * - * for (i = 0; i <= 9; i++) - * table['0' + i] = i + 26; - * - * table['-'] = 36; - * table['_'] = 37; - * - * for (i = 0; i < 256; i++) { - * if ((i >= 'a' && i <= 'z') || - * (i >= 'A' && i <= 'Z') || - * (i >= '0' && i <= '9') || - * (i == '-') || - * (i == '_')) - * continue; - * r = random() % left; - * table[i] = 38 + r; - * } - * } - */ -static unsigned char tableindex[256] = { - 0x3e, 0x3e, 0x33, 0x2d, 0x30, 0x38, 0x31, 0x3c, 0x2b, 0x33, 0x30, 0x3f, - 0x2d, 0x3c, 0x36, 0x3a, 0x28, 0x2c, 0x2a, 0x37, 0x3d, 0x34, 0x35, 0x2d, - 0x39, 0x2b, 0x2f, 0x2c, 0x3b, 0x32, 0x2b, 0x39, 0x30, 0x38, 0x28, 0x3c, - 0x32, 0x33, 0x39, 0x38, 0x27, 0x2b, 0x39, 0x30, 0x27, 0x24, 0x2f, 0x2b, - 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x3a, 0x29, 0x36, - 0x31, 0x3c, 0x35, 0x26, 0x31, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, - 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, - 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x3e, 0x3b, 0x39, 0x2f, 0x25, - 0x27, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, - 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, - 0x17, 0x18, 0x19, 0x36, 0x3b, 0x2f, 0x2f, 0x2e, 0x29, 0x33, 0x2a, 0x36, - 0x28, 0x3f, 0x2e, 0x29, 0x2c, 0x29, 0x36, 0x2d, 0x32, 0x3d, 0x33, 0x2a, - 0x2e, 0x2f, 0x3b, 0x30, 0x3d, 0x39, 0x2b, 0x36, 0x2a, 0x2f, 0x2c, 0x26, - 0x3a, 0x37, 0x30, 0x3d, 0x2a, 0x36, 0x33, 0x2c, 0x38, 0x3d, 0x32, 0x3e, - 0x26, 0x2a, 0x2c, 0x35, 0x27, 0x39, 0x3b, 0x31, 0x2a, 0x37, 0x3c, 0x27, - 0x32, 0x29, 0x39, 0x37, 0x34, 0x3f, 0x39, 0x2e, 0x38, 0x2b, 0x2c, 0x3e, - 0x3b, 0x3b, 0x2d, 0x33, 0x3b, 0x3b, 0x32, 0x3d, 0x3f, 0x3a, 0x34, 0x26, - 0x35, 0x30, 0x31, 0x39, 0x27, 0x2f, 0x3d, 0x35, 0x35, 0x36, 0x2e, 0x29, - 0x38, 0x27, 0x34, 0x32, 0x2c, 0x3c, 0x31, 0x28, 0x37, 0x38, 0x37, 0x34, - 0x33, 0x29, 0x32, 0x34, 0x3f, 0x26, 0x34, 0x34, 0x32, 0x27, 0x30, 0x33, - 0x33, 0x2d, 0x2b, 0x28, 0x3f, 0x33, 0x2b, 0x39, 0x37, 0x39, 0x2c, 0x3d, - 0x35, 0x39, 0x27, 0x2f -}; - -/*** - *** Compression - ***/ - -isc_result_t -dns_compress_init(dns_compress_t *cctx, isc_mem_t *mctx) { REQUIRE(cctx != NULL); - REQUIRE(mctx != NULL); /* See: rdataset.c:towiresorted(). */ + REQUIRE(mctx != NULL); + + if ((flags & DNS_COMPRESS_LARGE) != 0) { + size_t count = (1 << DNS_COMPRESS_LARGEBITS); + size_t size = count * sizeof(*set); + mask = count - 1; + set = isc_mem_allocatex(mctx, size, ISC_MEM_ZERO); + } else { + mask = ARRAY_SIZE(cctx->smallset) - 1; + set = cctx->smallset; + } /* - * not using a structure literal here to avoid large memset()s + * The lifetime of this object is limited to the stack frame of the + * caller, so we don't need to attach to the memory context. */ - cctx->mctx = mctx; - cctx->count = 0; - cctx->permitted = true; - cctx->disabled = false; - cctx->sensitive = false; - cctx->arena_off = 0; - - memset(&cctx->table[0], 0, sizeof(cctx->table)); - - cctx->magic = CCTX_MAGIC; - - return (ISC_R_SUCCESS); + *cctx = (dns_compress_t){ + .magic = CCTX_MAGIC, + .flags = flags | DNS_COMPRESS_PERMITTED, + .mctx = mctx, + .mask = mask, + .set = set, + }; } void dns_compress_invalidate(dns_compress_t *cctx) { - dns_compressnode_t *node; - unsigned int i; - - REQUIRE(VALID_CCTX(cctx)); - - for (i = 0; i < DNS_COMPRESS_TABLESIZE; i++) { - while (cctx->table[i] != NULL) { - node = cctx->table[i]; - cctx->table[i] = cctx->table[i]->next; - if ((node->offset & 0x8000) != 0) { - isc_mem_put(cctx->mctx, node->r.base, - node->r.length); - } - if (node->count < DNS_COMPRESS_INITIALNODES) { - continue; - } - isc_mem_put(cctx->mctx, node, sizeof(*node)); - } + REQUIRE(CCTX_VALID(cctx)); + if (cctx->set != cctx->smallset) { + isc_mem_free(cctx->mctx, cctx->set); } - - cctx->magic = 0; - cctx->permitted = false; - cctx->disabled = false; - cctx->sensitive = false; + *cctx = (dns_compress_t){ 0 }; } void dns_compress_setpermitted(dns_compress_t *cctx, bool permitted) { - REQUIRE(VALID_CCTX(cctx)); - cctx->permitted = permitted; + REQUIRE(CCTX_VALID(cctx)); + if (permitted) { + cctx->flags |= DNS_COMPRESS_PERMITTED; + } else { + cctx->flags &= ~DNS_COMPRESS_PERMITTED; + } } bool dns_compress_getpermitted(dns_compress_t *cctx) { - REQUIRE(VALID_CCTX(cctx)); - return (cctx->permitted); + REQUIRE(CCTX_VALID(cctx)); + return ((cctx->flags & DNS_COMPRESS_PERMITTED) != 0); } -void -dns_compress_disable(dns_compress_t *cctx) { - REQUIRE(VALID_CCTX(cctx)); - cctx->disabled = true; -} +/* + * Our hash value needs to cover the entire suffix of a name, and we need + * to calculate it one label at a time. So this function mixes a label into + * an existing hash. (We don't use isc_hash32() because the djb2 hash is a + * lot faster, and we limit the impact of collision attacks by restricting + * the size and occupancy of the hash set.) The accumulator is 32 bits to + * keep more of the fun mixing that happens in the upper bits. + */ +static uint16_t +hash_label(uint16_t init, uint8_t *ptr, bool sensitive) { + unsigned int len = ptr[0] + 1; + uint32_t hash = init; + + if (sensitive) { + while (len-- > 0) { + hash = hash * 33 + *ptr++; + } + } else { + /* using the autovectorize-friendly tolower() */ + while (len-- > 0) { + hash = hash * 33 + isc__ascii_tolower1(*ptr++); + } + } -void -dns_compress_setsensitive(dns_compress_t *cctx, bool sensitive) { - REQUIRE(VALID_CCTX(cctx)); - cctx->sensitive = sensitive; + return (isc_hash_bits32(hash, 16)); } -bool -dns_compress_getsensitive(dns_compress_t *cctx) { - REQUIRE(VALID_CCTX(cctx)); - return (cctx->sensitive); +static bool +match_wirename(uint8_t *a, uint8_t *b, unsigned int len, bool sensitive) { + if (sensitive) { + return (memcmp(a, b, len) == 0); + } else { + /* label lengths are < 'A' so unaffected by tolower() */ + return (isc_ascii_lowerequal(a, b, len)); + } } /* - * Find the longest match of name in the table. - * If match is found return true. prefix, suffix and offset are updated. - * If no match is found return false. + * We have found a hash set entry whose hash value matches the current + * suffix of our name, which is passed to this function via `sptr` and + * `slen`. We need to verify that the suffix in the message (referred to + * by `new_coff`) actually matches, in case of hash collisions. + * + * We know that the previous suffix of this name (after the first label) + * occurs in the message at `old_coff`, and all the compression offsets in + * the hash set and in the message refer to the first occurrence of a + * particular name or suffix. + * + * First, we need to match the label that was just added to our suffix, + * and second, verify that it is followed by the previous suffix. + * + * There are a few ways to match the previous suffix: + * + * When the first occurrence of this suffix is also the first occurrence + * of the previous suffix, `old_coff` points just after the new label. + * + * Otherwise, if this suffix occurs in a compressed name, it will be + * followed by a compression pointer that refers to the previous suffix, + * which must be equal to `old_coff`. + * + * The final possibility is that this suffix occurs in an uncompressed + * name, so we have to compare the rest of the suffix in full. + * + * A special case is when this suffix is a TLD. That can be handled by + * the case for uncompressed names, but it is common enough that it is + * worth taking a short cut. (In the TLD case, the `old_coff` will be + * zero, and the quick checks for the previous suffix will fail.) */ -bool -dns_compress_find(dns_compress_t *cctx, const dns_name_t *name, - dns_name_t *prefix, uint16_t *offset) { - dns_name_t tname; - dns_compressnode_t *node = NULL; - unsigned int labels, i, n; - unsigned int numlabels; - unsigned char *p; - - REQUIRE(VALID_CCTX(cctx)); - REQUIRE(dns_name_isabsolute(name)); - REQUIRE(offset != NULL); +static bool +match_suffix(isc_buffer_t *buffer, unsigned int new_coff, uint8_t *sptr, + unsigned int slen, unsigned int old_coff, bool sensitive) { + uint8_t pptr[] = { 0xC0 | (old_coff >> 8), old_coff & 0xff }; + uint8_t *bptr = isc_buffer_base(buffer); + unsigned int blen = isc_buffer_usedlength(buffer); + unsigned int llen = sptr[0] + 1; - if (cctx->disabled) { + INSIST(llen <= 64 && llen < slen); + + if (blen < new_coff + llen) { return (false); } - if (cctx->count == 0) { + blen -= new_coff; + bptr += new_coff; + + /* does the first label of the suffix appear here? */ + if (!match_wirename(bptr, sptr, llen, sensitive)) { return (false); } - labels = dns_name_countlabels(name); - INSIST(labels > 0); - - dns_name_init(&tname, NULL); + /* is this label followed by the previously matched suffix? */ + if (old_coff == new_coff + llen) { + return (true); + } - numlabels = labels > 3U ? 3U : labels; - p = name->ndata; + blen -= llen; + bptr += llen; + slen -= llen; + sptr += llen; - for (n = 0; n < numlabels - 1; n++) { - unsigned char ch, llen; - unsigned int firstoffset, length; + /* are both labels followed by the root label? */ + if (blen >= 1 && slen == 1 && bptr[0] == 0 && sptr[0] == 0) { + return (true); + } - firstoffset = (unsigned int)(p - name->ndata); - length = name->length - firstoffset; + /* is this label followed by a pointer to the previous match? */ + if (blen >= 2 && bptr[0] == pptr[0] && bptr[1] == pptr[1]) { + return (true); + } - /* - * We calculate the table index using the first - * character in the first label of the suffix name. - */ - ch = p[1]; - i = tableindex[ch]; - if (cctx->sensitive) { - for (node = cctx->table[i]; node != NULL; - node = node->next) { - if (node->name.length != length) { - continue; - } - - if (memcmp(node->name.ndata, p, length) == 0) { - goto found; - } - } - } else { - for (node = cctx->table[i]; node != NULL; - node = node->next) { - unsigned int l, count; - unsigned char *p1, *p2; - - if (node->name.length != length) { - continue; - } - - l = labels - n; - if (node->name.labels != l) { - continue; - } - - p1 = node->name.ndata; - p2 = p; - while (l-- > 0) { - count = *p1++; - if (count != *p2++) { - goto cont1; - } - - /* no bitstring support */ - INSIST(count <= 63); - - if (!isc_ascii_lowerequal(p1, p2, - count)) { - goto cont1; - } - p1 += count; - p2 += count; - } - break; - cont1: - continue; - } - } + /* is this label followed by a copy of the rest of the suffix? */ + return (blen >= slen && match_wirename(bptr, sptr, slen, sensitive)); +} - if (node != NULL) { - break; - } +/* + * Robin Hood hashing aims to minimize probe distance when inserting a + * new element by ensuring that the new element does not have a worse + * probe distance than any other element in its probe sequence. During + * insertion, if an existing element is encountered with a shorter + * probe distance, it is swapped with the new element, and insertion + * continues with the displaced element. + */ +static unsigned int +probe_distance(dns_compress_t *cctx, unsigned int slot) { + return ((slot - cctx->set[slot].hash) & cctx->mask); +} - llen = *p; - p += llen + 1; - } +static unsigned int +slot_index(dns_compress_t *cctx, unsigned int hash, unsigned int probe) { + return ((hash + probe) & cctx->mask); +} -found: +static bool +insert_label(dns_compress_t *cctx, isc_buffer_t *buffer, const dns_name_t *name, + unsigned int label, uint16_t hash, unsigned int probe) { /* - * If node == NULL, we found no match at all. + * hash set entries must have valid compression offsets + * and the hash set must not get too full (75% load) */ - if (node == NULL) { - return (false); + unsigned int prefix_len = name->offsets[label]; + unsigned int coff = isc_buffer_usedlength(buffer) + prefix_len; + if (coff >= 0x4000 || cctx->count > cctx->mask * 3 / 4) { + return false; } - - if (n == 0) { - dns_name_reset(prefix); - } else { - dns_name_getlabelsequence(name, 0, n, prefix); + for (;;) { + unsigned int slot = slot_index(cctx, hash, probe); + /* we can stop when we find an empty slot */ + if (cctx->set[slot].coff == 0) { + cctx->set[slot].hash = hash; + cctx->set[slot].coff = coff; + cctx->count++; + return true; + } + /* he steals from the rich and gives to the poor */ + if (probe > probe_distance(cctx, slot)) { + probe = probe_distance(cctx, slot); + ISC_SWAP(cctx->set[slot].hash, hash); + ISC_SWAP(cctx->set[slot].coff, coff); + } + probe++; } - - *offset = (node->offset & 0x7fff); - return (true); } -static unsigned int -name_length(const dns_name_t *name) { - isc_region_t r; - dns_name_toregion(name, &r); - return (r.length); +/* + * Add the unmatched prefix of the name to the hash set. + */ +static void +insert(dns_compress_t *cctx, isc_buffer_t *buffer, const dns_name_t *name, + unsigned int label, uint16_t hash, unsigned int probe) { + bool sensitive = (cctx->flags & DNS_COMPRESS_CASE) != 0; + /* + * this insertion loop continues from the search loop inside + * dns_compress_name() below, iterating over the remaining labels + * of the name and accumulating the hash in the same manner + */ + while (insert_label(cctx, buffer, name, label, hash, probe) && + label-- > 0) { + unsigned int prefix_len = name->offsets[label]; + uint8_t *suffix_ptr = name->ndata + prefix_len; + hash = hash_label(hash, suffix_ptr, sensitive); + probe = 0; + } } void -dns_compress_add(dns_compress_t *cctx, const dns_name_t *name, - const dns_name_t *prefix, uint16_t offset) { - dns_name_t tname, xname; - unsigned int start; - unsigned int n; - unsigned int count; - unsigned int i; - dns_compressnode_t *node; - unsigned int length; - unsigned int tlength; - uint16_t toffset; - unsigned char *tmp; - isc_region_t r; - bool allocated = false; - - REQUIRE(VALID_CCTX(cctx)); +dns_compress_name(dns_compress_t *cctx, isc_buffer_t *buffer, + const dns_name_t *name, unsigned int *return_prefix, + unsigned int *return_coff) { + REQUIRE(CCTX_VALID(cctx)); + REQUIRE(ISC_BUFFER_VALID(buffer)); REQUIRE(dns_name_isabsolute(name)); + REQUIRE(name->labels > 0); + REQUIRE(name->offsets != NULL); + REQUIRE(return_prefix != NULL); + REQUIRE(return_coff != NULL); + REQUIRE(*return_coff == 0); - if (cctx->disabled) { + if ((cctx->flags & DNS_COMPRESS_DISABLED) != 0) { return; } - if (offset >= 0x4000) { - return; - } - dns_name_init(&tname, NULL); - dns_name_init(&xname, NULL); + bool sensitive = (cctx->flags & DNS_COMPRESS_CASE) != 0; + + uint16_t hash = HASH_INIT_DJB2; + unsigned int label = name->labels - 1; /* skip the root label */ - n = dns_name_countlabels(name); - count = dns_name_countlabels(prefix); - if (dns_name_isabsolute(prefix)) { - count--; - } - if (count == 0) { - return; - } - start = 0; - dns_name_toregion(name, &r); - length = r.length; - if (cctx->arena_off + length < DNS_COMPRESS_ARENA_SIZE) { - tmp = &cctx->arena[cctx->arena_off]; - cctx->arena_off += length; - } else { - allocated = true; - tmp = isc_mem_get(cctx->mctx, length); - } /* - * Copy name data to 'tmp' and make 'r' use 'tmp'. + * find out how much of the name's suffix is in the hash set, + * stepping backwards from the end one label at a time */ - memmove(tmp, r.base, r.length); - r.base = tmp; - dns_name_fromregion(&xname, &r); - - if (count > 2U) { - count = 2U; - } - - while (count > 0) { - unsigned char ch; - - dns_name_getlabelsequence(&xname, start, n, &tname); - /* - * We calculate the table index using the first - * character in the first label of tname. - */ - ch = tname.ndata[1]; - i = tableindex[ch]; - tlength = name_length(&tname); - toffset = (uint16_t)(offset + (length - tlength)); - if (toffset >= 0x4000) { - break; - } - /* - * Create a new node and add it. - */ - if (cctx->count < DNS_COMPRESS_INITIALNODES) { - node = &cctx->initialnodes[cctx->count]; - } else { - node = isc_mem_get(cctx->mctx, - sizeof(dns_compressnode_t)); - } - node->count = cctx->count++; - /* - * 'node->r.base' becomes 'tmp' when start == 0. - * Record this by setting 0x8000 so it can be freed later. - */ - if (start == 0 && allocated) { - toffset |= 0x8000; - } - node->offset = toffset; - dns_name_toregion(&tname, &node->r); - dns_name_init(&node->name, NULL); - node->name.length = node->r.length; - node->name.ndata = node->r.base; - node->name.labels = tname.labels; - node->name.attributes = - (struct dns_name_attrs){ .absolute = true }; - node->next = cctx->table[i]; - cctx->table[i] = node; - start++; - n--; - count--; - } + while (label-- > 0) { + unsigned int prefix_len = name->offsets[label]; + unsigned int suffix_len = name->length - prefix_len; + uint8_t *suffix_ptr = name->ndata + prefix_len; + hash = hash_label(hash, suffix_ptr, sensitive); + + for (unsigned int probe = 0; true; probe++) { + unsigned int slot = slot_index(cctx, hash, probe); + unsigned int coff = cctx->set[slot].coff; + + /* + * if we would have inserted this entry here (as in + * insert_label() above), our suffix cannot be in the + * hash set, so stop searching and switch to inserting + * the rest of the name (its prefix) into the set + */ + if (coff == 0 || probe > probe_distance(cctx, slot)) { + insert(cctx, buffer, name, label, hash, probe); + return; + } - if (start == 0) { - if (!allocated) { - cctx->arena_off -= length; - } else { - isc_mem_put(cctx->mctx, tmp, length); + /* + * this slot matches, so provisionally set the + * return values and continue with the next label + */ + if (hash == cctx->set[slot].hash && + match_suffix(buffer, coff, suffix_ptr, suffix_len, + *return_coff, sensitive)) + { + *return_coff = coff; + *return_prefix = prefix_len; + break; + } } } } void -dns_compress_rollback(dns_compress_t *cctx, uint16_t offset) { - unsigned int i; - dns_compressnode_t *node; - - REQUIRE(VALID_CCTX(cctx)); +dns_compress_rollback(dns_compress_t *cctx, unsigned int coff) { + REQUIRE(CCTX_VALID(cctx)); - if (cctx->disabled) { - return; - } - - for (i = 0; i < DNS_COMPRESS_TABLESIZE; i++) { - node = cctx->table[i]; + for (unsigned int slot = 0; slot <= cctx->mask; slot++) { + if (cctx->set[slot].coff < coff) { + continue; + } /* - * This relies on nodes with greater offsets being - * closer to the beginning of the list, and the - * items with the greatest offsets being at the end - * of the initialnodes[] array. + * The next few elements might be part of the deleted element's + * probe sequence, so we slide them down to overwrite the entry + * we are deleting and preserve the probe sequence. Moving an + * element to the previous slot reduces its probe distance, so + * we stop when we find an element whose probe distance is zero. */ - while (node != NULL && (node->offset & 0x7fff) >= offset) { - cctx->table[i] = node->next; - if ((node->offset & 0x8000) != 0) { - isc_mem_put(cctx->mctx, node->r.base, - node->r.length); - } - if (node->count >= DNS_COMPRESS_INITIALNODES) { - isc_mem_put(cctx->mctx, node, sizeof(*node)); - } - cctx->count--; - node = cctx->table[i]; + unsigned int prev = slot; + unsigned int next = slot_index(cctx, prev, 1); + while (cctx->set[next].coff != 0 && + probe_distance(cctx, next) != 0) { + cctx->set[prev] = cctx->set[next]; + prev = next; + next = slot_index(cctx, prev, 1); } + cctx->set[prev].coff = 0; + cctx->set[prev].hash = 0; + cctx->count--; } } diff --git a/lib/dns/include/dns/compress.h b/lib/dns/include/dns/compress.h index 81e70f9e767..b5930617be8 100644 --- a/lib/dns/include/dns/compress.h +++ b/lib/dns/include/dns/compress.h @@ -34,44 +34,73 @@ ISC_LANG_BEGINDECLS * * The nameserver can be configured not to use compression at all using * \c dns_compress_disable(). + * + * DNS name compression only needs exact matches on (suffixes of) names. We + * could use a data structure that supports longest-match lookups, but that + * would introduce a lot of heavyweight machinery, and all we need is + * something that exists very briefly to store a few names before it is + * thrown away. + * + * In the abstract we need a map from DNS names to compression offsets. But + * a compression offset refers to a point in the message where the name has + * been written. So in fact all we need is a hash set of compression offsets. + * + * Typical messages do not contain more than a few dozen names, so by + * default our hash set is small (64 entries, 256 bytes). It can be + * enlarged when a message is likely to contain a lot of names, such as for + * outgoing zone transfers (which are handled in lib/ns/xfrout.c) and + * update requests (for which nsupdate uses DNS_REQUESTOPT_LARGE - see + * request.h). + */ + +/* + * Logarithms of hash set sizes. In the usual (small) case, allow for for a + * few dozen names in the hash set. (We can't actually use every slot because + * space is reserved for performance reasons.) For large messages, the number + * of names is limited by the minimum size of an RR (owner, type, class, ttl, + * length) which is 16 bytes when the owner has a new 3-character label + * before the compressed zone name. Divide the maximum compression offset + * 0x3FFF by 16 and you get roughly 1024. + */ +enum { + DNS_COMPRESS_SMALLBITS = 6, + DNS_COMPRESS_LARGEBITS = 10, +}; + +/* + * Compression context flags */ +enum dns_compress_flags { + /* affecting the whole message */ + DNS_COMPRESS_DISABLED = 0x00000001U, + DNS_COMPRESS_CASE = 0x00000002U, + DNS_COMPRESS_LARGE = 0x00000004U, + /* can toggle while rendering a message */ + DNS_COMPRESS_PERMITTED = 0x00000008U, +}; /* - * DNS_COMPRESS_TABLESIZE must be a power of 2. The compress code - * utilizes this assumption. + * The hash may be any 16 bit value. Unused slots have coff == 0. (Valid + * compression offsets cannot be zero because of the DNS message header.) */ -#define DNS_COMPRESS_TABLEBITS 6 -#define DNS_COMPRESS_TABLESIZE (1U << DNS_COMPRESS_TABLEBITS) -#define DNS_COMPRESS_TABLEMASK (DNS_COMPRESS_TABLESIZE - 1) -#define DNS_COMPRESS_INITIALNODES 24 -#define DNS_COMPRESS_ARENA_SIZE 640 - -typedef struct dns_compressnode dns_compressnode_t; - -struct dns_compressnode { - dns_compressnode_t *next; - uint16_t offset; - uint16_t count; - isc_region_t r; - dns_name_t name; +struct dns_compress_slot { + uint16_t hash; + uint16_t coff; }; struct dns_compress { - unsigned int magic; /*%< Magic number. */ - bool permitted; - bool disabled; - bool sensitive; - /*% Compression pointer table. */ - dns_compressnode_t *table[DNS_COMPRESS_TABLESIZE]; - /*% Preallocated arena for names. */ - unsigned char arena[DNS_COMPRESS_ARENA_SIZE]; - off_t arena_off; - /*% Preallocated nodes for the table. */ - dns_compressnode_t initialnodes[DNS_COMPRESS_INITIALNODES]; - uint16_t count; /*%< Number of nodes. */ - isc_mem_t *mctx; /*%< Memory context. */ + unsigned int magic; + dns_compress_flags_t flags; + uint16_t mask; + uint16_t count; + isc_mem_t *mctx; + dns_compress_slot_t *set; + dns_compress_slot_t smallset[1 << DNS_COMPRESS_SMALLBITS]; }; +/* + * Deompression context + */ enum dns_decompress { DNS_DECOMPRESS_DEFAULT, DNS_DECOMPRESS_PERMITTED, @@ -79,40 +108,45 @@ enum dns_decompress { DNS_DECOMPRESS_ALWAYS, }; -isc_result_t -dns_compress_init(dns_compress_t *cctx, isc_mem_t *mctx); +void +dns_compress_init(dns_compress_t *cctx, isc_mem_t *mctx, + dns_compress_flags_t flags); /*%< * Initialise the compression context structure pointed to by - * 'cctx'. A freshly initialized context has name compression - * enabled, but no methods are set. Please use \c - * dns_compress_setmethods() to set a compression method. + * 'cctx'. + * + * The `flags` argument is usually zero; or some combination of: + *\li DNS_COMPRESS_DISABLED, so the whole message is uncompressed + *\li DNS_COMPRESS_CASE, for case-sensitive compression + *\li DNS_COMPRESS_LARGE, for messages with many names + * + * (See also dns_request_create()'s options argument) * * Requires: - * \li 'cctx' is a valid dns_compress_t structure. - * \li 'mctx' is an initialized memory context. + *\li 'cctx' is a dns_compress_t structure on the stack. + *\li 'mctx' is an initialized memory context. * Ensures: - * \li 'cctx' is initialized. - * \li 'cctx->permitted' is true. - * - * Returns: - * \li #ISC_R_SUCCESS + *\li 'cctx' is initialized. + *\li 'dns_compress_getpermitted(cctx)' is true */ void dns_compress_invalidate(dns_compress_t *cctx); /*%< - * Invalidate the compression structure pointed to by cctx. + * Invalidate the compression structure pointed to by + * 'cctx', freeing any memory that has been allocated. * * Requires: - *\li 'cctx' to be initialized. + *\li 'cctx' is an initialized dns_compress_t */ void dns_compress_setpermitted(dns_compress_t *cctx, bool permitted); /*%< - * Sets whether compression is allowed, according to RFC 3597 + * Sets whether compression is allowed, according to RFC 3597. + * This can vary depending on the rdata type. * * Requires: *\li 'cctx' to be initialized. @@ -122,7 +156,7 @@ bool dns_compress_getpermitted(dns_compress_t *cctx); /*%< - * Gets allowed compression methods. + * Find out whether compression is allowed, according to RFC 3597. * * Requires: *\li 'cctx' to be initialized. @@ -132,74 +166,38 @@ dns_compress_getpermitted(dns_compress_t *cctx); */ void -dns_compress_disable(dns_compress_t *cctx); +dns_compress_name(dns_compress_t *cctx, isc_buffer_t *buffer, + const dns_name_t *name, unsigned int *return_prefix, + unsigned int *return_coff); /*%< - * Disables all name compression in the context. Once disabled, - * name compression cannot currently be re-enabled. + * Finds longest suffix matching 'name' in the compression table, + * and adds any remaining prefix of 'name' to the table. * - * Requires: - *\li 'cctx' to be initialized. - * - */ - -void -dns_compress_setsensitive(dns_compress_t *cctx, bool sensitive); - -/* - * Preserve the case of compressed domain names. - * - * Requires: - * 'cctx' to be initialized. - */ - -bool -dns_compress_getsensitive(dns_compress_t *cctx); -/* - * Return whether case is to be preserved when compressing - * domain names. - * - * Requires: - * 'cctx' to be initialized. - */ - -bool -dns_compress_find(dns_compress_t *cctx, const dns_name_t *name, - dns_name_t *prefix, uint16_t *offset); -/*%< - * Finds longest possible match of 'name' in the compression table. + * This is used by dns_name_towire() for both compressed and uncompressed + * names; for uncompressed names, dns_name_towire() does not need to know + * about the matching suffix, but it still needs to add the name for use + * by later compression pointers. For example, an owner name of a record + * in the additional section will often need to refer back to an RFC 3597 + * uncompressed name in the rdata of a record in the answer section. * * Requires: *\li 'cctx' to be initialized. + *\li 'buffer' contains the rendered message. *\li 'name' to be a absolute name. - *\li 'prefix' to be initialized. - *\li 'offset' to point to an uint16_t. + *\li 'return_prefix' points to an unsigned int. + *\li 'return_coff' points to an unsigned int, which must be zero. * * Ensures: - *\li 'prefix' and 'offset' are valid if true is returned. - * - * Returns: - *\li #true / #false - */ - -void -dns_compress_add(dns_compress_t *cctx, const dns_name_t *name, - const dns_name_t *prefix, uint16_t offset); -/*%< - * Add compression pointers for 'name' to the compression table, - * not replacing existing pointers. - * - * Requires: - *\li 'cctx' initialized - * - *\li 'name' must be initialized and absolute, and must remain - * valid until the message compression is complete. + *\li When no suffix is found, the return variables + * 'return_prefix' and 'return_coff' are unchanged * - *\li 'prefix' must be a prefix returned by - * dns_compress_find(), or the same as 'name'. + *\li Otherwise, '*return_prefix' is set to the length of the + * prefix of the name that did not match, and '*suffix_coff' + * is set to a nonzero compression offset of the match. */ void -dns_compress_rollback(dns_compress_t *cctx, uint16_t offset); +dns_compress_rollback(dns_compress_t *cctx, unsigned int offset); /*%< * Remove any compression pointers from the table that are >= offset. * diff --git a/lib/dns/include/dns/request.h b/lib/dns/include/dns/request.h index 5f72aef76e4..29624c72fc2 100644 --- a/lib/dns/include/dns/request.h +++ b/lib/dns/include/dns/request.h @@ -44,6 +44,7 @@ #define DNS_REQUESTOPT_TCP 0x00000001U #define DNS_REQUESTOPT_CASE 0x00000002U #define DNS_REQUESTOPT_FIXEDID 0x00000004U +#define DNS_REQUESTOPT_LARGE 0x00000008U typedef struct dns_requestevent { ISC_EVENT_COMMON(struct dns_requestevent); @@ -152,6 +153,9 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, *\li If the #DNS_REQUESTOPT_CASE option is set, use case sensitive * compression. * + *\li If the #DNS_REQUESTOPT_LARGE option is set, use a large + * compression context to accommodate more names. + * *\li When the request completes, successfully, due to a timeout, or * because it was canceled, a completion event will be sent to 'task'. * diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index b117018b7ff..04aa86ea8b1 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -53,6 +53,8 @@ typedef void dns_clientupdatetrans_t; typedef struct dns_cache dns_cache_t; typedef uint16_t dns_cert_t; typedef struct dns_compress dns_compress_t; +typedef enum dns_compress_flags dns_compress_flags_t; +typedef struct dns_compress_slot dns_compress_slot_t; typedef struct dns_db dns_db_t; typedef struct dns_dbimplementation dns_dbimplementation_t; typedef struct dns_dbiterator dns_dbiterator_t; diff --git a/lib/dns/message.c b/lib/dns/message.c index fddb791ee7b..ca72b6580f2 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -2012,9 +2012,7 @@ dns_message_rendersection(dns_message_t *msg, dns_section_t sectionid, msg->flags |= DNS_MESSAGEFLAG_TC; } if (result != ISC_R_SUCCESS) { - INSIST(st.used < 65536); - dns_compress_rollback(msg->cctx, - (uint16_t)st.used); + dns_compress_rollback(msg->cctx, st.used); *(msg->buffer) = st; /* rollback */ msg->buffer->length += msg->reserved; msg->counts[sectionid] += total; diff --git a/lib/dns/name.c b/lib/dns/name.c index 72fb8b4e2fd..d6158483211 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -1703,14 +1703,13 @@ dns_name_towire(const dns_name_t *name, dns_compress_t *cctx, isc_result_t dns_name_towire2(const dns_name_t *name, dns_compress_t *cctx, - isc_buffer_t *target, uint16_t *comp_offsetp) { + isc_buffer_t *target, uint16_t *name_coff) { bool compress; - bool found; - uint16_t here; /* start of the name we are adding to the message */ - uint16_t there; /* target of the compression pointer */ - dns_name_t prefix; dns_offsets_t clo; dns_name_t clname; + unsigned int here; + unsigned int prefix_length; + unsigned int suffix_coff; /* * Convert 'name' into wire format, compressing it as specified by the @@ -1725,103 +1724,60 @@ dns_name_towire2(const dns_name_t *name, dns_compress_t *cctx, dns_compress_getpermitted(cctx); /* - * If this exact name was already rendered before, and the - * offset of the previously rendered name is passed to us, write - * a compression pointer directly. + * Write a compression pointer directly if the caller passed us + * a pointer to this name's offset that we saved previously. */ - if (comp_offsetp != NULL && *comp_offsetp < 0x4000 && compress) { - if (target->length - target->used < 2) { + if (compress && name_coff != NULL && *name_coff < 0x4000) { + if (isc_buffer_availablelength(target) < 2) { return (ISC_R_NOSPACE); } - here = *comp_offsetp; - isc_buffer_putuint16(target, here | 0xc000); + isc_buffer_putuint16(target, *name_coff | 0xc000); return (ISC_R_SUCCESS); } - /* - * If 'name' doesn't have an offsets table, make a clone which - * has one. - */ if (name->offsets == NULL) { DNS_NAME_INIT(&clname, clo); dns_name_clone(name, &clname); name = &clname; } - DNS_NAME_INIT(&prefix, NULL); - - here = target->used; /*XXX*/ - - /* - * Never compress the root name. - */ - if (name->length == 1) { - found = false; - compress = false; - } else { - found = dns_compress_find(cctx, name, &prefix, &there); - } /* - * If the offset does not fit in a 14 bit compression pointer, - * we're out of luck. + * Always add the name to the compression context; if compression + * is off, reset the return values before writing the name. */ - if (found && there >= 0x4000) { - compress = false; + prefix_length = name->length; + suffix_coff = 0; + dns_compress_name(cctx, target, name, &prefix_length, &suffix_coff); + if (!compress) { + prefix_length = name->length; + suffix_coff = 0; } /* - * Will the compression pointer reduce the message size? + * Return this name's compression offset for use next time, provided + * it isn't too short for compression to help (i.e. it's the root) */ - if (found && (prefix.length + 2) >= name->length) { - compress = false; + here = isc_buffer_usedlength(target); + if (name_coff != NULL && here < 0x4000 && prefix_length > 1) { + *name_coff = (uint16_t)here; } - if (found && compress) { - if (target->length - target->used < prefix.length) { + if (prefix_length > 0) { + if (isc_buffer_availablelength(target) < prefix_length) { return (ISC_R_NOSPACE); } - if (prefix.length != 0) { - unsigned char *base = target->base; - (void)memmove(base + target->used, prefix.ndata, - (size_t)prefix.length); - } - isc_buffer_add(target, prefix.length); - if (target->length - target->used < 2) { - return (ISC_R_NOSPACE); + memmove(isc_buffer_used(target), name->ndata, prefix_length); + isc_buffer_add(target, prefix_length); + } + + if (suffix_coff > 0) { + if (name_coff != NULL && prefix_length == 0) { + *name_coff = suffix_coff; } - isc_buffer_putuint16(target, there | 0xc000); - } else { - if (target->length - target->used < name->length) { + if (isc_buffer_availablelength(target) < 2) { return (ISC_R_NOSPACE); } - if (name->length != 0) { - unsigned char *base = target->base; - (void)memmove(base + target->used, name->ndata, - (size_t)name->length); - } - isc_buffer_add(target, name->length); - } - - if (found && prefix.length == 0) { - here = there; - } - - if (here >= 0x4000) { - return (ISC_R_SUCCESS); - } - - if (found) { - dns_compress_add(cctx, name, &prefix, here); - } else { - dns_compress_add(cctx, name, name, here); - } - - /* - * Don't set the offset of the previously rendered name if the - * compression has been disabled. - */ - if (compress && comp_offsetp != NULL) { - *comp_offsetp = here; + isc_buffer_putuint16(target, suffix_coff | 0xc000); } return (ISC_R_SUCCESS); diff --git a/lib/dns/ncache.c b/lib/dns/ncache.c index 6b8b939176e..c97d31bf35c 100644 --- a/lib/dns/ncache.c +++ b/lib/dns/ncache.c @@ -408,8 +408,7 @@ dns_ncache_towire(dns_rdataset_t *rdataset, dns_compress_t *cctx, return (ISC_R_SUCCESS); rollback: - INSIST(savedbuffer.used < 65536); - dns_compress_rollback(cctx, (uint16_t)savedbuffer.used); + dns_compress_rollback(cctx, savedbuffer.used); *countp = 0; *target = savedbuffer; diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c index ccc79b1e464..cd1ee659ea2 100644 --- a/lib/dns/rdata.c +++ b/lib/dns/rdata.c @@ -897,8 +897,7 @@ dns_rdata_towire(dns_rdata_t *rdata, dns_compress_t *cctx, } if (result != ISC_R_SUCCESS) { *target = st; - INSIST(target->used < 65536); - dns_compress_rollback(cctx, (uint16_t)target->used); + dns_compress_rollback(cctx, target->used); } return (result); } diff --git a/lib/dns/rdataset.c b/lib/dns/rdataset.c index a2274e729de..edd56baa604 100644 --- a/lib/dns/rdataset.c +++ b/lib/dns/rdataset.c @@ -525,14 +525,12 @@ towiresorted(dns_rdataset_t *rdataset, const dns_name_t *owner_name, rollback: if (partial && result == ISC_R_NOSPACE) { - INSIST(rrbuffer.used < 65536); - dns_compress_rollback(cctx, (uint16_t)rrbuffer.used); + dns_compress_rollback(cctx, rrbuffer.used); *countp += added; *target = rrbuffer; goto cleanup; } - INSIST(savedbuffer.used < 65536); - dns_compress_rollback(cctx, (uint16_t)savedbuffer.used); + dns_compress_rollback(cctx, savedbuffer.used); *countp = 0; *target = savedbuffer; diff --git a/lib/dns/request.c b/lib/dns/request.c index 0a53f2b6205..1ceb1e741cf 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -748,7 +748,7 @@ req_render(dns_message_t *message, isc_buffer_t **bufferp, unsigned int options, isc_result_t result; isc_region_t r; dns_compress_t cctx; - bool cleanup_cctx = false; + unsigned int compflags; REQUIRE(bufferp != NULL && *bufferp == NULL); @@ -759,15 +759,14 @@ req_render(dns_message_t *message, isc_buffer_t **bufferp, unsigned int options, */ isc_buffer_allocate(mctx, &buf1, 65535); - result = dns_compress_init(&cctx, mctx); - if (result != ISC_R_SUCCESS) { - return (result); + compflags = 0; + if ((options & DNS_REQUESTOPT_LARGE) != 0) { + compflags |= DNS_COMPRESS_LARGE; } - cleanup_cctx = true; - if ((options & DNS_REQUESTOPT_CASE) != 0) { - dns_compress_setsensitive(&cctx, true); + compflags |= DNS_COMPRESS_CASE; } + dns_compress_init(&cctx, mctx, compflags); /* * Render message. @@ -797,9 +796,6 @@ req_render(dns_message_t *message, isc_buffer_t **bufferp, unsigned int options, goto cleanup; } - dns_compress_invalidate(&cctx); - cleanup_cctx = false; - /* * Copy rendered message to exact sized buffer. */ @@ -817,21 +813,20 @@ req_render(dns_message_t *message, isc_buffer_t **bufferp, unsigned int options, /* * Cleanup and return. */ + dns_compress_invalidate(&cctx); isc_buffer_free(&buf1); *bufferp = buf2; return (ISC_R_SUCCESS); cleanup: dns_message_renderreset(message); + dns_compress_invalidate(&cctx); if (buf1 != NULL) { isc_buffer_free(&buf1); } if (buf2 != NULL) { isc_buffer_free(&buf2); } - if (cleanup_cctx) { - dns_compress_invalidate(&cctx); - } return (result); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 6fc26b2d5ce..756878729df 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2493,7 +2493,6 @@ resquery_send(resquery_t *query) { dns_tsigkey_t *tsigkey = NULL; dns_peer_t *peer = NULL; dns_compress_t cctx; - bool cleanup_cctx = false; bool useedns; bool secure_domain; bool tcp = ((query->options & DNS_FETCHOPT_TCP) != 0); @@ -2572,11 +2571,7 @@ resquery_send(resquery_t *query) { /* * Convert the question to wire format. */ - result = dns_compress_init(&cctx, fctx->res->mctx); - if (result != ISC_R_SUCCESS) { - goto cleanup_message; - } - cleanup_cctx = true; + dns_compress_init(&cctx, fctx->res->mctx, 0); isc_buffer_init(&buffer, query->data, sizeof(query->data)); result = dns_message_renderbegin(fctx->qmessage, &cctx, &buffer); @@ -2847,9 +2842,6 @@ resquery_send(resquery_t *query) { } #endif /* HAVE_DNSTAP */ - dns_compress_invalidate(&cctx); - cleanup_cctx = false; - if (dns_message_gettsigkey(fctx->qmessage) != NULL) { dns_tsigkey_attach(dns_message_gettsigkey(fctx->qmessage), &query->tsigkey); @@ -2871,6 +2863,7 @@ resquery_send(resquery_t *query) { /* * We're now done with the query message. */ + dns_compress_invalidate(&cctx); dns_message_reset(fctx->qmessage, DNS_MESSAGE_INTENTRENDER); isc_buffer_usedregion(&buffer, &r); @@ -2902,9 +2895,7 @@ resquery_send(resquery_t *query) { return (ISC_R_SUCCESS); cleanup_message: - if (cleanup_cctx) { - dns_compress_invalidate(&cctx); - } + dns_compress_invalidate(&cctx); dns_message_reset(fctx->qmessage, DNS_MESSAGE_INTENTRENDER); @@ -9903,16 +9894,14 @@ rctx_logpacket(respctx_t *rctx) { * Log the response via dnstap. */ memset(&zr, 0, sizeof(zr)); - result = dns_compress_init(&cctx, fctx->res->mctx); + dns_compress_init(&cctx, fctx->res->mctx, 0); + dns_compress_setpermitted(&cctx, false); + isc_buffer_init(&zb, zone, sizeof(zone)); + result = dns_name_towire(fctx->domain, &cctx, &zb); if (result == ISC_R_SUCCESS) { - isc_buffer_init(&zb, zone, sizeof(zone)); - dns_compress_setpermitted(&cctx, false); - result = dns_name_towire(fctx->domain, &cctx, &zb); - if (result == ISC_R_SUCCESS) { - isc_buffer_usedregion(&zb, &zr); - } - dns_compress_invalidate(&cctx); + isc_buffer_usedregion(&zb, &zr); } + dns_compress_invalidate(&cctx); if ((fctx->qmessage->flags & DNS_MESSAGEFLAG_RD) != 0) { dtmsgtype = DNS_DTTYPE_FR; diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index c42574d2a94..91dd4993c9c 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -981,11 +981,9 @@ failure: static isc_result_t render(dns_message_t *msg, isc_mem_t *mctx, isc_buffer_t *buf) { dns_compress_t cctx; - bool cleanup_cctx = false; isc_result_t result; - CHECK(dns_compress_init(&cctx, mctx)); - cleanup_cctx = true; + dns_compress_init(&cctx, mctx, 0); CHECK(dns_message_renderbegin(msg, &cctx, buf)); CHECK(dns_message_rendersection(msg, DNS_SECTION_QUESTION, 0)); CHECK(dns_message_rendersection(msg, DNS_SECTION_ANSWER, 0)); @@ -994,9 +992,7 @@ render(dns_message_t *msg, isc_mem_t *mctx, isc_buffer_t *buf) { CHECK(dns_message_renderend(msg)); result = ISC_R_SUCCESS; failure: - if (cleanup_cctx) { - dns_compress_invalidate(&cctx); - } + dns_compress_invalidate(&cctx); return (result); } diff --git a/lib/ns/client.c b/lib/ns/client.c index 60da425efb1..4c1f90801bf 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -464,6 +464,7 @@ ns_client_send(ns_client_t *client) { isc_buffer_t buffer = { .magic = 0 }; isc_region_t r; dns_compress_t cctx; + unsigned int compflags; bool cleanup_cctx = false; unsigned int render_opts; unsigned int preferred_glue; @@ -531,11 +532,7 @@ ns_client_send(ns_client_t *client) { } client_allocsendbuf(client, &buffer, &data); - - result = dns_compress_init(&cctx, client->manager->mctx); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + compflags = 0; if (client->peeraddr_valid && client->view != NULL) { isc_netaddr_t netaddr; dns_name_t *name = NULL; @@ -549,13 +546,14 @@ ns_client_send(ns_client_t *client) { !dns_acl_allowed(&netaddr, name, client->view->nocasecompress, env)) { - dns_compress_setsensitive(&cctx, true); + compflags |= DNS_COMPRESS_CASE; } if (!client->view->msgcompression) { - dns_compress_disable(&cctx); + compflags = DNS_COMPRESS_DISABLED; } } + dns_compress_init(&cctx, client->manager->mctx, compflags); cleanup_cctx = true; result = dns_message_renderbegin(client->message, &cctx, &buffer); diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index b41c89a6213..d2c059a35bd 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -1525,8 +1525,7 @@ sendstream(xfrout_ctx_t *xfr) { if (is_tcp) { isc_region_t used; - CHECK(dns_compress_init(&cctx, xfr->mctx)); - dns_compress_setsensitive(&cctx, true); + dns_compress_init(&cctx, xfr->mctx, DNS_COMPRESS_CASE); cleanup_cctx = true; CHECK(dns_message_renderbegin(msg, &cctx, &xfr->txbuf)); CHECK(dns_message_rendersection(msg, DNS_SECTION_QUESTION, 0)); diff --git a/tests/dns/dnstap_test.c b/tests/dns/dnstap_test.c index 716f986f4ca..c6959189639 100644 --- a/tests/dns/dnstap_test.c +++ b/tests/dns/dnstap_test.c @@ -171,8 +171,7 @@ ISC_RUN_TEST_IMPL(dns_dt_send) { memset(&zr, 0, sizeof(zr)); isc_buffer_init(&zb, zone, sizeof(zone)); - result = dns_compress_init(&cctx, mctx); - assert_int_equal(result, ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, false); result = dns_name_towire(zname, &cctx, &zb); assert_int_equal(result, ISC_R_SUCCESS); diff --git a/tests/dns/name_test.c b/tests/dns/name_test.c index 37fb2a4577e..1fe4d802daa 100644 --- a/tests/dns/name_test.c +++ b/tests/dns/name_test.c @@ -136,6 +136,13 @@ compress_test(const dns_name_t *name1, const dns_name_t *name2, isc_buffer_init(&source, buf1, sizeof(buf1)); isc_buffer_init(&target, buf2, sizeof(buf2)); + /* + * compression offsets are not allowed to be zero so our + * names need to start after a little fake header + */ + isc_buffer_putuint16(&source, 0xEAD); + isc_buffer_putuint16(&target, 0xEAD); + if (rdata) { /* RDATA compression */ assert_int_equal(dns_name_towire(name1, cctx, &source), @@ -172,6 +179,7 @@ compress_test(const dns_name_t *name1, const dns_name_t *name2, isc_buffer_setactive(&source, source.used); dns_name_init(&name, NULL); + RUNTIME_CHECK(isc_buffer_getuint16(&source) == 0xEAD); RUNTIME_CHECK(dns_name_fromwire(&name, &source, dctx, 0, &target) == ISC_R_SUCCESS); RUNTIME_CHECK(dns_name_fromwire(&name, &source, dctx, 0, &target) == @@ -193,26 +201,37 @@ ISC_RUN_TEST_IMPL(compression) { dns_name_t name1; dns_name_t name2; dns_name_t name3; + dns_name_t name4; isc_region_t r; unsigned char plain1[] = "\003yyy\003foo"; unsigned char plain2[] = "\003bar\003yyy\003foo"; unsigned char plain3[] = "\003xxx\003bar\003foo"; - unsigned char plain[] = "\003yyy\003foo\0\003bar\003yyy\003foo\0\003" - "bar\003yyy\003foo\0\003xxx\003bar\003foo"; - /* - * Note: foo in xxx.bar.foo is not compressed because dns_compress_find - * only looks for the name and name less the leading label. - */ - unsigned char compressed[] = "\003yyy\003foo\0\003bar\xc0\x00\xc0\x09" - "\003xxx\003bar\003foo"; + unsigned char plain4[] = "\003xxx\003bar\003zzz"; + + unsigned char plain[] = "\x0E\xAD" + "\003yyy\003foo\0" + "\003bar\003yyy\003foo\0" + "\003bar\003yyy\003foo\0" + "\003xxx\003bar\003foo"; + + unsigned char compressed[29] = "\x0E\xAD" + "\003yyy\003foo\0" + "\003bar\xc0\x02" + "\xc0\x0B" + "\003xxx\003bar\xc0\x06"; /* * Only the second owner name is compressed. */ - unsigned char disabled_owner[] = - "\003yyy\003foo\0\003bar\003yyy\003foo\0" - "\xc0\x09\003xxx\003bar\003foo"; - unsigned char root_plain[] = "\003yyy\003foo\0\0\0" - "\003xxx\003bar\003foo"; + unsigned char disabled_owner[] = "\x0E\xAD" + "\003yyy\003foo\0" + "\003bar\003yyy\003foo\0" + "\xc0\x0B" + "\003xxx\003bar\003foo"; + + unsigned char root_plain[] = "\x0E\xAD" + "\003yyy\003foo\0" + "\0\0" + "\003xxx\003bar\003zzz"; UNUSED(state); @@ -231,9 +250,14 @@ ISC_RUN_TEST_IMPL(compression) { r.length = sizeof(plain3); dns_name_fromregion(&name3, &r); + dns_name_init(&name4, NULL); + r.base = plain4; + r.length = sizeof(plain3); + dns_name_fromregion(&name4, &r); + /* Test 1: off, rdata */ permitted = false; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); @@ -245,7 +269,7 @@ ISC_RUN_TEST_IMPL(compression) { /* Test2: on, rdata */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); @@ -257,9 +281,8 @@ ISC_RUN_TEST_IMPL(compression) { /* Test3: off, disabled, rdata */ permitted = false; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, DNS_COMPRESS_DISABLED); dns_compress_setpermitted(&cctx, permitted); - dns_compress_disable(&cctx); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); compress_test(&name1, &name2, &name3, plain, sizeof(plain), plain, @@ -270,9 +293,8 @@ ISC_RUN_TEST_IMPL(compression) { /* Test4: on, disabled, rdata */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, DNS_COMPRESS_DISABLED); dns_compress_setpermitted(&cctx, permitted); - dns_compress_disable(&cctx); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); compress_test(&name1, &name2, &name3, plain, sizeof(plain), plain, @@ -283,11 +305,11 @@ ISC_RUN_TEST_IMPL(compression) { /* Test5: on, rdata */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); - compress_test(&name1, dns_rootname, &name3, root_plain, + compress_test(&name1, dns_rootname, &name4, root_plain, sizeof(root_plain), root_plain, sizeof(root_plain), &cctx, dctx, true); @@ -296,7 +318,7 @@ ISC_RUN_TEST_IMPL(compression) { /* Test 6: off, owner */ permitted = false; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); @@ -308,7 +330,7 @@ ISC_RUN_TEST_IMPL(compression) { /* Test7: on, owner */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); @@ -320,9 +342,8 @@ ISC_RUN_TEST_IMPL(compression) { /* Test8: off, disabled, owner */ permitted = false; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, DNS_COMPRESS_DISABLED); dns_compress_setpermitted(&cctx, permitted); - dns_compress_disable(&cctx); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); compress_test(&name1, &name2, &name3, plain, sizeof(plain), plain, @@ -333,9 +354,8 @@ ISC_RUN_TEST_IMPL(compression) { /* Test9: on, disabled, owner */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, DNS_COMPRESS_DISABLED); dns_compress_setpermitted(&cctx, permitted); - dns_compress_disable(&cctx); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); compress_test(&name1, &name2, &name3, disabled_owner, @@ -347,11 +367,11 @@ ISC_RUN_TEST_IMPL(compression) { /* Test10: on, owner */ permitted = true; - assert_int_equal(dns_compress_init(&cctx, mctx), ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); dns_compress_setpermitted(&cctx, permitted); dctx = dns_decompress_setpermitted(DNS_DECOMPRESS_DEFAULT, permitted); - compress_test(&name1, dns_rootname, &name3, root_plain, + compress_test(&name1, dns_rootname, &name4, root_plain, sizeof(root_plain), root_plain, sizeof(root_plain), &cctx, dctx, false); diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c index acd964e4ef1..5282122b063 100644 --- a/tests/dns/rdata_test.c +++ b/tests/dns/rdata_test.c @@ -173,7 +173,7 @@ rdata_towire(dns_rdata_t *rdata, unsigned char *dst, size_t dstlen, /* * Try converting input data into uncompressed wire form. */ - dns_compress_init(&cctx, mctx); + dns_compress_init(&cctx, mctx, 0); result = dns_rdata_towire(rdata, &cctx, &target); dns_compress_invalidate(&cctx); diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index 303ed94e372..bf180c6aaa6 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -109,12 +109,10 @@ add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { unsigned char tsigbuf[1024]; unsigned int count; unsigned int sigsize = 0; - bool invalidate_ctx = false; memset(&tsig, 0, sizeof(tsig)); - CHECK(dns_compress_init(&cctx, mctx)); - invalidate_ctx = true; + dns_compress_init(&cctx, mctx, 0); tsig.common.rdclass = dns_rdataclass_any; tsig.common.rdtype = dns_rdatatype_tsig; @@ -169,9 +167,7 @@ cleanup: if (dynbuf != NULL) { isc_buffer_free(&dynbuf); } - if (invalidate_ctx) { - dns_compress_invalidate(&cctx); - } + dns_compress_invalidate(&cctx); return (result); } @@ -239,8 +235,7 @@ render(isc_buffer_t *buf, unsigned int flags, dns_tsigkey_t *key, dns_message_setquerytsig(msg, *tsigin); } - result = dns_compress_init(&cctx, mctx); - assert_int_equal(result, ISC_R_SUCCESS); + dns_compress_init(&cctx, mctx, 0); result = dns_message_renderbegin(msg, &cctx, buf); assert_int_equal(result, ISC_R_SUCCESS); diff --git a/tests/libtest/ns.c b/tests/libtest/ns.c index e79b72d4e39..3b4bf7ba5dd 100644 --- a/tests/libtest/ns.c +++ b/tests/libtest/ns.c @@ -305,7 +305,7 @@ attach_query_msg_to_client(ns_client_t *client, const char *qnamestr, /* * Render the query. */ - dns_compress_init(&cctx, mctx); + dns_compress_init(&cctx, mctx, 0); isc_buffer_init(&querybuf, query, sizeof(query)); result = dns_message_renderbegin(message, &cctx, &querybuf); if (result != ISC_R_SUCCESS) {