From 1be8dbfbce41dbc29ed0c5bc88595c0e7277d639 Mon Sep 17 00:00:00 2001
From: Thomas Quinot <thomas@quinot.org>
Date: Sat, 24 Feb 2018 13:50:25 +0100
Subject: [PATCH] Pass only validation domain name to plugin

Previously, the requested certificate domain and the corresponding
validation domain name (by default _acme-challenge.<domain>) were
passed to DNS plugins. However, if a CNAME is installed at the standard
validation location, and the validation domain name is overridden
accordingly, plugins must disregard the original domain, and install
the challenge TXT record at the overridden domain. To avoid any
confusion, pass the validation domain name (not the original domain)
as first argument to _perform and _cleanup.
---
 certbot/plugins/dns_common.py      | 26 ++++++++++++++++++++------
 certbot/plugins/dns_common_test.py | 10 ++++++++--
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/certbot/plugins/dns_common.py b/certbot/plugins/dns_common.py
index 580de8dfe..6e71bf6db 100644
--- a/certbot/plugins/dns_common.py
+++ b/certbot/plugins/dns_common.py
@@ -130,11 +130,15 @@ class DNSAuthenticator(common.Plugin):
 
         responses = []
         for achall in achalls:
-            domain = achall.domain
             validation_domain_name = self.validation_domain_name(achall)
             validation = achall.validation(achall.account_key)
 
-            self._perform(domain, validation_domain_name, validation)
+            # Note: achall.domain used to be passed as the first parameter to _perform.
+            # However, validation_domain_name may be overridden and be unrelated to the
+            # original domain (with _acme-challenge.<domain> being a CNAME). We now pass
+            # validation_domain_name for both parameters to avoid confusion in plugins.
+
+            self._perform(validation_domain_name, validation_domain_name, validation)
             responses.append(achall.response(achall.account_key))
 
         # DNS updates take time to propagate and checking to see if the update has occurred is not
@@ -149,11 +153,13 @@ class DNSAuthenticator(common.Plugin):
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
         if self._attempt_cleanup:
             for achall in achalls:
-                domain = achall.domain
                 validation_domain_name = self.validation_domain_name(achall)
                 validation = achall.validation(achall.account_key)
 
-                self._cleanup(domain, validation_domain_name, validation)
+                # Note: achall.domain used to be passed as the first parameter to _cleanup,
+                # see discussion in perform above.
+
+                self._cleanup(validation_domain_name, validation_domain_name, validation)
 
     @abc.abstractmethod
     def _setup_credentials(self):  # pragma: no cover
@@ -167,7 +173,11 @@ class DNSAuthenticator(common.Plugin):
         """
         Performs a dns-01 challenge by creating a DNS TXT record.
 
-        :param str domain: The domain being validated.
+        Note: the two parameters domain and validation_domain_name always
+        have the same value. "domain" is retained for backwards compatibility,
+        but should be ignored; plugins should only reference validation_domain_name.
+
+        :param str domain: The validation record domain name.
         :param str validation_domain_name: The validation record domain name.
         :param str validation: The validation record content.
         :raises errors.PluginError: If the challenge cannot be performed
@@ -181,7 +191,11 @@ class DNSAuthenticator(common.Plugin):
 
         Fails gracefully if no such record exists.
 
-        :param str domain: The domain being validated.
+        Note: the two parameters domain and validation_domain_name always
+        have the same value. "domain" is retained for backwards compatibility,
+        but should be ignored; plugins should only reference validation_domain_name.
+
+        :param str domain: The validation record domain name.
         :param str validation_domain_name: The validation record domain name.
         :param str validation: The validation record content.
         """
diff --git a/certbot/plugins/dns_common_test.py b/certbot/plugins/dns_common_test.py
index 832722bc4..15f5aa3b3 100644
--- a/certbot/plugins/dns_common_test.py
+++ b/certbot/plugins/dns_common_test.py
@@ -45,14 +45,20 @@ class DNSAuthenticatorTest(util.TempDirTestCase, dns_test_common.BaseAuthenticat
     def test_perform(self):
         self.auth.perform([self.achall])
 
-        self.auth._perform.assert_called_once_with(dns_test_common.DOMAIN, mock.ANY, mock.ANY)
+        self.auth._perform.assert_called_once_with(
+            "_acme-challenge." + dns_test_common.DOMAIN,
+            "_acme-challenge." + dns_test_common.DOMAIN,
+            mock.ANY)
 
     def test_cleanup(self):
         self.auth._attempt_cleanup = True
 
         self.auth.cleanup([self.achall])
 
-        self.auth._cleanup.assert_called_once_with(dns_test_common.DOMAIN, mock.ANY, mock.ANY)
+        self.auth._cleanup.assert_called_once_with(
+            "_acme-challenge." + dns_test_common.DOMAIN,
+            "_acme-challenge." + dns_test_common.DOMAIN,
+            mock.ANY)
 
     def test_validation_domain_name(self):
         # Validation domain name without override