From 78f6ead480ce6c2f00410acfee1bcf9895e2cc09 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Mon, 15 Feb 2021 19:28:50 -0300 Subject: [PATCH 1/5] Don't retry +tcp queries on failure if tries=1 or retries=0 Before this commit, a premature EOF (connection closed) on tcp queries was causing dig to automatically attempt to send the query again, even if +tries=1 or +retries=0 was provided on command line. This commit fix the problem by taking into account the no. of retries specified by the user when processing a premature EOF on tcp connections. --- bin/dig/dighost.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 178c241708..9d8a1e5a22 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3117,7 +3117,8 @@ force_next(dig_query_t *query) { */ static void requeue_or_update_exitcode(dig_lookup_t *lookup) { - if (lookup->eoferr == 0U) { + if (lookup->eoferr == 0U && lookup->retries > 1) { + --lookup->retries; /* * Peer closed the connection prematurely for the first time * for this lookup. Try again, keeping track of this failure. From e6808960037ba272f385256679623d0d8dea88cb Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 16 Feb 2021 12:36:45 -0300 Subject: [PATCH 2/5] Adjusted dig system tests Now that premature EOF on tcp connections take +tries and +retry into account, the dig system tests handling TCP EOF with +tries=1 were expecting dig to do a second attempt in handling the tcp query, which doesn't happen anymore. To make the test work as expected +tries value was adjusted to 2, to make it behave as before after the new update on dig. --- bin/tests/system/digdelv/tests.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh index 299d396b20..f0f1f4fa2e 100644 --- a/bin/tests/system/digdelv/tests.sh +++ b/bin/tests/system/digdelv/tests.sh @@ -801,7 +801,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (immediate -> immediate) ($n)" ret=0 echo "no_response no_response" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 && ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi @@ -811,7 +811,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> partial AXFR) ($n)" ret=0 echo "partial_axfr partial_axfr" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 && ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi @@ -821,7 +821,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (immediate -> partial AXFR) ($n)" ret=0 echo "no_response partial_axfr" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 && ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi @@ -831,7 +831,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> immediate) ($n)" ret=0 echo "partial_axfr no_response" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 && ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 2 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi @@ -841,7 +841,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (immediate -> complete AXFR) ($n)" ret=0 echo "no_response complete_axfr" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 || ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 || ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi @@ -851,7 +851,7 @@ if [ -x "$DIG" ] ; then echo_i "checking exit code for a retry upon TCP EOF (partial AXFR -> complete AXFR) ($n)" ret=0 echo "partial_axfr complete_axfr" | sendcmd 10.53.0.5 - dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 || ret=1 + dig_with_opts @10.53.0.5 example AXFR +tries=2 > dig.out.test$n 2>&1 || ret=1 # Sanity check: ensure ans5 behaves as expected. [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi From 4f82cc41cc46971f1ab95d55ee686fb9efce2f34 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 16 Feb 2021 13:13:07 -0300 Subject: [PATCH 3/5] Added tests for tries=1 and retry=0 on TCP EOF Added tests to ensure that dig won't retry sending a query over tcp (+tcp) when a TCP connection is closed prematurely (EOF is read) if either +tries=1 or retry=0 is specified on the command line. --- bin/tests/system/digdelv/tests.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh index f0f1f4fa2e..a30577139a 100644 --- a/bin/tests/system/digdelv/tests.sh +++ b/bin/tests/system/digdelv/tests.sh @@ -857,6 +857,25 @@ if [ -x "$DIG" ] ; then if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) + n=$((n+1)) + echo_i "checking +tries=1 won't retry twice upon TCP EOF ($n)" + ret=0 + echo "no_response no_response" | sendcmd 10.53.0.5 + dig_with_opts @10.53.0.5 example AXFR +tries=1 > dig.out.test$n 2>&1 && ret=1 + # Sanity check: ensure ans5 behaves as expected. + [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || ret=1 + if [ $ret -ne 0 ]; then echo_i "failed"; fi + status=$((status+ret)) + + n=$((n+1)) + echo_i "checking +retry=0 won't retry twice upon TCP EOF ($n)" + ret=0 + dig_with_opts @10.53.0.5 example AXFR +retry=0 > dig.out.test$n 2>&1 && ret=1 + # Sanity check: ensure ans5 behaves as expected. + [ `grep "communications error.*end of file" dig.out.test$n | wc -l` -eq 1 ] || ret=1 + if [ $ret -ne 0 ]; then echo_i "failed"; fi + status=$((status+ret)) + n=$((n+1)) echo_i "check that dig +expandaaaa works ($n)" ret=0 From 3b98c4d3112e5d5d5989f04423313eec85e93c38 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 16 Feb 2021 13:23:14 -0300 Subject: [PATCH 4/5] Update dig's man page Adjusted man page entries for +tries and +retry options to reflect the fact that now those options apply to TCP as well. --- bin/dig/dig.rst | 6 +++--- doc/man/dig.1in | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bin/dig/dig.rst b/bin/dig/dig.rst index c28b51e4ad..b54d22ef5c 100644 --- a/bin/dig/dig.rst +++ b/bin/dig/dig.rst @@ -480,8 +480,8 @@ abbreviation is unambiguous; for example, ``+cd`` is equivalent to ``+nssearch`` or ``+trace`` query option is used. ``+retry=T`` - This option sets the number of times to retry UDP queries to server to ``T`` - instead of the default, 2. Unlike ``+tries``, this does not include + This option sets the number of times to retry UDP and TCP queries to server to ``T`` + instead of the default, 2. Unlike ``+tries``, this does not include the initial query. ``+[no]rrcomments`` @@ -569,7 +569,7 @@ abbreviation is unambiguous; for example, ``+cd`` is equivalent to default queries from a name server. ``+tries=T`` - This option sets the number of times to try UDP queries to server to ``T`` + This option sets the number of times to try UDP and TCP queries to server to ``T`` instead of the default, 3. If ``T`` is less than or equal to zero, the number of tries is silently rounded up to 1. diff --git a/doc/man/dig.1in b/doc/man/dig.1in index e5820f5f42..bc42c4d543 100644 --- a/doc/man/dig.1in +++ b/doc/man/dig.1in @@ -492,8 +492,8 @@ recursive queries. Recursion is automatically disabled when the \fB+nssearch\fP or \fB+trace\fP query option is used. .TP .B \fB+retry=T\fP -This option sets the number of times to retry UDP queries to server to \fBT\fP -instead of the default, 2. Unlike \fB+tries\fP, this does not include +This option sets the number of times to retry UDP and TCP queries to server to \fBT\fP +instead of the default, 2. Unlike \fB+tries\fP, this does not include the initial query. .TP .B \fB+[no]rrcomments\fP @@ -581,7 +581,7 @@ the root zone name servers. default queries from a name server. .TP .B \fB+tries=T\fP -This option sets the number of times to try UDP queries to server to \fBT\fP +This option sets the number of times to try UDP and TCP queries to server to \fBT\fP instead of the default, 3. If \fBT\fP is less than or equal to zero, the number of tries is silently rounded up to 1. .TP From 04537633a7fea319049ad1b7dc843a49741d7903 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 16 Feb 2021 13:34:44 -0300 Subject: [PATCH 5/5] Add CHANGES note for [GL #2490] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 765a297a8c..c01583ed8c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5608. [bug] Dig now honors +retry=0 and +tries=1 when queries + are sent over TCP (+tcp) and the remote server closes + the connection prematurely. [GL #2490] + 5607. [bug] Rekey after 'rndc dnssec -checkds' or 'rndc dnssec -rollover' command is received, because such a command may influence the next key event. [GL #2488]