Add certbot-dns-rfc2136 integration testing (#8448)

* tests: add certbot-dns-rfc2136 integration tests

* dont use 'with' form of socket.socket

fixes py2 crash

* address some feedback:

- conftest: make DNS server a global resource
- conftest: add dns_xdist parameter into node config
- conftest: add --dns-server=bind flag
- conftest: if configured, point the ACME server to the DNS server
- dnsserver: make it sort-of compatible with xdist (future-proofing)
- context: parameterize dns-rfc2136 credentials file (future proofing)
- context: reduce dns-rfc2136 propagation time to speed up tests
- tox: add a integration-dns-rfc2136 target
- rfc2136: add a test/zone for subdelegation
- rfc2136: skip tests if no DNS server is configured

* try add integration-dns-rfc2136 to CI

* mock recursive dns via RPZ

* update --dns-server args and tox.ini args

* address more feedback:

- dns_server: rename rfc2136 creds file to .tpl
- dns_server: dont vary dns server port, instead we will vary zone names (#8455)
- dns_server: log error if bind9 fails to stop cleanly
- dns_server: replace assert with raise
- context: remove redundant _worker_id
- context: remove redundant cleanup override
- context: fix seek/flush in credentials context manager
- context: rename skip_if_no_server -> ...bind_server
- context: add newline EOF

* conftest: document _setup_primary_node sideeffects

* ci: rfc2136-integration from standard->nightly

* fix _stop_bind (function was renamed to stop)

* ignore errors from shutil.rmtree during cleanup

* dns_server: check for crash while polling

* remove --dry-run from rfc2136 test

.azure-pipelines/templates/jobs/extended-tests-jobs.yml

@@ -64,6 +64,10 @@ jobs:
           ACME_SERVER: boulder-v2
         nginx-compat:
           TOXENV: nginx_compat
+        linux-integration-rfc2136:
+          IMAGE_NAME: ubuntu-18.04
+          PYTHON_VERSION: 3.8
+          TOXENV: integration-dns-rfc2136
         le-auto-oraclelinux6:
           TOXENV: le_auto_oraclelinux6
         docker-dev:

certbot-ci/certbot_integration_tests/assets/bind-config/conf/named.conf

@@ -0,0 +1,60 @@
+options {
+  directory "/var/cache/bind";
+
+  // Running inside Docker. Bind address on Docker host is 127.0.0.1.
+  listen-on { any; };
+  listen-on-v6 { any; };
+
+  // We are allowing BIND to service recursive queries, but only in an extremely limimited sense
+  // where it is entirely disconnected from public DNS:
+  // - Iterative queries are disabled. Only forwarding to a non-existent forwarder.
+  // - The only recursive answers we can get (that will not be a SERVFAIL) will come from the
+  //   RPZ "mock-recursion" zone. Effectively this means we are mocking out the entirety of
+  //   public DNS.
+  allow-recursion { any; };       // BIND will only answer using RPZ if recursion is enabled
+  forwarders { 192.0.2.254; };    // Nobody is listening, this is TEST-NET-1
+  forward only;                   // Do NOT perform iterative queries from the root zone
+  dnssec-validation no;           // Do not bother fetching the root DNSKEY set (performance)
+  response-policy {               // All recursive queries will be served from here.
+    zone "mock-recursion"
+    log yes;
+  } recursive-only no             // Allow RPZs to affect authoritative zones too.
+    qname-wait-recurse no         // No real recursion.
+    nsip-wait-recurse no;         // No real recursion.
+
+  allow-transfer { none; };
+  allow-update { none; };
+};
+
+key "default-key." {
+  algorithm hmac-sha512;
+  secret "91CgOwzihr0nAVEHKFXJPQCbuBBbBI19Ks5VAweUXgbF40NWTD83naeg3c5y2MPdEiFRXnRLJxL6M+AfHCGLNw==";
+};
+
+zone "mock-recursion" {
+  type primary;
+  file "/var/lib/bind/rpz.mock-recursion";
+  allow-query {
+    none;
+  };
+};
+
+zone "example.com." {
+  type primary;
+  file "/var/lib/bind/db.example.com";
+  journal "/var/cache/bind/db.example.com.jnl";
+
+  update-policy {
+    grant default-key zonesub TXT;
+  };
+};
+
+zone "sub.example.com." {
+  type primary;
+  file "/var/lib/bind/db.sub.example.com";
+  journal "/var/cache/bind/db.sub.example.com.jnl";
+
+  update-policy {
+    grant default-key zonesub TXT;
+  };
+};

certbot-ci/certbot_integration_tests/assets/bind-config/rfc2136-credentials-default.ini.tpl

@@ -0,0 +1,10 @@
+# Target DNS server
+dns_rfc2136_server = {server_address}
+# Target DNS port
+dns_rfc2136_port = {server_port}
+# TSIG key name
+dns_rfc2136_name = default-key.
+# TSIG key secret
+dns_rfc2136_secret = 91CgOwzihr0nAVEHKFXJPQCbuBBbBI19Ks5VAweUXgbF40NWTD83naeg3c5y2MPdEiFRXnRLJxL6M+AfHCGLNw==
+# TSIG key algorithm
+dns_rfc2136_algorithm = HMAC-SHA512

certbot-ci/certbot_integration_tests/assets/bind-config/zones/db.example.com

@@ -0,0 +1,11 @@
+$ORIGIN example.com.
+$TTL 3600
+example.com.  IN  SOA   ns1.example.com. admin.example.com. ( 2020091025 7200 3600 1209600 3600 )
+
+example.com.  IN  NS    ns1
+example.com.  IN  NS    ns2
+
+ns1           IN  A     192.0.2.2
+ns2           IN  A     192.0.2.3
+
+@         IN  A     192.0.2.1

certbot-ci/certbot_integration_tests/assets/bind-config/zones/db.sub.example.com

@@ -0,0 +1,9 @@
+$ORIGIN sub.example.com.
+$TTL 3600
+sub.example.com.  IN  SOA   ns1.example.com. admin.example.com. ( 2020091025 7200 3600 1209600 3600 )
+
+sub.example.com.  IN  NS    ns1
+sub.example.com.  IN  NS    ns2
+
+ns1           IN  A     192.0.2.2
+ns2           IN  A     192.0.2.3

certbot-ci/certbot_integration_tests/assets/bind-config/zones/rpz.mock-recursion

@@ -0,0 +1,6 @@
+$TTL 3600
+
+@        SOA ns1.example.test. dummy.example.test. 1 12h 15m 3w 2h
+         NS  ns1.example.test.
+
+_acme-challenge.aliased.example   IN     CNAME   _acme-challenge.example.com.

certbot-ci/certbot_integration_tests/conftest.py

@@ -12,6 +12,8 @@ import subprocess
 import sys
 
 from certbot_integration_tests.utils import acme_server as acme_lib
+from certbot_integration_tests.utils import dns_server as dns_lib
+from certbot_integration_tests.utils.dns_server import DNSServer
 
 
 def pytest_addoption(parser):
@@ -23,6 +25,10 @@ def pytest_addoption(parser):
                      choices=['boulder-v1', 'boulder-v2', 'pebble'],
                      help='select the ACME server to use (boulder-v1, boulder-v2, '
                           'pebble), defaulting to pebble')
+    parser.addoption('--dns-server', default='challtestsrv',
+                     choices=['bind', 'challtestsrv'],
+                     help='select the DNS server to use (bind, challtestsrv), '
+                          'defaulting to challtestsrv')
 
 
 def pytest_configure(config):
@@ -32,7 +38,7 @@ def pytest_configure(config):
     """
     if not hasattr(config, 'slaveinput'):  # If true, this is the primary node
         with _print_on_err():
-            config.acme_xdist = _setup_primary_node(config)
+            _setup_primary_node(config)
 
 
 def pytest_configure_node(node):
@@ -41,6 +47,7 @@ def pytest_configure_node(node):
     :param node: current worker node
     """
     node.slaveinput['acme_xdist'] = node.config.acme_xdist
+    node.slaveinput['dns_xdist'] = node.config.dns_xdist
 
 
 @contextlib.contextmanager
@@ -61,12 +68,18 @@ def _print_on_err():
 def _setup_primary_node(config):
     """
     Setup the environment for integration tests.
-    Will:
+
+    This function will:
         - check runtime compatibility (Docker, docker-compose, Nginx)
         - create a temporary workspace and the persistent GIT repositories space
+        - configure and start a DNS server using Docker, if configured
         - configure and start paralleled ACME CA servers using Docker
-        - transfer ACME CA servers configurations to pytest nodes using env variables
+        - transfer ACME CA and DNS servers configurations to pytest nodes using env variables
-    :param config: Configuration of the pytest primary node
+
+    This function modifies `config` by injecting the ACME CA and DNS server configurations,
+    in addition to cleanup functions for those servers.
+
+    :param config: Configuration of the pytest primary node. Is modified by this function.
     """
     # Check for runtime compatibility: some tools are required to be available in PATH
     if 'boulder' in config.option.acme_server:
@@ -86,11 +99,26 @@ def _setup_primary_node(config):
     workers = ['primary'] if not config.option.numprocesses\
         else ['gw{0}'.format(i) for i in range(config.option.numprocesses)]
 
+    # If a non-default DNS server is configured, start it and feed it to the ACME server
+    dns_server = None
+    acme_dns_server = None
+    if config.option.dns_server == 'bind':
+        dns_server = dns_lib.DNSServer(workers)
+        config.add_cleanup(dns_server.stop)
+        print('DNS xdist config:\n{0}'.format(dns_server.dns_xdist))
+        dns_server.start()
+        acme_dns_server = '{}:{}'.format(
+            dns_server.dns_xdist['address'],
+            dns_server.dns_xdist['port']
+        )
+
     # By calling setup_acme_server we ensure that all necessary acme server instances will be
     # fully started. This runtime is reflected by the acme_xdist returned.
-    acme_server = acme_lib.ACMEServer(config.option.acme_server, workers)
+    acme_server = acme_lib.ACMEServer(config.option.acme_server, workers,
+                                      dns_server=acme_dns_server)
     config.add_cleanup(acme_server.stop)
     print('ACME xdist config:\n{0}'.format(acme_server.acme_xdist))
     acme_server.start()
 
-    return acme_server.acme_xdist
+    config.acme_xdist = acme_server.acme_xdist
+    config.dns_xdist = dns_server.dns_xdist if dns_server else None

certbot-ci/certbot_integration_tests/rfc2136_tests/context.py

@@ -0,0 +1,64 @@
+from contextlib import contextmanager
+from pytest import skip
+from pkg_resources import resource_filename
+import tempfile
+
+from certbot_integration_tests.certbot_tests import context as certbot_context
+from certbot_integration_tests.utils import certbot_call
+
+
+class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
+    """Integration test context for certbot-dns-rfc2136"""
+    def __init__(self, request):
+        super(IntegrationTestsContext, self).__init__(request)
+
+        self.request = request
+
+        self._dns_xdist = None
+        if hasattr(request.config, 'slaveinput'):  # Worker node
+            self._dns_xdist = request.config.slaveinput['dns_xdist']
+        else:  # Primary node
+            self._dns_xdist = request.config.dns_xdist
+
+    def certbot_test_rfc2136(self, args):
+        """
+        Main command to execute certbot using the RFC2136 DNS authenticator.
+        :param list args: list of arguments to pass to Certbot
+        """
+        command = ['--authenticator', 'dns-rfc2136', '--dns-rfc2136-propagation-seconds', '2']
+        command.extend(args)
+        return certbot_call.certbot_test(
+            command, self.directory_url, self.http_01_port, self.tls_alpn_01_port,
+            self.config_dir, self.workspace, force_renew=True)
+
+    @contextmanager
+    def rfc2136_credentials(self, label='default'):
+        # type: (str) -> str
+        """
+        Produces the contents of a certbot-dns-rfc2136 credentials file.
+        :param str label: which RFC2136 credential to use
+        :yields: Path to credentials file
+        :rtype: str
+        """
+        src_file = resource_filename('certbot_integration_tests',
+                                     'assets/bind-config/rfc2136-credentials-{}.ini.tpl'
+                                     .format(label))
+        contents = None
+
+        with open(src_file, 'r') as f:
+            contents = f.read().format(
+                server_address=self._dns_xdist['address'],
+                server_port=self._dns_xdist['port']
+            )
+
+        with tempfile.NamedTemporaryFile('w+', prefix='rfc2136-creds-{}'.format(label),
+                                         suffix='.ini', dir=self.workspace) as f:
+            f.write(contents)
+            f.flush()
+            yield f.name
+
+    def skip_if_no_bind9_server(self):
+        """Skips the test if there was no RFC2136-capable DNS server configured
+        in the test environment"""
+        if not self._dns_xdist:
+            skip('No RFC2136-capable DNS server is configured')

certbot-ci/certbot_integration_tests/rfc2136_tests/test_main.py

@@ -0,0 +1,25 @@
+"""Module executing integration tests against Certbot with the RFC2136 DNS authenticator."""
+import pytest
+
+from certbot_integration_tests.rfc2136_tests import context as rfc2136_context
+
+
+@pytest.fixture()
+def context(request):
+    # Fixture request is a built-in pytest fixture describing current test request.
+    integration_test_context = rfc2136_context.IntegrationTestsContext(request)
+    try:
+        yield integration_test_context
+    finally:
+        integration_test_context.cleanup()
+
+
+@pytest.mark.parametrize('domain', [('example.com'), ('sub.example.com')])
+def test_get_certificate(domain, context):
+    context.skip_if_no_bind9_server()
+
+    with context.rfc2136_credentials() as creds:
+        context.certbot_test_rfc2136([
+            'certonly', '--dns-rfc2136-credentials', creds,
+            '-d', domain, '-d', '*.{}'.format(domain)
+        ])

certbot-ci/certbot_integration_tests/utils/dns_server.py

@@ -0,0 +1,138 @@
+#!/usr/bin/env python
+"""Module to setup an RFC2136-capable DNS server"""
+import os
+import os.path
+from pkg_resources import resource_filename
+import shutil
+import socket
+import subprocess
+import sys
+import tempfile
+import time
+
+
+BIND_DOCKER_IMAGE = 'internetsystemsconsortium/bind9:9.16'
+BIND_BIND_ADDRESS = ('127.0.0.1', 45953)
+
+# A TCP DNS message which is a query for '. CH A' transaction ID 0xcb37. This is used
+# by _wait_until_ready to check that BIND is responding without depending on dnspython.
+BIND_TEST_QUERY = bytearray.fromhex('0011cb37000000010000000000000000010003')
+
+
+class DNSServer(object):
+    """
+    DNSServer configures and handles the lifetime of an RFC2136-capable server.
+    DNServer provides access to the dns_xdist parameter, listing the address and port
+    to use for each pytest node.
+
+    At this time, DNSServer should only be used with a single node, but may be expanded in
+    future to support parallelization (https://github.com/certbot/certbot/issues/8455).
+    """
+
+    def __init__(self, nodes, show_output=False):
+        """
+        Create an DNSServer instance.
+        :param list nodes: list of node names that will be setup by pytest xdist
+        :param bool show_output: if True, print the output of the DNS server
+        """
+
+        self.bind_root = tempfile.mkdtemp()
+
+        self.dns_xdist = {
+            'address': BIND_BIND_ADDRESS[0],
+            'port': BIND_BIND_ADDRESS[1]
+        }
+
+        # Unfortunately the BIND9 image forces everything to stderr with -g and we can't
+        # modify the verbosity.
+        self._output = sys.stderr if show_output else open(os.devnull, 'w')
+
+    def start(self):
+        """Start the DNS server"""
+        try:
+            self._configure_bind()
+            self._start_bind()
+        except:
+            self.stop()
+            raise
+
+    def stop(self):
+        """Stop the DNS server, and clean its resources"""
+        try:
+            self.process.terminate()
+            self.process.wait()
+        except BaseException as e:
+            print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr)
+
+        shutil.rmtree(self.bind_root, ignore_errors=True)
+
+        if self._output != sys.stderr:
+            self._output.close()
+
+    def _configure_bind(self):
+        """Configure the BIND9 server based on the prebaked configuration"""
+        bind_conf_src = resource_filename('certbot_integration_tests', 'assets/bind-config')
+        shutil.copytree(bind_conf_src, self.bind_root, dirs_exist_ok=True)
+
+    def _start_bind(self):
+        """Launch the BIND9 server as a Docker container"""
+        addr_str = '{}:{}'.format(BIND_BIND_ADDRESS[0], BIND_BIND_ADDRESS[1])
+        self.process = subprocess.Popen([
+          'docker', 'run', '--rm',
+          '-p', '{}:53/udp'.format(addr_str),
+          '-p', '{}:53/tcp'.format(addr_str),
+          '-v', '{}/conf:/etc/bind'.format(self.bind_root),
+          '-v', '{}/zones:/var/lib/bind'.format(self.bind_root),
+          BIND_DOCKER_IMAGE
+        ], stdout=self._output, stderr=self._output)
+
+        if self.process.poll():
+            raise("BIND9 server stopped unexpectedly")
+
+        try:
+          self._wait_until_ready()
+        except:
+          # The container might be running even if we think it isn't
+          self.stop()
+          raise
+
+    def _wait_until_ready(self, attempts=30):
+      # type: (int) -> None
+      """
+      Polls the DNS server over TCP until it gets a response, or until
+      it runs out of attempts and raises a ValueError.
+      The DNS response message must match the txn_id of the DNS query message,
+      but otherwise the contents are ignored.
+      :param int attempts: The number of attempts to make.
+      """
+      for _ in range(attempts):
+        if self.process.poll():
+          raise ValueError('BIND9 server stopped unexpectedly')
+
+        sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        sock.settimeout(5.0)
+        try:
+          sock.connect(BIND_BIND_ADDRESS)
+          sock.sendall(BIND_TEST_QUERY)
+          buf = sock.recv(1024)
+          # We should receive a DNS message with the same tx_id
+          if buf and len(buf) > 4 and buf[2:4] == BIND_TEST_QUERY[2:4]:
+            return
+          # If we got a response but it wasn't the one we wanted, wait a little
+          time.sleep(1)
+        except:
+          # If there was a network error, wait a little
+          time.sleep(1)
+          pass
+        finally:
+          sock.close()
+
+      raise ValueError(
+        'Gave up waiting for DNS server {} to respond'.format(BIND_BIND_ADDRESS))
+
+    def __enter__(self):
+        self.start()
+        return self.dns_xdist
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.stop()

tox.ini

@@ -240,6 +240,13 @@ commands =
         --cov-config=certbot-ci/certbot_integration_tests/.coveragerc
     coverage report --include 'certbot/*' --show-missing --fail-under=62
 
+[testenv:integration-dns-rfc2136]
+commands =
+    {[base]pip_install} acme certbot certbot-dns-rfc2136 certbot-ci
+    pytest certbot-ci/certbot_integration_tests/rfc2136_tests \
+        --acme-server=pebble --dns-server=bind \
+        --numprocesses=1
+
 [testenv:integration-external]
 # Run integration tests with Certbot outside of tox's virtual environment.
 commands =