Report information
The Basics
Id:
46389
Status:
open
Priority:
Low/Low
Queue:

People
Owner:
Nobody in particular
Requestors:
Stephen Morris <stephen@isc.org>(email delivery suspended)
Cc:
AdminCc:

BugTracker
Version Fixed:
(no value)
Version Found:
(no value)
Versions Affected:
(no value)
Versions Planned:
(no value)
Priority:
P2 Normal
Severity:
S3 Low
CVSS Score:
(no value)
CVE ID:
(no value)
Component:
BIND Utilities
Area:
test

Dates
Created:Tue, 24 Oct 2017 10:29:23 -0400
Updated:Wed, 08 Nov 2017 07:03:03 -0500
Closed:Not set



This bug tracker is no longer active.

Please go to our Gitlab to submit issues (both feature requests and bug reports) for active projects maintained by Internet Systems Consortium (ISC).

Due to security and confidentiality requirements, full access is limited to the primary maintainers.

Date: Tue, 24 Oct 2017 14:29:23 +0000
Subject: Fix/Extend testing for dnssec-cds
From: stephen@isc.org
To: bind9-public@isc.org
Reviewed the testing of the utility at commit b49042a6a5e7fd21f2147337c21a5d4d9b957204 (on master, post V9.12.0b1). Overall, the utility has a comprehensive set of tests. The following points should be addressed however: 1. The tests fail on FreeBSD-11. (This appears to be a peculiarity of the test script, not of the utility itself.) 2. There is no test for the operation of the "-c" switch (class of the zones). 3. Tests should check that "-s <arg>" outputs an error message when <arg> is not a valid date and time. 4. The tests only check -s with relative times, not absolute times. 5. Tests should check that "-T <arg>" outputs an error message when <arg> is not a valid TTL. 6. The algorithms tested are SHA1 and SHA256. The documentation states that the utility can also use GOST and SHA384 - these two should be tested as well. 7. No check is made that the algorithm argument is interpreted in a case-insensitive manner. 8. There is no test for the -V switch
I have two comments regarding point 1 (FreeBSD issues): 1. "tput setaf ..." returns with exit code 1 on FreeBSD. Combined with "set -e" in bin/tests/system/cds/setup.sh, this causes cds test setup to be silently interrupted when bin/tests/system/conf.sh is sourced. However, note that the Jenkins FreeBSD slave seems to have no problems with running the cds test. I logged into the slave, ran "tput setaf 1" and it returned with exit code 1, just like the VM I checked earlier. I have no idea what is happening here. I think we should drop "set -e" from bin/tests/system/cds/setup.sh as it does not seem to be widely used in bin/tests/system: $ git grep "set -e" bin/tests/system/ bin/tests/system/cds/setup.sh:set -eu bin/tests/system/rpz/ckdnsrps.sh:set -e bin/tests/system/rpz/setup.sh:set -e bin/tests/system/rpzrecurse/setup.sh:set -e Nevertheless, fixing bin/tests/system/conf.sh also would not hurt. 2. The cds test is overly optimistic about execution times. The checkmtime.pl script expects it will not take more than 2 seconds to set the test up and run the first 14 checks. This might be a reasonable assumption on a modern physical machine; my laptop usually runs the whole cds test in under a second. However, our FreeBSD VM is a lot slower, usually taking about 6 seconds to run the whole cds test. Not to mention our SPARC machine with Solaris 10, which consistently takes about 30 seconds (!) to complete the test. Given the timestamps used as input for the relevant checks (there are three of them and each one is an hour apart from the next), I think there is no harm in reducing the resolution of these checks to a minute, e.g. like this: diff --git a/bin/tests/system/cds/checkmtime.pl b/bin/tests/system/cds/checkmtime.pl index ea3fa5d27a..3a1347f7be 100644 --- a/bin/tests/system/cds/checkmtime.pl +++ b/bin/tests/system/cds/checkmtime.pl @@ -10,4 +10,4 @@ my $target = shift; my $file = shift; my $mtime = time - (stat $file)[9]; die "bad mtime $mtime" - unless abs($mtime - $target) < 3; + unless ($mtime - $target >= 0 && $mtime - $target < 60); This will ensure the test passes even on extreme laggards without making it moot. If this looks good to you, we should merge the above change with no CHANGES note to make Jenkins happy again. As a side note, checkmtime.pl currently uses abs(), which allows the test to pass when file modification time is *less* than an hour (or two hours for the second check) in the past. I do not think this should ever be possible and thus I removed abs() from the condition checked by checkmtime.pl.
On Tue Oct 31 10:14:53 2017, michal wrote: > I have two comments regarding point 1 (FreeBSD issues): > > 1. "tput setaf ..." returns with exit code 1 on FreeBSD. => tput is likely very dependent on the terminal set in the environment... I tried on a window: with the terminal changed from xterm-256color to vt220 "tput setaf 1" stops to turn to red and returns 1 BTW I agree about your proposal to fix conf.sh. Note that "tput setaf 0" returns 1 too with a black and white terminal as vt220.
CC:
To: "Francis Dupont via RT" <bind9-public@isc.org>
Date: Tue, 31 Oct 2017 17:25:52 +0000
Subject: Re: [ISC-Bugs #46389] Fix/Extend testing for dnssec-cds
From: "Evan Hunt" <each@isc.org>
> BTW I agree about your proposal to fix conf.sh. > Note that "tput setaf 0" returns 1 too with a black > and white terminal as vt220. I guess something like this would work? diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in index ecccbe4..1ce400a 100644 --- a/bin/tests/system/conf.sh.in +++ b/bin/tests/system/conf.sh.in @@ -142,7 +142,7 @@ NZD=@NZD_TOOLS@ # # Set up color-coded test output # -if test -t 1 && type tput > /dev/null 2>&1 ; then +if test -t 1 && type tput > /dev/null 2>&1 && tput sgr0 > /dev/null 2>&1 ; then COLOR_FAIL=`tput setaf 1` # red COLOR_WARN=`tput setaf 3` # yellow COLOR_PASS=`tput setaf 2` # green
On Tue Oct 31 17:25:55 2017, each@isc.org wrote: > > BTW I agree about your proposal to fix conf.sh. > > Note that "tput setaf 0" returns 1 too with a black > > and white terminal as vt220. > > I guess something like this would work? => like this but not this (tput sgr0 returns 0 with vt220) > diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in > -if test -t 1 && type tput > /dev/null 2>&1 ; then > +if test -t 1 && type tput > /dev/null 2>&1 && tput sgr0 > /dev/null 2>&1 ; => IMHO you need to use "tput setaf 0" too.
Okay, how about applying this without a CHANGES note: commit c1fdc8f5da Author: Michał Kępień <michal@isc.org> Date: Wed Nov 8 12:55:14 2017 +0100 [master] Make cds system test behave more nicely on FreeBSD and slower machines diff --git a/bin/tests/system/cds/checkmtime.pl b/bin/tests/system/cds/checkmtime.pl index ea3fa5d27a..3a1347f7be 100644 --- a/bin/tests/system/cds/checkmtime.pl +++ b/bin/tests/system/cds/checkmtime.pl @@ -10,4 +10,4 @@ my $target = shift; my $file = shift; my $mtime = time - (stat $file)[9]; die "bad mtime $mtime" - unless abs($mtime - $target) < 3; + unless ($mtime - $target >= 0 && $mtime - $target < 60); diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in index 6b2320e202..1ef20a09b9 100644 --- a/bin/tests/system/conf.sh.in +++ b/bin/tests/system/conf.sh.in @@ -142,7 +142,7 @@ NZD=@NZD_TOOLS@ # # Set up color-coded test output # -if test -t 1 && type tput > /dev/null 2>&1 ; then +if test -t 1 && type tput > /dev/null 2>&1 && tput setaf 0 > /dev/null 2>&1 ; then COLOR_FAIL=`tput setaf 1` # red COLOR_WARN=`tput setaf 3` # yellow COLOR_PASS=`tput setaf 2` # green