chg: test: Rewrite cipher-suites system test to pytest

The minimal dnspython version to run this test is 2.5.0.

Merge branch 'mnowak/pytest_rewrite_cipher-suites' into 'main'

See merge request isc-projects/bind9!8662

.gitlab-ci.yml

@@ -577,14 +577,16 @@ 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"
   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
@@ -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/"
 

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

bin/tests/system/cipher-suites/tests.sh

@@ -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

bin/tests/system/cipher-suites/tests_cipher_suites.py

@@ -0,0 +1,93 @@
+# 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 re
+
+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.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",
+    [
+        ("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,
+        ),
+    ],
+)
+# 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
+    if rcode == dns.rcode.NOERROR:
+        assert ans.answer != []
+    elif rcode == dns.rcode.SERVFAIL:
+        assert ans.answer == []

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("*"):

bin/tests/system/isctest/mark.py

@@ -41,11 +41,15 @@ def with_tsan(*args):  # pylint: disable=unused-argument
     return feature_test("--tsan")
 
 
-have_libxml2 = pytest.mark.skipif(
+without_fips = pytest.mark.skipif(
+    feature_test("--have-fips-mode"), reason="FIPS support enabled in the build"
+)
+
+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"
 )
 

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

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",

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*",

bin/tests/system/vulture_ignore_list.py

@@ -9,15 +9,5 @@
 # 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()
+transfers_complete  # unused function (cipher-suites/tests_cipher_suites.py:31)
+transfers_complete  # unused variable (cipher-suites/tests_cipher_suites.py:86)