From dbcf683c1a57f49876e329fca183cb39d20ca3a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 2 Oct 2020 08:41:43 +0200 Subject: [PATCH 1/4] Allow "order none" in "rrset-order" rules named-checkconf treats the following configuration as valid: options { rrset-order { order none; }; }; Yet, the above configuration causes named to crash on startup with: order.c:74: REQUIRE(mode == 0x00000800 || mode == 0x00000400 || mode == 0x00800000) failed, back trace Add DNS_RDATASETATTR_NONE to the list of RRset ordering modes accepted by dns_order_add() to allow "order none" to be used in "rrset-order" rules. This both prevents the aforementioned crashes and addresses the discrepancy between named-checkconf and named. --- lib/dns/order.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dns/order.c b/lib/dns/order.c index 463df94d01..fbd1c9f6da 100644 --- a/lib/dns/order.c +++ b/lib/dns/order.c @@ -73,7 +73,8 @@ dns_order_add(dns_order_t *order, const dns_name_t *name, REQUIRE(DNS_ORDER_VALID(order)); REQUIRE(mode == DNS_RDATASETATTR_RANDOMIZE || mode == DNS_RDATASETATTR_FIXEDORDER || - mode == DNS_RDATASETATTR_CYCLIC); + mode == DNS_RDATASETATTR_CYCLIC || + mode == DNS_RDATASETATTR_NONE); ent = isc_mem_get(order->mctx, sizeof(*ent)); From abdd4c89fc7f4bf8f2eb502fa00e9d19f92bbdc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 2 Oct 2020 08:41:43 +0200 Subject: [PATCH 2/4] Add tests for "order none" RRset ordering rules Make sure "order none" RRset ordering rules are tested in the "rrsetorder" system test just like all other rule types are. As the check for the case of no "rrset-order" rule matching a given RRset also tests "order none" (rather than "order random", as the test code may suggest at first glance), replace the test code for that case so that it matches other "order none" tests. --- .../checkconf/good-rrset-order-none.conf | 5 + bin/tests/system/rrsetorder/clean.sh | 2 +- bin/tests/system/rrsetorder/ns1/named.conf.in | 1 + bin/tests/system/rrsetorder/ns1/root.db | 5 + bin/tests/system/rrsetorder/ns2/named.conf.in | 1 + bin/tests/system/rrsetorder/ns3/named.conf.in | 1 + bin/tests/system/rrsetorder/tests.sh | 103 +++++++++++++----- 7 files changed, 90 insertions(+), 28 deletions(-) create mode 100644 bin/tests/system/checkconf/good-rrset-order-none.conf diff --git a/bin/tests/system/checkconf/good-rrset-order-none.conf b/bin/tests/system/checkconf/good-rrset-order-none.conf new file mode 100644 index 0000000000..afd0ab660b --- /dev/null +++ b/bin/tests/system/checkconf/good-rrset-order-none.conf @@ -0,0 +1,5 @@ +options { + rrset-order { + order none; + }; +}; diff --git a/bin/tests/system/rrsetorder/clean.sh b/bin/tests/system/rrsetorder/clean.sh index ea90d8f67f..97c025ae32 100644 --- a/bin/tests/system/rrsetorder/clean.sh +++ b/bin/tests/system/rrsetorder/clean.sh @@ -10,7 +10,7 @@ # information regarding copyright ownership. rm -f dig.out.test* -rm -f dig.out.cyclic dig.out.fixed dig.out.random dig.out.nomatch +rm -f dig.out.cyclic dig.out.fixed dig.out.random dig.out.nomatch dig.out.none rm -f dig.out.0 dig.out.1 dig.out.2 dig.out.3 rm -f dig.out.cyclic2 rm -f ns2/root.bk diff --git a/bin/tests/system/rrsetorder/ns1/named.conf.in b/bin/tests/system/rrsetorder/ns1/named.conf.in index bccd32a4f8..d4ee9ed195 100644 --- a/bin/tests/system/rrsetorder/ns1/named.conf.in +++ b/bin/tests/system/rrsetorder/ns1/named.conf.in @@ -24,6 +24,7 @@ options { name "fixed.example" order fixed; name "random.example" order random; name "cyclic.example" order cyclic; + name "none.example" order none; type NS order random; order cyclic; }; diff --git a/bin/tests/system/rrsetorder/ns1/root.db b/bin/tests/system/rrsetorder/ns1/root.db index b5a8f8f982..2367f1025f 100644 --- a/bin/tests/system/rrsetorder/ns1/root.db +++ b/bin/tests/system/rrsetorder/ns1/root.db @@ -42,3 +42,8 @@ nomatch.example. A 1.2.3.1 nomatch.example. A 1.2.3.2 nomatch.example. A 1.2.3.3 nomatch.example. A 1.2.3.4 +; +none.example. A 1.2.3.1 +none.example. A 1.2.3.2 +none.example. A 1.2.3.3 +none.example. A 1.2.3.4 diff --git a/bin/tests/system/rrsetorder/ns2/named.conf.in b/bin/tests/system/rrsetorder/ns2/named.conf.in index 7d0ecfbf0a..a3742cf79b 100644 --- a/bin/tests/system/rrsetorder/ns2/named.conf.in +++ b/bin/tests/system/rrsetorder/ns2/named.conf.in @@ -24,6 +24,7 @@ options { name "fixed.example" order fixed; name "random.example" order random; name "cyclic.example" order cyclic; + name "none.example" order none; type NS order random; order cyclic; }; diff --git a/bin/tests/system/rrsetorder/ns3/named.conf.in b/bin/tests/system/rrsetorder/ns3/named.conf.in index 7343a7e618..1f3b54ac25 100644 --- a/bin/tests/system/rrsetorder/ns3/named.conf.in +++ b/bin/tests/system/rrsetorder/ns3/named.conf.in @@ -24,6 +24,7 @@ options { name "fixed.example" order fixed; name "random.example" order random; name "cyclic.example" order cyclic; + name "none.example" order none; type NS order random; order cyclic; }; diff --git a/bin/tests/system/rrsetorder/tests.sh b/bin/tests/system/rrsetorder/tests.sh index 5adcca20a0..1c63711051 100644 --- a/bin/tests/system/rrsetorder/tests.sh +++ b/bin/tests/system/rrsetorder/tests.sh @@ -142,6 +142,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +echo_i "Checking order none (primary)" +ret=0 +# Fetch the "reference" response and ensure it contains the expected records. +$DIGCMD @10.53.0.1 none.example > dig.out.none || ret=1 +for i in 1 2 3 4; do + grep -F -q 1.2.3.$i dig.out.none || ret=1 +done +# Ensure 20 further queries result in the same response as the "reference" one. +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + $DIGCMD @10.53.0.1 none.example > dig.out.test$i || ret=1 + $DIFF dig.out.none dig.out.test$i >/dev/null || ret=1 +done +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + # # # @@ -236,6 +251,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +echo_i "Checking order none (secondary)" +ret=0 +# Fetch the "reference" response and ensure it contains the expected records. +$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1 +for i in 1 2 3 4; do + grep -F -q 1.2.3.$i dig.out.none || ret=1 +done +# Ensure 20 further queries result in the same response as the "reference" one. +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + $DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1 + $DIFF dig.out.none dig.out.test$i >/dev/null || ret=1 +done +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + echo_i "Shutting down secondary" (cd ..; $SHELL stop.sh rrsetorder ns2 ) @@ -346,6 +376,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` +echo_i "Checking order none (secondary loaded from disk)" +ret=0 +# Fetch the "reference" response and ensure it contains the expected records. +$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1 +for i in 1 2 3 4; do + grep -F -q 1.2.3.$i dig.out.none || ret=1 +done +# Ensure 20 further queries result in the same response as the "reference" one. +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + $DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1 + $DIFF dig.out.none dig.out.test$i >/dev/null || ret=1 +done +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + # # # @@ -443,6 +488,21 @@ echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 3 if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi +echo_i "Checking order none (cache)" +ret=0 +# Fetch the "reference" response and ensure it contains the expected records. +$DIGCMD @10.53.0.3 none.example > dig.out.none || ret=1 +for i in 1 2 3 4; do + grep -F -q 1.2.3.$i dig.out.none || ret=1 +done +# Ensure 20 further queries result in the same response as the "reference" one. +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + $DIGCMD @10.53.0.3 none.example > dig.out.test$i || ret=1 + $DIFF dig.out.none dig.out.test$i >/dev/null || ret=1 +done +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + echo_i "Checking default order (cache)" ret=0 for i in $GOOD_RANDOM @@ -468,33 +528,22 @@ done echo_i "Default selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi - -echo_i "Checking default order no match in rrset-order (no shuffling)" -ret=0 -for i in $GOOD_RANDOM -do - eval match$i=0 -done -for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 -do -$DIGCMD @10.53.0.4 nomatch.example > dig.out.nomatch|| ret=1 - match=0 - for j in $GOOD_RANDOM - do - eval "$DIFF dig.out.nomatch dig.out.random.good$j >/dev/null && match$j=1 match=1" - if [ $match -eq 1 ]; then break; fi - done - if [ $match -eq 0 ]; then ret=1; fi -done -match=0 -for i in $GOOD_RANDOM -do -eval "match=\`expr \$match + \$match$i\`" -done -echo_i "Consistent selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -ne 1 ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi - status=`expr $status + $ret` + +echo_i "Checking default order no match in rrset-order (cache)" +ret=0 +# Fetch the "reference" response and ensure it contains the expected records. +$DIGCMD @10.53.0.4 nomatch.example > dig.out.nomatch || ret=1 +for i in 1 2 3 4; do + grep -F -q 1.2.3.$i dig.out.nomatch || ret=1 +done +# Ensure 20 further queries result in the same response as the "reference" one. +for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do + $DIGCMD @10.53.0.4 nomatch.example > dig.out.test$i || ret=1 + $DIFF dig.out.nomatch dig.out.test$i >/dev/null || ret=1 +done +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 From 2ac04dc9305438f424a3a11c540d505e44db013e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 2 Oct 2020 08:41:43 +0200 Subject: [PATCH 3/4] Rework "rrset-order" documentation Certain parts of the existing documentation for the "rrset-order" statement are incorrect, others are ambiguous. Rework the relevant section of the ARM to make it clear and up-to-date with the source code. --- doc/arm/reference.rst | 110 ++++++++++++++++++++++++++++++------------ 1 file changed, 78 insertions(+), 32 deletions(-) diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index f9bb269d3d..2289559e3a 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -3139,58 +3139,104 @@ are not sorted. RRset Ordering ^^^^^^^^^^^^^^ -When multiple records are returned in an answer, it may be useful to -configure the order of the records placed into the response. The -``rrset-order`` statement permits configuration of the ordering of the -records in a multiple-record response. See also the ``sortlist`` -statement, :ref:`the_sortlist_statement`. +.. note:: -An ``order_spec`` is defined as follows: + While alternating the order of records in a DNS response between + subsequent queries is a known load distribution technique, certain + caveats apply (mostly stemming from caching) which usually make it a + suboptimal choice for load balancing purposes when used on its own. -[class *class_name*] [type *type_name*] [name "*domain_name*"] order *ordering* +The ``rrset-order`` statement permits configuration of the ordering of +the records in a multiple-record response. See also: +:ref:`the_sortlist_statement`. -If no class is specified, the default is ``ANY``. If no type is -specified, the default is ``ANY``. If no name is specified, the default -is ``*`` (asterisk). +Each rule in an ``rrset-order`` statement is defined as follows: + +:: + + [class ] [type ] [name ""] order + +The default qualifiers for each rule are: + + - If no ``class`` is specified, the default is ``ANY``. + - If no ``type`` is specified, the default is ``ANY``. + - If no ``name`` is specified, the default is ``*`` (asterisk). + +```` only matches the name itself, not any of its +subdomains. To make a rule match all subdomains of a given name, a +wildcard name (``*.``) must be used. Note that +``*.`` does *not* match ```` itself; to +specify RRset ordering for a name and all of its subdomains, two +separate rules must be defined: one for ```` and one for +``*.``. The legal values for ``ordering`` are: ``fixed`` - Records are returned in the order they are defined in the zone file. This option is only available if BIND is configured with ``--enable-fixed-rrset`` at compile time. + Records are returned in the order they are defined in the zone file. + +.. note:: + + The ``fixed`` option is only available if BIND is configured with + ``--enable-fixed-rrset`` at compile time. ``random`` Records are returned in a random order. ``cyclic`` - Records are returned in a cyclic round-robin order, rotating by one record per query. If BIND is configured with ``--enable-fixed-rrset`` at compile time, the initial ordering of the RRset matches the one specified in the zone file; otherwise the initial ordering is indeterminate. + Records are returned in a cyclic round-robin order, rotating by one + record per query. ``none`` - Records are returned in whatever order they were retrieved from the database. This order is indeterminate, but remains consistent as long as the database is not modified. When no ordering is specified, this is the default. + Records are returned in the order they were retrieved from the + database. This order is indeterminate, but remains consistent as + long as the database is not modified. -For example: +The default RRset order used depends on whether any ``rrset-order`` +statements are present in the configuration file used by ``named``: + + - If no ``rrset-order`` statement is present in the configuration + file, the implicit default is to return all records in ``random`` + order. + + - If any ``rrset-order`` statements are present in the configuration + file, but no ordering rule specified in these statements matches a + given RRset, the default order for that RRset is ``none``. + +Note that if multiple ``rrset-order`` statements are present in the +configuration file (at both the ``options`` and ``view`` levels), they +are *not* combined; instead, the more-specific one (``view``) replaces +the less-specific one (``options``). + +If multiple rules within a single ``rrset-order`` statement match a +given RRset, the first matching rule is applied. + +Example: :: - rrset-order { - class IN type A name "host.example.com" order random; - order cyclic; - }; + rrset-order { + type A name "foo.isc.org" order random; + type AAAA name "foo.isc.org" order cyclic; + name "bar.isc.org" order fixed; + name "*.bar.isc.org" order random; + name "*.baz.isc.org" order cyclic; + }; -causes any responses for type A records in class IN, that have -``host.example.com`` as a suffix, to always be returned in random -order. All other records are returned in cyclic order. +With the above configuration, the following RRset ordering is used: -If multiple ``rrset-order`` statements appear, they are not combined; -the last one applies. - -By default, records are returned in ``random`` order. - -.. note:: - - "Fixed" ordering of the ``rrset-order`` statement by default is not - currently supported in BIND 9. Fixed ordering can be enabled at - compile time by specifying "--enable-fixed-rrset" on the "configure" - command line. +=================== ======== =========== +QNAME QTYPE RRset Order +=================== ======== =========== +``foo.isc.org`` ``A`` ``random`` +``foo.isc.org`` ``AAAA`` ``cyclic`` +``foo.isc.org`` ``TXT`` ``none`` +``sub.foo.isc.org`` all ``none`` +``bar.isc.org`` all ``fixed`` +``sub.bar.isc.org`` all ``random`` +``baz.isc.org`` all ``none`` +``sub.baz.isc.org`` all ``cyclic`` +=================== ======== =========== .. _tuning: From 27c815a220a1b18756cc643f3ec1c16635fedda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 2 Oct 2020 08:41:43 +0200 Subject: [PATCH 4/4] Add CHANGES entries --- CHANGES | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 83de584f5b..10f1cde154 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,11 @@ +5513. [doc] The ARM section describing the "rrset-order" statement + was rewritten to make it unambiguous and up-to-date with + the source code. [GL #2139] + +5512. [bug] "rrset-order" rules using "order none" were causing + named to crash despite named-checkconf treating them as + valid. [GL #2139] + 5511. [bug] 'dig -u +yaml' failed to display timestamps to the microsecond. [GL #2190]