From 338cf3e4677e1ad793c5ff789b0b3a4d7e453426 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 13 Jun 2022 19:20:08 +0300 Subject: [PATCH] Add isc_dnsstream_assembler_t implementation This commit adds the implementation for an "isc_dnsstream_assembler_t" object. The object is built on top of "isc_dnsbuffer_t" and is intended to encapsulate the state machine used for handling DNS messages received in the format used for messages transmitted over TCP. The idea is that the object accepts the input data received from a socket, tries to assemble DNS messages from the incoming data and calls the callback which contains the status of the incoming data as well as a pointer to the memory region referencing the data of the assembled message. It is capable of assembling DNS messages no matter how torn apart they are when sent over network. The following statuses might be passed to the callback: * ISC_R_SUCCESS - a message has been successfully assembled; * ISC_R_NOMORE - not enough data has been processed to assemble a message; * ISC_R_RANGE - there was an attempt to process a zero-sized DNS message (someone attempts to send us junk data). One could say that the object replaces the implementation of "isc__nm_*_processbuffer()" functions used by the old TCP DNS and TLS DNS transports with a better defined state machine completely decoupled from the networking code itself. Such a design makes it trivial to write unit tests for it, leading to better verification of its correctness. Another important difference is directly related to the fact that it is built on top of "isc_dnsbuffer_t", which tries to manage memory in a smart way. In particular: * It tries to use a static buffer for smaller messages, reducing pressure on the memory manager (hot path); * When allocating dynamic memory for larger messages, it tries to allocate memory conservatively (generic path). These characteristics is a significant upgrade over the older logic where a 64KB(+2 bytes) buffer was allocated from dynamic memory regardless of the fact if we need a buffer this large or not. That is, lesser memory usage is expected in a generic case for DNS transports built on top of "isc_dnsstream_assembler_t." --- .gitlab-ci.yml | 1 + lib/isc/Makefile.am | 1 + lib/isc/include/isc/dnsstream.h | 378 ++++++++++++++++++++++++++++++++ 3 files changed, 380 insertions(+) create mode 100644 lib/isc/include/isc/dnsstream.h diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6e14d6d634..dd97506400 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1522,6 +1522,7 @@ gcov: - find bin lib -maxdepth 1 -mindepth 1 -type d -exec cp -f lib/isc/include/isc/buffer.h "{}" \; - cp -f lib/isc/include/isc/buffer.h lib/isc/netmgr/buffer.h - cp -f lib/isc/include/isc/dnsbuffer.h lib/isc/netmgr/dnsbuffer.h + - cp -f lib/isc/include/isc/dnsstream.h lib/isc/netmgr/dnsstream.h # Help gcovr find dlz_dbi.c file - for DST in ldap mysql mysqldyn sqlite3 wildcard; do cp contrib/dlz/modules/common/dlz_dbi.c "contrib/dlz/modules/${DST}"; done # Generate XML file in the Cobertura XML format suitable for use by GitLab diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index 3d55874dab..50d94e40d8 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -26,6 +26,7 @@ libisc_la_HEADERS = \ include/isc/deprecated.h \ include/isc/dir.h \ include/isc/dnsbuffer.h \ + include/isc/dnsstream.h \ include/isc/endian.h \ include/isc/entropy.h \ include/isc/errno.h \ diff --git a/lib/isc/include/isc/dnsstream.h b/lib/isc/include/isc/dnsstream.h new file mode 100644 index 0000000000..6e4ede334a --- /dev/null +++ b/lib/isc/include/isc/dnsstream.h @@ -0,0 +1,378 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ +#pragma once + +#include + +typedef struct isc_dnsstream_assembler isc_dnsstream_assembler_t; +/*!< + * \brief The 'isc_dnsstream_assembler_t' object is built on top of + * 'isc_dnsbuffer_t' and intended to encapsulate the state machine + * used for handling DNS messages received in the format used for + * messages transmitted over TCP. + * + * The idea is that the object accepts the input data received from a + * socket (or anywhere else, for that matter), tries to assemble DNS + * messages from the incoming data and calls the callback passing it + * the status of the incoming data as well as a pointer to the memory + * region referencing the data of the assembled message (in the case + * there is enough data to assemble the message). It is capable of + * assembling DNS messages no matter how "torn apart" they are when + * sent over network. + * + * The implementation is completely decoupled from the networking code + * itself makes it trivial to write unit tests for it, leading to + * better verification of its correctness. Another important aspect + * of its functioning is directly related to the fact that it is built + * on top of 'isc_dnsbuffer_t', which tries to manage memory in a + * smart way. In particular: + * + *\li It tries to use a static buffer for smaller messages, reducing + * pressure on the memory manager (hot path); + * + *\li When allocating dynamic memory for larger messages, it tries to + * allocate memory conservatively (generic path). + * + * That is, when using 'isc_dnsstream_assembler_t', we allocate memory + * conservatively, avoiding any allocations whatsoever for small DNS + * messages (whose size is lesser of equal to 512 bytes). The last + * characteristic is important in the context of DNS, as most of DNS + * messages are small. + */ + +typedef bool (*isc_dnsstream_assembler_cb_t)(isc_dnsstream_assembler_t *dnsasm, + const isc_result_t result, + isc_region_t *restrict region, + void *cbarg, void *userarg); +/*!< + * /brief The type of callback called when processing the data passed to a + * 'isc_dnsstream_assembler_t' type. + * + * The callback accepts the following arguments: + * + *\li 'isc_dnsstream_assembler_t *dnsasm' - a pointer to the + * 'isc_dnsstream_assembler_t' object in use; + *\li 'isc_result_t result' - processing status; + *\li 'isc_region_t *region' - the region referencing the DNS message if + * assembled, empty otherwise; + *\li 'void *cbarg' - the callback argument, set during the object + * initialisation or when setting the callback; + *\li 'void *userarg' - the callback argument passed to it when processing the + * current chunk of data; + * + * Return values: + * + *\li 'true' - continue processing data, if there is any non-processed data + * left; + *\li 'false' - stop processing data regardless of non-processed data + * availability. + * + * Processing status values: + * + *\li 'ISC_R_SUCCESS' - a message has been successfully assembled; + *\li 'ISC_R_NOMORE' - not enough data to assemble a DNS message, need to get +more; + *\li 'ISC_R_RANGE' - there was an attempt to process a zero-sized DNS +message (i.e. someone attempts to send us junk data). + */ + +struct isc_dnsstream_assembler { + isc_dnsbuffer_t dnsbuf; /*!< Internal buffer for assembling DNS + messages. */ + isc_dnsstream_assembler_cb_t onmsg_cb; /*!< Data processing callback. */ + void *cbarg; /*!< Callback argument. */ + bool calling_cb; /*mctx); + isc_dnsbuffer_init(&dnsasm->dnsbuf, memctx); +} + +static inline void +isc_dnsstream_assembler_uninit(isc_dnsstream_assembler_t *restrict dnsasm) { + REQUIRE(dnsasm != NULL); + /* + * Uninitialising the object from withing the callback does not + * make any sense. + */ + INSIST(dnsasm->calling_cb == false); + isc_dnsbuffer_uninit(&dnsasm->dnsbuf); + if (dnsasm->mctx != NULL) { + isc_mem_detach(&dnsasm->mctx); + } +} + +static inline isc_dnsstream_assembler_t * +isc_dnsstream_assembler_new(isc_mem_t *memctx, isc_dnsstream_assembler_cb_t cb, + void *cbarg) { + isc_dnsstream_assembler_t *newasm; + + REQUIRE(memctx != NULL); + REQUIRE(cb != NULL); + + newasm = isc_mem_get(memctx, sizeof(*newasm)); + isc_dnsstream_assembler_init(newasm, memctx, cb, cbarg); + + return (newasm); +} + +static inline void +isc_dnsstream_assembler_free(isc_dnsstream_assembler_t **restrict dnsasm) { + isc_dnsstream_assembler_t *restrict oldasm = NULL; + isc_mem_t *memctx = NULL; + REQUIRE(dnsasm != NULL && *dnsasm != NULL); + + oldasm = *dnsasm; + + isc_mem_attach(oldasm->mctx, &memctx); + isc_dnsstream_assembler_uninit(oldasm); + isc_mem_putanddetach(&memctx, oldasm, sizeof(*oldasm)); + + *dnsasm = NULL; +} + +static inline void +isc_dnsstream_assembler_setcb(isc_dnsstream_assembler_t *restrict dnsasm, + isc_dnsstream_assembler_cb_t cb, void *cbarg) { + REQUIRE(dnsasm != NULL); + REQUIRE(cb != NULL); + dnsasm->onmsg_cb = cb; + dnsasm->cbarg = cbarg; +} + +static inline bool +isc__dnsstream_assembler_handle_message( + isc_dnsstream_assembler_t *restrict dnsasm, void *userarg) { + bool cont = false; + isc_region_t region = { 0 }; + isc_result_t result; + uint16_t dnslen = isc_dnsbuffer_peek_uint16be(&dnsasm->dnsbuf); + + INSIST(dnsasm->calling_cb == false); + + if (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf) < sizeof(uint16_t)) { + result = ISC_R_NOMORE; + } else if (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf) >= + sizeof(uint16_t) && + dnslen == 0) + { + /* + * Someone seems to send us binary junk or output from /dev/zero + */ + result = ISC_R_RANGE; + isc_dnsbuffer_clear(&dnsasm->dnsbuf); + } else if (dnslen <= (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf) - + sizeof(uint16_t))) + { + result = ISC_R_SUCCESS; + } else { + result = ISC_R_NOMORE; + } + + dnsasm->result = result; + dnsasm->calling_cb = true; + if (result == ISC_R_SUCCESS) { + (void)isc_dnsbuffer_consume_uint16be(&dnsasm->dnsbuf); + isc_dnsbuffer_remainingregion(&dnsasm->dnsbuf, ®ion); + region.length = dnslen; + cont = dnsasm->onmsg_cb(dnsasm, ISC_R_SUCCESS, ®ion, + dnsasm->cbarg, userarg); + if (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf) >= dnslen) { + isc_dnsbuffer_consume(&dnsasm->dnsbuf, dnslen); + } + } else { + cont = false; + (void)dnsasm->onmsg_cb(dnsasm, result, NULL, dnsasm->cbarg, + userarg); + } + dnsasm->calling_cb = false; + + return (cont); +} + +static inline void +isc_dnsstream_assembler_incoming(isc_dnsstream_assembler_t *restrict dnsasm, + void *userarg, void *restrict buf, + const unsigned int buf_size) { + REQUIRE(dnsasm != NULL); + INSIST(!dnsasm->calling_cb); + + if (buf_size == 0) { + INSIST(buf == NULL); + } else { + INSIST(buf != NULL); + isc_dnsbuffer_putmem(&dnsasm->dnsbuf, buf, buf_size); + } + + while (isc__dnsstream_assembler_handle_message(dnsasm, userarg)) { + if (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf) == 0) { + break; + } + } + isc_dnsbuffer_trycompact(&dnsasm->dnsbuf); +} + +static inline isc_result_t +isc_dnsstream_assembler_result( + const isc_dnsstream_assembler_t *restrict dnsasm) { + REQUIRE(dnsasm != NULL); + + return (dnsasm->result); +} + +static inline size_t +isc_dnsstream_assembler_remaininglength( + const isc_dnsstream_assembler_t *restrict dnsasm) { + REQUIRE(dnsasm != NULL); + + return (isc_dnsbuffer_remaininglength(&dnsasm->dnsbuf)); +} + +static inline void +isc_dnsstream_assembler_clear(isc_dnsstream_assembler_t *restrict dnsasm) { + REQUIRE(dnsasm != NULL); + + isc_dnsbuffer_clear(&dnsasm->dnsbuf); + dnsasm->result = ISC_R_UNSET; +}