From 72ca219097b3ac13af154d8f9487dda4e22bd6a3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 16 Sep 2016 15:05:39 -0700 Subject: [PATCH 1/4] fix typo -- domains should be domain --- certbot/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/util.py b/certbot/util.py index e78ae664c..324c0b26a 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -395,7 +395,7 @@ def enforce_domain_sanity(domain): 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 From f2e0afc96c1fc1e40f44f983ce8274cb8dd311c4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 16 Sep 2016 16:08:51 -0700 Subject: [PATCH 2/4] add enforce_le_validity function --- certbot/tests/util_test.py | 25 +++++++++++++++++++++++++ certbot/util.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 4aa6d3ff3..47d764ed5 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -330,6 +330,31 @@ 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_valid_domain(self): + self.assertEqual(self._call(u"example.com"), u"example.com") + + class EnforceDomainSanityTest(unittest.TestCase): """Test enforce_domain_sanity.""" diff --git a/certbot/util.py b/certbot/util.py index 324c0b26a..0b1431d98 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -390,6 +390,34 @@ 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)) + for label in domain.split("."): + 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. From 275e3f748e610740e99c12b503110b6cbbd3f666 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 16 Sep 2016 16:47:02 -0700 Subject: [PATCH 3/4] filter names returned by get_all_names --- certbot-nginx/certbot_nginx/configurator.py | 20 ++++++++++++++++++- .../certbot_nginx/tests/configurator_test.py | 6 ++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 444e48903..a89e276c5 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -308,7 +308,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 diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 9e0c0dda5..84f5e2e3b 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/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", - "alias", "localhost", ".example.com", r"~^(www\.)?(example|bar)\.", - "155.225.50.69.nephoscale.net", "*.www.example.com", - "example.*", "www.example.org", "myhost"])) + ["somename", "another.alias", "alias", "localhost", + "155.225.50.69.nephoscale.net", "www.example.org", "myhost"])) def test_supported_enhancements(self): self.assertEqual(['redirect'], self.config.supported_enhancements()) From 307b2e5307002070ec33e3efabbc26775b3a8973 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 16 Sep 2016 16:53:25 -0700 Subject: [PATCH 4/4] Reject domains with only one label --- certbot-nginx/certbot_nginx/tests/configurator_test.py | 4 ++-- certbot/tests/util_test.py | 3 +++ certbot/util.py | 7 ++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 84f5e2e3b..d3bf52a2e 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -66,8 +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( - ["somename", "another.alias", "alias", "localhost", - "155.225.50.69.nephoscale.net", "www.example.org", "myhost"])) + ["155.225.50.69.nephoscale.net", + "www.example.org", "another.alias"])) def test_supported_enhancements(self): self.assertEqual(['redirect'], self.config.supported_enhancements()) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 47d764ed5..6f06c8306 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -351,6 +351,9 @@ class EnforceLeValidity(unittest.TestCase): 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") diff --git a/certbot/util.py b/certbot/util.py index 0b1431d98..5cb4b4cad 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -406,7 +406,12 @@ def enforce_le_validity(domain): raise errors.ConfigurationError( "{0} contains an invalid character. " "Valid characters are A-Z, a-z, 0-9, ., and -.".format(domain)) - for label in domain.split("."): + + 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(