Merge branch '3655-decompress-faster-v9_18' into 'v9_18'

Simplify and speed up DNS name decompression

See merge request isc-projects/bind9!7099
This commit is contained in:
Tony Finch 2022-11-21 13:43:41 +00:00
commit a2f4f9bb92
3 changed files with 145 additions and 157 deletions

View file

@ -1,3 +1,6 @@
6022. [performance] The decompression implementation in dns_name_fromwire()
is now smaller and faster. [GL #3655]
6021. [bug] Use the current domain name when checking answers from
a dual-stack-server. [GL #3607]

View file

@ -701,9 +701,6 @@ dns_name_fromwire(dns_name_t *name, isc_buffer_t *source,
* Notes:
* \li Decompression policy is controlled by 'dctx'.
*
* \li If DNS_NAME_DOWNCASE is set, any uppercase letters in 'source' will be
* downcased when they are copied into 'target'.
*
* Security:
*
* \li *** WARNING ***
@ -724,13 +721,12 @@ dns_name_fromwire(dns_name_t *name, isc_buffer_t *source,
*
* \li 'dctx' is a valid decompression context.
*
* \li DNS_NAME_DOWNCASE is not set.
*
* Ensures:
*
* If result is success:
* \li If 'target' is not NULL, 'name' is attached to it.
*
* \li Uppercase letters are downcased in the copy iff
* DNS_NAME_DOWNCASE is set in options.
* \li If 'target' is not NULL, 'name' is attached to it.
*
* \li The current location in source is advanced, and the used space
* in target is updated.
@ -743,7 +739,6 @@ dns_name_fromwire(dns_name_t *name, isc_buffer_t *source,
* \li Bad Form: Compression type not allowed
* \li Bad Form: Bad compression pointer
* \li Bad Form: Input too short
* \li Resource Limit: Too many compression pointers
* \li Resource Limit: Not enough space in buffer
*/

View file

@ -1735,185 +1735,175 @@ set_offsets(const dns_name_t *name, unsigned char *offsets,
}
isc_result_t
dns_name_fromwire(dns_name_t *name, isc_buffer_t *source,
dns_decompress_t *dctx, unsigned int options,
dns_name_fromwire(dns_name_t *const name, isc_buffer_t *const source,
dns_decompress_t *const dctx, unsigned int options,
isc_buffer_t *target) {
unsigned char *cdata, *ndata;
unsigned int cused; /* Bytes of compressed name data used */
unsigned int nused, labels, n, nmax;
unsigned int current, new_current, biggest_pointer;
bool done;
fw_state state = fw_start;
unsigned int c;
unsigned char *offsets;
dns_offsets_t odata;
bool downcase;
bool seen_pointer;
/*
* Copy the possibly-compressed name at source into target,
* decompressing it. Loop prevention is performed by checking
* the new pointer against biggest_pointer.
* Copy the name at source into target, decompressing it.
*
* *** WARNING ***
*
* dns_name_fromwire() deals with raw network data. An error in this
* routine could result in the failure or hijacking of the server.
*
* The description of name compression in RFC 1035 section 4.1.4 is
* subtle wrt certain edge cases. The first important sentence is:
*
* > In this scheme, an entire domain name or a list of labels at the
* > end of a domain name is replaced with a pointer to a prior
* > occurance of the same name.
*
* The key word is "prior". This says that compression pointers must
* point strictly earlier in the message (before our "marker" variable),
* which is enough to prevent DoS attacks due to compression loops.
*
* The next important sentence is:
*
* > If a domain name is contained in a part of the message subject to a
* > length field (such as the RDATA section of an RR), and compression
* > is used, the length of the compressed name is used in the length
* > calculation, rather than the length of the expanded name.
*
* When decompressing, this means that the amount of the source buffer
* that we consumed (which is checked wrt the container's length field)
* is the length of the compressed name. A compressed name is defined as
* a sequence of labels ending with the root label or a compression
* pointer, that is, the segment of the name that dns_name_fromwire()
* examines first.
*
* This matters when handling names that play dirty tricks, like:
*
* +---+---+---+---+---+---+
* | 4 | 1 |'a'|192| 0 | 0 |
* +---+---+---+---+---+---+
*
* We start at octet 1. There is an ordinary single character label "a",
* followed by a compression pointer that refers back to octet zero.
* Here there is a label of length 4, which weirdly re-uses the octets
* we already examined as the data for the label. It is followed by the
* root label,
*
* The specification says that the compressed name ends after the first
* zero octet (after the compression pointer) not the second zero octet,
* even though the second octet is later in the message. This shows the
* correct way to set our "consumed" variable.
*/
REQUIRE((options & DNS_NAME_DOWNCASE) == 0);
REQUIRE(VALID_NAME(name));
REQUIRE(BINDABLE(name));
REQUIRE(dctx != NULL);
REQUIRE((target != NULL && ISC_BUFFER_VALID(target)) ||
(target == NULL && ISC_BUFFER_VALID(name->buffer)));
downcase = ((options & DNS_NAME_DOWNCASE) != 0);
if (target == NULL && name->buffer != NULL) {
target = name->buffer;
isc_buffer_clear(target);
}
REQUIRE(dctx != NULL);
REQUIRE(BINDABLE(name));
uint8_t *const name_buf = isc_buffer_used(target);
const uint32_t name_max = ISC_MIN(DNS_NAME_MAXWIRE,
isc_buffer_availablelength(target));
uint32_t name_len = 0;
MAKE_EMPTY(name); /* in case of failure */
dns_offsets_t odata;
uint8_t *offsets = NULL;
uint32_t labels = 0;
INIT_OFFSETS(name, offsets, odata);
/*
* Make 'name' empty in case of failure.
* After chasing a compression pointer, these variables refer to the
* source buffer as follows:
*
* sb --- mr --- cr --- st --- cd --- sm
*
* sb = source_buf (const)
* mr = marker
* cr = cursor
* st = start (const)
* cd = consumed
* sm = source_max (const)
*
* The marker hops backwards for each pointer.
* The cursor steps forwards for each label.
* The amount of the source we consumed is set once.
*/
MAKE_EMPTY(name);
const uint8_t *const source_buf = isc_buffer_base(source);
const uint8_t *const source_max = isc_buffer_used(source);
const uint8_t *const start = isc_buffer_current(source);
const uint8_t *marker = start;
const uint8_t *cursor = start;
const uint8_t *consumed = NULL;
/*
* Initialize things to make the compiler happy; they're not required.
* One iteration per label.
*/
n = 0;
new_current = 0;
/*
* Set up.
*/
labels = 0;
done = false;
ndata = isc_buffer_used(target);
nused = 0;
seen_pointer = false;
/*
* Find the maximum number of uncompressed target name
* bytes we are willing to generate. This is the smaller
* of the available target buffer length and the
* maximum legal domain name length (255).
*/
nmax = isc_buffer_availablelength(target);
if (nmax > DNS_NAME_MAXWIRE) {
nmax = DNS_NAME_MAXWIRE;
}
cdata = isc_buffer_current(source);
cused = 0;
current = source->current;
biggest_pointer = current;
/*
* Note: The following code is not optimized for speed, but
* rather for correctness. Speed will be addressed in the future.
*/
while (current < source->active && !done) {
c = *cdata++;
current++;
if (!seen_pointer) {
cused++;
}
switch (state) {
case fw_start:
if (c < 64) {
offsets[labels] = nused;
labels++;
if (nused + c + 1 > nmax) {
goto full;
}
nused += c + 1;
*ndata++ = c;
if (c == 0) {
done = true;
}
n = c;
state = fw_ordinary;
} else if (c >= 128 && c < 192) {
/*
* 14 bit local compression pointer.
* Local compression is no longer an
* IETF draft.
*/
return (DNS_R_BADLABELTYPE);
} else if (c >= 192) {
/*
* Ordinary 14-bit pointer.
*/
if ((dctx->allowed & DNS_COMPRESS_GLOBAL14) ==
0) {
return (DNS_R_DISALLOWED);
}
new_current = c & 0x3F;
state = fw_newcurrent;
} else {
return (DNS_R_BADLABELTYPE);
while (cursor < source_max) {
const uint8_t label_len = *cursor++;
if (label_len < 64) {
/*
* Normal label: record its offset, and check bounds on
* the name length, which also ensures we don't overrun
* the offsets array. Don't touch any source bytes yet!
* The source bounds check will happen when we loop.
*/
offsets[labels++] = name_len;
/* and then a step to the ri-i-i-i-i-ight */
cursor += label_len;
name_len += label_len + 1;
if (name_len > name_max) {
return (name_max == DNS_NAME_MAXWIRE
? DNS_R_NAMETOOLONG
: ISC_R_NOSPACE);
} else if (label_len == 0) {
goto root_label;
}
break;
case fw_ordinary:
if (downcase) {
c = maptolower[c];
}
*ndata++ = c;
n--;
if (n == 0) {
state = fw_start;
}
break;
case fw_newcurrent:
new_current *= 256;
new_current += c;
if (new_current >= biggest_pointer) {
} else if (label_len < 192) {
return (DNS_R_BADLABELTYPE);
} else if ((dctx->allowed & DNS_COMPRESS_GLOBAL14) == 0) {
return (DNS_R_DISALLOWED);
} else if (cursor < source_max) {
/*
* Compression pointer. Ensure it does not loop.
*
* Copy multiple labels in one go, to make the most of
* memmove() performance. Start at the marker and finish
* just before the pointer's hi+lo bytes, before the
* cursor. Bounds were already checked.
*/
const uint32_t hi = label_len & 0x3F;
const uint32_t lo = *cursor++;
const uint8_t *pointer = source_buf + (256 * hi + lo);
if (pointer >= marker) {
return (DNS_R_BADPOINTER);
}
biggest_pointer = new_current;
current = new_current;
cdata = (unsigned char *)source->base + current;
seen_pointer = true;
state = fw_start;
break;
default:
FATAL_ERROR("Unknown state %d", state);
/* Does not return. */
const uint32_t copy_len = (cursor - 2) - marker;
uint8_t *const dest = name_buf + name_len - copy_len;
memmove(dest, marker, copy_len);
consumed = consumed != NULL ? consumed : cursor;
/* it's just a jump to the left */
cursor = marker = pointer;
}
}
return (ISC_R_UNEXPECTEDEND);
root_label:;
/*
* Copy labels almost like we do for compression pointers,
* from the marker up to and including the root label.
*/
const uint32_t copy_len = cursor - marker;
memmove(name_buf + name_len - copy_len, marker, copy_len);
consumed = consumed != NULL ? consumed : cursor;
isc_buffer_forward(source, consumed - start);
if (!done) {
return (ISC_R_UNEXPECTEDEND);
}
name->ndata = (unsigned char *)target->base + target->used;
name->labels = labels;
name->length = nused;
name->attributes |= DNS_NAMEATTR_ABSOLUTE;
isc_buffer_forward(source, cused);
isc_buffer_add(target, name->length);
name->ndata = name_buf;
name->labels = labels;
name->length = name_len;
isc_buffer_add(target, name_len);
return (ISC_R_SUCCESS);
full:
if (nmax == DNS_NAME_MAXWIRE) {
/*
* The name did not fit even though we had a buffer
* big enough to fit a maximum-length name.
*/
return (DNS_R_NAMETOOLONG);
} else {
/*
* The name might fit if only the caller could give us a
* big enough buffer.
*/
return (ISC_R_NOSPACE);
}
}
isc_result_t