From 2d5d4a65c45ea379f847bcd9effd9c05a4b50556 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 15:07:27 +0200 Subject: [PATCH 01/13] Moved domain check to le_util --- letsencrypt/configuration.py | 40 +++--------------------------------- letsencrypt/le_util.py | 34 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 69778f5f0..6de529981 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -8,6 +8,7 @@ import zope.interface from letsencrypt import constants from letsencrypt import errors from letsencrypt import interfaces +from letsencrypt import le_util class NamespaceConfig(object): @@ -123,40 +124,5 @@ def check_config_sanity(config): # Domain checks if config.namespace.domains is not None: - _check_config_domain_sanity(config.namespace.domains) - - -def _check_config_domain_sanity(domains): - """Helper method for check_config_sanity which validates - domain flag values and errors out if the requirements are not met. - - :param domains: List of domains - :type domains: `list` of `string` - :raises ConfigurationError: for invalid domains and cases where Let's - Encrypt currently will not issue certificates - - """ - # Check if there's a wildcard domain - if any(d.startswith("*.") for d in domains): - raise errors.ConfigurationError( - "Wildcard domains are not supported") - # Punycode - if any("xn--" in d for d in domains): - raise errors.ConfigurationError( - "Punycode domains are not supported") - - # Unicode - try: - for domain in domains: - domain.encode('ascii') - except UnicodeDecodeError: - raise errors.ConfigurationError( - "Internationalized domain names are not supported") - - # FQDN checks from - # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ - # Characters used, domain parts < 63 chars, tld > 1 < 64 chars - # first and last char is not "-" - fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? 1 < 64 chars + # first and last char is not "-" + fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Mon, 7 Dec 2015 15:37:09 +0200 Subject: [PATCH 02/13] Added domain checks for apache installer --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 98b0b8820..50e5ed6be 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -369,7 +369,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost_macro = [] for vhost in self.vhosts: - all_names.update(vhost.get_names()) + # Check domains for validity + for name in vhost.get_names(): + try: + le_util.check_domain_sanity(name) + all_names.add(name) + except errors.ConfigurationError: + pass if vhost.modmacro: vhost_macro.append(vhost.filep) From 82f71cba9ba0df6f639c996a64dcd4e5122a39a6 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 7 Dec 2015 16:02:27 +0200 Subject: [PATCH 03/13] Linter fixes --- letsencrypt/configuration.py | 1 - letsencrypt/le_util.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 6de529981..afd5edbe4 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -1,7 +1,6 @@ """Let's Encrypt user-supplied configuration.""" import os import urlparse -import re import zope.interface diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 97f983ea2..e5e252871 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -313,4 +313,4 @@ def check_domain_sanity(domain): # first and last char is not "-" fqdn = re.compile("^((?!-)[A-Za-z0-9-]{1,63}(? Date: Mon, 7 Dec 2015 16:05:53 +0200 Subject: [PATCH 04/13] Corrected tests to reflect the removal of wildcard domains etc. --- .../letsencrypt_apache/tests/configurator_test.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index fcccfaae2..986b060f5 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -64,7 +64,7 @@ class TwoVhost80Test(util.ApacheTest): mock_getutility.notification = mock.MagicMock(return_value=True) names = self.config.get_all_names() self.assertEqual(names, set( - ["letsencrypt.demo", "encryption-example.demo", "ip-172-30-0-17"])) + ["letsencrypt.demo", "encryption-example.demo"])) @mock.patch("zope.component.getUtility") @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") @@ -82,7 +82,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.vhosts.append(vhost) names = self.config.get_all_names() - self.assertEqual(len(names), 5) + self.assertEqual(len(names), 4) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("letsencrypt.demo" in names) @@ -90,10 +90,17 @@ class TwoVhost80Test(util.ApacheTest): def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) + self.config.parser.add_dir( + self.vh_truth[0].path, "ServerAlias", ["working.example.com"]) + self.config._add_servernames(self.vh_truth[2]) # pylint: disable=protected-access + self.config._add_servernames(self.vh_truth[0]) # pylint: disable=protected-access self.assertEqual( self.vh_truth[2].get_names(), set(["*.le.co", "ip-172-30-0-17"])) + self.assertEqual( + self.vh_truth[0].get_names(), set(["working.example.com", + "encryption-example.demo"])) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found. From 51a5d7ceb085ac03593adafc63bfb6357d96eb34 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:05:11 +0200 Subject: [PATCH 05/13] Move validation code to main client --- letsencrypt/display/ops.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index 038ad6fdc..ca9c8c126 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -186,7 +186,8 @@ def choose_names(installer): logger.debug("No installer, picking names manually") return _choose_names_manually() - names = list(installer.get_all_names()) + domains = list(installer.get_all_names()) + names = get_valid_domains(domains) if not names: manual = util(interfaces.IDisplay).yesno( @@ -207,6 +208,22 @@ def choose_names(installer): else: return [] +def get_valid_domains(self, domains): + """Helper method for choose_names that implements basic checks + on domain names + + :param list domains: Domain names to validate + :return: List of valid domains + :rtype: list + """ + valid_domains = [] + for domain in domains: + try: + le_util.check_domain_sanity(domain) + valid_domains.append(domain) + except errors.ConfigurationError: + continue + return valid_domains def _filter_names(names): """Determine which names the user would like to select from a list. From d8b83bc478ac3ff875a604ce315c16c593e36170 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:05:33 +0200 Subject: [PATCH 06/13] Revert "Corrected tests to reflect the removal of wildcard domains etc." This reverts commit 53a4d0725dbf0c5f728094dc318f491c9478effd. --- .../letsencrypt_apache/tests/configurator_test.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 986b060f5..fcccfaae2 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -64,7 +64,7 @@ class TwoVhost80Test(util.ApacheTest): mock_getutility.notification = mock.MagicMock(return_value=True) names = self.config.get_all_names() self.assertEqual(names, set( - ["letsencrypt.demo", "encryption-example.demo"])) + ["letsencrypt.demo", "encryption-example.demo", "ip-172-30-0-17"])) @mock.patch("zope.component.getUtility") @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") @@ -82,7 +82,7 @@ class TwoVhost80Test(util.ApacheTest): self.config.vhosts.append(vhost) names = self.config.get_all_names() - self.assertEqual(len(names), 4) + self.assertEqual(len(names), 5) self.assertTrue("zombo.com" in names) self.assertTrue("google.com" in names) self.assertTrue("letsencrypt.demo" in names) @@ -90,17 +90,10 @@ class TwoVhost80Test(util.ApacheTest): def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) - self.config.parser.add_dir( - self.vh_truth[0].path, "ServerAlias", ["working.example.com"]) - self.config._add_servernames(self.vh_truth[2]) # pylint: disable=protected-access - self.config._add_servernames(self.vh_truth[0]) # pylint: disable=protected-access self.assertEqual( self.vh_truth[2].get_names(), set(["*.le.co", "ip-172-30-0-17"])) - self.assertEqual( - self.vh_truth[0].get_names(), set(["working.example.com", - "encryption-example.demo"])) def test_get_virtual_hosts(self): """Make sure all vhosts are being properly found. From e891624cb1159c498167fa76c8225c3176fcf4b4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:07:17 +0200 Subject: [PATCH 07/13] Revert "Added domain checks for apache installer" This reverts commit 5dcd5088273dcf4dcb402bcd8a69a655d29fa383. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 50e5ed6be..98b0b8820 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -369,13 +369,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost_macro = [] for vhost in self.vhosts: - # Check domains for validity - for name in vhost.get_names(): - try: - le_util.check_domain_sanity(name) - all_names.add(name) - except errors.ConfigurationError: - pass + all_names.update(vhost.get_names()) if vhost.modmacro: vhost_macro.append(vhost.filep) From 3c1c3c3e8dbf6b0ae7e50032c76a25a4fba7f992 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:31:47 +0200 Subject: [PATCH 08/13] Import and non-classmethod fix --- letsencrypt/display/ops.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index ca9c8c126..941a0a114 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -4,6 +4,7 @@ import os import zope.component +from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt.display import util as display_util @@ -208,7 +209,7 @@ def choose_names(installer): else: return [] -def get_valid_domains(self, domains): +def get_valid_domains(domains): """Helper method for choose_names that implements basic checks on domain names From 0fb4f7dc8bde20afc6746b14438aa65c356eff07 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:32:02 +0200 Subject: [PATCH 09/13] Tests for domain validation --- letsencrypt/tests/display/ops_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index b0b905c33..60874a007 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -385,7 +385,17 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(self._call(self.mock_install), []) + def test_get_valid_domains(self): + from letsencrypt.display_ops import get_valid_domains + all_valid = ["example.com", "second.example.com", + "also.example.com"] + all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] + two_valid = ["example.com", "xn--ls8h.tld", "also.example.com"] + self.assertEqual(get_valid_domains(all_valid), all_valid) + self.assertEqual(get_valid_domains(all_invalid), []) + self.assertEqual(len(get_valid_domains(two_valid)), 2) + class SuccessInstallationTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Test the success installation message.""" From 5c8b493eda2e5fa707ed6299d2ee932e7921a467 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 09:54:56 +0200 Subject: [PATCH 10/13] Whitespace and dot fix --- letsencrypt/tests/display/ops_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 60874a007..5958b37a1 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -386,7 +386,7 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(self._call(self.mock_install), []) def test_get_valid_domains(self): - from letsencrypt.display_ops import get_valid_domains + from letsencrypt.display.ops import get_valid_domains all_valid = ["example.com", "second.example.com", "also.example.com"] all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] @@ -395,7 +395,7 @@ class ChooseNamesTest(unittest.TestCase): self.assertEqual(get_valid_domains(all_invalid), []) self.assertEqual(len(get_valid_domains(two_valid)), 2) - + class SuccessInstallationTest(unittest.TestCase): # pylint: disable=too-few-public-methods """Test the success installation message.""" From 0cb80bf7534660d3df30bc12d4b8350e4468dd24 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 14:23:46 +0200 Subject: [PATCH 11/13] Better test coverage --- letsencrypt/tests/display/ops_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 5958b37a1..30183b955 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -1,3 +1,4 @@ +# coding=utf-8 """Test letsencrypt.display.ops.""" import os import sys @@ -389,7 +390,8 @@ class ChooseNamesTest(unittest.TestCase): from letsencrypt.display.ops import get_valid_domains all_valid = ["example.com", "second.example.com", "also.example.com"] - all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN"] + all_invalid = ["xn--ls8h.tld", "*.wildcard.com", "notFQDN", + "uniçodé.com"] two_valid = ["example.com", "xn--ls8h.tld", "also.example.com"] self.assertEqual(get_valid_domains(all_valid), all_valid) self.assertEqual(get_valid_domains(all_invalid), []) From 006edfdbd1953183d09d8624aeb13bba8e07babe Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 21:08:30 +0200 Subject: [PATCH 12/13] Wording change to error messages --- letsencrypt/le_util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index e5e252871..7c7f0b7f7 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -294,18 +294,18 @@ def check_domain_sanity(domain): # Check if there's a wildcard domain if domain.startswith("*."): raise errors.ConfigurationError( - "Wildcard domains are not supported") + "Wildcard domains are not presently supported") # Punycode if "xn--" in domain: raise errors.ConfigurationError( - "Punycode domains are not supported") + "Punycode domains are not presently supported") # Unicode try: domain.encode('ascii') except UnicodeDecodeError: raise errors.ConfigurationError( - "Internationalized domain names are not supported") + "Internationalized domain names are not presently supported") # FQDN checks from # http://www.mkyong.com/regular-expressions/domain-name-regular-expression-example/ From 05723cbb4afb275ffd27e090b7801ea6d42ef5cf Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 8 Dec 2015 21:28:23 +0200 Subject: [PATCH 13/13] Fixed wording --- letsencrypt/le_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 7c7f0b7f7..fe63c70af 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -294,7 +294,7 @@ def check_domain_sanity(domain): # Check if there's a wildcard domain if domain.startswith("*."): raise errors.ConfigurationError( - "Wildcard domains are not presently supported") + "Wildcard domains are not supported") # Punycode if "xn--" in domain: raise errors.ConfigurationError(