From 02d1c5fc8914a3656d7dc124a43dc1273da441c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Thu, 2 Nov 2023 16:29:46 +0100 Subject: [PATCH 1/8] Silent pylint's line-too-long warning as it's handled better by black Black allows the lines with string literals to be longer, which is convenient for descriptive error messages. --- .pylintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.pylintrc b/.pylintrc index 07d503514d..b5ea55a5e5 100644 --- a/.pylintrc +++ b/.pylintrc @@ -5,6 +5,7 @@ disable= C0115, # missing-class-docstring C0116, # missing-function-docstring C0209, # consider-using-f-string + C0301, # line-too-long, handled better by black C0415, # import-outside-toplevel R0801, # duplicate-code R0903, # too-few-public-methods From e7d46ad8ba17725a934ec84512644f837513b8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Thu, 21 Dec 2023 20:25:20 +0100 Subject: [PATCH 2/8] Extend isctest package with more utility functions Check for more rcodes and various properties needed in the wildcard test. Add a `name` module for various dns.name.Name operations (with `prepend_label` function only now). Expose `timeout` as a parameter of `query.tcp`/`query.udp`. --- bin/tests/system/isctest/__init__.py | 1 + bin/tests/system/isctest/check.py | 18 ++++++++++++++++++ bin/tests/system/isctest/name.py | 16 ++++++++++++++++ bin/tests/system/isctest/query.py | 6 ++++-- 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/isctest/name.py diff --git a/bin/tests/system/isctest/__init__.py b/bin/tests/system/isctest/__init__.py index 73ac413013..afeb55bf79 100644 --- a/bin/tests/system/isctest/__init__.py +++ b/bin/tests/system/isctest/__init__.py @@ -12,6 +12,7 @@ from . import check from . import instance from . import query +from . import name from . import rndc from . import run from . import log diff --git a/bin/tests/system/isctest/check.py b/bin/tests/system/isctest/check.py index 2ef37ed05e..28eb16d5dd 100644 --- a/bin/tests/system/isctest/check.py +++ b/bin/tests/system/isctest/check.py @@ -101,3 +101,21 @@ def zones_equal( def is_executable(cmd: str, errmsg: str) -> None: executable = shutil.which(cmd) assert executable is not None, errmsg + + +def nxdomain(message: dns.message.Message) -> None: + rcode(message, dns.rcode.NXDOMAIN) + + +def single_question(message: dns.message.Message) -> None: + assert len(message.question) == 1, str(message) + + +def empty_answer(message: dns.message.Message) -> None: + assert not message.answer, str(message) + + +def is_response_to(response: dns.message.Message, query: dns.message.Message) -> None: + single_question(response) + single_question(query) + assert query.is_response(response), str(response) diff --git a/bin/tests/system/isctest/name.py b/bin/tests/system/isctest/name.py new file mode 100644 index 0000000000..f5b6de52c3 --- /dev/null +++ b/bin/tests/system/isctest/name.py @@ -0,0 +1,16 @@ +# 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 dns.name + + +def prepend_label(label: str, name: dns.name.Name) -> dns.name.Name: + return dns.name.Name((label,) + name.labels) diff --git a/bin/tests/system/isctest/query.py b/bin/tests/system/isctest/query.py index 329558d272..46fd9b85f9 100644 --- a/bin/tests/system/isctest/query.py +++ b/bin/tests/system/isctest/query.py @@ -24,10 +24,11 @@ def udp( ip: str, port: Optional[int] = None, source: Optional[str] = None, + timeout: int = QUERY_TIMEOUT, ) -> dns.message.Message: if port is None: port = int(os.environ["PORT"]) - return dns.query.udp(message, ip, QUERY_TIMEOUT, port=port, source=source) + return dns.query.udp(message, ip, timeout, port=port, source=source) def tcp( @@ -35,7 +36,8 @@ def tcp( ip: str, port: Optional[int] = None, source: Optional[str] = None, + timeout: int = QUERY_TIMEOUT, ) -> dns.message.Message: if port is None: port = int(os.environ["PORT"]) - return dns.query.tcp(message, ip, QUERY_TIMEOUT, port=port, source=source) + return dns.query.tcp(message, ip, timeout, port=port, source=source) From 5d738cd9edacbd5b8f83f28aefeccf0b2913cbab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Thu, 2 Nov 2023 16:00:46 +0100 Subject: [PATCH 3/8] Add hypothesis strategies for generating DNS names and company The most important being `dns_names` that generates dns.name.Name objects based on given paramaters. No guarantees are given when it comes the uniformity of generated samples, however it plays nicely with the hypothesis' shrinking algorithm. Once we use hypothesis more widely (in at least one more test) this file should be moved for it to be reused easily. --- bin/tests/system/wildcard/strategies.py | 165 ++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 bin/tests/system/wildcard/strategies.py diff --git a/bin/tests/system/wildcard/strategies.py b/bin/tests/system/wildcard/strategies.py new file mode 100644 index 0000000000..2de911d5f6 --- /dev/null +++ b/bin/tests/system/wildcard/strategies.py @@ -0,0 +1,165 @@ +#!/usr/bin/python3 + +# 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. + +from typing import List +from warnings import warn + +from hypothesis.strategies import ( + binary, + builds, + composite, + integers, + just, + nothing, + permutations, +) + +import dns.name +import dns.message +import dns.rdataclass +import dns.rdatatype + +# LATER: Move this file so it can be easily reused. + + +@composite +def dns_names( + draw, + *, + prefix: dns.name.Name = dns.name.empty, + suffix: dns.name.Name = dns.name.root, + min_labels: int = 1, + max_labels: int = 128, +) -> dns.name.Name: + """ + This is a hypothesis strategy to be used for generating DNS names with given `prefix`, `suffix` + and with total number of labels specified by `min_labels` and `max labels`. + + For example, calling + ``` + dns_names( + prefix=dns.name.from_text("test"), + suffix=dns.name.from_text("isc.org"), + max_labels=6 + ).example() + ``` + will result in names like `test.abc.isc.org.` or `test.abc.def.isc.org`. + + There is no attempt to make the distribution of the generated names uniform in any way. + The strategy however minimizes towards shorter names with shorter labels. + + It can be used with to build compound strategies, like this one which generates random DNS queries. + + ``` + dns_queries = builds( + dns.message.make_query, + qname=dns_names(), + rdtype=dns_rdatatypes, + rdclass=dns_rdataclasses, + ) + ``` + """ + + prefix = prefix.relativize(dns.name.root) + suffix = suffix.derelativize(dns.name.root) + + try: + outer_name = prefix + suffix + remaining_bytes = 255 - len(outer_name.to_wire()) + assert remaining_bytes >= 0 + except dns.name.NameTooLong: + warn( + "Maximal length name of name execeeded by prefix and suffix. Strategy won't generate any names.", + RuntimeWarning, + ) + return draw(nothing()) + + minimum_number_of_labels_to_generate = max(0, min_labels - len(outer_name.labels)) + maximum_number_of_labels_to_generate = max_labels - len(outer_name.labels) + if maximum_number_of_labels_to_generate < 0: + warn( + "Maximal number of labels execeeded by prefix and suffix. Strategy won't generate any names.", + RuntimeWarning, + ) + return draw(nothing()) + + maximum_number_of_labels_to_generate = min( + maximum_number_of_labels_to_generate, remaining_bytes // 2 + ) + if maximum_number_of_labels_to_generate < minimum_number_of_labels_to_generate: + warn( + f"Minimal number set to {minimum_number_of_labels_to_generate}, but in {remaining_bytes} bytes there is only space for maximum of {maximum_number_of_labels_to_generate} labels.", + RuntimeWarning, + ) + return draw(nothing()) + + if remaining_bytes == 0 or maximum_number_of_labels_to_generate == 0: + warn( + f"Strategy will return only one name ({outer_name}) as it exactly matches byte or label length limit.", + RuntimeWarning, + ) + return draw(just(outer_name)) + + chosen_number_of_labels_to_generate = draw( + integers( + minimum_number_of_labels_to_generate, maximum_number_of_labels_to_generate + ) + ) + chosen_number_of_bytes_to_partion = draw( + integers(2 * chosen_number_of_labels_to_generate, remaining_bytes) + ) + chosen_lengths_of_labels = draw( + _partition_bytes_to_labels( + chosen_number_of_bytes_to_partion, chosen_number_of_labels_to_generate + ) + ) + generated_labels = tuple( + draw(binary(min_size=l - 1, max_size=l - 1)) for l in chosen_lengths_of_labels + ) + + return dns.name.Name(prefix.labels + generated_labels + suffix.labels) + + +RDATACLASS_MAX = RDATATYPE_MAX = 65535 +dns_rdataclasses = builds(dns.rdataclass.RdataClass, integers(0, RDATACLASS_MAX)) +dns_rdataclasses_without_meta = dns_rdataclasses.filter(dns.rdataclass.is_metaclass) +dns_rdatatypes = builds(dns.rdatatype.RdataType, integers(0, RDATATYPE_MAX)) + +# NOTE: This should really be `dns_rdatatypes_without_meta = dns_rdatatypes_without_meta.filter(dns.rdatatype.is_metatype()`, +# but hypothesis then complains about the filter being too strict, so it is done in a “constructive” way. +dns_rdatatypes_without_meta = integers(0, dns.rdatatype.OPT - 1) | integers(dns.rdatatype.OPT + 1, 127) | integers(256, RDATATYPE_MAX) # type: ignore + + +@composite +def _partition_bytes_to_labels( + draw, remaining_bytes: int, number_of_labels: int +) -> List[int]: + two_bytes_reserved_for_label = 2 + + # Reserve two bytes for each label + partition = [two_bytes_reserved_for_label] * number_of_labels + remaining_bytes -= two_bytes_reserved_for_label * number_of_labels + + assert remaining_bytes >= 0 + + # Add a random number between 0 and the remainder to each partition + for i in range(number_of_labels): + added = draw( + integers(0, min(remaining_bytes, 64 - two_bytes_reserved_for_label)) + ) + partition[i] += added + remaining_bytes -= added + + # NOTE: Some of the remaining bytes will usually not be assigned to any label, but we don't care. + + return draw(permutations(partition)) From d0cfbd398e786b1565c80216ccca974e5fb2fec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Thu, 2 Nov 2023 16:03:54 +0100 Subject: [PATCH 4/8] Expand the wildcard system test with wider use of hypothesis The queries are now generated more generally (i. e. they have multiple labels, etc.). --- bin/tests/system/wildcard/tests_wildcard.py | 119 +++++++++++--------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/bin/tests/system/wildcard/tests_wildcard.py b/bin/tests/system/wildcard/tests_wildcard.py index d73fd28cd6..e5ea5c1b91 100755 --- a/bin/tests/system/wildcard/tests_wildcard.py +++ b/bin/tests/system/wildcard/tests_wildcard.py @@ -11,6 +11,7 @@ # See the COPYRIGHT file distributed with this work for additional # information regarding copyright ownership. + """ Example property-based test for wildcard synthesis. Verifies that otherwise-empty zone with single wildcard record * A 192.0.2.1 @@ -18,8 +19,6 @@ produces synthesized answers for .test. A, and returns NODATA for .test. when rdtype is not A. Limitations - untested properties: - - expansion works with multiple labels - - asterisk in qname does not cause expansion - empty non-terminals prevent expansion - or more generally any existing node prevents expansion - DNSSEC record inclusion @@ -28,9 +27,10 @@ Limitations - untested properties: - flags beyond RCODE - special behavior of rdtypes like CNAME """ + import pytest -pytest.importorskip("dns") +pytest.importorskip("dns", minversion="2.0.0") import dns.message import dns.name import dns.query @@ -47,73 +47,88 @@ try: pytest.importorskip("hypothesis") except ValueError: pytest.importorskip("hypothesis", minversion="4.41.2") -from hypothesis import given -from hypothesis.strategies import binary, integers +from hypothesis import assume, example, given + +from strategies import dns_names, dns_rdatatypes_without_meta +import isctest.check +import isctest.name +import isctest.query # labels of a zone with * A 192.0.2.1 wildcard -WILDCARD_ZONE = ("allwild", "test", "") +SUFFIX = dns.name.from_text("allwild.test.") WILDCARD_RDTYPE = dns.rdatatype.A WILDCARD_RDATA = "192.0.2.1" -IPADDR = "10.53.0.1" +IP_ADDR = "10.53.0.1" TIMEOUT = 5 # seconds, just a sanity check -# Helpers -def is_nonexpanding_rdtype(rdtype): - """skip meta types to avoid weird rcodes caused by AXFR etc.; RFC 6895""" - return not ( - rdtype == WILDCARD_RDTYPE - or dns.rdatatype.is_metatype(rdtype) # known metatypes: OPT ... - or 128 <= rdtype <= 255 - ) # unknown meta types +@given(name=dns_names(suffix=SUFFIX), rdtype=dns_rdatatypes_without_meta) +def test_wildcard_rdtype_mismatch( + name: dns.name.Name, rdtype: dns.rdatatype.RdataType, named_port: int +) -> None: + """Any label non-matching rdtype must result in NODATA.""" + assume(rdtype != WILDCARD_RDTYPE) + + # NS and SOA are present in the zone and DS gets answered from parent. + assume( + not ( + name == SUFFIX + and rdtype in (dns.rdatatype.SOA, dns.rdatatype.NS, dns.rdatatype.DS) + ) + ) + + # Subdomains of *.allwild.test. are not to be synthesized. + # See RFC 4592 section 2.2.1. + assume(name == SUFFIX or name.labels[-len(SUFFIX) - 1] != b"*") + + query_msg = dns.message.make_query(name, rdtype) + response_msg = isctest.query.tcp(query_msg, IP_ADDR, named_port, timeout=TIMEOUT) + + isctest.check.is_response_to(response_msg, query_msg) + isctest.check.noerror(response_msg) + isctest.check.empty_answer(response_msg) -def tcp_query(where, port, qname, qtype): - querymsg = dns.message.make_query(qname, qtype) - assert len(querymsg.question) == 1 - return querymsg, dns.query.tcp(querymsg, where, port=port, timeout=TIMEOUT) +@given(name=dns_names(suffix=SUFFIX, min_labels=len(SUFFIX) + 1)) +def test_wildcard_match(name: dns.name.Name, named_port: int) -> None: + """Any label with maching rdtype must result in wildcard data in answer.""" + # Subdomains of *.allwild.test. are not to be synthesized. + # See RFC 4592 section 2.2.1. + assume(name.labels[-len(SUFFIX) - 1] != b"*") -def query(where, port, label, rdtype): - labels = (label,) + WILDCARD_ZONE - qname = dns.name.Name(labels) - return tcp_query(where, port, qname, rdtype) + query_msg = dns.message.make_query(name, WILDCARD_RDTYPE) + response_msg = isctest.query.tcp(query_msg, IP_ADDR, named_port, timeout=TIMEOUT) - -# Tests -@given( - label=binary(min_size=1, max_size=63), - rdtype=integers(min_value=0, max_value=65535).filter(is_nonexpanding_rdtype), -) -def test_wildcard_rdtype_mismatch(label, rdtype, named_port): - """any label non-matching rdtype must result in to NODATA""" - check_answer_nodata(*query(IPADDR, named_port, label, rdtype)) - - -def check_answer_nodata(querymsg, answer): - assert querymsg.is_response(answer), str(answer) - assert answer.rcode() == dns.rcode.NOERROR, str(answer) - assert answer.answer == [], str(answer) - - -@given(label=binary(min_size=1, max_size=63)) -def test_wildcard_match(label, named_port): - """any label with maching rdtype must result in wildcard data in answer""" - check_answer_noerror(*query(IPADDR, named_port, label, WILDCARD_RDTYPE)) - - -def check_answer_noerror(querymsg, answer): - assert querymsg.is_response(answer), str(answer) - assert answer.rcode() == dns.rcode.NOERROR, str(answer) - assert len(querymsg.question) == 1, str(answer) + isctest.check.is_response_to(response_msg, query_msg) + isctest.check.noerror(response_msg) expected_answer = [ dns.rrset.from_text( - querymsg.question[0].name, + query_msg.question[0].name, 300, # TTL, ignored by dnspython comparison dns.rdataclass.IN, WILDCARD_RDTYPE, WILDCARD_RDATA, ) ] - assert answer.answer == expected_answer, str(answer) + assert response_msg.answer == expected_answer, str(response_msg) + + +# Force the `*.*.allwild.test.` corner case to be checked. +@example(name=isctest.name.prepend_label("*", isctest.name.prepend_label("*", SUFFIX))) +@given( + name=dns_names( + suffix=isctest.name.prepend_label("*", SUFFIX), min_labels=len(SUFFIX) + 2 + ) +) +def test_wildcard_with_star_not_synthesized( + name: dns.name.Name, named_port: int +) -> None: + """RFC 4592 section 2.2.1 ghost.*.example.""" + query_msg = dns.message.make_query(name, WILDCARD_RDTYPE) + response_msg = isctest.query.tcp(query_msg, IP_ADDR, named_port, timeout=TIMEOUT) + + isctest.check.is_response_to(response_msg, query_msg) + isctest.check.nxdomain(response_msg) + isctest.check.empty_answer(query_msg) From 9943172566e44ee4a93944e257717eb116b337f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Mon, 29 Jan 2024 18:56:32 +0100 Subject: [PATCH 5/8] Test nested wildcard special case in the respective system test See final remark of RFC 4592 2.2.1. for details. --- bin/tests/system/wildcard/ns1/named.conf.in | 6 ++ .../system/wildcard/ns1/nestedwild.db.in | 16 +++++ bin/tests/system/wildcard/ns1/sign.sh | 1 + bin/tests/system/wildcard/tests_wildcard.py | 59 +++++++++++++++++++ 4 files changed, 82 insertions(+) create mode 100644 bin/tests/system/wildcard/ns1/nestedwild.db.in diff --git a/bin/tests/system/wildcard/ns1/named.conf.in b/bin/tests/system/wildcard/ns1/named.conf.in index ac02abf8dc..7ae63ac1b9 100644 --- a/bin/tests/system/wildcard/ns1/named.conf.in +++ b/bin/tests/system/wildcard/ns1/named.conf.in @@ -34,6 +34,12 @@ zone "example" { type primary; file "example.db"; }; zone "nsec" { type primary; file "nsec.db.signed"; }; zone "private.nsec" { type primary; file "private.nsec.db.signed"; }; +zone "nestedwild.test" { + type primary; + file "nestedwild.db"; + check-names ignore; +}; + /* * The contents of nsec3 and private.nsec3 are specially chosen to * have separate NSEC3 records for the "no qname proof" and the diff --git a/bin/tests/system/wildcard/ns1/nestedwild.db.in b/bin/tests/system/wildcard/ns1/nestedwild.db.in new file mode 100644 index 0000000000..02278a8e52 --- /dev/null +++ b/bin/tests/system/wildcard/ns1/nestedwild.db.in @@ -0,0 +1,16 @@ +; 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. + +$ORIGIN nestedwild.test. +nestedwild.test. 3600 IN SOA . . 0 0 0 0 0 +nestedwild.test. 3600 NS ns.example.test. +*.nestedwild.test. 3600 A 192.0.2.1 +*.*.*.nestedwild.test. 3600 A 192.0.2.1 \ No newline at end of file diff --git a/bin/tests/system/wildcard/ns1/sign.sh b/bin/tests/system/wildcard/ns1/sign.sh index 7af0b83f72..cc01160757 100755 --- a/bin/tests/system/wildcard/ns1/sign.sh +++ b/bin/tests/system/wildcard/ns1/sign.sh @@ -18,6 +18,7 @@ dssets= # RFC 4592 example zone. cp allwild.db.in allwild.db cp example.db.in example.db +cp nestedwild.db.in nestedwild.db zone=nsec infile=nsec.db.in diff --git a/bin/tests/system/wildcard/tests_wildcard.py b/bin/tests/system/wildcard/tests_wildcard.py index e5ea5c1b91..cd5b278d28 100755 --- a/bin/tests/system/wildcard/tests_wildcard.py +++ b/bin/tests/system/wildcard/tests_wildcard.py @@ -132,3 +132,62 @@ def test_wildcard_with_star_not_synthesized( isctest.check.is_response_to(response_msg, query_msg) isctest.check.nxdomain(response_msg) isctest.check.empty_answer(query_msg) + + +NESTED_SUFFIX = dns.name.from_text("*.*.nestedwild.test.") + + +# Force `*.*.*.nestedwild.test.` to be checked. +@example(name=isctest.name.prepend_label("*", NESTED_SUFFIX)) +@given(name=dns_names(suffix=NESTED_SUFFIX, min_labels=len(NESTED_SUFFIX) + 1)) +def test_name_in_between_wildcards(name: dns.name.Name, named_port: int) -> None: + """Check nested wildcard cases. + + There are `*.nestedwild.test. A` and `*.*.*.nestedwild.test. A` records present in their zone. + This means that `foo.*.nestedwild.test. A` must not be synthetized (see test above) + but `foo.*.*.nestedwild.test A` must. + """ + + # `*.*.*.nestedwild.test.` and `*.foo.*.*.nestedwild.test.` must be NOERROR + # `foo.*.*.*.nestedwild.test` must be NXDOMAIN (see test below). + assume( + len(name) == len(NESTED_SUFFIX) + 1 + or name.labels[-len(NESTED_SUFFIX) - 1] != b"*" + ) + + query_msg = dns.message.make_query(name, WILDCARD_RDTYPE) + response_msg = isctest.query.tcp(query_msg, IP_ADDR, named_port, timeout=TIMEOUT) + + isctest.check.is_response_to(response_msg, query_msg) + isctest.check.noerror(response_msg) + expected_answer = [ + dns.rrset.from_text( + query_msg.question[0].name, + 300, # TTL, ignored by dnspython comparison + dns.rdataclass.IN, + WILDCARD_RDTYPE, + WILDCARD_RDATA, + ) + ] + assert response_msg.answer == expected_answer, str(response_msg) + + +@given( + name=dns_names( + suffix=isctest.name.prepend_label("*", NESTED_SUFFIX), + min_labels=len(NESTED_SUFFIX) + 2, + ) +) +def test_name_nested_wildcard_subdomains_not_synthesized( + name: dns.name.Name, named_port: int +): + """Check nested wildcard cases. + + `foo.*.*.*.nestedwild.test. A` must not be synthesized. + """ + query_msg = dns.message.make_query(name, WILDCARD_RDTYPE) + response_msg = isctest.query.tcp(query_msg, IP_ADDR, named_port, timeout=TIMEOUT) + + isctest.check.is_response_to(response_msg, query_msg) + isctest.check.nxdomain(response_msg) + isctest.check.empty_answer(query_msg) From f55cacbbfd5eeaeb8e521895a0ffab8addeaad6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Tue, 23 Apr 2024 15:48:18 +0200 Subject: [PATCH 6/8] Disable deadlines for hypothesis tests when running in CI The times it takes to run tests CI vary significantly enough that it makes hypothesis test reach their deadlines and fail randomly marking the tests as flaky. This commit disables the deadlines when running in CI. --- .gitlab-ci.yml | 2 ++ bin/tests/system/wildcard/tests_wildcard.py | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 72b64b8433..0396e175db 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -55,6 +55,8 @@ variables: BIND_STRESS_TEST_OS: linux BIND_STRESS_TEST_ARCH: amd64 + HYPOTHESIS_PROFILE: "ci" + default: # Allow all running CI jobs to be automatically canceled when a new # version of a branch is pushed. diff --git a/bin/tests/system/wildcard/tests_wildcard.py b/bin/tests/system/wildcard/tests_wildcard.py index cd5b278d28..f304f966b3 100755 --- a/bin/tests/system/wildcard/tests_wildcard.py +++ b/bin/tests/system/wildcard/tests_wildcard.py @@ -28,6 +28,7 @@ Limitations - untested properties: - special behavior of rdtypes like CNAME """ +import os import pytest pytest.importorskip("dns", minversion="2.0.0") @@ -47,7 +48,7 @@ try: pytest.importorskip("hypothesis") except ValueError: pytest.importorskip("hypothesis", minversion="4.41.2") -from hypothesis import assume, example, given +from hypothesis import assume, example, given, settings from strategies import dns_names, dns_rdatatypes_without_meta import isctest.check @@ -62,6 +63,10 @@ WILDCARD_RDATA = "192.0.2.1" IP_ADDR = "10.53.0.1" TIMEOUT = 5 # seconds, just a sanity check +# Timing of hypothesis tests is flaky in the CI, so we disable deadlines. +settings.register_profile("ci", deadline=None) +settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default")) + @given(name=dns_names(suffix=SUFFIX), rdtype=dns_rdatatypes_without_meta) def test_wildcard_rdtype_mismatch( From bb1e5cfa092cc18da43fbf37db3fcfb5f7c8713c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Tue, 23 Apr 2024 16:45:45 +0200 Subject: [PATCH 7/8] Move hypothesis strategies to isctest for later reuse `isctest.hypothesis` seems to be a nice place to have these. --- bin/tests/system/isctest/__init__.py | 1 + .../system/isctest/hypothesis/__init__.py | 13 +++++++++++++ .../system/isctest/hypothesis/settings.py | 18 ++++++++++++++++++ .../hypothesis}/strategies.py | 11 +++++++---- bin/tests/system/wildcard/tests_wildcard.py | 9 ++------- 5 files changed, 41 insertions(+), 11 deletions(-) create mode 100644 bin/tests/system/isctest/hypothesis/__init__.py create mode 100644 bin/tests/system/isctest/hypothesis/settings.py rename bin/tests/system/{wildcard => isctest/hypothesis}/strategies.py (92%) diff --git a/bin/tests/system/isctest/__init__.py b/bin/tests/system/isctest/__init__.py index afeb55bf79..5426e450fe 100644 --- a/bin/tests/system/isctest/__init__.py +++ b/bin/tests/system/isctest/__init__.py @@ -17,6 +17,7 @@ from . import rndc from . import run from . import log from . import vars # pylint: disable=redefined-builtin +from . import hypothesis # isctest.mark module is intentionally NOT imported, because it relies on # environment variables which might not be set at the time of import of the diff --git a/bin/tests/system/isctest/hypothesis/__init__.py b/bin/tests/system/isctest/hypothesis/__init__.py new file mode 100644 index 0000000000..0bb73f3bab --- /dev/null +++ b/bin/tests/system/isctest/hypothesis/__init__.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. + +from . import settings +from . import strategies diff --git a/bin/tests/system/isctest/hypothesis/settings.py b/bin/tests/system/isctest/hypothesis/settings.py new file mode 100644 index 0000000000..5eae01063b --- /dev/null +++ b/bin/tests/system/isctest/hypothesis/settings.py @@ -0,0 +1,18 @@ +# 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 os + +from hypothesis import settings + +# Timing of hypothesis tests is flaky in the CI, so we disable deadlines. +settings.register_profile("ci", deadline=None) +settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default")) diff --git a/bin/tests/system/wildcard/strategies.py b/bin/tests/system/isctest/hypothesis/strategies.py similarity index 92% rename from bin/tests/system/wildcard/strategies.py rename to bin/tests/system/isctest/hypothesis/strategies.py index 2de911d5f6..1e05cc1932 100644 --- a/bin/tests/system/wildcard/strategies.py +++ b/bin/tests/system/isctest/hypothesis/strategies.py @@ -29,8 +29,6 @@ import dns.message import dns.rdataclass import dns.rdatatype -# LATER: Move this file so it can be easily reused. - @composite def dns_names( @@ -131,9 +129,14 @@ def dns_names( RDATACLASS_MAX = RDATATYPE_MAX = 65535 -dns_rdataclasses = builds(dns.rdataclass.RdataClass, integers(0, RDATACLASS_MAX)) +try: + dns_rdataclasses = builds(dns.rdataclass.RdataClass, integers(0, RDATACLASS_MAX)) + dns_rdatatypes = builds(dns.rdatatype.RdataType, integers(0, RDATATYPE_MAX)) +except AttributeError: + # In old dnspython versions, RDataTypes and RDataClasses are int and not enums. + dns_rdataclasses = integers(0, RDATACLASS_MAX) # type: ignore + dns_rdatatypes = integers(0, RDATATYPE_MAX) # type: ignore dns_rdataclasses_without_meta = dns_rdataclasses.filter(dns.rdataclass.is_metaclass) -dns_rdatatypes = builds(dns.rdatatype.RdataType, integers(0, RDATATYPE_MAX)) # NOTE: This should really be `dns_rdatatypes_without_meta = dns_rdatatypes_without_meta.filter(dns.rdatatype.is_metatype()`, # but hypothesis then complains about the filter being too strict, so it is done in a “constructive” way. diff --git a/bin/tests/system/wildcard/tests_wildcard.py b/bin/tests/system/wildcard/tests_wildcard.py index f304f966b3..cc5c1571a2 100755 --- a/bin/tests/system/wildcard/tests_wildcard.py +++ b/bin/tests/system/wildcard/tests_wildcard.py @@ -28,7 +28,6 @@ Limitations - untested properties: - special behavior of rdtypes like CNAME """ -import os import pytest pytest.importorskip("dns", minversion="2.0.0") @@ -48,9 +47,9 @@ try: pytest.importorskip("hypothesis") except ValueError: pytest.importorskip("hypothesis", minversion="4.41.2") -from hypothesis import assume, example, given, settings +from hypothesis import assume, example, given -from strategies import dns_names, dns_rdatatypes_without_meta +from isctest.hypothesis.strategies import dns_names, dns_rdatatypes_without_meta import isctest.check import isctest.name import isctest.query @@ -63,10 +62,6 @@ WILDCARD_RDATA = "192.0.2.1" IP_ADDR = "10.53.0.1" TIMEOUT = 5 # seconds, just a sanity check -# Timing of hypothesis tests is flaky in the CI, so we disable deadlines. -settings.register_profile("ci", deadline=None) -settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default")) - @given(name=dns_names(suffix=SUFFIX), rdtype=dns_rdatatypes_without_meta) def test_wildcard_rdtype_mismatch( From 9584a7bdcd5978165a14348247fa670025172257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Bal=C3=A1=C5=BEik?= Date: Tue, 14 May 2024 16:26:44 +0200 Subject: [PATCH 8/8] Add a helper for uncompressed length of dnspython's dns.name.Name This is useful for generating using hypothesis but also for other cases. --- bin/tests/system/isctest/hypothesis/strategies.py | 4 +++- bin/tests/system/isctest/name.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/isctest/hypothesis/strategies.py b/bin/tests/system/isctest/hypothesis/strategies.py index 1e05cc1932..d26a90b1c2 100644 --- a/bin/tests/system/isctest/hypothesis/strategies.py +++ b/bin/tests/system/isctest/hypothesis/strategies.py @@ -29,6 +29,8 @@ import dns.message import dns.rdataclass import dns.rdatatype +import isctest.name + @composite def dns_names( @@ -73,7 +75,7 @@ def dns_names( try: outer_name = prefix + suffix - remaining_bytes = 255 - len(outer_name.to_wire()) + remaining_bytes = 255 - isctest.name.len_wire_uncompressed(outer_name) assert remaining_bytes >= 0 except dns.name.NameTooLong: warn( diff --git a/bin/tests/system/isctest/name.py b/bin/tests/system/isctest/name.py index f5b6de52c3..d992ae3fce 100644 --- a/bin/tests/system/isctest/name.py +++ b/bin/tests/system/isctest/name.py @@ -14,3 +14,7 @@ import dns.name def prepend_label(label: str, name: dns.name.Name) -> dns.name.Name: return dns.name.Name((label,) + name.labels) + + +def len_wire_uncompressed(name: dns.name.Name) -> int: + return len(name) + sum(map(len, name.labels))