From 295dc5a2a95396eb8888031a4c3bf79f9a088c30 Mon Sep 17 00:00:00 2001
From: osirisinferi <github@flut.demon.nl>
Date: Mon, 23 Aug 2021 01:38:14 +0200
Subject: [PATCH] certbot-dns-rfc2136: catch error when a hostname is being
 used for `dns_rfc2136_server` (#8990)

* Raise separate error when a hostname is being used for `dns_rfc2136_server`

* Explicitly say IP address instead of hostname in docs

* Don't catch ValueError, but actually check the server value

* Add tests

* Add CHANGELOG entry
---
 .../certbot_dns_rfc2136/__init__.py           |  2 +-
 .../_internal/dns_rfc2136.py                  |  9 ++++++--
 certbot-dns-rfc2136/tests/dns_rfc2136_test.py | 21 +++++++++++++++++++
 certbot/CHANGELOG.md                          |  4 +++-
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py b/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py
index 3c574835f..077a73185 100644
--- a/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py
+++ b/certbot-dns-rfc2136/certbot_dns_rfc2136/__init__.py
@@ -33,7 +33,7 @@ different to HMAC-MD5.
    :name: credentials.ini
    :caption: Example credentials file:
 
-   # Target DNS server
+   # Target DNS server (IPv4 or IPv6 address, not a hostname)
    dns_rfc2136_server = 192.0.2.1
    # Target DNS port
    dns_rfc2136_port = 53
diff --git a/certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py b/certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py
index 77007a971..9fa68c9d9 100644
--- a/certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py
+++ b/certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py
@@ -3,6 +3,7 @@ import logging
 from typing import Optional
 
 import dns.flags
+from dns.inet import is_address
 import dns.message
 import dns.name
 import dns.query
@@ -54,7 +55,11 @@ class Authenticator(dns_common.DNSAuthenticator):
         return 'This plugin configures a DNS TXT record to respond to a dns-01 challenge using ' + \
                'RFC 2136 Dynamic Updates.'
 
-    def _validate_algorithm(self, credentials):
+    def _validate_credentials(self, credentials):
+        server = credentials.conf('server')
+        if not is_address(server):
+            raise errors.PluginError("The configured target DNS server ({0}) is not a valid IPv4 "
+                                     "or IPv6 address. A hostname is not allowed.".format(server))
         algorithm = credentials.conf('algorithm')
         if algorithm:
             if not self.ALGORITHMS.get(algorithm.upper()):
@@ -69,7 +74,7 @@ class Authenticator(dns_common.DNSAuthenticator):
                 'secret': 'TSIG key secret',
                 'server': 'The target DNS server'
             },
-            self._validate_algorithm
+            self._validate_credentials
         )
 
     def _perform(self, _domain, validation_name, validation):
diff --git a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py
index ec424c6d9..72ea6d9a4 100644
--- a/certbot-dns-rfc2136/tests/dns_rfc2136_test.py
+++ b/certbot-dns-rfc2136/tests/dns_rfc2136_test.py
@@ -74,6 +74,27 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
 
         self.auth.perform([self.achall])
 
+    def test_invalid_server_raises(self):
+        config = VALID_CONFIG.copy()
+        config["rfc2136_server"] = "example.com"
+        dns_test_common.write(config, self.config.rfc2136_credentials)
+
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+    @test_util.patch_display_util()
+    def test_valid_server_passes(self, unused_mock_get_utility):
+        config = VALID_CONFIG.copy()
+        dns_test_common.write(config, self.config.rfc2136_credentials)
+
+        self.auth.perform([self.achall])
+
+        config["rfc2136_server"] = "2001:db8:3333:4444:cccc:dddd:eeee:ffff"
+        dns_test_common.write(config, self.config.rfc2136_credentials)
+
+        self.auth.perform([self.achall])
+
 
 class RFC2136ClientTest(unittest.TestCase):
 
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index cdbe12fb5..88e5e84b4 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -6,7 +6,9 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* The certbot-dns-rfc2136 plugin always assumed the use of an IP address as the
+  target server, but this was never checked. Until now. The plugin raises an error
+  if the configured target server is not a valid IPv4 or IPv6 address.
 
 ### Changed