From: Ondřej Surý Date: Thu, 28 Aug 2025 10:24:17 +0000 (+0200) Subject: Remove the random ordering of resource records in RRset X-Git-Tag: v9.21.14~57^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7dc6048f9372e7b9781aae36637c9210ce561161;p=thirdparty%2Fbind9.git Remove the random ordering of resource records in RRset The rrset-order random doesn't offer uniform distribution of all permutations and it isn't superior to cyclic order in any way. Make the random ordering an alias to the cyclic ordering. --- diff --git a/bin/named/config.c b/bin/named/config.c index b6718a4cef2..6e7d481aad6 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -105,7 +105,7 @@ options {\n\ request-zoneversion false;\n\ resolver-query-timeout 10;\n\ # responselog ;\n\ - rrset-order { order random; };\n\ + rrset-order { order cyclic; };\n\ secroots-file \"named.secroots\";\n\ send-cookie true;\n\ serial-query-rate 20;\n\ diff --git a/bin/named/server.c b/bin/named/server.c index e93d252c1d0..cce0be6d7bf 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1360,7 +1360,10 @@ configure_order(dns_order_t *order, const cfg_obj_t *ent) { INSIST(cfg_obj_isstring(obj)); str = cfg_obj_asstring(obj); if (!strcasecmp(str, "random")) { - mode = dns_order_randomize; + cfg_obj_log(obj, ISC_LOG_WARNING, + "random ordering is obsolete; " + "cyclic ordering is being used instead"); + mode = dns_order_cyclic; } else if (!strcasecmp(str, "cyclic")) { mode = dns_order_cyclic; } else if (!strcasecmp(str, "none")) { diff --git a/bin/tests/system/rrsetorder/ns1/named.conf.j2 b/bin/tests/system/rrsetorder/ns1/named.conf.j2 index 213a006545b..21c94f0197a 100644 --- a/bin/tests/system/rrsetorder/ns1/named.conf.j2 +++ b/bin/tests/system/rrsetorder/ns1/named.conf.j2 @@ -24,7 +24,6 @@ options { dnssec-validation no; notify no; rrset-order { - name "random.example" order random; name "cyclic.example" order cyclic; name "none.example" order none; type NS order random; diff --git a/bin/tests/system/rrsetorder/tests.sh b/bin/tests/system/rrsetorder/tests.sh index 0358601d1d3..5064d7ebaf1 100644 --- a/bin/tests/system/rrsetorder/tests.sh +++ b/bin/tests/system/rrsetorder/tests.sh @@ -76,28 +76,6 @@ diff dig.out.2 dig.out.3 >/dev/null && ret=1 if [ $matches -ne 16 ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) -echo_i "Checking order random (primary)" -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 - dig_cmd @10.53.0.1 random.example >dig.out.random || ret=1 - match=0 - for j in $GOOD_RANDOM; do - eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true" - 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=\$((match + match$i))" -done -echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status + ret)) echo_i "Checking order none (primary)" ret=0 @@ -164,29 +142,6 @@ if [ $matches -ne 16 ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) -echo_i "Checking order random (secondary)" -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 - dig_cmd @10.53.0.2 random.example >dig.out.random || ret=1 - match=0 - for j in $GOOD_RANDOM; do - eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true" - 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=\$((match + match$i))" -done -echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status + ret)) - echo_i "Checking order none (secondary)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. @@ -267,29 +222,6 @@ if [ $matches -ne 16 ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) -echo_i "Checking order random (secondary loaded from disk)" -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 - dig_cmd @10.53.0.2 random.example >dig.out.random || ret=1 - match=0 - for j in $GOOD_RANDOM; do - eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true" - 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=\$((match + match$i))" -done -echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status + ret)) - echo_i "Checking order none (secondary loaded from disk)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. @@ -359,29 +291,6 @@ if [ $matches -ne 16 ]; then ret=1; fi if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) -echo_i "Checking order random (cache)" -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 - dig_cmd @10.53.0.3 random.example >dig.out.random || ret=1 - match=0 - for j in $GOOD_RANDOM; do - eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true" - 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=\$((match + match$i))" -done -echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status + ret)) - echo_i "Checking order none (cache)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. @@ -397,29 +306,6 @@ done if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) -echo_i "Checking default order (cache)" -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 - dig_cmd @10.53.0.5 random.example >dig.out.random || ret=1 - match=0 - for j in $GOOD_RANDOM; do - eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true" - 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=\$((match + match$i))" -done -echo_i "Default selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples" -if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((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. diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index 6f64db05e6d..5467a099aff 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -4050,8 +4050,8 @@ RRset Ordering The legal values for ```` are: ``random`` - Records are returned in a non-deterministic order. The random ordering - doesn't guarantee uniform distribution of all permutations. + This value has been deprecated and using ``random`` behaves + exactly like ``cyclic``. ``cyclic`` Records are returned in a cyclic round-robin order, rotating by one @@ -4086,9 +4086,9 @@ RRset Ordering :: rrset-order { - type A name "foo.isc.org" order random; + type A name "foo.isc.org" order none; type AAAA name "foo.isc.org" order cyclic; - name "*.bar.isc.org" order random; + name "*.bar.isc.org" order none; name "*.baz.isc.org" order cyclic; }; @@ -4097,11 +4097,11 @@ RRset Ordering =================== ======== =========== QNAME QTYPE RRset Order =================== ======== =========== - ``foo.isc.org`` ``A`` ``random`` + ``foo.isc.org`` ``A`` ``none`` ``foo.isc.org`` ``AAAA`` ``cyclic`` ``foo.isc.org`` ``TXT`` ``none`` ``sub.foo.isc.org`` all ``none`` - ``sub.bar.isc.org`` all ``random`` + ``sub.bar.isc.org`` all ``none`` ``baz.isc.org`` all ``none`` ``sub.baz.isc.org`` all ``cyclic`` =================== ======== =========== diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 11eab4baf73..d45a877bf84 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -238,7 +238,6 @@ typedef enum { typedef enum { dns_order_none, dns_order_cyclic, - dns_order_randomize } dns_orderopt_t; typedef enum { diff --git a/lib/dns/rdataset.c b/lib/dns/rdataset.c index fb7a919459d..db5d8c9322e 100644 --- a/lib/dns/rdataset.c +++ b/lib/dns/rdataset.c @@ -209,21 +209,8 @@ dns_rdataset_current(dns_rdataset_t *rdataset, dns_rdata_t *rdata) { } #define MAX_SHUFFLE 32 -#define WANT_RANDOM(r) (((r)->attributes.order == dns_order_randomize)) #define WANT_CYCLIC(r) (((r)->attributes.order == dns_order_cyclic)) -struct towire_sort { - int key; - dns_rdata_t *rdata; -}; - -static void -swap_rdata(dns_rdata_t *in, unsigned int a, unsigned int b) { - dns_rdata_t rdata = in[a]; - in[a] = in[b]; - in[b] = rdata; -} - static isc_result_t towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, dns_compress_t *cctx, isc_buffer_t *target, bool partial, @@ -236,11 +223,11 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, unsigned int headlen; bool question = false; bool shuffle = false; - bool want_random, want_cyclic; + bool want_cyclic; dns_rdata_t in_fixed[MAX_SHUFFLE]; dns_rdata_t *in = in_fixed; - struct towire_sort out_fixed[MAX_SHUFFLE]; - struct towire_sort *out = out_fixed; + dns_rdata_t *out_fixed[MAX_SHUFFLE]; + dns_rdata_t **out = out_fixed; dns_fixedname_t fixed; dns_name_t *name = NULL; @@ -254,7 +241,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, REQUIRE(countp != NULL); REQUIRE(cctx != NULL && cctx->mctx != NULL); - want_random = WANT_RANDOM(rdataset); want_cyclic = WANT_CYCLIC(rdataset); if (rdataset->attributes.question) { @@ -287,7 +273,7 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, * Do we want to shuffle this answer? */ if (!question && count > 1 && rdataset->type != dns_rdatatype_rrsig) { - if (want_random || want_cyclic) { + if (want_cyclic) { shuffle = true; } } @@ -303,7 +289,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, } if (shuffle) { - uint32_t seed = 0; unsigned int j = 0; /* @@ -322,10 +307,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, } INSIST(i == count); - if (want_random) { - seed = isc_random32(); - } - if (want_cyclic && (rdataset->count != DNS_RDATASET_COUNT_UNDEFINED)) { @@ -333,12 +314,7 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, } for (i = 0; i < count; i++) { - if (want_random) { - swap_rdata(in, j, j + seed % (count - j)); - } - - out[i].key = 0; - out[i].rdata = &in[j]; + out[i] = &in[j]; if (++j == count) { j = 0; } @@ -379,7 +355,8 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, isc_buffer_putuint16(target, rdataset->type); isc_buffer_putuint16(target, rdataset->rdclass); if (!question) { - dns_rdata_t rdata = DNS_RDATA_INIT; + dns_rdata_t rdata_s = DNS_RDATA_INIT; + dns_rdata_t *rdata = &rdata_s; isc_buffer_putuint32(target, rdataset->ttl); @@ -393,12 +370,12 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name, * Copy out the rdata */ if (shuffle) { - rdata = *(out[i].rdata); + rdata = out[i]; } else { - dns_rdata_reset(&rdata); - dns_rdataset_current(rdataset, &rdata); + dns_rdata_reset(&rdata_s); + dns_rdataset_current(rdataset, &rdata_s); } - result = dns_rdata_towire(&rdata, cctx, target); + result = dns_rdata_towire(rdata, cctx, target); if (result != ISC_R_SUCCESS) { goto rollback; }