From: Nick Porter Date: Wed, 21 Jul 2021 20:46:34 +0000 (+0100) Subject: v4: Convert %(unbound: ) to new xlat API (#4122) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2cb193661ee08b10571aefeb6a85e3db7d510bb2;p=thirdparty%2Ffreeradius-server.git v4: Convert %(unbound: ) to new xlat API (#4122) * Remove un-needed char[] variables * Define and initialise thread specific data for rlm_unbound * Define xlat thread data for unbound * Initialise unbound xlat thread data * Add unbound_request_t to hold state of running unbound request * Define xlat_unbound_callback() to be called when unbound event completes * Define unbound xlat resume callback * Define xlat_unbound_signal() for cancelling unbound requests * Define new generic unbound xlat * Remove old unbound xlats * Remove un-used items from module inst and un-needed instantiate / detach * Remove unused rrlabels_tostr() Parsing of DNS labels done by functions from util/dns.c * Remove un-used ub_common_fail() * Add test for rlm_unbound * Amend docs for unbound xlat * Remove old unbound callback * Remove un-used ub_common_wait() * Always inform unbound when its timeout event happens This is used for unbound to maintain its statistics about rtt for given servers. * Remove unused code * Add timeout for unbound calls The timeouts within unbound itself are on individual network calls. This is our overall timeout on the whole resolution. * Add resolvconf and hosts options to rlm_unbound resolvconf - file name of a resolv.conf file to load hosts - file name of a hosts file to load * Add destructor for unbound_request_t to cancel pending requests * Temporarily disable test which trips bug in libunbound * Ensure ub_cancel() is not called after the request is completed --- diff --git a/doc/antora/modules/raddb/pages/mods-available/unbound.adoc b/doc/antora/modules/raddb/pages/mods-available/unbound.adoc index a7162b6c160..63cf5fe228d 100644 --- a/doc/antora/modules/raddb/pages/mods-available/unbound.adoc +++ b/doc/antora/modules/raddb/pages/mods-available/unbound.adoc @@ -11,8 +11,49 @@ FQDNs to be resolved during request processing. ## Configuration Settings -filename = "${raddbdir}/mods-config/unbound/default.conf" -timeout = 3000 +filename:: libunbound configuration file. See `man unbound.conf` for +details of options which can be set in this file. + +Defaults to `"${raddbdir}/mods-config/unbound/default.conf"` + +timeout:: maximum time to wait for a DNS resolution. + +Defaults to 3000 + +resolvconf:: resolv.conf file to load. Without this set unbound will +query root servers. + +hosts:: hosts file to load. + +## xlat for DNS resolution + +An xlat based on the instance name can be used to perform DNS lookups. + +.Example + +``` +%(dns:www.example.com A) +%(dns:www.example.com AAAA) +``` + +Given an instance `dns` the above xlats will perform A record and +AAA record lookups on www.example.com + +.Example + +``` +%(dns:1.1.168.192.in-addr.arpa PTR) +``` + +The above example will perform reverse DNS lookup on 192.168.1.1 + +.Example +``` +%(dns:example.com MX 1) +``` + +The above example will perform an MX lookup on example.com only +returning the first record. == Default Configuration diff --git a/raddb/mods-available/unbound b/raddb/mods-available/unbound index 6a9102b0db0..b817e31098e 100644 --- a/raddb/mods-available/unbound +++ b/raddb/mods-available/unbound @@ -15,6 +15,58 @@ # ## Configuration Settings # unbound dns { + # + # File to read unbound configuration details from. + # # filename = "${raddbdir}/mods-config/unbound/default.conf" + + # + # Timeout for unbound queries. + # # timeout = 3000 + + # + # resolv.conf file to instruct unbound to load resolvers from. + # Defaults to not set. + # Without this set, unbound queries root DNS servers. If a local + # caching DNS server is available that will improve performance. + # + # resolvconf = "/etc/resolv.conf" + + # + # hosts file to load data from. Defaults to not set. + # + # hosts = "/etc/hosts" } + +# +# ## xlat for DNS resolution +# +# An xlat based on the instance name can be used to perform DNS lookups. +# +# .Example +# +# ``` +# %(dns:www.example.com A) +# %(dns:www.example.com AAAA) +# ``` +# +# Given an instance `dns` the above xlats will perform A record and +# AAAA record lookups on www.example.com. +# +# .Example +# +# ``` +# %(dns:1.1.168.192.in-addr.arpa PTR) +# ``` +# +# The above example will perform reverse DNS lookup on 192.168.1.1 +# +# .Example +# +# ``` +# %(dns:example.com MX 1) +# ``` +# +# The above example will perform an MX lookup on example.com returning +# just the first result. diff --git a/src/modules/rlm_unbound/io.c b/src/modules/rlm_unbound/io.c index d4b10d3a006..aaf301f4a27 100644 --- a/src/modules/rlm_unbound/io.c +++ b/src/modules/rlm_unbound/io.c @@ -145,13 +145,15 @@ static void _unbound_io_event_free(struct ub_event *ub_ev) /** Timeout fired * + * Unbound uses these timeouts as part of its mechanism to measure rtt from + * candidate DNS servers, working out which is the fastest to use for any + * given query. The timeout happening causes the timeout against the server + * to be increased for any subsequent queries sent to it. */ static void _unbound_io_service_timer_expired(UNUSED fr_event_list_t *el, UNUSED fr_time_t now, void *uctx) { unbound_io_event_t *ev = talloc_get_type_abort(uctx, unbound_io_event_t); - fr_assert(ev->active); /* must be active */ - DEBUG4("unbound event %p - Timeout", ev); ev->cb(-1, UB_EV_TIMEOUT, ev->uctx); /* Inform libunbound */ diff --git a/src/modules/rlm_unbound/log.c b/src/modules/rlm_unbound/log.c index 35454a8757d..789ee12ce98 100644 --- a/src/modules/rlm_unbound/log.c +++ b/src/modules/rlm_unbound/log.c @@ -128,7 +128,6 @@ static int _unbound_log_free(unbound_log_t *u_log) */ int unbound_log_init(TALLOC_CTX *ctx, unbound_log_t **u_log_out, struct ub_ctx *ub) { - char opt[64]; /* To silence const warns until newer unbound in distros */ char *val; unbound_log_t *u_log; int ret; @@ -138,22 +137,16 @@ int unbound_log_init(TALLOC_CTX *ctx, unbound_log_t **u_log_out, struct ub_ctx * * a log destination, and disable it * if they did. */ - strcpy(opt, "use-syslog"); - ret = ub_ctx_get_option(ub, opt, &val); + ret = ub_ctx_get_option(ub, "use-syslog", &val); if ((ret != 0) || !val) { ERROR("Failed retrieving unbound syslog settings: %s", ub_strerror(ret)); return -1; } if (strcmp(val, "yes") == 0) { - char vbuff[3]; + WARN("Disabling unbound syslog output (use-syslog: %s) > (use-syslog: no)", val); - strcpy(opt, "use-syslog:"); - strcpy(vbuff, "no"); - - WARN("Disabling unbound syslog output (%s %s) > (%s %s)", opt, val, opt, vbuff); - - ret = ub_ctx_set_option(ub, opt, vbuff); + ret = ub_ctx_set_option(ub, "use-syslog:", "no"); if (ret != 0) { ERROR("Failed disabling unbound syslog output: %s", ub_strerror(ret)); free(val); @@ -162,21 +155,16 @@ int unbound_log_init(TALLOC_CTX *ctx, unbound_log_t **u_log_out, struct ub_ctx * } free(val); - strcpy(opt, "logfile"); - ret = ub_ctx_get_option(ub, opt, &val); + ret = ub_ctx_get_option(ub, "logfile", &val); if ((ret != 0) || !val) { ERROR("Failed retrieving unbound logfile settings: %s", ub_strerror(ret)); return -1; } if (strcmp(val, "yes") == 0) { - char vbuff[3]; - - WARN("Disabling unbound logfile output (%s %s) > (%s %s)", opt, val, opt, vbuff); - strcpy(opt, "logfile:"); - strcpy(vbuff, "no"); + WARN("Disabling unbound logfile output (logfile: %s) > (logfile: no)", val); - ret = ub_ctx_set_option(ub, opt, vbuff); + ret = ub_ctx_set_option(ub, "logfile:", "no"); if (ret != 0) { ERROR("Failed disabling unbound logfile output: %s", ub_strerror(ret)); free(val); diff --git a/src/modules/rlm_unbound/rlm_unbound.5 b/src/modules/rlm_unbound/rlm_unbound.5 index e269af4eaff..196d070668f 100644 --- a/src/modules/rlm_unbound/rlm_unbound.5 +++ b/src/modules/rlm_unbound/rlm_unbound.5 @@ -36,14 +36,20 @@ any libunbound configuration values. .PP An instance named, for example, "dns" will provide the following xlat functionalities: -.IP %{dns-a:} -Performs an A lookup for the owner name, returning a stringified IPv4 -address. Only the first A record in the RRSET will be returned. -.IP %{dns-aaaa:} -Performs an AAAA lookup for the owner name, returning a stringified IPv6 -address. Only the first AAAA record in the RRSET will be returned. -.IP %{dns-ptr:} +.IP %(dns: []) +Perform the lookup of against returning up to + results. The returned data type is appropriate to the record +type being queried. +.IP %(dns: A) +Performs an A lookup for the owner name, returning the results as IPv4 +addresses. +.IP %(dns: AAAA) +Performs an AAAA lookup for the owner name, returning the results as IPv6 +addresses. +.IP %(dns: PTR) Performs a PTR lookup for the owner. +.IP %(dns: MX 1) +Performs an MX lookup for the owner, returning just the first result. .PP .SH CAVEATS Logging from rlm_unbound can be problematic, especialy if more than one diff --git a/src/modules/rlm_unbound/rlm_unbound.c b/src/modules/rlm_unbound/rlm_unbound.c index 16e9d16f291..290b1c2840b 100644 --- a/src/modules/rlm_unbound/rlm_unbound.c +++ b/src/modules/rlm_unbound/rlm_unbound.c @@ -35,399 +35,460 @@ RCSID("$Id$") #include "log.h" typedef struct { - struct ub_ctx *ub; /* This must come first. Do not move */ - char const *name; - char const *xlat_a_name; - char const *xlat_aaaa_name; - char const *xlat_ptr_name; uint32_t timeout; - char const *filename; - - unbound_log_t *u_log; + char const *filename; //!< Unbound configuration file + char const *resolvconf; //!< resolv.conf file to use + char const *hosts; //!< hosts file to load } rlm_unbound_t; +typedef struct { + unbound_io_event_base_t *ev_b; //!< Unbound event base + rlm_unbound_t *inst; //!< Instance data + unbound_log_t *u_log; //!< Unbound log structure +} rlm_unbound_thread_t; + +typedef struct { + rlm_unbound_t *inst; //!< Instance data + rlm_unbound_thread_t *t; //!< Thread structure +} unbound_xlat_thread_inst_t; + +typedef struct { + int async_id; //!< Id of async query + request_t *request; //!< Current request being processed + rlm_unbound_thread_t *t; //!< Thread running this request + int done; //!< Indicator that the callback has been called + ///< Negative values indicate errors. + fr_type_t return_type; //!< Data type to parse results into + bool has_priority; //!< Does the returned data start with a priority field + uint16_t count; //!< Number of results to return + fr_value_box_list_t list; //!< Where to put the parsed results + TALLOC_CTX *out_ctx; //!< CTX to allocate parsed results in + fr_event_timer_t const *ev; //!< Event for timeout +} unbound_request_t; + /* * A mapping of configuration file names to internal variables. */ static const CONF_PARSER module_config[] = { { FR_CONF_OFFSET("filename", FR_TYPE_FILE_INPUT | FR_TYPE_REQUIRED, rlm_unbound_t, filename), .dflt = "${modconfdir}/unbound/default.conf" }, { FR_CONF_OFFSET("timeout", FR_TYPE_UINT32, rlm_unbound_t, timeout), .dflt = "3000" }, + { FR_CONF_OFFSET("resolvconf", FR_TYPE_FILE_INPUT, rlm_unbound_t, resolvconf) }, + { FR_CONF_OFFSET("hosts", FR_TYPE_FILE_INPUT, rlm_unbound_t, hosts) }, CONF_PARSER_TERMINATOR }; -/* - * Callback sent to libunbound for xlat functions. Simply links the - * new ub_result via a pointer that has been allocated from the heap. - * This pointer has been pre-initialized to a magic value. - */ -static void link_ubres(void *my_arg, int err, struct ub_result *result) +static int _unbound_request_free(unbound_request_t *ur) { - struct ub_result **ubres = (struct ub_result **)my_arg; - /* - * Note that while result will be NULL on error, we are explicit - * here because that is actually a behavior that is suboptimal - * and only documented in the examples. It could change. + * Cancel an outstanding async unbound call if the request is being freed */ - if (err) { - ERROR("%s", ub_strerror(err)); - *ubres = NULL; - } else { - *ubres = result; - } + if ((ur->async_id != 0) && (ur->done == 0)) ub_cancel(ur->t->ev_b->ub, ur->async_id); + + return 0; } -/* - * Convert labels as found in a DNS result to a NULL terminated string. +/** Callback called by unbound when resolution started with ub_resolve_event() completes * - * Result is written to memory pointed to by "out" but no result will - * be written unless it and its terminating NULL character fit in "left" - * bytes. Returns the number of bytes written excluding the terminating - * NULL, or -1 if nothing was written because it would not fit or due - * to a violation in the labels format. + * @param mydata the request tracking structure set up before ub_resolve_event() was called + * @param rcode should be the rcode from the reply packet, but appears not to be + * @param packet wire format reply packet + * @param packet_len length of wire format packet + * @param sec DNSSEC status code + * @param why_bogus String describing DNSSEC issue if sec = 1 + * @param rate_limited Was the request rate limited due to unbound workload */ -static int rrlabels_tostr(char *out, char *rr, size_t left) +static void xlat_unbound_callback(void *mydata, int rcode, void *packet, int packet_len, int sec, + char* why_bogus, UNUSED int rate_limited) { - int offset = 0; + unbound_request_t *ur = talloc_get_type_abort(mydata, unbound_request_t); + request_t *request = ur->request; + fr_dbuff_t dbuff; + uint16_t qdcount = 0, ancount = 0, i, rdlength = 0; + uint8_t pktrcode = 0, skip = 0; + ssize_t used; + fr_value_box_t *vb; /* - * TODO: verify that unbound results (will) always use this label - * format, and review the specs on this label format for nuances. + * Request has completed remove timeout event and set + * async_id to 0 so ub_cancel() is not called when ur is freed */ + if (ur->ev) (void)fr_event_timer_delete(&ur->ev); + ur->async_id = 0; - if (!left) { - return -1; - } - if (left > 253) { - left = 253; /* DNS length limit */ - } - /* As a whole this should be "NULL terminated" by the 0-length label */ - if (strnlen(rr, left) > left - 1) { - return -1; + /* + * Bogus responses have the "sec" flag set to 1 + */ + if (sec == 1) { + RERROR("%s", why_bogus); + ur->done = -16; + goto resume; } - /* It will fit, but does it it look well formed? */ - while (1) { - size_t count; + RHEXDUMP4((uint8_t const *)packet, packet_len, "Unbound callback called with packet [length %d]", packet_len); - count = *((unsigned char *)(rr + offset)); - if (!count) break; + fr_dbuff_init(&dbuff, (uint8_t const *)packet, (size_t)packet_len); - offset++; - if (count > 63 || strlen(rr + offset) < count) { - return -1; - } - offset += count; - } + /* Skip initial header entries */ + fr_dbuff_advance(&dbuff, 3); - /* Data is valid and fits. Copy it. */ - offset = 0; - while (1) { - int count; + /* + * Extract rcode - it doesn't appear to be passed in as a + * parameter, contrary to the documentation... + */ + fr_dbuff_out(&pktrcode, &dbuff); + rcode = pktrcode & 0x0f; + if (rcode != 0) { + ur->done = 0 - rcode; + REDEBUG("DNS rcode is %d", rcode); + goto resume; + } - count = *((unsigned char *)(rr)); - if (!count) break; + fr_dbuff_out(&qdcount, &dbuff); + if (qdcount > 1) { + RERROR("DNS results packet with multiple questions"); + ur->done = -32; + goto resume; + } - if (offset) { - *(out + offset) = '.'; - offset++; + /* How many answer records do we have? */ + fr_dbuff_out(&ancount, &dbuff); + RDEBUG4("Unbound returned %d answers", ancount); + + /* Skip remaining header entries */ + fr_dbuff_advance(&dbuff, 4); + + /* Skip the QNAME */ + fr_dbuff_out(&skip, &dbuff); + while (skip > 0) { + if (skip > 63) { + /* + * This is a pointer to somewhere else in the the packet + * Pointers use two octets + * Just move past the pointer to the next label in the question + */ + fr_dbuff_advance(&dbuff, 1); + } else { + if (fr_dbuff_remaining(&dbuff) < skip) break; + fr_dbuff_advance(&dbuff, skip); } - - rr++; - memcpy(out + offset, rr, count); - rr += count; - offset += count; + fr_dbuff_out(&skip, &dbuff); } - *(out + offset) = '\0'; - return offset; -} - -static int ub_common_wait(rlm_unbound_t const *inst, request_t *request, - char const *name, struct ub_result **ub, int async_id) -{ - useconds_t iv, waited; - - iv = inst->timeout > 64 ? 64000 : inst->timeout * 1000; - ub_process(inst->ub); - - for (waited = 0; (void const *)*ub == (void const *)inst; waited += iv, iv *= 2) { - - if (waited + iv > (useconds_t)inst->timeout * 1000) { - usleep(inst->timeout * 1000 - waited); - ub_process(inst->ub); + /* Skip QTYPE and QCLASS */ + fr_dbuff_advance(&dbuff, 4); + + /* We only want a limited number of replies */ + if (ancount > ur->count) ancount = ur->count; + + fr_value_box_list_init(&ur->list); + + /* Read the answer RRs */ + for (i = 0; i < ancount; i++) { + fr_dbuff_out(&skip, &dbuff); + if (skip > 63) fr_dbuff_advance(&dbuff, 1); + + /* Skip TYPE, CLASS and TTL */ + fr_dbuff_advance(&dbuff, 8); + + fr_dbuff_out(&rdlength, &dbuff); + RDEBUG4("RDLENGTH is %d", rdlength); + + vb = fr_value_box_alloc_null(ur->out_ctx); + switch (ur->return_type) { + case FR_TYPE_IPV4_ADDR: + case FR_TYPE_IPV6_ADDR: + case FR_TYPE_OCTETS: + if (fr_value_box_from_network(ur->out_ctx, vb, ur->return_type, NULL, + (uint8_t *)fr_dbuff_current(&dbuff), rdlength, true) < 0) { + error: + talloc_free(vb); + fr_dlist_talloc_free(&ur->list); + ur->done = -32; + goto resume; + } + fr_dbuff_advance(&dbuff, rdlength); break; - } - - usleep(iv); - /* Check if already handled by event loop */ - if ((void const *)*ub != (void const *)inst) { + case FR_TYPE_STRING: + if (ur->has_priority) { + /* + * This record type has a priority before the label + * add the priority first as a separate box + */ + fr_value_box_t *priority_vb; + if (rdlength < 3) { + REDEBUG("%s - Invalid data returned", ur->t->inst->name); + goto error; + } + priority_vb = fr_value_box_alloc_null(ur->out_ctx); + if (fr_value_box_from_network(ur->out_ctx, priority_vb, FR_TYPE_UINT16, NULL, + (uint8_t *)fr_dbuff_current(&dbuff), 2, true) < 0) { + talloc_free(priority_vb); + goto error; + } + fr_dlist_insert_tail(&ur->list, priority_vb); + fr_dbuff_advance(&dbuff, 2); + } + + /* String types require decoding of dns format labels */ + used = fr_dns_label_to_value_box(ur->out_ctx, vb, (uint8_t const *)packet, packet_len, + (uint8_t const *)fr_dbuff_current(&dbuff), true); + if (used < 0) goto error; + fr_dbuff_advance(&dbuff, (size_t)used); break; + + default: + RERROR("No meaningful output type set"); + goto error; } - /* In case we are running single threaded */ - ub_process(inst->ub); + fr_dlist_insert_tail(&ur->list, vb); + } - if ((void const *)*ub == (void const *)inst) { - int res; + ur->done = 1; - REDEBUG2("%s - DNS took too long", name); +resume: + unlang_interpret_mark_runnable(ur->request); +} - res = ub_cancel(inst->ub, async_id); - if (res) { - REDEBUG("%s - ub_cancel: %s", name, ub_strerror(res)); - } - return -1; - } +/** Callback from our timeout event to cancel a request + * + */ +static void xlat_unbound_timeout(UNUSED fr_event_list_t *el, UNUSED fr_time_t now, void *uctx) +{ + unbound_request_t *ur = talloc_get_type_abort(uctx, unbound_request_t); + request_t *request = ur->request; - return 0; + REDEBUG("Timeout waiting for DNS resolution"); + talloc_free(ur); + unlang_interpret_mark_runnable(request); } -static int ub_common_fail(request_t *request, char const *name, struct ub_result *ub) +/* + * Xlat signal callback if an unbound request needs cancelling + */ +static void xlat_unbound_signal(request_t *request, UNUSED void *instance, UNUSED void *thread, + void *rctx, fr_state_signal_t action) { - if (ub->bogus) { - RWDEBUG("%s - Bogus DNS response", name); - return -1; - } + unbound_request_t *ur = talloc_get_type_abort(rctx, unbound_request_t); - if (ub->nxdomain) { - RDEBUG2("%s - NXDOMAIN", name); - return -1; - } + if (action != FR_SIGNAL_CANCEL) return; - if (!ub->havedata) { - RDEBUG2("%s - Empty result", name); - return -1; - } + if (ur->ev) (void)fr_event_timer_delete(&ur->ev); - return 0; + RDEBUG2("Forcefully cancelling pending unbound request"); + talloc_free(ur); } -/** Perform a DNS lookup for an A record - * - * @ingroup xlat_functions +/* + * Xlat resume callback after unbound has either returned or timed out + * Move the parsed results to the xlat output cursor */ -static ssize_t xlat_a(TALLOC_CTX *ctx, char **out, size_t outlen, - void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t xlat_unbound_resume(UNUSED TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, + UNUSED void const *xlat_inst, UNUSED void *xlat_thread_inst, + UNUSED fr_value_box_list_t *in, void *rctx) { - rlm_unbound_t const *inst = mod_inst; - struct ub_result **ubres; - int async_id; - char *fmt2; /* For const warnings. Keep till new libunbound ships. */ - - /* This has to be on the heap, because threads. */ - ubres = talloc(inst, struct ub_result *); - - /* Used and thus impossible value from heap to designate incomplete */ - memcpy(ubres, &mod_inst, sizeof(*ubres)); - - fmt2 = talloc_typed_strdup(ctx, fmt); - ub_resolve_async(inst->ub, fmt2, 1, 1, ubres, link_ubres, &async_id); - talloc_free(fmt2); + fr_value_box_t *vb; + unbound_request_t *ur = talloc_get_type_abort(rctx, unbound_request_t); + +#define RCODEERROR(_code, _message) case _code: \ + REDEBUG(_message, ur->t->inst->name); \ + goto error + + /* Check for unbound errors */ + switch (ur->done) { + case 1: + break; + + default: + REDEBUG("%s - Unknown DNS error", ur->t->inst->name); + error: + talloc_free(ur); + return XLAT_ACTION_FAIL; - if (ub_common_wait(inst, request, inst->xlat_a_name, ubres, async_id)) { - goto error0; + RCODEERROR(0, "%s - No result"); + RCODEERROR(-1, "%s - Query format error"); + RCODEERROR(-2, "%s - DNS server failure"); + RCODEERROR(-3, "%s - Nonexistent domain name"); + RCODEERROR(-4, "%s - DNS server does not support query type"); + RCODEERROR(-5, "%s - DNS server refused query"); + RCODEERROR(-16, "%s - Bogus DNS response"); + RCODEERROR(-32, "%s - Error parsing DNS response"); } - if (*ubres) { - if (ub_common_fail(request, inst->xlat_a_name, *ubres)) { - goto error1; - } - - if (!inet_ntop(AF_INET, (*ubres)->data[0], *out, outlen)) { - goto error1; - }; - - ub_resolve_free(*ubres); - talloc_free(ubres); - return strlen(*out); + /* + * Move parsed results into xlat cursor + */ + while ((vb = fr_dlist_pop_head(&ur->list))) { + fr_dcursor_append(out, vb); } - RWDEBUG("%s - No result", inst->xlat_a_name); + talloc_free(ur); + return XLAT_ACTION_DONE; +} - error1: - ub_resolve_free(*ubres); /* Handles NULL gracefully */ - error0: - talloc_free(ubres); - return -1; -} +static xlat_arg_parser_t const xlat_unbound_args[] = { + { .required = true, .concat = true, .type = FR_TYPE_STRING }, + { .required = true, .concat = true, .type = FR_TYPE_STRING }, + { .single = true, .type = FR_TYPE_UINT16 }, + XLAT_ARG_PARSER_TERMINATOR +}; -/** Perform a DNS lookup for an AAAA record +/** Perform a DNS lookup using libunbound * * @ingroup xlat_functions */ -static ssize_t xlat_aaaa(TALLOC_CTX *ctx, char **out, size_t outlen, - void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t xlat_unbound(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, + UNUSED void const *xlat_inst, void *xlat_thread_inst, + fr_value_box_list_t *in) { - rlm_unbound_t const *inst = mod_inst; - struct ub_result **ubres; - int async_id; - char *fmt2; /* For const warnings. Keep till new libunbound ships. */ + fr_value_box_t *host_vb = fr_dlist_head(in); + fr_value_box_t *query_vb = fr_dlist_next(in, host_vb); + fr_value_box_t *count_vb = fr_dlist_next(in, query_vb); + unbound_xlat_thread_inst_t *xt = talloc_get_type_abort(xlat_thread_inst, unbound_xlat_thread_inst_t); + unbound_request_t *ur; + + if (host_vb->length == 0) { + REDEBUG("Can't resolve zero length host"); + return XLAT_ACTION_FAIL; + } - /* This has to be on the heap, because threads. */ - ubres = talloc(inst, struct ub_result *); + MEM(ur = talloc_zero(unlang_interpret_frame_talloc_ctx(request), unbound_request_t)); + talloc_set_destructor(ur, _unbound_request_free); - /* Used and thus impossible value from heap to designate incomplete */ - memcpy(ubres, &mod_inst, sizeof(*ubres)); + /* + * Set the maximum number of records we want to return + */ + if ((count_vb) && (count_vb->type == FR_TYPE_UINT16) && (count_vb->vb_uint16 > 0)) { + ur->count = count_vb->vb_uint16; + } else { + ur->count = UINT16_MAX; + } - fmt2 = talloc_typed_strdup(ctx, fmt); - ub_resolve_async(inst->ub, fmt2, 28, 1, ubres, link_ubres, &async_id); - talloc_free(fmt2); + ur->request = request; + ur->t = xt->t; + ur->out_ctx = ctx; - if (ub_common_wait(inst, request, inst->xlat_aaaa_name, ubres, async_id)) { - goto error0; +#define UB_QUERY(_record, _rrvalue, _return, _hasprio) \ + if (strcmp(query_vb->vb_strvalue, _record) == 0) { \ + ur->return_type = _return; \ + ur->has_priority = _hasprio; \ + ub_resolve_event(xt->t->ev_b->ub, host_vb->vb_strvalue, _rrvalue, 1, ur, \ + xlat_unbound_callback, &ur->async_id); \ } - if (*ubres) { - if (ub_common_fail(request, inst->xlat_aaaa_name, *ubres)) { - goto error1; - } - if (!inet_ntop(AF_INET6, (*ubres)->data[0], *out, outlen)) { - goto error1; - }; - ub_resolve_free(*ubres); - talloc_free(ubres); - return strlen(*out); + UB_QUERY("A", 1, FR_TYPE_IPV4_ADDR, false) + else UB_QUERY("AAAA", 28, FR_TYPE_IPV6_ADDR, false) + else UB_QUERY("PTR", 12, FR_TYPE_STRING, false) + else UB_QUERY("MX", 15, FR_TYPE_STRING, true) + else UB_QUERY("SRV", 33, FR_TYPE_STRING, true) + else UB_QUERY("TXT", 16, FR_TYPE_STRING, false) + else UB_QUERY("CERT", 37, FR_TYPE_OCTETS, false) + else { + REDEBUG("Invalid / unsupported DNS query type"); + return XLAT_ACTION_FAIL; } - RWDEBUG("%s - No result", inst->xlat_aaaa_name); - -error1: - ub_resolve_free(*ubres); /* Handles NULL gracefully */ - -error0: - talloc_free(ubres); - return -1; -} - -typedef struct { - struct ub_result *result; //!< The result from the previous operation. -} dns_resume_ctx_t; - -/* -static xlat_action_t xlat_ptr(TALLOC_CTX *ctx, fr_cursor_t *out, - request_t *request, void const *xlat_inst, void *xlat_thread_inst, - fr_value_box_t **in) -{ - if (!*in) return XLAT_ACTION_DONE; + /* + * unbound returned before we yielded - run the callback + * This is when serving results from local data + */ + if (ur->async_id == 0) return xlat_unbound_resume(NULL, out, request, NULL, NULL, NULL, ur); - if (fr_value_box_list_concat(ctx, *in, in, FR_TYPE_STRING, true) < 0) { - RPEDEBUG("Failed concatenating input string for attribute reference"); + if (fr_event_timer_in(ur, ur->t->ev_b->el, &ur->ev, fr_time_delta_from_msec(xt->inst->timeout), + xlat_unbound_timeout, ur) < 0) { + REDEBUG("Unable to attach unbound timeout_envent"); + ub_cancel(xt->t->ev_b->ub, ur->async_id); return XLAT_ACTION_FAIL; } - yield_to - + return unlang_xlat_yield(request, xlat_unbound_resume, xlat_unbound_signal, ur); } -*/ -/** Perform a DNS lookup for a PTR record - * - * @ingroup xlat_functions - */ -static ssize_t xlat_ptr(TALLOC_CTX *ctx, char **out, size_t outlen, - void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static int mod_xlat_thread_instantiate(UNUSED void *xlat_inst, void *xlat_thread_inst, + UNUSED xlat_exp_t const *exp, void *uctx) { - rlm_unbound_t const *inst = mod_inst; - struct ub_result **ubres; - int async_id; - char *fmt2; /* For const warnings. Keep till new libunbound ships. */ - - /* This has to be on the heap, because threads. */ - ubres = talloc(inst, struct ub_result *); - - /* Used and thus impossible value from heap to designate incomplete */ - memcpy(ubres, &mod_inst, sizeof(*ubres)); - - fmt2 = talloc_typed_strdup(ctx, fmt); - ub_resolve_async(inst->ub, fmt2, 12, 1, ubres, link_ubres, &async_id); - talloc_free(fmt2); - - if (ub_common_wait(inst, request, inst->xlat_ptr_name, - ubres, async_id)) { - goto error0; - } - - if (*ubres) { - if (ub_common_fail(request, inst->xlat_ptr_name, *ubres)) { - goto error1; - } - if (rrlabels_tostr(*out, (*ubres)->data[0], outlen) < 0) { - goto error1; - } - ub_resolve_free(*ubres); - talloc_free(ubres); - return strlen(*out); - } + rlm_unbound_t *inst = talloc_get_type_abort(uctx, rlm_unbound_t); + unbound_xlat_thread_inst_t *xt = talloc_get_type_abort(xlat_thread_inst, unbound_xlat_thread_inst_t); - RWDEBUG("%s - No result", inst->xlat_ptr_name); + xt->inst = inst; + xt->t = talloc_get_type_abort(module_thread_by_data(inst)->data, rlm_unbound_thread_t); -error1: - ub_resolve_free(*ubres); /* Handles NULL gracefully */ - -error0: - talloc_free(ubres); - return -1; + return 0; } -static int mod_instantiate(void *instance, CONF_SECTION *conf) +static int mod_thread_instantiate(UNUSED CONF_SECTION const *cs, void *instance, fr_event_list_t *el, void *thread) { - rlm_unbound_t *inst = instance; - int res; - char k[64]; /* To silence const warns until newer unbound in distros */ + rlm_unbound_t *inst = talloc_get_type_abort(instance, rlm_unbound_t); + rlm_unbound_thread_t *t = talloc_get_type_abort(thread, rlm_unbound_thread_t); + int res; + + t->inst = inst; + if (unbound_io_init(t, &t->ev_b, el) < 0) { + PERROR("Unable to create unbound event base"); + return -1; + } /* - * @todo - move this to the thread-instantiate function + * Ensure unbound uses threads */ - inst->ub = ub_ctx_create(); - if (!inst->ub) { - cf_log_err(conf, "ub_ctx_create failed"); + res = ub_ctx_async(t->ev_b->ub, 1); + if (res) { + error: + PERROR("%s", ub_strerror(res)); return -1; } /* - * Note unbound threads WILL happen with -s option, if it matters. - * We cannot tell from here whether that option is in effect. + * Load settings from the unbound config file */ - res = ub_ctx_async(inst->ub, 1); + res = ub_ctx_config(t->ev_b->ub, UNCONST(char *, inst->filename)); if (res) goto error; - /* Now load the config file, which can override gleaned settings. */ - res = ub_ctx_config(inst->ub, UNCONST(char *, inst->filename)); - if (res) goto error; + if (unbound_log_init(t, &t->u_log, t->ev_b->ub) < 0) { + PERROR("Failed to initialise unbound log"); + return -1; + } - if (unbound_log_init(inst, &inst->u_log, inst->ub) < 0) goto error; + /* + * Load resolv.conf if specified + */ + if (inst->resolvconf) ub_ctx_resolvconf(t->ev_b->ub, inst->resolvconf); + + /* + * Load hosts file if specified + */ + if (inst->hosts) ub_ctx_hosts(t->ev_b->ub, inst->hosts); /* - * Now we need to finalize the context. - * - * There's no clean API to just finalize the context made public - * in libunbound. But we can trick it by trying to delete data - * which as it happens fails quickly and quietly even though the - * data did not exist. + * The unbound context needs to be "finalised" to fix its settings. + * The API does not expose a method to do this, rather it happens on first + * use. A quick workround is to delete data which won't be present */ - strcpy(k, "notar33lsite.foo123.nottld A 127.0.0.1"); - ub_ctx_data_remove(inst->ub, k); + ub_ctx_data_remove(t->ev_b->ub, "notar33lsite.foo123.nottld A 127.0.0.1"); + return 0; +} + +static int mod_thread_detach(UNUSED fr_event_list_t *el, void *thread) +{ + rlm_unbound_thread_t *t = talloc_get_type_abort(thread, rlm_unbound_thread_t); - error: - cf_log_err(conf, "%s", ub_strerror(res)); + talloc_free(t->u_log); + talloc_free(t->ev_b); - return -1; + return 0; } static int mod_bootstrap(void *instance, CONF_SECTION *conf) { - rlm_unbound_t *inst = instance; + rlm_unbound_t *inst = instance; + xlat_t *xlat; inst->name = cf_section_name2(conf); if (!inst->name) inst->name = cf_section_name1(conf); @@ -437,34 +498,9 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) return -1; } - MEM(inst->xlat_a_name = talloc_typed_asprintf(inst, "%s-a", inst->name)); - MEM(inst->xlat_aaaa_name = talloc_typed_asprintf(inst, "%s-aaaa", inst->name)); - MEM(inst->xlat_ptr_name = talloc_typed_asprintf(inst, "%s-ptr", inst->name)); - - if (xlat_register_legacy(inst, inst->xlat_a_name, xlat_a, NULL, NULL, 0, XLAT_DEFAULT_BUF_LEN) || - xlat_register_legacy(inst, inst->xlat_aaaa_name, xlat_aaaa, NULL, NULL, 0, XLAT_DEFAULT_BUF_LEN) || - xlat_register_legacy(inst, inst->xlat_ptr_name, xlat_ptr, NULL, NULL, 0, XLAT_DEFAULT_BUF_LEN)) { - cf_log_err(conf, "Failed registering xlats"); - return -1; - } - - return 0; -} - -static int mod_detach(void *instance) -{ - rlm_unbound_t *inst = instance; - - ub_process(inst->ub); - - /* - * This can hang/leave zombies currently - * see upstream bug #519 - * ...so expect valgrind to complain with -m - */ - talloc_free(inst->u_log); /* Free logging first */ - - ub_ctx_delete(inst->ub); + if(!(xlat = xlat_register(NULL, inst->name, xlat_unbound, true))) return -1; + xlat_func_args(xlat, xlat_unbound_args); + xlat_async_thread_instantiate_set(xlat, mod_xlat_thread_instantiate, unbound_xlat_thread_inst_t, NULL, inst); return 0; } @@ -477,6 +513,9 @@ module_t rlm_unbound = { .inst_size = sizeof(rlm_unbound_t), .config = module_config, .bootstrap = mod_bootstrap, - .instantiate = mod_instantiate, - .detach = mod_detach + + .thread_inst_size = sizeof(rlm_unbound_thread_t), + .thread_inst_type = "rlm_unbound_thread_t", + .thread_instantiate = mod_thread_instantiate, + .thread_detach = mod_thread_detach, }; diff --git a/src/tests/modules/unbound/all.mk b/src/tests/modules/unbound/all.mk new file mode 100644 index 00000000000..d64039f8e10 --- /dev/null +++ b/src/tests/modules/unbound/all.mk @@ -0,0 +1,3 @@ +# +# Test the "unbound" module +# diff --git a/src/tests/modules/unbound/dns.attrs b/src/tests/modules/unbound/dns.attrs new file mode 100644 index 00000000000..24fef056dc8 --- /dev/null +++ b/src/tests/modules/unbound/dns.attrs @@ -0,0 +1,11 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = "bob" +User-Password = "hello" + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/unbound/dns.unlang b/src/tests/modules/unbound/dns.unlang new file mode 100644 index 00000000000..b13a0208e7a --- /dev/null +++ b/src/tests/modules/unbound/dns.unlang @@ -0,0 +1,115 @@ +# Use builtin "local" zone +update request { + &Tmp-IP-Address-0 := "%(dns:localhost A)" +} + +if (&Tmp-IP-Address-0 != 127.0.0.1) { + test_fail +} + +update request { + &Tmp-String-0 := "%(dns:localhost AAAA)" +} + +if (&Tmp-String-0 != "::1") { + test_fail +} + +update request { + &Tmp-String-1 := "%(dns:1.0.0.127.in-addr.arpa PTR)" +} + +if (&Tmp-String-1 != "localhost") { + test_fail +} + +# Use local data in module config to allow for dotted names +update request { + &Tmp-IP-Address-0 := "%(dns:www.example.com A)" +} + +if (&Tmp-IP-Address-0 != 192.168.1.1) { + test_fail +} + +update request { + &Tmp-String-0 := "%(dns:1.1.168.192.in-addr.arpa PTR)" +} + +if (&Tmp-String-0 != "www.example.com") { + test_fail +} + +# Try a real, known, network response +# Temporarily disabled while there is a bug in unbound +#update request { +# &Tmp-String-0 := "%(dns:8.8.8.8.in-addr.arpa PTR)" +#} + +#if (&Tmp-String-0 != "dns.google") { +# test_fail +#} + +# Invalid query +update request { + &Tmp-String-0 := "%(dns:www.example.com ABC)" +} + +if (&Tmp-String-0 != "") { + test_fail +} + +if (&Module-Failure-Message != "Invalid / unsupported DNS query type") { + test_fail +} + +update request { + &Tmp-String-0 := "" +} + +update request { + &Tmp-String-1 := "%(dns:%{Tmp-String-0} A)" +} + +if (&Tmp-String-1 != "") { + test_fail +} + +if (&Module-Failure-Message != "Can't resolve zero length host") { + test_fail +} + +update request { + &Tmp-String-1 := "%(dns:example.com MX)" +} + +# Until we can handle multiple boxes in xlat expansion, the results +# are concatenated into a single string +# DNS results are not in a specified order +if ((&Tmp-String-1 != '10mail.example.com20mail2.example.com') && (&Tmp-String-1 != '20mail2.example.com10mail.example.com')) { + test_fail +} + +# Just return a single record +# As results are not in a specified order, it could be either. +update request { + &Tmp-String-1 := "%(dns:example.com MX 1)" +} + +if ((&Tmp-String-1 != '10mail.example.com') && (&Tmp-String-1 != '20mail2.example.com')) { + test_fail +} + +update request { + &Tmp-String-1 := "%(dns:n0nex1stent.d0ma1n A)" +} + +if (&Tmp-String-1 != "") { + test_fail +} + +if (&Module-Failure-Message != "dns - Nonexistent domain name") { + test_fail +} + +test_pass diff --git a/src/tests/modules/unbound/module.conf b/src/tests/modules/unbound/module.conf new file mode 100644 index 00000000000..c0430d24d68 --- /dev/null +++ b/src/tests/modules/unbound/module.conf @@ -0,0 +1,4 @@ +unbound dns { + filename = "$ENV{MODULE_TEST_DIR}/unbound.conf" + timeout = 3000 +} diff --git a/src/tests/modules/unbound/unbound.conf b/src/tests/modules/unbound/unbound.conf new file mode 100644 index 00000000000..33fc4616b82 --- /dev/null +++ b/src/tests/modules/unbound/unbound.conf @@ -0,0 +1,6 @@ +server: + num-threads: 2 + local-data: 'www.example.com. A 192.168.1.1' + local-data: 'example.com. MX 10 mail.example.com' + local-data: 'example.com. MX 20 mail2.example.com' + local-data-ptr: '192.168.1.1 www.example.com'