From 55d4365a46cef56228851ac9f7e6c47846eaff88 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 21:30:05 -0800 Subject: [PATCH 01/35] Expect a fixed standalone challenge preference --- letsencrypt/plugins/standalone_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 26a040c2e..91b7b56a0 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -104,8 +104,8 @@ class AuthenticatorTest(unittest.TestCase): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) def test_get_chall_pref(self): - self.assertEqual(set(self.auth.get_chall_pref(domain=None)), - set([challenges.TLSSNI01, challenges.HTTP01])) + self.assertEqual(self.auth.get_chall_pref(domain=None), + [challenges.TLSSNI01, challenges.HTTP01]) @mock.patch("letsencrypt.plugins.standalone.util") def test_perform_alredy_listening(self, mock_util): From 5054a3dd79b752b9839ba85f120dc43d452078d1 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 21:57:31 -0800 Subject: [PATCH 02/35] Fix typo in test name --- letsencrypt/plugins/standalone_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 91b7b56a0..b4aac76b2 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -108,7 +108,7 @@ class AuthenticatorTest(unittest.TestCase): [challenges.TLSSNI01, challenges.HTTP01]) @mock.patch("letsencrypt.plugins.standalone.util") - def test_perform_alredy_listening(self, mock_util): + def test_perform_already_listening(self, mock_util): for chall, port in ((challenges.TLSSNI01.typ, 1234), (challenges.HTTP01.typ, 4321)): mock_util.already_listening.return_value = True From 144a678473fd3d2634092adcb72b411c1a3274fb Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:01:55 -0800 Subject: [PATCH 03/35] Encode challenge preference order in constant --- letsencrypt/plugins/standalone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 8b8612fd1..9efb5d301 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -108,7 +108,7 @@ class ServerManager(object): in six.iteritems(self._instances)) -SUPPORTED_CHALLENGES = set([challenges.TLSSNI01, challenges.HTTP01]) +SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] def supported_challenges_validator(data): From a0142dbe44e2e8f1530fa35361b044232ec91b2c Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:13:05 -0800 Subject: [PATCH 04/35] Don't randomize challenge preference --- letsencrypt/plugins/standalone.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 9efb5d301..496ea7ded 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -198,9 +198,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - chall_pref = list(self.supported_challenges) - random.shuffle(chall_pref) # 50% for each challenge - return chall_pref + return SUPPORTED_CHALLENGES def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From fa558715983956eefec8ed41fab3208a390afea4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:14:32 -0800 Subject: [PATCH 05/35] Remove dead import --- letsencrypt/plugins/standalone.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 496ea7ded..dc52cf5e7 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -2,7 +2,6 @@ import argparse import collections import logging -import random import socket import threading From d5511971aa4ea64d67c7181aa16720fce51481c4 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Wed, 2 Dec 2015 22:50:32 -0800 Subject: [PATCH 06/35] Update plugin help string --- letsencrypt/plugins/standalone.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index dc52cf5e7..3cd8cd95f 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -165,10 +165,10 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): - add("supported-challenges", help="Supported challenges, " - "order preferences are randomly chosen.", - type=supported_challenges_validator, default=",".join( - sorted(chall.typ for chall in SUPPORTED_CHALLENGES))) + add("supported-challenges", + help="Supported challenges. Prefers tls-sni-01.", + type=supported_challenges_validator, + default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) @property def supported_challenges(self): From 8cf47e3aba7dba8090ef8c18c325ff5177dc42ed Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 3 Dec 2015 00:13:09 -0800 Subject: [PATCH 07/35] Add tests to check that configuration is used The existing tests use the case in which the (configured) supported challenges are equal to the defaults, and in the same (now-fixed) order. These additional tests check that, if we have configured a subset of the supported challenges, then we actually _use_ that configuration. --- letsencrypt/plugins/standalone_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index b4aac76b2..1833a55fe 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -100,6 +100,11 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.supported_challenges, set([challenges.TLSSNI01, challenges.HTTP01])) + def test_supported_challenges_configured(self): + self.config.standalone_supported_challenges = "tls-sni-01" + self.assertEqual(self.auth.supported_challenges, + set([challenges.TLSSNI01])) + def test_more_info(self): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) @@ -107,6 +112,11 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.get_chall_pref(domain=None), [challenges.TLSSNI01, challenges.HTTP01]) + def test_get_chall_pref_configured(self): + self.config.standalone_supported_challenges = "tls-sni-01" + self.assertEqual(self.auth.get_chall_pref(domain=None), + [challenges.TLSSNI01]) + @mock.patch("letsencrypt.plugins.standalone.util") def test_perform_already_listening(self, mock_util): for chall, port in ((challenges.TLSSNI01.typ, 1234), From dbf181ebacd9edf00196dbcfc85fdf2794e2f3c9 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Thu, 3 Dec 2015 00:28:12 -0800 Subject: [PATCH 08/35] Respect config when stating challenge preferences --- letsencrypt/plugins/standalone.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 3cd8cd95f..1bca3c036 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -197,7 +197,8 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return SUPPORTED_CHALLENGES + return [chall for chall in SUPPORTED_CHALLENGES + if chall in self.supported_challenges] def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From b9b634b6029012d999ade0e5ed4d8a20eaeb3460 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:42:08 -0800 Subject: [PATCH 09/35] Merge another augeas lens fix From: https://github.com/hercules-team/augeas/pull/329 Fixes: https://github.com/letsencrypt/letsencrypt/issues/1294#issuecomment-161805063 --- letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 30d8ca501..6d15486f8 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -51,7 +51,7 @@ let sep_osp = Sep.opt_space let sep_eq = del /[ \t]*=[ \t]*/ "=" let nmtoken = /[a-zA-Z:_][a-zA-Z0-9:_.-]*/ -let word = /[a-zA-Z][a-zA-Z0-9._-]*/ +let word = /[a-z][a-z0-9._-]*/i let comment = Util.comment let eol = Util.doseol From 5ccfb7c37bc6683a1da0c80fef6261cf96f3013b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:50:38 -0800 Subject: [PATCH 10/35] Add another failing case --- .../failing/missing-double-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/failing/missing-double-quote-1724.conf diff --git a/tests/apache-conf-files/failing/missing-double-quote-1724.conf b/tests/apache-conf-files/failing/missing-double-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/failing/missing-double-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From edf3a4ed732e1123cf257bd9c999c7ca6e468fb4 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Mon, 7 Dec 2015 10:49:24 +0100 Subject: [PATCH 11/35] Make webroot usable also when running as non-root (GH #1795) Thanks to @aburch's suggestions, the logic has been changed: - Set umask before creating folders and files - Leverage os.makedirs' mode option in conjunction with umask The program still tries to change owner / group, but in case of errors it continues gracefully. Tests have been updated, and they pass. --- letsencrypt/plugins/webroot.py | 51 +++++++++++++++++++---------- letsencrypt/plugins/webroot_test.py | 13 +++++--- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 0b81d45b5..392d1fc2c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -60,23 +60,38 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) try: - os.makedirs(self.full_roots[name]) - # Set permissions as parent directory (GH #1389) - # We don't use the parameters in makedirs because it - # may not always work + # Change the permissiosn to be writable (GH #1389) + # We also set umask because os.makedirs's mode parameter does + # not always work: # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python + # We set the umask instead of going the chmod route to ensure the client + # can also run as non-root (GH #1795) + stat_path = os.stat(path) - filemode = stat.S_IMODE(stat_path.st_mode) - os.chmod(self.full_roots[name], filemode) - # Set owner and group, too - os.chown(self.full_roots[name], stat_path.st_uid, - stat_path.st_gid) + old_umask = os.umask(0o022) + os.makedirs(self.full_roots[name], 0o0755) + + # Set owner as parent directory if possible + + try: + stat_path = os.stat(path) + os.chown(self.full_roots[name], stat_path.st_uid, + stat_path.st_gid) + except OSError as exception: + if exception.errno == errno.EACCES: + logger.debug("Insufficient permissions to change owner and uid - ignoring") + else: + raise errors.PluginError( + "Couldn't create root for {0} http-01 " + "challenge responses: {1}", name, exception) except OSError as exception: if exception.errno != errno.EEXIST: raise errors.PluginError( "Couldn't create root for {0} http-01 " "challenge responses: {1}", name, exception) + finally: + os.umask(old_umask) def perform(self, achalls): # pylint: disable=missing-docstring assert self.full_roots, "Webroot plugin appears to be missing webroot map" @@ -95,18 +110,18 @@ to serve all files under specified web root ({0}).""" def _perform_single(self, achall): response, validation = achall.response_and_validation() + path = self._path_for_achall(achall) logger.debug("Attempting to save validation to %s", path) - with open(path, "w") as validation_file: - validation_file.write(validation.encode()) - # Set permissions as parent directory (GH #1389) - parent_path = self.full_roots[achall.domain] - stat_parent_path = os.stat(parent_path) - filemode = stat.S_IMODE(stat_parent_path.st_mode) - # Remove execution bit (not needed for this file) - os.chmod(path, filemode & ~stat.S_IEXEC) - os.chown(path, stat_parent_path.st_uid, stat_parent_path.st_gid) + old_umask = os.umask(0o022) + + try: + with open(path, "w") as validation_file: + # Change permissions to be world-readable, owner-writable (GH #1795) + validation_file.write(validation.encode()) + finally: + os.umask(old_umask) return response diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index e7f96b50d..2e88c1756 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -75,12 +75,17 @@ class AuthenticatorTest(unittest.TestCase): # Remove exec bit from permission check, so that it # matches the file self.auth.perform([self.achall]) - parent_permissions = (stat.S_IMODE(os.stat(self.path).st_mode) & - ~stat.S_IEXEC) + path_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode) + self.assertEqual(path_permissions, 0o644) - actual_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode) + # Check permissions of the directories + + for dirpath, dirnames, filenames in os.walk(self.path): + for directory in dirnames: + full_path = os.path.join(dirpath, directory) + dir_permissions = stat.S_IMODE(os.stat(full_path).st_mode) + self.assertEqual(dir_permissions, 0o755) - self.assertEqual(parent_permissions, actual_permissions) parent_gid = os.stat(self.path).st_gid parent_uid = os.stat(self.path).st_uid From 2b942d97b27eab9ef8f5fee263a5dbbd95b432f8 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Mon, 7 Dec 2015 11:17:29 +0100 Subject: [PATCH 12/35] Address review comments - move the umask call before the try/except block - move comment in _prepare_single to the umask call Simplify the code comments, too. Tests still pass. --- letsencrypt/plugins/webroot.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 392d1fc2c..c4072c3f9 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -59,16 +59,19 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) + + # Change the permissiosn to be writable (GH #1389) + # Umask is used instead of chmod to ensure the client can also + # run as non-root (GH #1795) + old_umask = os.umask(0o022) + try: - # Change the permissiosn to be writable (GH #1389) - # We also set umask because os.makedirs's mode parameter does - # not always work: - # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python - # We set the umask instead of going the chmod route to ensure the client - # can also run as non-root (GH #1795) stat_path = os.stat(path) - old_umask = os.umask(0o022) + # This is coupled with the "umask" call above because + # os.makedirs's "mode" parameter may not always work: + # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python + os.makedirs(self.full_roots[name], 0o0755) # Set owner as parent directory if possible @@ -114,11 +117,11 @@ to serve all files under specified web root ({0}).""" path = self._path_for_achall(achall) logger.debug("Attempting to save validation to %s", path) + # Change permissions to be world-readable, owner-writable (GH #1795) old_umask = os.umask(0o022) try: with open(path, "w") as validation_file: - # Change permissions to be world-readable, owner-writable (GH #1795) validation_file.write(validation.encode()) finally: os.umask(old_umask) From fe6e9be6a2d3c0090cdd91c7a19825c980deb1cb Mon Sep 17 00:00:00 2001 From: Alcaro Date: Wed, 9 Dec 2015 22:04:29 +0100 Subject: [PATCH 13/35] Update reference to deprecated directives https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile > SSLCertificateChainFile became obsolete with version 2.4.8, when SSLCertificateFile was extended to also load intermediate CA certificates from the server certificate file. --- docs/using.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 51426183d..687901191 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -224,21 +224,23 @@ The following files are available: ``cert.pem`` Server certificate only. - This is what Apache needs for `SSLCertificateFile + This is what Apache < 2.4.8 needs for `SSLCertificateFile `_. ``chain.pem`` All certificates that need to be served by the browser **excluding** server certificate, i.e. root and intermediate certificates only. - This is what Apache needs for `SSLCertificateChainFile + This is what Apache < 2.4.8 needs for `SSLCertificateChainFile `_. ``fullchain.pem`` All certificates, **including** server certificate. This is concatenation of ``chain.pem`` and ``cert.pem``. - This is what nginx needs for `ssl_certificate + This is what Apache >= 2.4.8 needs for `SSLCertificateFile + `_, + and what nginx needs for `ssl_certificate `_. From 45d20847e64c80e81f14c5f239ec57c095ad7202 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 9 Dec 2015 18:21:27 -0800 Subject: [PATCH 14/35] Implement --staging - Fixes #1667 - Should help with rate limit problems --- letsencrypt/cli.py | 13 +++++++++++++ letsencrypt/tests/cli_test.py | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 348818368..f57b1eb0b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -696,6 +696,15 @@ class HelpfulArgumentParser(object): parsed_args = self.parser.parse_args(self.args) parsed_args.func = self.VERBS[self.verb] + # Do any post-parsing homework here + + # argparse seemingly isn't flexible enough to give us this behaviour easily... + staging_uri = 'https://acme-staging.api.letsencrypt.org/directory' + if parsed_args.staging: + if parsed_args.server not in (flag_default("server"), staging_uri): + raise errors.Error("--server value conflicts with --staging") + parsed_args.server = staging_uri + return parsed_args @@ -1037,6 +1046,10 @@ def _paths_parser(helpful): help="Logs directory.") add("paths", "--server", default=flag_default("server"), help=config_help("server")) + # overwrites server, handled in HelpfulArgumentParser.parse_args() + add("testing", "--staging", action='store_true', + help='Use the staging server to obtain test (invalid) certs; equivalent' + ' to --server https://acme-staging.api.letsencrypt.org/directory ') def _plugins_parsing(helpful, plugins): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 462d37a87..60a8e9440 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -343,6 +343,20 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = cli.prepare_and_parse_args(plugins, long_args) self.assertEqual(namespace.domains, ['example.com', 'another.net']) + def test_parse_server(self): + plugins = disco.PluginsRegistry.find_all() + short_args = ['--server', 'example.com'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.server, 'example.com') + + short_args = ['--staging'] + namespace = cli.prepare_and_parse_args(plugins, short_args) + self.assertEqual(namespace.server, + 'https://acme-staging.api.letsencrypt.org/directory') + + short_args = ['--staging', '--server', 'example.com'] + self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) + def test_parse_webroot(self): plugins = disco.PluginsRegistry.find_all() webroot_args = ['--webroot', '-w', '/var/www/example', From 5804d033363d0005134c2298c5c6da1013ad1e3f Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Thu, 10 Dec 2015 14:36:25 +0000 Subject: [PATCH 15/35] Add failing test from ticket #1724 Augeas fails to parse the ErrorDocument argument that starts with a double quote and ends with an EOL. --- .../drupal-errordocument-arg-1724.conf | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf diff --git a/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf b/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf new file mode 100644 index 000000000..4733ffa4a --- /dev/null +++ b/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf @@ -0,0 +1,116 @@ +# +# Apache/PHP/Drupal settings: +# + +# Protect files and directories from prying eyes. + + Order allow,deny + + +# Don't show directory listings for URLs which map to a directory. +Options -Indexes + +# Follow symbolic links in this directory. +Options +FollowSymLinks + +# Make Drupal handle any 404 errors. +ErrorDocument 404 /index.php + +# Force simple error message for requests for non-existent favicon.ico. + + # There is no end quote below, for compatibility with Apache 1.3. + ErrorDocument 404 "The requested file favicon.ico was not found. + + +# Set the default handler. +DirectoryIndex index.php + +# Override PHP settings. More in sites/default/settings.php +# but the following cannot be changed at runtime. + +# PHP 4, Apache 1. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# PHP 4, Apache 2. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# PHP 5, Apache 1 and 2. + + php_value magic_quotes_gpc 0 + php_value register_globals 0 + php_value session.auto_start 0 + php_value mbstring.http_input pass + php_value mbstring.http_output pass + php_value mbstring.encoding_translation 0 + + +# Requires mod_expires to be enabled. + + # Enable expirations. + ExpiresActive On + + # Cache all files for 2 weeks after access (A). + ExpiresDefault A1209600 + + + # Do not allow PHP scripts to be cached unless they explicitly send cache + # headers themselves. Otherwise all scripts would have to overwrite the + # headers set by mod_expires if they want another caching behavior. This may + # fail if an error occurs early in the bootstrap process, and it may cause + # problems if a non-Drupal PHP file is installed in a subdirectory. + ExpiresActive Off + + + +# Various rewrite rules. + + RewriteEngine on + + # If your site can be accessed both with and without the 'www.' prefix, you + # can use one of the following settings to redirect users to your preferred + # URL, either WITH or WITHOUT the 'www.' prefix. Choose ONLY one option: + # + # To redirect all users to access the site WITH the 'www.' prefix, + # (http://example.com/... will be redirected to http://www.example.com/...) + # adapt and uncomment the following: + # RewriteCond %{HTTP_HOST} ^example\.com$ [NC] + # RewriteRule ^(.*)$ http://www.example.com/$1 [L,R=301] + # + # To redirect all users to access the site WITHOUT the 'www.' prefix, + # (http://www.example.com/... will be redirected to http://example.com/...) + # uncomment and adapt the following: + # RewriteCond %{HTTP_HOST} ^www\.example\.com$ [NC] + # RewriteRule ^(.*)$ http://example.com/$1 [L,R=301] + + # Modify the RewriteBase if you are using Drupal in a subdirectory or in a + # VirtualDocumentRoot and the rewrite rules are not working properly. + # For example if your site is at http://example.com/drupal uncomment and + # modify the following line: + # RewriteBase /drupal + # + # If your site is running in a VirtualDocumentRoot at http://example.com/, + # uncomment the following line: + # RewriteBase / + + # Rewrite URLs of the form 'x' to the form 'index.php?q=x'. + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteCond %{REQUEST_URI} !=/favicon.ico + RewriteRule ^(.*)$ index.php?q=$1 [L,QSA] + + +# $Id$ From 6b4031331192c86910714b38ff49454602ff4784 Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Thu, 10 Dec 2015 14:37:58 +0000 Subject: [PATCH 16/35] Merge Augeas lens fix for partially quoted arguments From https://github.com/hercules-team/augeas/commit/50fb756580477e9195946133ec2f0d1f0f6786c7 Closes: #1724 --- .../letsencrypt_apache/augeas_lens/httpd.aug | 11 +++++++++-- .../drupal-errordocument-arg-1724.conf | 0 2 files changed, 9 insertions(+), 2 deletions(-) rename tests/apache-conf-files/{failing => passing}/drupal-errordocument-arg-1724.conf (100%) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug index 0669896a0..732ba5468 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug +++ b/letsencrypt-apache/letsencrypt_apache/augeas_lens/httpd.aug @@ -68,6 +68,9 @@ let cl = /\\\\\n/ let dquot = let no_dquot = /[^"\\\r\n]/ in /"/ . (no_dquot|cdot|cl)* . /"/ +let dquot_msg = + let no_dquot = /([^ \t"\\\r\n]|[^"\\\r\n]+[^ \t"\\\r\n])/ + in /"/ . (no_dquot|cdot|cl)* let squot = let no_squot = /[^'\\\r\n]/ in /'/ . (no_squot|cdot|cl)* . /'/ @@ -78,6 +81,8 @@ let comp = /[<>=]?=/ *****************************************************************) let arg_dir = [ label "arg" . store (char_arg_dir+|dquot|squot) ] +(* message argument starts with " but ends at EOL *) +let arg_dir_msg = [ label "arg" . store dquot_msg ] let arg_sec = [ label "arg" . store (char_arg_sec+|comp|dquot|squot) ] let arg_wl = [ label "arg" . store (char_arg_wl+|dquot|squot) ] @@ -90,8 +95,10 @@ let arg_wordlist = let argv (l:lens) = l . (sep_spc . l)* -let directive = [ indent . label "directive" . store word . - (sep_spc . argv (arg_dir|arg_wordlist))? . eol ] +let directive = + (* arg_dir_msg may be the last or only argument *) + let dir_args = (argv (arg_dir|arg_wordlist) . (sep_spc . arg_dir_msg)?) | arg_dir_msg + in [ indent . label "directive" . store word . (sep_spc . dir_args)? . eol ] let section (body:lens) = (* opt_eol includes empty lines *) diff --git a/tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf b/tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf similarity index 100% rename from tests/apache-conf-files/failing/drupal-errordocument-arg-1724.conf rename to tests/apache-conf-files/passing/drupal-errordocument-arg-1724.conf From 3b5810995ddc161c5263cd77d044d2a0be14b5c2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 14:11:26 -0800 Subject: [PATCH 17/35] The flag name --test-cert may make sense to most people --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f57b1eb0b..16283a84a 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1047,7 +1047,7 @@ def _paths_parser(helpful): add("paths", "--server", default=flag_default("server"), help=config_help("server")) # overwrites server, handled in HelpfulArgumentParser.parse_args() - add("testing", "--staging", action='store_true', + add("testing", "--test-cert", "--staging", action='store_true', dest='staging', help='Use the staging server to obtain test (invalid) certs; equivalent' ' to --server https://acme-staging.api.letsencrypt.org/directory ') From 88956dfba8f5109c5b168b3207b72db9d4ae4e4a Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Dec 2015 19:37:22 -0800 Subject: [PATCH 18/35] Move staging URI into constants.py --- letsencrypt/cli.py | 7 +++---- letsencrypt/constants.py | 3 ++- letsencrypt/tests/cli_test.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 16283a84a..5be3c1696 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -699,11 +699,10 @@ class HelpfulArgumentParser(object): # Do any post-parsing homework here # argparse seemingly isn't flexible enough to give us this behaviour easily... - staging_uri = 'https://acme-staging.api.letsencrypt.org/directory' if parsed_args.staging: - if parsed_args.server not in (flag_default("server"), staging_uri): + if parsed_args.server not in (flag_default("server"), constants.STAGING_URI): raise errors.Error("--server value conflicts with --staging") - parsed_args.server = staging_uri + parsed_args.server = constants.STAGING_URI return parsed_args @@ -1049,7 +1048,7 @@ def _paths_parser(helpful): # overwrites server, handled in HelpfulArgumentParser.parse_args() add("testing", "--test-cert", "--staging", action='store_true', dest='staging', help='Use the staging server to obtain test (invalid) certs; equivalent' - ' to --server https://acme-staging.api.letsencrypt.org/directory ') + ' to --server ' + constants.STAGING_URI) def _plugins_parsing(helpful, plugins): diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 40155abd7..a1dccd1ea 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -30,8 +30,9 @@ CLI_DEFAULTS = dict( auth_chain_path="./chain.pem", strict_permissions=False, ) -"""Defaults for CLI flags and `.IConfig` attributes.""" +STAGING_URI = "https://acme-staging.api.letsencrypt.org/directory" +"""Defaults for CLI flags and `.IConfig` attributes.""" RENEWER_DEFAULTS = dict( renewer_enabled="yes", diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 60a8e9440..e7ae5de23 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -15,6 +15,7 @@ from acme import jose from letsencrypt import account from letsencrypt import cli from letsencrypt import configuration +from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util @@ -351,8 +352,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods short_args = ['--staging'] namespace = cli.prepare_and_parse_args(plugins, short_args) - self.assertEqual(namespace.server, - 'https://acme-staging.api.letsencrypt.org/directory') + self.assertEqual(namespace.server, constants.STAGING_URI) short_args = ['--staging', '--server', 'example.com'] self.assertRaises(errors.Error, cli.prepare_and_parse_args, plugins, short_args) From a878e48624e4d6c6bd4352e300433beb32b04acc Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 4 Dec 2015 11:50:38 -0800 Subject: [PATCH 19/35] Add another failing case --- .../failing/missing-double-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/failing/missing-double-quote-1724.conf diff --git a/tests/apache-conf-files/failing/missing-double-quote-1724.conf b/tests/apache-conf-files/failing/missing-double-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/failing/missing-double-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From 2321237d1ece6dbfcdf0293f338b2b8a7c7211ef Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 8 Dec 2015 22:30:17 -0800 Subject: [PATCH 20/35] Embodiement of Apache bug #1755 --- .../passing/example-1755.conf | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/apache-conf-files/passing/example-1755.conf diff --git a/tests/apache-conf-files/passing/example-1755.conf b/tests/apache-conf-files/passing/example-1755.conf new file mode 100644 index 000000000..260029576 --- /dev/null +++ b/tests/apache-conf-files/passing/example-1755.conf @@ -0,0 +1,36 @@ + + # The ServerName directive sets the request scheme, hostname and port that + # the server uses to identify itself. This is used when creating + # redirection URLs. In the context of virtual hosts, the ServerName + # specifies what hostname must appear in the request's Host: header to + # match this virtual host. For the default virtual host (this file) this + # value is not decisive as it is used as a last resort host regardless. + # However, you must set it for any further virtual host explicitly. + ServerName www.example.com + ServerAlias example.com +SetOutputFilter DEFLATE +# Do not attempt to compress the following extensions +SetEnvIfNoCase Request_URI \ +\.(?:gif|jpe?g|png|swf|flv|zip|gz|tar|mp3|mp4|m4v)$ no-gzip dont-vary + + ServerAdmin webmaster@localhost + DocumentRoot /var/www/proof + + # Available loglevels: trace8, ..., trace1, debug, info, notice, warn, + # error, crit, alert, emerg. + # It is also possible to configure the loglevel for particular + # modules, e.g. + #LogLevel info ssl:warn + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + # For most configuration files from conf-available/, which are + # enabled or disabled at a global level, it is possible to + # include a line for only one particular virtual host. For example the + # following line enables the CGI configuration for this host only + # after it has been globally disabled with "a2disconf". + #Include conf-available/serve-cgi-bin.conf + + +# vim: syntax=apache ts=4 sw=4 sts=4 sr noet From bdfca70d55a657ba7f79b63f01e89797229ba43e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 8 Dec 2015 23:04:13 -0800 Subject: [PATCH 21/35] Another #1531 --- .../apache-conf-files/passing/1626-1531.conf | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/apache-conf-files/passing/1626-1531.conf diff --git a/tests/apache-conf-files/passing/1626-1531.conf b/tests/apache-conf-files/passing/1626-1531.conf new file mode 100644 index 000000000..4a298857e --- /dev/null +++ b/tests/apache-conf-files/passing/1626-1531.conf @@ -0,0 +1,37 @@ + + ServerAdmin denver@ossguy.com + ServerName c-beta.ossguy.com + + Alias /robots.txt /home/denver/www/c-beta.ossguy.com/static/robots.txt + Alias /favicon.ico /home/denver/www/c-beta.ossguy.com/static/favicon.ico + + AliasMatch /(.*\.css) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.js) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.png) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.gif) /home/denver/www/c-beta.ossguy.com/static/$1 + AliasMatch /(.*\.jpg) /home/denver/www/c-beta.ossguy.com/static/$1 + + WSGIScriptAlias / /home/denver/www/c-beta.ossguy.com/django.wsgi + WSGIDaemonProcess c-beta-ossguy user=www-data group=www-data home=/var/www processes=5 threads=10 maximum-requests=1000 umask=0007 display-name=c-beta-ossguy + WSGIProcessGroup c-beta-ossguy + WSGIApplicationGroup %{GLOBAL} + + DocumentRoot /home/denver/www/c-beta.ossguy.com/static + + + Options -Indexes +FollowSymLinks -MultiViews + Require all granted + AllowOverride None + + + + Options +Indexes +FollowSymLinks -MultiViews + Require all granted + AllowOverride None + + + # Custom log file locations + LogLevel warn + ErrorLog /home/denver/www/logs/c-beta.ossguy.com/error.log + CustomLog /home/denver/www/logs/c-beta.ossguy.com/access.log combined + From de9e43de0cf35b5eecb0f91ba043cd6b285f3bff Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 9 Dec 2015 17:03:01 -0800 Subject: [PATCH 22/35] Update apache conf library --- tests/apache-conf-files/passing/1626-1531.conf | 4 ++-- tests/apache-conf-files/passing/README.modules | 2 ++ ...equire-wordlist.conf => sslrequire-wordlist-1827.htaccess} | 0 3 files changed, 4 insertions(+), 2 deletions(-) rename tests/apache-conf-files/passing/{sslrequire-wordlist.conf => sslrequire-wordlist-1827.htaccess} (100%) diff --git a/tests/apache-conf-files/passing/1626-1531.conf b/tests/apache-conf-files/passing/1626-1531.conf index 4a298857e..1622a57df 100644 --- a/tests/apache-conf-files/passing/1626-1531.conf +++ b/tests/apache-conf-files/passing/1626-1531.conf @@ -32,6 +32,6 @@ # Custom log file locations LogLevel warn - ErrorLog /home/denver/www/logs/c-beta.ossguy.com/error.log - CustomLog /home/denver/www/logs/c-beta.ossguy.com/access.log combined + ErrorLog /tmp/error.log + CustomLog /tmp/access.log combined diff --git a/tests/apache-conf-files/passing/README.modules b/tests/apache-conf-files/passing/README.modules index 9c5853061..7edbd3e84 100644 --- a/tests/apache-conf-files/passing/README.modules +++ b/tests/apache-conf-files/passing/README.modules @@ -3,3 +3,5 @@ Modules required to parse these conf files: ssl rewrite macro +wsgi +deflate diff --git a/tests/apache-conf-files/passing/sslrequire-wordlist.conf b/tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess similarity index 100% rename from tests/apache-conf-files/passing/sslrequire-wordlist.conf rename to tests/apache-conf-files/passing/sslrequire-wordlist-1827.htaccess From 6fc65505f770915cb6071011170cc0bdeabd241c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Dec 2015 12:18:27 -0800 Subject: [PATCH 23/35] Add test case for #1724 --- .../passing/missing-quote-1724.conf | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/apache-conf-files/passing/missing-quote-1724.conf diff --git a/tests/apache-conf-files/passing/missing-quote-1724.conf b/tests/apache-conf-files/passing/missing-quote-1724.conf new file mode 100644 index 000000000..7d97b23d0 --- /dev/null +++ b/tests/apache-conf-files/passing/missing-quote-1724.conf @@ -0,0 +1,52 @@ + + ServerAdmin webmaster@localhost + ServerAlias www.example.com + ServerName example.com + DocumentRoot /var/www/example.com/www/ + SSLEngine on + + SSLProtocol all -SSLv2 -SSLv3 + SSLCipherSuite "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRS$ + SSLCertificateFile /etc/ssl/certs/ssl-cert-snakeoil.pem + SSLCertificateKeyFile /etc/ssl/private/ssl-cert-snakeoil.key + + + Options FollowSymLinks + AllowOverride All + + + Options Indexes FollowSymLinks MultiViews + AllowOverride All + Order allow,deny + allow from all + # This directive allows us to have apache2's default start page + # in /apache2-default/, but still have / go to the right place + + + ScriptAlias /cgi-bin/ /usr/lib/cgi-bin/ + + AllowOverride None + Options +ExecCGI -MultiViews +SymLinksIfOwnerMatch + Order allow,deny + Allow from all + + + ErrorLog /var/log/apache2/error.log + + # Possible values include: debug, info, notice, warn, error, crit, + # alert, emerg. + LogLevel warn + + CustomLog /var/log/apache2/access.log combined + ServerSignature On + + Alias /apache_doc/ "/usr/share/doc/" + + Options Indexes MultiViews FollowSymLinks + AllowOverride None + Order deny,allow + Deny from all + Allow from 127.0.0.0/255.0.0.0 ::1/128 + + + From 0fa4b4c93fb6db5145e65d9470021d79cf4ab0fb Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Dec 2015 12:18:41 -0800 Subject: [PATCH 24/35] This is a hackish script to run all of these "tests". --- tests/apache-conf-files/hackish-apache-test | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100755 tests/apache-conf-files/hackish-apache-test diff --git a/tests/apache-conf-files/hackish-apache-test b/tests/apache-conf-files/hackish-apache-test new file mode 100755 index 000000000..c6663551e --- /dev/null +++ b/tests/apache-conf-files/hackish-apache-test @@ -0,0 +1,28 @@ +#!/bin/bash + +# A hackish script to see if the client is behaving as expected +# with each of the "passing" conf files. + +# TODO presently this requires interaction and human judgement to +# assess, but it should be automated +export EA=/etc/apache2/ +TESTDIR="`dirname $0`" +LEROOT="`realpath \"$TESTDIR/../../\"`" +cd $TESTDIR/passing + +function CleanupExit() { + echo control c, exiting tests... + if [ "$f" != "" ] ; then + sudo rm /etc/apache2/sites-{enabled,available}/"$f" + fi + exit 1 +} + +trap CleanupExit INT +for f in *.conf ; do + echo testing "$f" + sudo cp "$f" "$EA"/sites-available/ + sudo ln -s "$EA/sites-available/$f" "$EA/sites-enabled/$f" + sudo "$LEROOT"/venv/bin/letsencrypt --apache certonly -t + sudo rm /etc/apache2/sites-{enabled,available}/"$f" +done From 0bde2007d0bb949b6cd9f6f2119868dca5e68590 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 15:11:16 -0800 Subject: [PATCH 25/35] Remove closing tag --- .../two_vhost_80/apache2/conf-available/bad_conf_file.conf | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf index 1aad6a9f4..8e9178803 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf @@ -1,5 +1,3 @@ ServerName invalid.net - - From 9248ba1e9696e4f7b3d79fdda40edce5494a0a26 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:06:12 -0800 Subject: [PATCH 26/35] Fix deprecation bug --- letsencrypt/le_util.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index fe63c70af..c630f4fe7 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -1,6 +1,7 @@ """Utilities for all Let's Encrypt.""" import argparse import collections +import configargparse import errno import logging import os @@ -278,9 +279,11 @@ def add_deprecated_argument(add_argument, argument_name, nargs): sys.stderr.write( "Use of {0} is deprecated.\n".format(option_string)) + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE.add(ShowWarning) add_argument(argument_name, action=ShowWarning, help=argparse.SUPPRESS, nargs=nargs) + def check_domain_sanity(domain): """Method which validates domain value and errors out if the requirements are not met. From 75bc227cfac6bb4de7246597d78a400d1b788b1a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:13:06 -0800 Subject: [PATCH 27/35] Test setting agree-dev-preview in config file --- letsencrypt/tests/cli_test.py | 5 +++++ letsencrypt/tests/testdata/cli.ini | 1 + 2 files changed, 6 insertions(+) create mode 100644 letsencrypt/tests/testdata/cli.ini diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e7ae5de23..2b56c5abe 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -551,6 +551,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(path, os.path.abspath(path)) self.assertEqual(contents, test_contents) + def test_agree_dev_preview_config(self): + with MockedVerb('run') as mocked_run: + self._call(['-c', test_util.vector_path('cli.ini')]) + self.assertTrue(mocked_run.called) + class DetermineAccountTest(unittest.TestCase): """Tests for letsencrypt.cli._determine_account.""" diff --git a/letsencrypt/tests/testdata/cli.ini b/letsencrypt/tests/testdata/cli.ini new file mode 100644 index 000000000..8ef506071 --- /dev/null +++ b/letsencrypt/tests/testdata/cli.ini @@ -0,0 +1 @@ +agree-dev-preview = True From 3521c92be3b2c186ea56fdd3e57b21997ba1dd4a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 16:14:36 -0800 Subject: [PATCH 28/35] Fixed import ordering --- letsencrypt/le_util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index c630f4fe7..64295a80f 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -1,7 +1,6 @@ """Utilities for all Let's Encrypt.""" import argparse import collections -import configargparse import errno import logging import os @@ -11,6 +10,8 @@ import stat import subprocess import sys +import configargparse + from letsencrypt import errors From c66c6bae1d5edcfbe0716b7e5b3f870e8595e0da Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:00:33 -0800 Subject: [PATCH 29/35] Make `supported_challenges` return a list, not set --- letsencrypt/plugins/standalone.py | 4 ++-- letsencrypt/plugins/standalone_test.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 1bca3c036..139764601 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -173,8 +173,8 @@ class Authenticator(common.Plugin): @property def supported_challenges(self): """Challenges supported by this plugin.""" - return set(challenges.Challenge.TYPES[name] for name in - self.conf("supported-challenges").split(",")) + return [challenges.Challenge.TYPES[name] for name in + self.conf("supported-challenges").split(",")] @property def _necessary_ports(self): diff --git a/letsencrypt/plugins/standalone_test.py b/letsencrypt/plugins/standalone_test.py index 1833a55fe..1e39dee57 100644 --- a/letsencrypt/plugins/standalone_test.py +++ b/letsencrypt/plugins/standalone_test.py @@ -98,12 +98,12 @@ class AuthenticatorTest(unittest.TestCase): def test_supported_challenges(self): self.assertEqual(self.auth.supported_challenges, - set([challenges.TLSSNI01, challenges.HTTP01])) + [challenges.TLSSNI01, challenges.HTTP01]) def test_supported_challenges_configured(self): self.config.standalone_supported_challenges = "tls-sni-01" self.assertEqual(self.auth.supported_challenges, - set([challenges.TLSSNI01])) + [challenges.TLSSNI01]) def test_more_info(self): self.assertTrue(isinstance(self.auth.more_info(), six.string_types)) From 74927613e9e9f9fa02f0dfbe2a2f75707a850b5d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 18:03:52 -0800 Subject: [PATCH 30/35] Fixed lint issues --- letsencrypt/plugins/webroot.py | 5 ++--- letsencrypt/plugins/webroot_test.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index c4072c3f9..b4c877c78 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -2,7 +2,6 @@ import errno import logging import os -import stat import zope.interface @@ -105,10 +104,10 @@ to serve all files under specified web root ({0}).""" path = self.full_roots[achall.domain] except IndexError: raise errors.PluginError("Missing --webroot-path for domain: {1}" - .format(achall.domain)) + .format(achall.domain)) if not os.path.exists(path): raise errors.PluginError("Mysteriously missing path {0} for domain: {1}" - .format(path, achall.domain)) + .format(path, achall.domain)) return os.path.join(path, achall.chall.encode("token")) def _perform_single(self, achall): diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2e88c1756..2ebbf01a6 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -48,7 +48,7 @@ class AuthenticatorTest(unittest.TestCase): def test_add_parser_arguments(self): add = mock.MagicMock() self.auth.add_parser_arguments(add) - self.assertEqual(0, add.call_count) # became 0 when we moved the args to cli.py! + self.assertEqual(0, add.call_count) # args moved to cli.py! def test_prepare_bad_root(self): self.config.webroot_path = os.path.join(self.path, "null") @@ -80,7 +80,7 @@ class AuthenticatorTest(unittest.TestCase): # Check permissions of the directories - for dirpath, dirnames, filenames in os.walk(self.path): + for dirpath, dirnames, _ in os.walk(self.path): for directory in dirnames: full_path = os.path.join(dirpath, directory) dir_permissions = stat.S_IMODE(os.stat(full_path).st_mode) From 2f904a41e060e468891df53114b5df2ce735fba3 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:06:11 -0800 Subject: [PATCH 31/35] Derive preference order from `supported_challenges` order --- letsencrypt/plugins/standalone.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 139764601..70bf92dbb 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -197,8 +197,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return [chall for chall in SUPPORTED_CHALLENGES - if chall in self.supported_challenges] + return self.supported_challenges def perform(self, achalls): # pylint: disable=missing-docstring if any(util.already_listening(port) for port in self._necessary_ports): From f4d499dbad41ea16759deffd95290a01836bd914 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Fri, 11 Dec 2015 18:07:25 -0800 Subject: [PATCH 32/35] Make help message indicate derived challenge preference --- letsencrypt/plugins/standalone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 70bf92dbb..4319e51f9 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -166,7 +166,7 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): add("supported-challenges", - help="Supported challenges. Prefers tls-sni-01.", + help="Supported challenges. Preferred in the order they are listed.", type=supported_challenges_validator, default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) From 2d525594663ecf5037cd31a4b6f67bcb54d2e516 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 18:12:46 -0800 Subject: [PATCH 33/35] Cleanup comment --- letsencrypt/plugins/webroot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index b4c877c78..075930c8b 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -59,8 +59,8 @@ to serve all files under specified web root ({0}).""" logger.debug("Creating root challenges validation dir at %s", self.full_roots[name]) - # Change the permissiosn to be writable (GH #1389) - # Umask is used instead of chmod to ensure the client can also + # Change the permissions to be writable (GH #1389) + # Umask is used instead of chmod to ensure the client can also # run as non-root (GH #1795) old_umask = os.umask(0o022) From d45865a60110f9df383b58d307b06cea6f7cb42a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 19:14:23 -0800 Subject: [PATCH 34/35] Cleanup --- letsencrypt/plugins/webroot.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 075930c8b..0679bc349 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -65,16 +65,12 @@ to serve all files under specified web root ({0}).""" old_umask = os.umask(0o022) try: - - stat_path = os.stat(path) # This is coupled with the "umask" call above because # os.makedirs's "mode" parameter may not always work: # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python - os.makedirs(self.full_roots[name], 0o0755) # Set owner as parent directory if possible - try: stat_path = os.stat(path) os.chown(self.full_roots[name], stat_path.st_uid, From 1a7dd76288204a6bc1c979149162fa5440b04156 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 11 Dec 2015 19:31:50 -0800 Subject: [PATCH 35/35] Added test coverage --- letsencrypt/plugins/webroot_test.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 2ebbf01a6..9f5b6bba8 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -1,9 +1,10 @@ """Tests for letsencrypt.plugins.webroot.""" +import errno import os import shutil +import stat import tempfile import unittest -import stat import mock @@ -35,7 +36,6 @@ class AuthenticatorTest(unittest.TestCase): self.config = mock.MagicMock(webroot_path=self.path, webroot_map={"thing.com": self.path}) self.auth = Authenticator(self.config, "webroot") - self.auth.prepare() def tearDown(self): shutil.rmtree(self.path) @@ -70,7 +70,18 @@ class AuthenticatorTest(unittest.TestCase): self.assertRaises(errors.PluginError, self.auth.prepare) os.chmod(self.path, 0o700) + @mock.patch("letsencrypt.plugins.webroot.os.chown") + def test_failed_chown_eacces(self, mock_chown): + mock_chown.side_effect = OSError(errno.EACCES, "msg") + self.auth.prepare() # exception caught and logged + + @mock.patch("letsencrypt.plugins.webroot.os.chown") + def test_failed_chown_not_eacces(self, mock_chown): + mock_chown.side_effect = OSError() + self.assertRaises(errors.PluginError, self.auth.prepare) + def test_prepare_permissions(self): + self.auth.prepare() # Remove exec bit from permission check, so that it # matches the file @@ -93,6 +104,7 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid) def test_perform_cleanup(self): + self.auth.prepare() responses = self.auth.perform([self.achall]) self.assertEqual(1, len(responses)) self.assertTrue(os.path.exists(self.validation_path))