From feecbd8e777fa4fe4c6d329f375b8b2c4d60754d Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Wed, 17 Jan 2024 20:44:27 +0100 Subject: [PATCH 1/7] Add "without_fips" mark The "without_fips" mark disables test function when BIND 9 was built with the FIPS mode enabled as not everything works in FIPS-enabled builds. --- bin/tests/system/isctest/mark.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/tests/system/isctest/mark.py b/bin/tests/system/isctest/mark.py index 95435dfd3a..ac5480834c 100644 --- a/bin/tests/system/isctest/mark.py +++ b/bin/tests/system/isctest/mark.py @@ -41,6 +41,10 @@ def with_tsan(*args): # pylint: disable=unused-argument return feature_test("--tsan") +without_fips = pytest.mark.skipif( + feature_test("--have-fips-mode"), reason="FIPS support enabled in the build" +) + have_libxml2 = pytest.mark.skipif( not feature_test("--have-libxml2"), reason="libxml2 support disabled in the build" ) From df8c419058e1e43265928b59f4ae54293f9a32de Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Wed, 17 Jan 2024 20:47:42 +0100 Subject: [PATCH 2/7] Add isctest.query.tls() function When explicitly set to True, the "verify" argument lets dnspython verify certificates used for the connection. As most certificates in the system test will inevitably be self-signed, the "verify" argument defaults to False. The "verify" argument is present in dnspython since the version 2.5.0. --- bin/tests/system/isctest/query.py | 42 +++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/bin/tests/system/isctest/query.py b/bin/tests/system/isctest/query.py index 4c389af2d9..6e84c8188d 100644 --- a/bin/tests/system/isctest/query.py +++ b/bin/tests/system/isctest/query.py @@ -31,22 +31,39 @@ def generic_query( timeout: int = QUERY_TIMEOUT, attempts: int = 10, expected_rcode: dns_rcode = None, + verify: bool = False, ) -> Any: if port is None: - port = int(os.environ["PORT"]) + if query_func.__name__ == "tls": + port = int(os.environ["TLSPORT"]) + else: + port = int(os.environ["PORT"]) + + query_args = { + "q": message, + "where": ip, + "timeout": timeout, + "port": port, + "source": source, + } + if query_func.__name__ == "tls": + query_args["verify"] = verify + res = None for attempt in range(attempts): + isctest.log.debug( + f"{query_func.__name__}(): ip={ip}, port={port}, source={source}, " + f"timeout={timeout}, attempts left={attempts-attempt}" + ) try: - isctest.log.debug( - f"{query_func.__name__}(): ip={ip}, port={port}, source={source}, " - f"timeout={timeout}, attempts left={attempts-attempt}" - ) - res = query_func(message, ip, timeout, port=port, source=source) + res = query_func(**query_args) + except (dns.exception.Timeout, ConnectionRefusedError) as e: + isctest.log.debug(f"{query_func.__name__}(): the '{e}' exception raised") + else: if res.rcode() == expected_rcode or expected_rcode is None: return res - except (dns.exception.Timeout, ConnectionRefusedError) as e: - isctest.log.debug(f"{query_func.__name__}(): the '{e}' exceptio raised") time.sleep(1) + if expected_rcode is not None: last_rcode = dns_rcode.to_text(res.rcode()) if res else None isctest.log.debug( @@ -61,3 +78,12 @@ def udp(*args, **kwargs) -> Any: def tcp(*args, **kwargs) -> Any: return generic_query(dns.query.tcp, *args, **kwargs) + + +def tls(*args, **kwargs) -> Any: + try: + return generic_query(dns.query.tls, *args, **kwargs) + except TypeError as e: + raise RuntimeError( + "dnspython 2.5.0 or newer is required for isctest.query.tls()" + ) from e From b2964cc922f9630d4800d1409ed5b7e253ca3324 Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Thu, 10 Oct 2024 19:46:22 +0200 Subject: [PATCH 3/7] Use Debian "sid" for pylint and mypy jobs to get recent dnspython The base image tends to have a rather old dnspython version and when used with pylint and mypy it produces errors about newer dnspython features the old version does not know about. $ mypy "bin/tests/system/isctest/" bin/tests/system/isctest/query.py:55: error: Unexpected keyword argument "verify" for "tls" [call-arg] /usr/lib/python3/dist-packages/dns/query.py:958: note: "tls" defined here $ pylint --rcfile $CI_PROJECT_DIR/.pylintrc --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE 'ans\.py') ************* Module isctest.query bin/tests/system/isctest/query.py:55:11: E1123: Unexpected keyword argument 'verify' in function call (unexpected-keyword-arg) --- .gitlab-ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2ccaf19244..7034b9aa0f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -577,7 +577,9 @@ coccinelle: - if test "$(git status --porcelain | grep -Ev '\?\?' | wc -l)" -gt "0"; then git status --short; exit 1; fi pylint: - <<: *precheck_job + <<: *default_triggering_rules + <<: *debian_sid_amd64_image + stage: precheck needs: [] variables: PYTHONPATH: "${CI_PROJECT_DIR}/bin/tests/system" @@ -630,7 +632,9 @@ checkbashisms: - checkbashisms $(find . -path './.git' -prune -o -type f -exec sh -c 'head -n 1 "{}" | grep -qsF "#!/bin/sh"' \; -print) mypy: - <<: *precheck_job + <<: *default_triggering_rules + <<: *debian_sid_amd64_image + stage: precheck script: - mypy "bin/tests/system/isctest/" From 100b75986322d5105734bd9c1e70a2e5a987dc2d Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Wed, 17 Jan 2024 20:43:21 +0100 Subject: [PATCH 4/7] Rewrite cipher-suites system test to pytest The minimal required dnspython version is 2.5.0 because of the need for the "verify" argument in dns.query.tls(). --- bin/tests/system/cipher-suites/setup.sh | 8 +- bin/tests/system/cipher-suites/tests.sh | 96 ------------------- .../cipher-suites/tests_cipher_suites.py | 79 +++++++++++++++ .../cipher-suites/tests_sh_cipher_suites.py | 23 ----- 4 files changed, 86 insertions(+), 120 deletions(-) delete mode 100644 bin/tests/system/cipher-suites/tests.sh create mode 100644 bin/tests/system/cipher-suites/tests_cipher_suites.py delete mode 100644 bin/tests/system/cipher-suites/tests_sh_cipher_suites.py diff --git a/bin/tests/system/cipher-suites/setup.sh b/bin/tests/system/cipher-suites/setup.sh index 9d7d0a928e..71b94b9fb7 100644 --- a/bin/tests/system/cipher-suites/setup.sh +++ b/bin/tests/system/cipher-suites/setup.sh @@ -13,7 +13,13 @@ . ../conf.sh -$SHELL "${TOP_SRCDIR}/bin/tests/system/genzone.sh" 2 >ns1/example.db +# Drop unusual RR sets dnspython can't handle. For more information +# see https://github.com/rthalley/dnspython/issues/1034#issuecomment-1896541899. +$SHELL "${TOP_SRCDIR}/bin/tests/system/genzone.sh" 2 \ + | sed \ + -e '/AMTRELAY.*\# 2 0004/d' \ + -e '/GPOS.*"" "" ""/d' \ + -e '/URI.*30 40 ""/d' >ns1/example.db copy_setports ns1/named.conf.in ns1/named.conf copy_setports ns2/named.conf.in ns2/named.conf diff --git a/bin/tests/system/cipher-suites/tests.sh b/bin/tests/system/cipher-suites/tests.sh deleted file mode 100644 index f5b28b79df..0000000000 --- a/bin/tests/system/cipher-suites/tests.sh +++ /dev/null @@ -1,96 +0,0 @@ -#!/bin/sh - -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -set -e - -# shellcheck disable=SC1091 -. ../conf.sh - -testing="testing zone transfer over TLS (XoT): " - -common_dig_options="+noadd +nosea +nostat +noquest +nocmd" - -status=0 -n=0 - -dig_with_tls_opts() { - # shellcheck disable=SC2086 - "$DIG" +tls $common_dig_options -p "${TLSPORT}" "$@" -} - -wait_for_tls_xfer() ( - srv_number="$1" - shift - zone_name="$1" - shift - # Let's bind to .10 to make it possible to easily distinguish dig from NSs in packet traces - dig_with_tls_opts -b 10.53.0.10 "@10.53.0.$srv_number" "${zone_name}." AXFR >"dig.out.ns$srv_number.${zone_name}.test$n" || return 1 - grep "^;" "dig.out.ns$srv_number.${zone_name}.test$n" >/dev/null && return 1 - return 0 -) - -tls_xfer_expect_success() { - test_message="$1" - shift - n=$((n + 1)) - echo_i "$test_message - zone \"$2\" at \"ns$1\" ($n)" - ret=0 - retry_quiet 10 wait_for_tls_xfer "$@" || ret=1 - if [ $ret != 0 ]; then echo_i "failed"; fi - status=$((status + ret)) -} - -tls_xfer_expect_failure() { - test_message="$1" - shift - n=$((n + 1)) - echo_i "$test_message - zone \"$2\" at \"ns$1\", failure expected ($n)" - ret=0 - retry_quiet 10 wait_for_tls_xfer "$@" && ret=1 - if [ $ret != 0 ]; then echo_i "failed"; fi - status=$((status + ret)) -} - -tls_xfer_expect_success "$testing" 2 example -tls_xfer_expect_success "$testing" 3 example -tls_xfer_expect_success "$testing" 4 example - -tls_xfer_expect_success "$testing" 2 example-aes-128 -tls_xfer_expect_success "$testing" 3 example-aes-256 -if ! $FEATURETEST --have-fips-mode; then - tls_xfer_expect_success "$testing" 4 example-chacha-20 -fi - -tls_xfer_expect_failure "$testing" 2 example-aes-256 -if ! $FEATURETEST --have-fips-mode; then - tls_xfer_expect_failure "$testing" 2 example-chacha-20 -fi - -tls_xfer_expect_failure "$testing" 3 example-aes-128 -if ! $FEATURETEST --have-fips-mode; then - tls_xfer_expect_failure "$testing" 3 example-chacha-20 -fi - -tls_xfer_expect_failure "$testing" 4 example-aes-128 -tls_xfer_expect_failure "$testing" 4 example-aes-256 - -# NS5 tries to download the zone over TLSv1.2 -tls_xfer_expect_failure "$testing" 5 example -tls_xfer_expect_failure "$testing" 5 example-aes-128 -tls_xfer_expect_failure "$testing" 5 example-aes-256 -if ! $FEATURETEST --have-fips-mode; then - tls_xfer_expect_failure "$testing" 5 example-chacha-20 -fi - -echo_i "exit status: $status" -[ $status -eq 0 ] || exit 1 diff --git a/bin/tests/system/cipher-suites/tests_cipher_suites.py b/bin/tests/system/cipher-suites/tests_cipher_suites.py new file mode 100644 index 0000000000..255fc326ac --- /dev/null +++ b/bin/tests/system/cipher-suites/tests_cipher_suites.py @@ -0,0 +1,79 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +import pytest + +pytest.importorskip("dns", minversion="2.5.0") + +import dns.message + +import isctest +import isctest.mark + + +pytestmark = pytest.mark.extra_artifacts( + [ + "ns*/example*.db", + ] +) + + +@pytest.mark.requires_zones_loaded("ns1", "ns2", "ns3", "ns4", "ns5") +@pytest.mark.parametrize( + "qname,ns,rcode", + [ + ("example.", 2, dns.rcode.NOERROR), + ("example.", 3, dns.rcode.NOERROR), + ("example.", 4, dns.rcode.NOERROR), + ("example-aes-128.", 2, dns.rcode.NOERROR), + ("example-aes-256.", 3, dns.rcode.NOERROR), + pytest.param( + "example-chacha-20.", + 4, + dns.rcode.NOERROR, + marks=isctest.mark.without_fips, + ), + ("example-aes-256", 2, dns.rcode.SERVFAIL), + pytest.param( + "example-chacha-20", + 2, + dns.rcode.SERVFAIL, + marks=isctest.mark.without_fips, + ), + ("example-aes-128", 3, dns.rcode.SERVFAIL), + pytest.param( + "example-chacha-20", + 3, + dns.rcode.SERVFAIL, + marks=isctest.mark.without_fips, + ), + ("example-aes-128", 4, dns.rcode.SERVFAIL), + ("example-aes-256", 4, dns.rcode.SERVFAIL), + # NS5 tries to download the zone over TLSv1.2 + ("example", 5, dns.rcode.SERVFAIL), + ("example-aes-128", 5, dns.rcode.SERVFAIL), + ("example-aes-256", 5, dns.rcode.SERVFAIL), + pytest.param( + "example-chacha-20", + 5, + dns.rcode.SERVFAIL, + marks=isctest.mark.without_fips, + ), + ], +) +def test_cipher_suites_tls_xfer(qname, ns, rcode): + msg = dns.message.make_query(qname, "AXFR") + ans = isctest.query.tls(msg, f"10.53.0.{ns}") + assert ans.rcode() == rcode + if rcode == dns.rcode.NOERROR: + assert ans.answer != [] + elif rcode == dns.rcode.SERVFAIL: + assert ans.answer == [] diff --git a/bin/tests/system/cipher-suites/tests_sh_cipher_suites.py b/bin/tests/system/cipher-suites/tests_sh_cipher_suites.py deleted file mode 100644 index 65a4b82591..0000000000 --- a/bin/tests/system/cipher-suites/tests_sh_cipher_suites.py +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright (C) Internet Systems Consortium, Inc. ("ISC") -# -# SPDX-License-Identifier: MPL-2.0 -# -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, you can obtain one at https://mozilla.org/MPL/2.0/. -# -# See the COPYRIGHT file distributed with this work for additional -# information regarding copyright ownership. - -import pytest - -pytestmark = pytest.mark.extra_artifacts( - [ - "dig.out.*", - "ns*/example*.db", - ] -) - - -def test_cipher_suites(run_tests_sh): - run_tests_sh() From a72ff9fd574b4a3276c545237a378a85e65de12a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Wed, 28 Aug 2024 15:00:02 +0200 Subject: [PATCH 5/7] Make servers fixture in pytest module-wide The servers are setup and torn down once per each test module. All the logs and server state persists between individual tests within the same module. The servers fixture representing these servers should be module-wide as well. --- bin/tests/system/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tests/system/conftest.py b/bin/tests/system/conftest.py index 7530473909..1bb40cf7c9 100644 --- a/bin/tests/system/conftest.py +++ b/bin/tests/system/conftest.py @@ -662,7 +662,7 @@ def system_test( request.node.stash[FIXTURE_OK] = True -@pytest.fixture +@pytest.fixture(scope="module") def servers(system_test_dir): instances = {} for entry in system_test_dir.rglob("*"): From 23fb6159636241c1e7e66ba16d5728c28bcd6b97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicki=20K=C5=99=C3=AD=C5=BEek?= Date: Wed, 28 Aug 2024 15:03:27 +0200 Subject: [PATCH 6/7] Test cipher-suites after zone transfers complete Ensure the zone transfers have completed (successfully or not) before running the test cases, because they assume zone transfers have been done. --- .gitlab-ci.yml | 2 +- .../system/cipher-suites/tests_cipher_suites.py | 16 +++++++++++++++- bin/tests/system/vulture_ignore_list.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/vulture_ignore_list.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7034b9aa0f..0b4637f9b9 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -586,7 +586,7 @@ pylint: script: - pylint --rcfile $CI_PROJECT_DIR/.pylintrc $(git ls-files '*.py' | grep -vE '(ans\.py|dangerfile\.py|^bin/tests/system/|^contrib/)') # Ignore Pylint wrong-import-position error in system test to enable use of pytest.importorskip - - pylint --rcfile $CI_PROJECT_DIR/.pylintrc --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE 'ans\.py') + - pylint --rcfile $CI_PROJECT_DIR/.pylintrc --disable=wrong-import-position $(git ls-files 'bin/tests/system/*.py' | grep -vE '(ans\.py|vulture_ignore_list\.py)') reuse: <<: *precheck_job diff --git a/bin/tests/system/cipher-suites/tests_cipher_suites.py b/bin/tests/system/cipher-suites/tests_cipher_suites.py index 255fc326ac..1be3aafbbc 100644 --- a/bin/tests/system/cipher-suites/tests_cipher_suites.py +++ b/bin/tests/system/cipher-suites/tests_cipher_suites.py @@ -9,6 +9,8 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. +import re + import pytest pytest.importorskip("dns", minversion="2.5.0") @@ -26,6 +28,17 @@ pytestmark = pytest.mark.extra_artifacts( ) +@pytest.fixture(scope="module") +def transfers_complete(servers): + for zone in ["example", "example-aes-128", "example-aes-256", "example-chacha-20"]: + pattern = re.compile( + f"transfer of '{zone}/IN' from 10.53.0.1#[0-9]+: Transfer completed" + ) + for ns in ["ns2", "ns3", "ns4", "ns5"]: + with servers[ns].watch_log_from_start() as watcher: + watcher.wait_for_line(pattern) + + @pytest.mark.requires_zones_loaded("ns1", "ns2", "ns3", "ns4", "ns5") @pytest.mark.parametrize( "qname,ns,rcode", @@ -69,7 +82,8 @@ pytestmark = pytest.mark.extra_artifacts( ), ], ) -def test_cipher_suites_tls_xfer(qname, ns, rcode): +# pylint: disable=redefined-outer-name,unused-argument +def test_cipher_suites_tls_xfer(qname, ns, rcode, transfers_complete): msg = dns.message.make_query(qname, "AXFR") ans = isctest.query.tls(msg, f"10.53.0.{ns}") assert ans.rcode() == rcode diff --git a/bin/tests/system/vulture_ignore_list.py b/bin/tests/system/vulture_ignore_list.py new file mode 100644 index 0000000000..7682a478e6 --- /dev/null +++ b/bin/tests/system/vulture_ignore_list.py @@ -0,0 +1,13 @@ +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# SPDX-License-Identifier: MPL-2.0 +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +transfers_complete # unused function (cipher-suites/tests_cipher_suites.py:31) +transfers_complete # unused variable (cipher-suites/tests_cipher_suites.py:86) From df7e9f4ac31a0d31b6c70db7cc30232f980e06fe Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Fri, 11 Oct 2024 11:30:26 +0200 Subject: [PATCH 7/7] Rename have_* marks to with_* Marks starting with "with" or "without" make more sense linguistically than those starting with "have" or "have_not". --- bin/tests/system/isctest/mark.py | 4 ++-- bin/tests/system/statschannel/tests_json.py | 2 +- bin/tests/system/statschannel/tests_xml.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/isctest/mark.py b/bin/tests/system/isctest/mark.py index ac5480834c..43c4629e43 100644 --- a/bin/tests/system/isctest/mark.py +++ b/bin/tests/system/isctest/mark.py @@ -45,11 +45,11 @@ without_fips = pytest.mark.skipif( feature_test("--have-fips-mode"), reason="FIPS support enabled in the build" ) -have_libxml2 = pytest.mark.skipif( +with_libxml2 = pytest.mark.skipif( not feature_test("--have-libxml2"), reason="libxml2 support disabled in the build" ) -have_json_c = pytest.mark.skipif( +with_json_c = pytest.mark.skipif( not feature_test("--have-json-c"), reason="json-c support disabled in the build" ) diff --git a/bin/tests/system/statschannel/tests_json.py b/bin/tests/system/statschannel/tests_json.py index 62f8bcab14..ddd6d6210a 100755 --- a/bin/tests/system/statschannel/tests_json.py +++ b/bin/tests/system/statschannel/tests_json.py @@ -23,7 +23,7 @@ import generic requests = pytest.importorskip("requests") pytestmark = [ - isctest.mark.have_json_c, + isctest.mark.with_json_c, pytest.mark.extra_artifacts( [ "ns2/*.jnl", diff --git a/bin/tests/system/statschannel/tests_xml.py b/bin/tests/system/statschannel/tests_xml.py index e301f6bf60..4271636101 100755 --- a/bin/tests/system/statschannel/tests_xml.py +++ b/bin/tests/system/statschannel/tests_xml.py @@ -24,7 +24,7 @@ import generic requests = pytest.importorskip("requests") pytestmark = [ - isctest.mark.have_libxml2, + isctest.mark.with_libxml2, pytest.mark.extra_artifacts( [ "ns2/K*",