Merge pull request #3507 from certbot/only-suggest-valid-names

Only suggest names LE will accept

certbot-nginx/certbot_nginx/configurator.py

@@ -281,7 +281,25 @@ class NginxConfigurator(common.Plugin):
                     except (socket.error, socket.herror, socket.timeout):
                         continue
 
-        return all_names
+        return self._get_filtered_names(all_names)
+
+    def _get_filtered_names(self, all_names):
+        """Removes names that aren't considered valid by Let's Encrypt.
+
+        :param set all_names: all names found in the Nginx configuration
+
+        :returns: all found names that are considered valid by LE
+        :rtype: set
+
+        """
+        filtered_names = set()
+        for name in all_names:
+            try:
+                filtered_names.add(util.enforce_le_validity(name))
+            except errors.ConfigurationError as error:
+                logger.debug('Not suggesting name "%s"', name)
+                logger.debug(error)
+        return filtered_names
 
     def _get_snakeoil_paths(self):
         # TODO: generate only once

certbot-nginx/certbot_nginx/tests/configurator_test.py

@@ -66,10 +66,8 @@ class NginxConfiguratorTest(util.NginxTest):
         mock_gethostbyaddr.return_value = ('155.225.50.69.nephoscale.net', [], [])
         names = self.config.get_all_names()
         self.assertEqual(names, set(
-            ["*.www.foo.com", "somename", "another.alias",
+            ["155.225.50.69.nephoscale.net",
-             "alias", "localhost", ".example.com", r"~^(www\.)?(example|bar)\.",
+             "www.example.org", "another.alias"]))
-             "155.225.50.69.nephoscale.net", "*.www.example.com",
-             "example.*", "www.example.org", "myhost"]))
 
     def test_supported_enhancements(self):
         self.assertEqual(['redirect', 'staple-ocsp'],

certbot/tests/util_test.py

@@ -330,6 +330,34 @@ class AddDeprecatedArgumentTest(unittest.TestCase):
         self.assertTrue("--old-option" not in stdout.getvalue())
 
 
+class EnforceLeValidity(unittest.TestCase):
+    """Test enforce_le_validity."""
+    def _call(self, domain):
+        from certbot.util import enforce_le_validity
+        return enforce_le_validity(domain)
+
+    def test_sanity(self):
+        self.assertRaises(errors.ConfigurationError, self._call, u"..")
+
+    def test_invalid_chars(self):
+        self.assertRaises(
+            errors.ConfigurationError, self._call, u"hello_world.example.com")
+
+    def test_leading_hyphen(self):
+        self.assertRaises(
+            errors.ConfigurationError, self._call, u"-a.example.com")
+
+    def test_trailing_hyphen(self):
+        self.assertRaises(
+            errors.ConfigurationError, self._call, u"a-.example.com")
+
+    def test_one_label(self):
+        self.assertRaises(errors.ConfigurationError, self._call, u"com")
+
+    def test_valid_domain(self):
+        self.assertEqual(self._call(u"example.com"), u"example.com")
+
+
 class EnforceDomainSanityTest(unittest.TestCase):
     """Test enforce_domain_sanity."""
 

certbot/util.py

@@ -390,12 +390,45 @@ def add_deprecated_argument(add_argument, argument_name, nargs):
                  help=argparse.SUPPRESS, nargs=nargs)
 
 
+def enforce_le_validity(domain):
+    """Checks that Let's Encrypt will consider domain to be valid.
+
+    :param str domain: FQDN to check
+    :type domain: `str` or `unicode`
+    :returns: The domain cast to `str`, with ASCII-only contents
+    :rtype: str
+    :raises ConfigurationError: for invalid domains and cases where Let's
+                                Encrypt currently will not issue certificates
+
+    """
+    domain = enforce_domain_sanity(domain)
+    if not re.match("^[A-Za-z0-9.-]*$", domain):
+        raise errors.ConfigurationError(
+            "{0} contains an invalid character. "
+            "Valid characters are A-Z, a-z, 0-9, ., and -.".format(domain))
+
+    labels = domain.split(".")
+    if len(labels) < 2:
+        raise errors.ConfigurationError(
+            "{0} needs at least two labels".format(domain))
+    for label in labels:
+        if label.startswith("-"):
+            raise errors.ConfigurationError(
+                'label "{0}" in domain "{1}" cannot start with "-"'.format(
+                    label, domain))
+        if label.endswith("-"):
+            raise errors.ConfigurationError(
+                'label "{0}" in domain "{1}" cannot end with "-"'.format(
+                    label, domain))
+    return domain
+
+
 def enforce_domain_sanity(domain):
     """Method which validates domain value and errors out if
     the requirements are not met.
 
     :param domain: Domain to check
-    :type domains: `str` or `unicode`
+    :type domain: `str` or `unicode`
     :raises ConfigurationError: for invalid domains and cases where Let's
                                 Encrypt currently will not issue certificates