Optionally sign initial SOA query (#9672)

* Optionally sign initial SOA query

Added configuration file option to enable signing of the initial SOA query when determining the authoritative nameserver for the zone. Default is disabled.

* Better handling of sign_query configuration and fix lint issues

* Update str casting to match 5503d12

* Update certbot/CHANGELOG.md

Co-authored-by: alexzorin <alex@zorin.au>

* Update certbot/CHANGELOG.md

Co-authored-by: alexzorin <alex@zorin.au>

* Update dns_rfc2136.py

Updated with feedback from #9672

---------

Co-authored-by: alexzorin <alex@zorin.au>

AUTHORS.md

@@ -227,6 +227,7 @@ Authors
 * [Rémy HUBSCHER](https://github.com/Natim)
 * [Rémy Léone](https://github.com/sieben)
 * [Richard Barnes](https://github.com/r-barnes)
+* [Richard Harman](https://github.com/warewolf)
 * [Richard Panek](https://github.com/kernelpanek)
 * [Robert Buchholz](https://github.com/rbu)
 * [Robert Dailey](https://github.com/pahrohfit)

certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py

@@ -26,8 +26,8 @@
 
 Use of this plugin requires a configuration file containing the target DNS
 server and optional port that supports RFC 2136 Dynamic Updates, the name
-of the TSIG key, the TSIG key secret itself and the algorithm used if it's
-different to HMAC-MD5.
+of the TSIG key, the TSIG key secret itself, the algorithm used if it's
+different to HMAC-MD5, and optionally whether to sign the initial SOA query.
 
 .. code-block:: ini
    :name: credentials.ini
@@ -44,6 +44,9 @@
 AmKd7ak51vWKgSl12ib86oQRPkpDjg==
    # TSIG key algorithm
    dns_rfc2136_algorithm = HMAC-SHA512
+   # TSIG sign SOA query (optional, default: false)
+   dns_rfc2136_sign_query = false
+
 
 The path to this file can be provided interactively or using the
 ``--dns-rfc2136-credentials`` command-line argument. Certbot records the
@@ -194,4 +197,34 @@
      };
    };
 
+Another solution is to add `dns_rfc2136_sign_query = true` to the configuration file and then
+add the key to the `match-clients` list within the external zone view. All queries signed with
+this key should then be directed to this view, regardless of source IP.
+
+.. code-block:: none
+   :caption: Split-view BIND configuration with key-based ACLs
+
+   key "keyname." {
+     algorithm hmac-sha512;
+     secret "4q4wM/2I180UXoMyN4INVhJNi8V9BCV+jMw2mXgZw/CSuxUT8C7NKKFs \
+AmKd7ak51vWKgSl12ib86oQRPkpDjg==";
+   };
+
+   // adjust internal-addresses to suit your needs
+   acl internal-address { 127.0.0.0/8; 10.0.0.0/8; 192.168.0.0/16; 172.16.0.0/12; };
+
+   acl certbot-keys { key keyname.; }
+
+   view "external" {
+     match-clients { acl certbot-keys; !internal-addresses; any; };
+
+     zone "example.com." IN {
+       type master;
+       file "named.example.com";
+       update-policy {
+         grant keyname. name _acme-challenge.example.com. txt;
+       };
+     };
+   };
+
 """

certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py

@@ -90,26 +90,30 @@ def _cleanup(self, _domain: str, validation_name: str, validation: str) -> None:
     def _get_rfc2136_client(self) -> "_RFC2136Client":
         if not self.credentials:  # pragma: no cover
             raise errors.Error("Plugin has not been prepared.")
+
         return _RFC2136Client(cast(str, self.credentials.conf('server')),
                               int(cast(str, self.credentials.conf('port')) or self.PORT),
                               cast(str, self.credentials.conf('name')),
                               cast(str, self.credentials.conf('secret')),
                               self.ALGORITHMS.get(self.credentials.conf('algorithm') or '',
-                                                  dns.tsig.HMAC_MD5))
+                                                  dns.tsig.HMAC_MD5),
+                              (self.credentials.conf('sign_query') or '').upper() == "TRUE")
 
 
 class _RFC2136Client:
     """
     Encapsulates all communication with the target DNS server.
     """
     def __init__(self, server: str, port: int, key_name: str, key_secret: str,
-                 key_algorithm: dns.name.Name, timeout: int = DEFAULT_NETWORK_TIMEOUT) -> None:
+                 key_algorithm: dns.name.Name, sign_query: bool,
+                 timeout: int = DEFAULT_NETWORK_TIMEOUT) -> None:
         self.server = server
         self.port = port
         self.keyring = dns.tsigkeyring.from_text({
             key_name: key_secret
         })
         self.algorithm = key_algorithm
+        self.sign_query = sign_query
         self._default_timeout = timeout
 
     def add_txt_record(self, record_name: str, record_content: str, record_ttl: int) -> None:
@@ -217,8 +221,9 @@ def _query_soa(self, domain_name: str) -> bool:
         request = dns.message.make_query(domain, dns.rdatatype.SOA, dns.rdataclass.IN)
         # Turn off Recursion Desired bit in query
         request.flags ^= dns.flags.RD
-        # Use our TSIG keyring
-        request.use_tsig(self.keyring, algorithm=self.algorithm)
+        # Use our TSIG keyring if configured
+        if self.sign_query:
+            request.use_tsig(self.keyring, algorithm=self.algorithm)
 
         try:
             try:

certbot/CHANGELOG.md

@@ -13,6 +13,16 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
+* Optionally sign the SOA query for dns-rfc2136, to help resolve problems with split-view
+  DNS setups and hidden primary setups.
+  * Certbot versions prior to v1.32.0 did not sign queries with the specified TSIG key
+    resulting in difficulty with split-horizon implementations.
+  * Certbot v1.32.0 through v2.5.0 signed queries by default, potentially causing
+    incompatibility with hidden primary setups with `allow-update-forwarding` enabled
+    if the secondary did not also have the TSIG key within its config.
+  * Certbot v2.6.0 and later no longer signs queries by default, but allows
+    the user to optionally sign these queries by explicit configuration using the
+    `dns_rfc2136_sign_query` option in the credentials .ini file.
 * Lineage name validity is performed for new lineages. `--cert-name` may no longer contain
   filepath separators (i.e. `/` or `\`, depending on the platform).
 * `certbot-dns-google` now loads credentials using the standard [Application Default