diff --git a/CHANGES b/CHANGES index 280ccc71a6..a7c5f1c51e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5956. [func] Make RRL code treat all QNAMEs that are subject to + wildcard processing within a given zone as the same + name. [GL #3459] + 5955. [port] The libxml2 library has deprecated the usage of xmlInitThreads() and xmlCleanupThreads() functions. Use xmlInitParser() and xmlCleanupParser() instead. diff --git a/bin/tests/system/rrl/tests.sh b/bin/tests/system/rrl/tests.sh index ff03a3176d..e3207ca1cd 100644 --- a/bin/tests/system/rrl/tests.sh +++ b/bin/tests/system/rrl/tests.sh @@ -172,9 +172,7 @@ burst 3 a1.tld2 sleep 1 burst 10 a1.tld2 # Request 30 different qnames to try a wildcard. -burst 30 'x$CNT.a2.tld2' -# These should be counted and limited but are not. See RT33138. -burst 10 'y.x$CNT.a2.tld2' +burst 30 'y.x$CNT.a2.tld2' # IP TC drop NXDOMAIN SERVFAIL NOERROR # referrals to "." @@ -182,12 +180,9 @@ ck_result a1.tld3 x 0 1 2 0 0 2 # check 13 results including 1 second delay that allows an additional response ck_result a1.tld2 192.0.2.1 3 4 6 0 0 8 -# Check the wild card answers. -# The parent name of the 30 requests is counted. -ck_result 'x*.a2.tld2' 192.0.2.2 2 10 18 0 0 12 - -# These should be limited but are not. See RT33138. -ck_result 'y.x*.a2.tld2' 192.0.2.2 10 0 0 0 0 10 +# Check the wildcard answers. +# The zone origin name of the 30 requests is counted. +ck_result 'y.x*.a2.tld2' 192.0.2.2 2 10 18 0 0 12 ######### sec_start diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index 3b64f07bb4..c6fd9f505e 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -5497,7 +5497,9 @@ Response Rate Limiting All non-empty responses for a valid domain name (qname) and record type (qtype) are identical and have a limit specified with - :any:`responses-per-second` (default 0 or no limit). + :any:`responses-per-second` (default 0 or no limit). All valid wildcard + domain names are interpreted as the zone's origin name concatenated to the + "*" name. .. namedconf:statement:: nodata-per-second :tags: query diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index e97eb97108..e758ca8966 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -37,6 +37,10 @@ Removed Features Feature Changes ~~~~~~~~~~~~~~~ +- Response Rate Limiting (RRL) code now treats all QNAMEs that are + subject to wildcard processing within a given zone as the same name, + to prevent circumventing the limits enforced by RRL. :gl:`#3459` + - Zones using ``dnssec-policy`` now require dynamic DNS or ``inline-signing`` to be configured explicitly :gl:`#3381`. diff --git a/lib/dns/include/dns/rrl.h b/lib/dns/include/dns/rrl.h index 66c29ebe26..bdffed99ac 100644 --- a/lib/dns/include/dns/rrl.h +++ b/lib/dns/include/dns/rrl.h @@ -255,8 +255,8 @@ typedef enum { } dns_rrl_result_t; dns_rrl_result_t -dns_rrl(dns_view_t *view, const isc_sockaddr_t *client_addr, bool is_tcp, - dns_rdataclass_t rdclass, dns_rdatatype_t qtype, +dns_rrl(dns_view_t *view, dns_zone_t *zone, const isc_sockaddr_t *client_addr, + bool is_tcp, dns_rdataclass_t rdclass, dns_rdatatype_t qtype, const dns_name_t *qname, isc_result_t resp_result, isc_stdtime_t now, bool wouldlog, char *log_buf, unsigned int log_buf_len); diff --git a/lib/dns/rrl.c b/lib/dns/rrl.c index 599e76b004..9ecc6c5dc5 100644 --- a/lib/dns/rrl.c +++ b/lib/dns/rrl.c @@ -30,11 +30,13 @@ #include #include +#include #include #include #include #include #include +#include static void log_end(dns_rrl_t *rrl, dns_rrl_entry_t *e, bool early, char *log_buf, @@ -413,12 +415,10 @@ hash_key(const dns_rrl_key_t *key) { */ static void make_key(const dns_rrl_t *rrl, dns_rrl_key_t *key, - const isc_sockaddr_t *client_addr, dns_rdatatype_t qtype, - const dns_name_t *qname, dns_rdataclass_t qclass, - dns_rrl_rtype_t rtype) { - dns_name_t base; - dns_offsets_t base_offsets; - int labels, i; + const isc_sockaddr_t *client_addr, dns_zone_t *zone, + dns_rdatatype_t qtype, const dns_name_t *qname, + dns_rdataclass_t qclass, dns_rrl_rtype_t rtype) { + int i; memset(key, 0, sizeof(*key)); @@ -436,15 +436,30 @@ make_key(const dns_rrl_t *rrl, dns_rrl_key_t *key, } if (qname != NULL && qname->labels != 0) { - /* - * Ignore the first label of wildcards. - */ + dns_name_t *origin = NULL; + if ((qname->attributes & DNS_NAMEATTR_WILDCARD) != 0 && - (labels = dns_name_countlabels(qname)) > 1) + zone != NULL && (origin = dns_zone_getorigin(zone)) != NULL) { - dns_name_init(&base, base_offsets); - dns_name_getlabelsequence(qname, 1, labels - 1, &base); - key->s.qname_hash = dns_name_fullhash(&base, false); + dns_fixedname_t fixed; + dns_name_t *wild; + isc_result_t result; + + /* + * Put all wildcard names in one bucket using the zone's + * origin name concatenated to the "*" name. + */ + wild = dns_fixedname_initname(&fixed); + result = dns_name_concatenate(dns_wildcardname, origin, + wild, NULL); + if (result != ISC_R_SUCCESS) { + /* + * Fallback to use the zone's origin name + * instead of the concatenated name. + */ + wild = origin; + } + key->s.qname_hash = dns_name_fullhash(wild, false); } else { key->s.qname_hash = dns_name_fullhash(qname, false); } @@ -509,7 +524,7 @@ response_balance(dns_rrl_t *rrl, const dns_rrl_entry_t *e, int age) { * Search for an entry for a response and optionally create it. */ static dns_rrl_entry_t * -get_entry(dns_rrl_t *rrl, const isc_sockaddr_t *client_addr, +get_entry(dns_rrl_t *rrl, const isc_sockaddr_t *client_addr, dns_zone_t *zone, dns_rdataclass_t qclass, dns_rdatatype_t qtype, const dns_name_t *qname, dns_rrl_rtype_t rtype, isc_stdtime_t now, bool create, char *log_buf, unsigned int log_buf_len) { @@ -520,7 +535,7 @@ get_entry(dns_rrl_t *rrl, const isc_sockaddr_t *client_addr, dns_rrl_bin_t *new_bin, *old_bin; int probes, age; - make_key(rrl, &key, client_addr, qtype, qname, qclass, rtype); + make_key(rrl, &key, client_addr, zone, qtype, qname, qclass, rtype); hval = hash_key(&key); /* @@ -650,9 +665,9 @@ debit_rrl_entry(dns_rrl_t *rrl, dns_rrl_entry_t *e, double qps, double scale, /* * The limit for clients that have used TCP is not scaled. */ - credit_e = get_entry(rrl, client_addr, 0, dns_rdatatype_none, - NULL, DNS_RRL_RTYPE_TCP, now, false, - log_buf, log_buf_len); + credit_e = get_entry( + rrl, client_addr, NULL, 0, dns_rdatatype_none, NULL, + DNS_RRL_RTYPE_TCP, now, false, log_buf, log_buf_len); if (credit_e != NULL) { age = get_age(rrl, e, now); if (age < rrl->window) { @@ -1027,10 +1042,10 @@ log_stops(dns_rrl_t *rrl, isc_stdtime_t now, int limit, char *log_buf, * Main rate limit interface. */ dns_rrl_result_t -dns_rrl(dns_view_t *view, const isc_sockaddr_t *client_addr, bool is_tcp, - dns_rdataclass_t qclass, dns_rdatatype_t qtype, const dns_name_t *qname, - isc_result_t resp_result, isc_stdtime_t now, bool wouldlog, - char *log_buf, unsigned int log_buf_len) { +dns_rrl(dns_view_t *view, dns_zone_t *zone, const isc_sockaddr_t *client_addr, + bool is_tcp, dns_rdataclass_t qclass, dns_rdatatype_t qtype, + const dns_name_t *qname, isc_result_t resp_result, isc_stdtime_t now, + bool wouldlog, char *log_buf, unsigned int log_buf_len) { dns_rrl_t *rrl; dns_rrl_rtype_t rtype; dns_rrl_entry_t *e; @@ -1103,9 +1118,10 @@ dns_rrl(dns_view_t *view, const isc_sockaddr_t *client_addr, bool is_tcp, */ if (is_tcp) { if (scale < 1.0) { - e = get_entry(rrl, client_addr, 0, dns_rdatatype_none, - NULL, DNS_RRL_RTYPE_TCP, now, true, - log_buf, log_buf_len); + e = get_entry(rrl, client_addr, NULL, 0, + dns_rdatatype_none, NULL, + DNS_RRL_RTYPE_TCP, now, true, log_buf, + log_buf_len); if (e != NULL) { e->responses = -(rrl->window + 1); set_age(rrl, e, now); @@ -1136,8 +1152,8 @@ dns_rrl(dns_view_t *view, const isc_sockaddr_t *client_addr, bool is_tcp, rtype = DNS_RRL_RTYPE_ERROR; break; } - e = get_entry(rrl, client_addr, qclass, qtype, qname, rtype, now, true, - log_buf, log_buf_len); + e = get_entry(rrl, client_addr, zone, qclass, qtype, qname, rtype, now, + true, log_buf, log_buf_len); if (e == NULL) { UNLOCK(&rrl->lock); return (DNS_RRL_RESULT_OK); @@ -1171,8 +1187,8 @@ dns_rrl(dns_view_t *view, const isc_sockaddr_t *client_addr, bool is_tcp, dns_rrl_entry_t *e_all; dns_rrl_result_t rrl_all_result; - e_all = get_entry(rrl, client_addr, 0, dns_rdatatype_none, NULL, - DNS_RRL_RTYPE_ALL, now, true, log_buf, + e_all = get_entry(rrl, client_addr, zone, 0, dns_rdatatype_none, + NULL, DNS_RRL_RTYPE_ALL, now, true, log_buf, log_buf_len); if (e_all == NULL) { UNLOCK(&rrl->lock); diff --git a/lib/ns/client.c b/lib/ns/client.c index 6476edd59d..6c491afa59 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -836,10 +836,11 @@ ns_client_error(ns_client_t *client, isc_result_t result) { loglevel = ISC_LOG_DEBUG(1); } wouldlog = isc_log_wouldlog(ns_lctx, loglevel); - rrl_result = dns_rrl( - client->view, &client->peeraddr, TCP_CLIENT(client), - dns_rdataclass_in, dns_rdatatype_none, NULL, result, - client->now, wouldlog, log_buf, sizeof(log_buf)); + rrl_result = dns_rrl(client->view, NULL, &client->peeraddr, + TCP_CLIENT(client), dns_rdataclass_in, + dns_rdatatype_none, NULL, result, + client->now, wouldlog, log_buf, + sizeof(log_buf)); if (rrl_result != DNS_RRL_RESULT_OK) { /* * Log dropped errors in the query category diff --git a/lib/ns/query.c b/lib/ns/query.c index 93d2d6c353..beeae0e653 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -7028,10 +7028,10 @@ query_checkrrl(query_ctx_t *qctx, isc_result_t result) { } rrl_result = dns_rrl( - qctx->view, &qctx->client->peeraddr, TCP(qctx->client), - qctx->client->message->rdclass, qctx->qtype, constname, - resp_result, qctx->client->now, wouldlog, log_buf, - sizeof(log_buf)); + qctx->view, qctx->zone, &qctx->client->peeraddr, + TCP(qctx->client), qctx->client->message->rdclass, + qctx->qtype, constname, resp_result, qctx->client->now, + wouldlog, log_buf, sizeof(log_buf)); if (rrl_result != DNS_RRL_RESULT_OK) { /* * Log dropped or slipped responses in the query