From 8ca45ba01af487896eb095589055d006d70fc0fd Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 13 Dec 2016 15:47:03 +1100 Subject: [PATCH] 4533. [bug] dns_client_update should terminate on prerequiste failures (NXDOMAIN, YXDOMAIN, NXRRSET, YXRRSET) and also on BADZONE. [RT #43865] --- CHANGES | 4 + bin/tests/system/conf.sh.in | 2 + bin/tests/system/nsupdate/clean.sh | 5 + bin/tests/system/nsupdate/ns1/named.conf | 6 + bin/tests/system/nsupdate/ns1/sample.db.in | 8 ++ bin/tests/system/nsupdate/ns2/named.conf | 6 + bin/tests/system/nsupdate/ns2/sample.db.in | 10 ++ bin/tests/system/nsupdate/setup.sh | 3 + bin/tests/system/nsupdate/tests.sh | 71 ++++++++++++ lib/dns/client.c | 17 ++- lib/samples/sample-update.c | 126 ++++++++++----------- 11 files changed, 188 insertions(+), 70 deletions(-) create mode 100644 bin/tests/system/nsupdate/ns1/sample.db.in create mode 100644 bin/tests/system/nsupdate/ns2/sample.db.in diff --git a/CHANGES b/CHANGES index a7d7674304..f6b8d7a166 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4533. [bug] dns_client_update should terminate on prerequiste + failures (NXDOMAIN, YXDOMAIN, NXRRSET, YXRRSET) + and also on BADZONE. [RT #43865] + 4532. [contrib] Make gen-data-queryperf.py python 3 compatible. [RT #43836] diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in index 78440ee476..ff7c46b8fb 100644 --- a/bin/tests/system/conf.sh.in +++ b/bin/tests/system/conf.sh.in @@ -65,6 +65,7 @@ KEYDELETE=$TOP/bin/tests/system/tkey/keydelete LWTEST=$TOP/bin/tests/system/lwresd/lwtest MAKEJOURNAL=$TOP/bin/tests/makejournal PIPEQUERIES=$TOP/bin/tests/system/pipelined/pipequeries +SAMPLEUPDATE=$TOP/lib/samples/sample-update # The "stress" test is not run by default since it creates enough # load on the machine to make it unusable to other users. @@ -166,6 +167,7 @@ export RANDFILE export RESOLVE export RNDC export RRCHECKER +export SAMPLEUPDATE export SIGNER export SUBDIRS export TESTSOCK6 diff --git a/bin/tests/system/nsupdate/clean.sh b/bin/tests/system/nsupdate/clean.sh index 5c9d0405b7..a72ef61bbe 100644 --- a/bin/tests/system/nsupdate/clean.sh +++ b/bin/tests/system/nsupdate/clean.sh @@ -34,3 +34,8 @@ rm -f ns3/nsec3param.test.db rm -f ns3/too-big.test.db rm -f nsupdate.out* rm -f typelist.out.* +rm -f ns1/sample.db +rm -f ns2/sample.db +rm -f update.out.* +rm -f check.out.* +rm -f update.out.* diff --git a/bin/tests/system/nsupdate/ns1/named.conf b/bin/tests/system/nsupdate/ns1/named.conf index 5d89e91889..350792837e 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf +++ b/bin/tests/system/nsupdate/ns1/named.conf @@ -124,3 +124,9 @@ zone "many.test" { allow-update { any; }; file "many.test.db"; }; + +zone "sample" { + type master; + allow-update { any; }; + file "sample.db"; +}; diff --git a/bin/tests/system/nsupdate/ns1/sample.db.in b/bin/tests/system/nsupdate/ns1/sample.db.in new file mode 100644 index 0000000000..6881eb0162 --- /dev/null +++ b/bin/tests/system/nsupdate/ns1/sample.db.in @@ -0,0 +1,8 @@ +@ 0 SOA ns1 . 0 0 0 0 0 +@ 0 NS ns1 +ns1 0 A 10.53.0.1 +; a RRset that exists +exists 0 TXT This RRset exists. +; nxdomain +; A named without a TXT RRset +no-txt 0 A 1.2.3.4 diff --git a/bin/tests/system/nsupdate/ns2/named.conf b/bin/tests/system/nsupdate/ns2/named.conf index 9de2e80fa7..a14af0e2d9 100644 --- a/bin/tests/system/nsupdate/ns2/named.conf +++ b/bin/tests/system/nsupdate/ns2/named.conf @@ -55,4 +55,10 @@ view primary { file "update.bk"; allow-transfer { any; }; }; + + zone "sample" { + type master; + allow-update { any; }; + file "sample.db"; + }; }; diff --git a/bin/tests/system/nsupdate/ns2/sample.db.in b/bin/tests/system/nsupdate/ns2/sample.db.in new file mode 100644 index 0000000000..fde9dae8d6 --- /dev/null +++ b/bin/tests/system/nsupdate/ns2/sample.db.in @@ -0,0 +1,10 @@ +@ 0 SOA ns2 . 0 0 0 0 0 +@ 0 NS ns2 +ns2 0 A 10.53.0.2 +; +; These prerequistes are reversed, relative to ns1/sample.db.in: +; 'exists' does not exist. +nxdomain 0 TXT This RRset exists. + +; a name with a TXT RRset +no-txt 0 TXT This RRset exists diff --git a/bin/tests/system/nsupdate/setup.sh b/bin/tests/system/nsupdate/setup.sh index c5f0e26fce..3ca59fcea1 100644 --- a/bin/tests/system/nsupdate/setup.sh +++ b/bin/tests/system/nsupdate/setup.sh @@ -58,3 +58,6 @@ $DDNSCONFGEN -q -r $RANDFILE -a hmac-sha512 -k sha512-key -z keytests.nil > ns1/ cp -f ns1/many.test.db.in ns1/many.test.db rm -f ns1/many.test.db.jnl + +cp ns1/sample.db.in ns1/sample.db +cp ns2/sample.db.in ns2/sample.db diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 8a1e4797e0..c771bd8e4f 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -701,5 +701,76 @@ grep "status: NXDOMAIN" dig.out.ns3.test$n > /dev/null || ret=1 grep "records in zone (4) exceeds max-records (3)" ns3/named.run > /dev/null || ret=1 [ $ret = 0 ] || { echo I:failed; status=1; } +# +# Add client library tests here +# +n=`expr $n + 1` +echo "I:check that dns_client_update handles prerequisite NXDOMAIN failure ($n)" +$SAMPLEUPDATE -P 5300 -a 10.53.0.1 -a 10.53.0.2 -p "nxdomain exists.sample" \ + add "nxdomain-exists.sample 0 in a 1.2.3.4" > update.out.test$n 2>&1 +$SAMPLEUPDATE -P 5300 -a 10.53.0.2 -p "nxdomain exists.sample" \ + add "check-nxdomain-exists.sample 0 in a 1.2.3.4" > update.out.check$n 2>&1 +$DIG +tcp @10.53.0.1 -p 5300 a nxdomain-exists.sample > dig.out.ns1.test$n +$DIG +tcp @10.53.0.2 -p 5300 a nxdomain-exists.sample > dig.out.ns2.test$n +$DIG +tcp @10.53.0.2 -p 5300 a check-nxdomain-exists.sample > check.out.ns2.test$n +grep "update failed: YXDOMAIN" update.out.test$n > /dev/null || ret=1 +grep "update succeeded" update.out.check$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns1.test$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns2.test$n > /dev/null || ret=1 +grep "status: NOERROR" check.out.ns2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo I:failed; status=1; } + +n=`expr $n + 1` +echo "I:check that dns_client_update handles prerequisite YXDOMAIN failure ($n)" +$SAMPLEUPDATE -P 5300 -a 10.53.0.1 -a 10.53.0.2 -p "yxdomain nxdomain.sample" \ + add "yxdomain-nxdomain.sample 0 in a 1.2.3.4" > update.out.test$n 2>&1 +$SAMPLEUPDATE -P 5300 -a 10.53.0.2 -p "yxdomain nxdomain.sample" \ + add "check-yxdomain-nxdomain.sample 0 in a 1.2.3.4" > update.out.check$n 2>&1 +$DIG +tcp @10.53.0.1 -p 5300 a nxdomain-exists.sample > dig.out.ns1.test$n +$DIG +tcp @10.53.0.2 -p 5300 a nxdomain-exists.sample > dig.out.ns2.test$n +$DIG +tcp @10.53.0.2 -p 5300 a check-nxdomain-exists.sample > check.out.ns2.test$n +grep "update failed: NXDOMAIN" update.out.test$n > /dev/null || ret=1 +grep "update succeeded" update.out.check$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns1.test$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns2.test$n > /dev/null || ret=1 +grep "status: NOERROR" check.out.ns2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo I:failed; status=1; } + +n=`expr $n + 1` +echo "I:check that dns_client_update handles prerequisite NXRRSET failure ($n)" +$SAMPLEUPDATE -P 5300 -a 10.53.0.1 -a 10.53.0.2 -p "nxrrset exists.sample TXT This RRset exists." \ + add "nxrrset-exists.sample 0 in a 1.2.3.4" > update.out.test$n 2>&1 +$SAMPLEUPDATE -P 5300 -a 10.53.0.2 -p "nxrrset exists.sample TXT This RRset exists." \ + add "check-nxrrset-exists.sample 0 in a 1.2.3.4" > update.out.check$n 2>&1 +$DIG +tcp @10.53.0.1 -p 5300 a nxrrset-exists.sample > dig.out.ns1.test$n +$DIG +tcp @10.53.0.2 -p 5300 a nxrrset-exists.sample > dig.out.ns2.test$n +$DIG +tcp @10.53.0.2 -p 5300 a check-nxrrset-exists.sample > check.out.ns2.test$n +grep "update failed: YXRRSET" update.out.test$n > /dev/null || ret=1 +grep "update succeeded" update.out.check$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns1.test$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns2.test$n > /dev/null || ret=1 +grep "status: NOERROR" check.out.ns2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo I:failed; status=1; } + +n=`expr $n + 1` +echo "I:check that dns_client_update handles prerequisite YXRRSET failure ($n)" +$SAMPLEUPDATE -P 5300 -a 10.53.0.1 -a 10.53.0.2 -p "yxrrset no-txt.sample TXT" \ + add "yxrrset-nxrrset.sample 0 in a 1.2.3.4" > update.out.test$n 2>&1 +$SAMPLEUPDATE -P 5300 -a 10.53.0.2 -p "yxrrset no-txt.sample TXT" \ + add "check-yxrrset-nxrrset.sample 0 in a 1.2.3.4" > update.out.check$n 2>&1 +$DIG +tcp @10.53.0.1 -p 5300 a yxrrset-nxrrset.sample > dig.out.ns1.test$n +$DIG +tcp @10.53.0.2 -p 5300 a yxrrset-nxrrset.sample > dig.out.ns2.test$n +$DIG +tcp @10.53.0.2 -p 5300 a check-yxrrset-nxrrset.sample > check.out.ns2.test$n +grep "update failed: NXRRSET" update.out.test$n > /dev/null || ret=1 +grep "update succeeded" update.out.check$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns1.test$n > /dev/null || ret=1 +grep "status: NXDOMAIN" dig.out.ns2.test$n > /dev/null || ret=1 +grep "status: NOERROR" check.out.ns2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo I:failed; status=1; } + +# +# End client library tests here +# + echo "I:exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/lib/dns/client.c b/lib/dns/client.c index 93294501fe..0ec1953eb1 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -1978,8 +1978,21 @@ update_done(isc_task_t *task, isc_event_t *event) { LOCK(&uctx->lock); uctx->currentserver = ISC_LIST_NEXT(uctx->currentserver, link); dns_request_destroy(&uctx->updatereq); - if (result != ISC_R_SUCCESS && !uctx->canceled && - uctx->currentserver != NULL) { + /* + * Moving on to the next server shouldn't change the result + * for NXDOMAIN, YXDOMAIN, NXRRSET and YXRRSET as they + * indicate a prerequisite failure. REFUSED should also + * be consistent across all servers but often isn't as that + * is policy rather that zone content driven (slaves that + * aren't willing to forward should return NOTIMPL). NOTZONE + * indicates that we stuffed up the request construction so + * don't retry. + */ + if (result != ISC_R_SUCCESS && result != DNS_R_NXDOMAIN && + result != DNS_R_YXDOMAIN && result != DNS_R_YXRRSET && + result != DNS_R_NXRRSET && result != DNS_R_NOTZONE && + !uctx->canceled && uctx->currentserver != NULL) + { dns_message_renderreset(uctx->updatemsg); dns_message_settsigkey(uctx->updatemsg, NULL); diff --git a/lib/samples/sample-update.c b/lib/samples/sample-update.c index 2a3020467a..06d52dd423 100644 --- a/lib/samples/sample-update.c +++ b/lib/samples/sample-update.c @@ -60,6 +60,8 @@ static const dns_rdataclass_t default_rdataclass = dns_rdataclass_in; static isc_bufferlist_t usedbuffers; static ISC_LIST(dns_rdatalist_t) usedrdatalists; +static const char *port = "53"; + static void setup_tsec(char *keyfile, isc_mem_t *mctx); static void update_addordelete(isc_mem_t *mctx, char *cmdline, isc_boolean_t isdelete, dns_name_t *name); @@ -80,20 +82,49 @@ usage(void) { exit(1); } +static isc_boolean_t +addserver(const char *server, isc_sockaddrlist_t *list, + isc_sockaddr_t *sockaddr) +{ + struct addrinfo hints, *res; + int gaierror; + + memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + hints.ai_protocol = IPPROTO_UDP; +#ifdef AI_NUMERICHOST + hints.ai_flags |= AI_NUMERICHOST; +#endif +#ifdef AI_NUMERICSERV + hints.ai_flags |= AI_NUMERICSERV; +#endif + gaierror = getaddrinfo(server, port, &hints, &res); + if (gaierror != 0) { + fprintf(stderr, "getaddrinfo(%s) failed: %s\n", + server, gai_strerror(gaierror)); + return (ISC_FALSE); + } + INSIST(res->ai_addrlen <= sizeof(sockaddr->type)); + memmove(&sockaddr->type, res->ai_addr, res->ai_addrlen); + freeaddrinfo(res); + sockaddr->length = (unsigned int)res->ai_addrlen; + ISC_LINK_INIT(sockaddr, link); + ISC_LIST_APPEND(*list, sockaddr, link); + return (ISC_TRUE); +} + int main(int argc, char *argv[]) { int ch; - struct addrinfo hints, *res; - int gaierror; dns_client_t *client = NULL; char *zonenamestr = NULL; char *keyfilename = NULL; char *prereqstr = NULL; - isc_sockaddrlist_t auth_servers; - char *auth_server = NULL; - char *recursive_server = NULL; - isc_sockaddr_t sa_auth, sa_recursive; + isc_sockaddr_t sa_auth[10], sa_recursive[10]; + unsigned int nsa_auth = 0, nsa_recursive = 0; isc_sockaddrlist_t rec_servers; + isc_sockaddrlist_t auth_servers; isc_result_t result; isc_boolean_t isdelete; isc_buffer_t b, *buf; @@ -106,19 +137,32 @@ main(int argc, char *argv[]) { dns_namelist_t updatelist, prereqlist, *prereqlistp = NULL; isc_mem_t *umctx = NULL; - while ((ch = isc_commandline_parse(argc, argv, "a:k:p:r:z:")) != EOF) { + ISC_LIST_INIT(auth_servers); + ISC_LIST_INIT(rec_servers); + + while ((ch = isc_commandline_parse(argc, argv, "a:k:p:P:r:z:")) != EOF) { switch (ch) { case 'k': keyfilename = isc_commandline_argument; break; case 'a': - auth_server = isc_commandline_argument; + if (nsa_auth < sizeof(sa_auth)/sizeof(*sa_auth) && + addserver(isc_commandline_argument, &auth_servers, + &sa_auth[nsa_auth])) + nsa_auth++; break; case 'p': prereqstr = isc_commandline_argument; break; + case 'P': + port = isc_commandline_argument; + break; case 'r': - recursive_server = isc_commandline_argument; + if (nsa_recursive < + sizeof(sa_recursive)/sizeof(*sa_recursive) && + addserver(isc_commandline_argument, &rec_servers, + &sa_recursive[nsa_recursive])) + nsa_recursive++; break; case 'z': zonenamestr = isc_commandline_argument; @@ -143,8 +187,9 @@ main(int argc, char *argv[]) { exit(1); } - if (auth_server == NULL && recursive_server == NULL) { - fprintf(stderr, "authoritative or recursive server " + if (ISC_LIST_HEAD(auth_servers) == NULL && + ISC_LIST_HEAD(rec_servers) == NULL) { + fprintf(stderr, "authoritative or recursive servers " "must be specified\n"); usage(); } @@ -153,7 +198,6 @@ main(int argc, char *argv[]) { ISC_LIST_INIT(usedbuffers); ISC_LIST_INIT(usedrdatalists); ISC_LIST_INIT(prereqlist); - ISC_LIST_INIT(auth_servers); isc_lib_register(); result = dns_lib_init(); if (result != ISC_R_SUCCESS) { @@ -172,60 +216,6 @@ main(int argc, char *argv[]) { exit(1); } - /* Set the authoritative server */ - if (auth_server != NULL) { - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_DGRAM; - hints.ai_protocol = IPPROTO_UDP; -#ifdef AI_NUMERICHOST - hints.ai_flags = AI_NUMERICHOST; -#endif - gaierror = getaddrinfo(auth_server, "53", &hints, &res); - if (gaierror != 0) { - fprintf(stderr, "getaddrinfo failed: %s\n", - gai_strerror(gaierror)); - exit(1); - } - INSIST(res->ai_addrlen <= sizeof(sa_auth.type)); - memmove(&sa_auth.type, res->ai_addr, res->ai_addrlen); - freeaddrinfo(res); - sa_auth.length = (unsigned int)res->ai_addrlen; - ISC_LINK_INIT(&sa_auth, link); - - ISC_LIST_APPEND(auth_servers, &sa_auth, link); - } - - /* Set the recursive server */ - if (recursive_server != NULL) { - memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_DGRAM; - hints.ai_protocol = IPPROTO_UDP; -#ifdef AI_NUMERICHOST - hints.ai_flags = AI_NUMERICHOST; -#endif - gaierror = getaddrinfo(recursive_server, "53", &hints, &res); - if (gaierror != 0) { - fprintf(stderr, "getaddrinfo failed: %s\n", - gai_strerror(gaierror)); - exit(1); - } - INSIST(res->ai_addrlen <= sizeof(sa_recursive.type)); - memmove(&sa_recursive.type, res->ai_addr, res->ai_addrlen); - freeaddrinfo(res); - sa_recursive.length = (unsigned int)res->ai_addrlen; - ISC_LINK_INIT(&sa_recursive, link); - ISC_LIST_INIT(rec_servers); - ISC_LIST_APPEND(rec_servers, &sa_recursive, link); - result = dns_client_setservers(client, dns_rdataclass_in, - NULL, &rec_servers); - if (result != ISC_R_SUCCESS) { - fprintf(stderr, "set server failed: %d\n", result); - exit(1); - } - } - /* Construct zone name */ zname = NULL; if (zonenamestr != NULL) { @@ -264,8 +254,8 @@ main(int argc, char *argv[]) { result = dns_client_update(client, default_rdataclass, /* XXX: fixed */ zname, prereqlistp, &updatelist, - (auth_server == NULL) ? NULL : - &auth_servers, tsec, 0); + (ISC_LIST_HEAD(auth_servers) == NULL) ? + NULL : &auth_servers, tsec, 0); if (result != ISC_R_SUCCESS) { fprintf(stderr, "update failed: %s\n", dns_result_totext(result));