From 33f1c301924cc6b5c022bf8c9113b04290d2da0a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 22 Sep 2015 16:49:07 -0700 Subject: [PATCH 01/40] Updated a2enmod.sh --- .../configurators/apache/a2enmod.sh | 34 +++++-------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh index f822a1f7b..0a8ade4c2 100755 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh @@ -3,31 +3,15 @@ # httpd docker image. First argument is the server_root and the second is the # module to be enabled. -APACHE_CONFDIR=$1 +confdir=$1 +module=$2 -enable () { - echo "LoadModule "$1"_module /usr/local/apache2/modules/mod_"$1".so" >> \ - $APACHE_CONFDIR"/test.conf" - available_base="/mods-available/"$1".conf" - available_conf=$APACHE_CONFDIR$available_base - enabled_dir=$APACHE_CONFDIR"/mods-enabled" - enabled_conf=$enabled_dir"/"$1".conf" - if [ -e "$available_conf" -a -d "$enabled_dir" -a ! -e "$enabled_conf" ] - then - ln -s "..$available_base" $enabled_conf - fi -} - -if [ $2 == "ssl" ] +echo "LoadModule ${module}_module " \ + "/usr/local/apache2/modules/mod_${module}.so" >> "${confdir}/test.conf" +available_conf=$APACHE_CONFDIR"/mods-available/${module}.conf" +enabled_dir=$APACHE_CONFDIR"/mods-enabled" +enabled_conf=$enabled_dir"/"$1".conf" +if [ -e "$available_conf" -a -d "$enabled_dir" -a ! -e "$enabled_conf" ] then - # Enables ssl and all its dependencies - enable "setenvif" - enable "mime" - enable "socache_shmcb" - enable "ssl" -elif [ $2 == "rewrite" ] -then - enable "rewrite" -else - exit 1 + ln -s "..$available_base" $enabled_conf fi From a4d0188d215e6394edce152b750105e284d911e2 Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sat, 3 Oct 2015 12:50:18 -0400 Subject: [PATCH 02/40] Add Mac compatibility to integration tests --- letsencrypt/plugins/manual.py | 1 + letsencrypt/plugins/standalone/authenticator.py | 11 +++++++---- tests/boulder-integration.sh | 6 +++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 3f7276725..f532b8fbc 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -159,6 +159,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") # don't care about setting stdout and stderr, # we're in test mode anyway shell=True, + executable="/bin/bash", # "preexec_fn" is UNIX specific, but so is "command" preexec_fn=os.setsid) except OSError as error: # ValueError should not happen! diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index 968063781..f7c24f5e5 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -301,11 +301,14 @@ class StandaloneAuthenticator(common.Plugin): :param int port: The TCP port in question. :returns: True or False.""" - listeners = [conn.pid for conn in psutil.net_connections() - if conn.status == 'LISTEN' and - conn.type == socket.SOCK_STREAM and - conn.laddr[1] == port] try: + + # net_connections() can raise AccessDenied on certain OSs + listeners = [conn.pid for conn in psutil.net_connections() + if conn.status == 'LISTEN' and + conn.type == socket.SOCK_STREAM and + conn.laddr[1] == port] + if listeners and listeners[0] is not None: # conn.pid may be None if the current process doesn't have # permission to identify the listening process! Additionally, diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index ed877d136..efd16ef6c 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -50,7 +50,11 @@ dir="$root/conf/archive/le1.wtf" for x in cert chain fullchain privkey; do latest="$(ls -1t $dir/ | grep -e "^${x}" | head -n1)" - live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")" + if [ `uname` == 'Darwin' ]; then + live="$(greadlink -f "$root/conf/live/le1.wtf/${x}.pem")" + else + live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")" + fi [ "${dir}/${latest}" = "$live" ] # renewer fails this test done From 034af2003c45b50e4047b332f7bcdff725fe8fec Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 4 Oct 2015 20:07:00 -0400 Subject: [PATCH 03/40] Only ask for bash on OS X --- letsencrypt/plugins/manual.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index f532b8fbc..179a7b49f 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -154,12 +154,18 @@ binary for temporary key/certificate generation.""".replace("\n", "") if self.conf("test-mode"): logger.debug("Test mode. Executing the manual command: %s", command) try: + # sh shipped with OS X does't support echo -n + if sys.platform == "darwin": + executable = "/bin/bash" + else: + executable = "/bin/sh" + self._httpd = subprocess.Popen( command, # don't care about setting stdout and stderr, # we're in test mode anyway shell=True, - executable="/bin/bash", + executable=executable, # "preexec_fn" is UNIX specific, but so is "command" preexec_fn=os.setsid) except OSError as error: # ValueError should not happen! From 7420a78296371c63e61c242d75e0761eb153ffd6 Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 4 Oct 2015 20:08:15 -0400 Subject: [PATCH 04/40] Shrink AccessDenied error handler and inform --- .../plugins/standalone/authenticator.py | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index f7c24f5e5..cdf9a6f04 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -1,4 +1,5 @@ """Standalone authenticator.""" +import logging import os import psutil import signal @@ -19,6 +20,9 @@ from letsencrypt import interfaces from letsencrypt.plugins import common +logger = logging.getLogger(__name__) + + class StandaloneAuthenticator(common.Plugin): # pylint: disable=too-many-instance-attributes """Standalone authenticator. @@ -302,13 +306,21 @@ class StandaloneAuthenticator(common.Plugin): :returns: True or False.""" try: + net_connections = psutil.net_connections() + except psutil.AccessDenied as error: + logger.info("Access denied when trying to list network " + "connections: %s. Are you root?", error) + # this function is just a pre-check that often causes false + # positives and problems in testing (c.f. #680 on Mac, #255 + # generally); we will fail later in bind() anyway + return False - # net_connections() can raise AccessDenied on certain OSs - listeners = [conn.pid for conn in psutil.net_connections() - if conn.status == 'LISTEN' and - conn.type == socket.SOCK_STREAM and - conn.laddr[1] == port] + listeners = [conn.pid for conn in net_connections + if conn.status == 'LISTEN' and + conn.type == socket.SOCK_STREAM and + conn.laddr[1] == port] + try: if listeners and listeners[0] is not None: # conn.pid may be None if the current process doesn't have # permission to identify the listening process! Additionally, From c9e28309ed2d3efb3c2c21301f88796fe87169ab Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 4 Oct 2015 20:08:38 -0400 Subject: [PATCH 05/40] Define constants for OS specific executables --- tests/boulder-integration.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index efd16ef6c..633d426e3 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -14,6 +14,12 @@ export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx export GOPATH="${GOPATH:-/tmp/go}" export PATH="$GOPATH/bin:$PATH" +if [ `uname` == 'Darwin' ]; then + readlink="greadlink" +else + readlink="readlink" +fi + common() { letsencrypt_test \ --authenticator standalone \ @@ -50,11 +56,7 @@ dir="$root/conf/archive/le1.wtf" for x in cert chain fullchain privkey; do latest="$(ls -1t $dir/ | grep -e "^${x}" | head -n1)" - if [ `uname` == 'Darwin' ]; then - live="$(greadlink -f "$root/conf/live/le1.wtf/${x}.pem")" - else - live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")" - fi + live="$($readlink -f "$root/conf/live/le1.wtf/${x}.pem")" [ "${dir}/${latest}" = "$live" ] # renewer fails this test done From 2737c3299d41cf8dd1a386626846e09316e27bab Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 4 Oct 2015 20:13:07 -0400 Subject: [PATCH 06/40] That shouldn't be in the try block --- letsencrypt/plugins/manual.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 179a7b49f..f1170eea7 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -153,13 +153,13 @@ binary for temporary key/certificate generation.""".replace("\n", "") ct=response.CONTENT_TYPE, port=port) if self.conf("test-mode"): logger.debug("Test mode. Executing the manual command: %s", command) + # sh shipped with OS X does't support echo -n + if sys.platform == "darwin": + executable = "/bin/bash" + else: + executable = "/bin/sh" + try: - # sh shipped with OS X does't support echo -n - if sys.platform == "darwin": - executable = "/bin/bash" - else: - executable = "/bin/sh" - self._httpd = subprocess.Popen( command, # don't care about setting stdout and stderr, From 973cd6ce42b837175087f4e2a4af4f978fbe9df2 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Mon, 5 Oct 2015 18:19:36 -0500 Subject: [PATCH 07/40] Add instructions for submitting a PR. --- docs/contributing.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/contributing.rst b/docs/contributing.rst index 3959ccee1..d2bb2ebd5 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -267,6 +267,17 @@ Please: .. _PEP 8 - Style Guide for Python Code: https://www.python.org/dev/peps/pep-0008 +Submitting a pull request +========================= +Steps: + +1. Write your code! +2. Make sure your environment is set up properly and that you're in your virtualenv (this is a **very important** step). +3. Run ``./pep8.travis.sh`` to do a cursory check of your code style. Fix any errors. +4. Run ``tox -e lint`` to check for pylint errors. Fix any errors. +5. Run ``tox`` to run the unit tests. Fix any errors. +6. :ref:`Run the integration tests `. +7. Submit the PR. Updating the documentation ========================== From a4e5f298565e9b21c4d551bcd3122a404f3fc729 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Mon, 5 Oct 2015 18:25:33 -0500 Subject: [PATCH 08/40] Add link to instructions for running integration tests --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index d2bb2ebd5..a76d76cd8 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -276,7 +276,7 @@ Steps: 3. Run ``./pep8.travis.sh`` to do a cursory check of your code style. Fix any errors. 4. Run ``tox -e lint`` to check for pylint errors. Fix any errors. 5. Run ``tox`` to run the unit tests. Fix any errors. -6. :ref:`Run the integration tests `. +6. Run the integration tests, see `integration`_. 7. Submit the PR. Updating the documentation From e8118c862b3b9d9dbe34b36a52c1d6990226652e Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 6 Oct 2015 18:54:05 +0000 Subject: [PATCH 09/40] Renewer logging setup (fixes #897) --- letsencrypt/cli.py | 50 ++++++++++++++++++------------- letsencrypt/renewer.py | 29 +++++++++++++++--- letsencrypt/tests/renewer_test.py | 34 +++++++++++++-------- 3 files changed, 76 insertions(+), 37 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0bd5f537e..66254d2b0 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -841,9 +841,23 @@ def _plugins_parsing(helpful, plugins): helpful.add_plugin_args(plugins) -def _setup_logging(args): - level = -args.verbose_count * 10 - fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" +def setup_log_file_handler(args, logfile, fmt): + """Setup file debug logging.""" + log_file_path = os.path.join(args.logs_dir, logfile) + handler = logging.handlers.RotatingFileHandler( + log_file_path, maxBytes=2 ** 20, backupCount=10) + # rotate on each invocation, rollover only possible when maxBytes + # is nonzero and backupCount is nonzero, so we set maxBytes as big + # as possible not to overrun in single CLI invocation (1MB). + handler.doRollover() # TODO: creates empty letsencrypt.log.1 file + handler.setLevel(logging.DEBUG) + handler_formatter = logging.Formatter(fmt=fmt) + handler_formatter.converter = time.gmtime # don't use localtime + handler.setFormatter(handler_formatter) + return handler, log_file_path + + +def _cli_log_handler(args, level, fmt): if args.text_mode: handler = colored_logging.StreamHandler() handler.setFormatter(logging.Formatter(fmt)) @@ -852,30 +866,26 @@ def _setup_logging(args): # dialog box is small, display as less as possible handler.setFormatter(logging.Formatter("%(message)s")) handler.setLevel(level) + return handler + + +def setup_logging(args, cli_handler_factory, logfile): + """Setup logging.""" + fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" + level = -args.verbose_count * 10 + file_handler, log_file_path = setup_log_file_handler( + args, logfile=logfile, fmt=fmt) + cli_handler = cli_handler_factory(args, level, fmt) # TODO: use fileConfig? - # unconditionally log to file for debugging purposes - # TODO: change before release? - log_file_name = os.path.join(args.logs_dir, 'letsencrypt.log') - file_handler = logging.handlers.RotatingFileHandler( - log_file_name, maxBytes=2 ** 20, backupCount=10) - # rotate on each invocation, rollover only possible when maxBytes - # is nonzero and backupCount is nonzero, so we set maxBytes as big - # as possible not to overrun in single CLI invocation (1MB). - file_handler.doRollover() # TODO: creates empty letsencrypt.log.1 file - file_handler.setLevel(logging.DEBUG) - file_handler_formatter = logging.Formatter(fmt=fmt) - file_handler_formatter.converter = time.gmtime # don't use localtime - file_handler.setFormatter(file_handler_formatter) - root_logger = logging.getLogger() root_logger.setLevel(logging.DEBUG) # send all records to handlers - root_logger.addHandler(handler) + root_logger.addHandler(cli_handler) root_logger.addHandler(file_handler) logger.debug("Root logging level set at %d", level) - logger.info("Saving debug log to %s", log_file_name) + logger.info("Saving debug log to %s", log_file_path) def _handle_exception(exc_type, exc_value, trace, args): @@ -944,7 +954,7 @@ def main(cli_args=sys.argv[1:]): # private key! #525 le_util.make_or_verify_dir( args.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args) - _setup_logging(args) + setup_logging(args, _cli_log_handler, logfile='letsencrypt.log') # do not log `args`, as it contains sensitive data (e.g. revoke --key)! logger.debug("Arguments: %r", cli_args) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 1c9cddc95..98ecc83b3 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -8,6 +8,7 @@ within lineages of successor certificates, according to configuration. """ import argparse +import logging import os import sys @@ -17,10 +18,12 @@ import zope.component from letsencrypt import account from letsencrypt import configuration +from letsencrypt import colored_logging from letsencrypt import cli from letsencrypt import client from letsencrypt import crypto_util from letsencrypt import errors +from letsencrypt import le_util from letsencrypt import notify from letsencrypt import storage @@ -28,6 +31,9 @@ from letsencrypt.display import util as display_util from letsencrypt.plugins import disco as plugins_disco +logger = logging.getLogger(__name__) + + class _AttrDict(dict): """Attribute dictionary. @@ -104,6 +110,12 @@ def renew(cert, old_version): # (where fewer than all names were renewed) +def _cli_log_handler(args, level, fmt): # pylint: disable=unused-argument + handler = colored_logging.StreamHandler() + handler.setFormatter(logging.Formatter(fmt)) + return handler + + def _paths_parser(parser): add = parser.add_argument_group("paths").add_argument add("--config-dir", default=cli.flag_default("config_dir"), @@ -119,11 +131,16 @@ def _paths_parser(parser): def _create_parser(): parser = argparse.ArgumentParser() #parser.add_argument("--cron", action="store_true", help="Run as cronjob.") - # pylint: disable=protected-access + parser.add_argument( + "-v", "--verbose", dest="verbose_count", action="count", + default=cli.flag_default("verbose_count"), help="This flag can be used " + "multiple times to incrementally increase the verbosity of output, " + "e.g. -vvv.") + return _paths_parser(parser) -def main(config=None, args=sys.argv[1:]): +def main(config=None, cli_args=sys.argv[1:]): """Main function for autorenewer script.""" # TODO: Distinguish automated invocation from manual invocation, # perhaps by looking at sys.argv[0] and inhibiting automated @@ -133,8 +150,12 @@ def main(config=None, args=sys.argv[1:]): zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) - cli_config = configuration.RenewerConfiguration( - _create_parser().parse_args(args)) + args = _create_parser().parse_args(cli_args) + + le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid()) + cli.setup_logging(args, _cli_log_handler, logfile='renewer.log') + + cli_config = configuration.RenewerConfiguration(args) config = storage.config_with_defaults(config) # Now attempt to read the renewer config file and augment or replace diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 6f115abf9..bc8afbcb5 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -44,8 +44,15 @@ class BaseRenewableCertTest(unittest.TestCase): self.tempdir = tempfile.mkdtemp() self.cli_config = configuration.RenewerConfiguration( - namespace=mock.MagicMock(config_dir=self.tempdir)) + namespace=mock.MagicMock( + config_dir=self.tempdir, + work_dir=self.tempdir, + logs_dir=self.tempdir, + ) + ) + # TODO: maybe provide RenewerConfiguration.make_dirs? + # TODO: main() should create those dirs, c.f. #902 os.makedirs(os.path.join(self.tempdir, "live", "example.org")) os.makedirs(os.path.join(self.tempdir, "archive", "example.org")) os.makedirs(os.path.join(self.tempdir, "renewal")) @@ -62,6 +69,9 @@ class BaseRenewableCertTest(unittest.TestCase): self.test_rc = storage.RenewableCert( self.config, self.defaults, self.cli_config) + def tearDown(self): + shutil.rmtree(self.tempdir) + def _write_out_ex_kinds(self): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) @@ -79,11 +89,6 @@ class BaseRenewableCertTest(unittest.TestCase): class RenewableCertTests(BaseRenewableCertTest): # pylint: disable=too-many-public-methods """Tests for letsencrypt.renewer.*.""" - def setUp(self): - super(RenewableCertTests, self).setUp() - - def tearDown(self): - shutil.rmtree(self.tempdir) def test_initialization(self): self.assertEqual(self.test_rc.lineagename, "example.org") @@ -665,11 +670,17 @@ class RenewableCertTests(BaseRenewableCertTest): # This should fail because the renewal itself appears to fail self.assertFalse(renewer.renew(self.test_rc, 1)) + def _common_cli_args(self): + return [ + "--config-dir", self.cli_config.config_dir, + "--work-dir", self.cli_config.work_dir, + "--logs-dir", self.cli_config.logs_dir, + ] + @mock.patch("letsencrypt.renewer.notify") @mock.patch("letsencrypt.storage.RenewableCert") @mock.patch("letsencrypt.renewer.renew") def test_main(self, mock_renew, mock_rc, mock_notify): - """Test for main() function.""" from letsencrypt import renewer mock_rc_instance = mock.MagicMock() mock_rc_instance.should_autodeploy.return_value = True @@ -691,8 +702,7 @@ class RenewableCertTests(BaseRenewableCertTest): "example.com.conf"), "w") as f: f.write("cert = cert.pem\nprivkey = privkey.pem\n") f.write("chain = chain.pem\nfullchain = fullchain.pem\n") - renewer.main(self.defaults, args=[ - '--config-dir', self.cli_config.config_dir]) + renewer.main(self.defaults, cli_args=self._common_cli_args()) self.assertEqual(mock_rc.call_count, 2) self.assertEqual(mock_rc_instance.update_all_links_to.call_count, 2) self.assertEqual(mock_notify.notify.call_count, 4) @@ -705,8 +715,7 @@ class RenewableCertTests(BaseRenewableCertTest): mock_happy_instance.should_autorenew.return_value = False mock_happy_instance.latest_common_version.return_value = 10 mock_rc.return_value = mock_happy_instance - renewer.main(self.defaults, args=[ - '--config-dir', self.cli_config.config_dir]) + renewer.main(self.defaults, cli_args=self._common_cli_args()) self.assertEqual(mock_rc.call_count, 4) self.assertEqual(mock_happy_instance.update_all_links_to.call_count, 0) self.assertEqual(mock_notify.notify.call_count, 4) @@ -717,8 +726,7 @@ class RenewableCertTests(BaseRenewableCertTest): with open(os.path.join(self.cli_config.renewal_configs_dir, "bad.conf"), "w") as f: f.write("incomplete = configfile\n") - renewer.main(self.defaults, args=[ - '--config-dir', self.cli_config.config_dir]) + renewer.main(self.defaults, cli_args=self._common_cli_args()) # The errors.CertStorageError is caught inside and nothing happens. From 9e1477faa48148ad5b2c3664375a3520d589eb94 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 8 Oct 2015 19:28:55 +0000 Subject: [PATCH 10/40] Release 0.0.0.dev20151008 --- acme/setup.py | 2 +- letsencrypt-apache/setup.py | 2 +- letsencrypt-nginx/setup.py | 2 +- letsencrypt/__init__.py | 2 +- letshelp-letsencrypt/setup.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 6448b7fe9..36a724f97 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.0.0.dev20151008' install_requires = [ # load_pem_private/public_key (>=0.6) diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index 626e700b2..ef9299b2c 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.0.0.dev20151008' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index a37b8222b..dde7243a9 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.0.0.dev20151008' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt/__init__.py b/letsencrypt/__init__.py index 1155a5b0c..d7ec9c5e3 100644 --- a/letsencrypt/__init__.py +++ b/letsencrypt/__init__.py @@ -1,4 +1,4 @@ """Let's Encrypt client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.1.0.dev0' +__version__ = '0.0.0.dev20151008' diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index a83fc8843..ad517c74e 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.0.0.dev20151008' install_requires = [ 'setuptools', # pkg_resources From 744afe9cea0be2fbb5f8c155e65060d4277de524 Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Thu, 8 Oct 2015 16:15:09 -0400 Subject: [PATCH 11/40] PEP8 E128 up in here. Don't assume sh exists --- letsencrypt/plugins/manual.py | 2 +- letsencrypt/plugins/standalone/authenticator.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index f19861d9d..65d7d6876 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -133,7 +133,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") if sys.platform == "darwin": executable = "/bin/bash" else: - executable = "/bin/sh" + executable = None try: self._httpd = subprocess.Popen( diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index cdf9a6f04..a9972fba2 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -309,7 +309,7 @@ class StandaloneAuthenticator(common.Plugin): net_connections = psutil.net_connections() except psutil.AccessDenied as error: logger.info("Access denied when trying to list network " - "connections: %s. Are you root?", error) + "connections: %s. Are you root?", error) # this function is just a pre-check that often causes false # positives and problems in testing (c.f. #680 on Mac, #255 # generally); we will fail later in bind() anyway From 6dfb75b96f6e5126e590b38e608ebedb06c7b2bc Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 8 Oct 2015 20:32:15 +0000 Subject: [PATCH 12/40] Fix #928 test_json_dumps_pretty py3 compat. --- acme/acme/jose/interfaces_test.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/acme/acme/jose/interfaces_test.py b/acme/acme/jose/interfaces_test.py index 91e6f4416..84dc2a1be 100644 --- a/acme/acme/jose/interfaces_test.py +++ b/acme/acme/jose/interfaces_test.py @@ -1,6 +1,8 @@ """Tests for acme.jose.interfaces.""" import unittest +import six + class JSONDeSerializableTest(unittest.TestCase): # pylint: disable=too-many-instance-attributes @@ -90,8 +92,9 @@ class JSONDeSerializableTest(unittest.TestCase): self.assertEqual('["foo1", "foo2"]', self.seq.json_dumps()) def test_json_dumps_pretty(self): - self.assertEqual( - self.seq.json_dumps_pretty(), '[\n "foo1", \n "foo2"\n]') + filler = ' ' if six.PY2 else '' + self.assertEqual(self.seq.json_dumps_pretty(), + '[\n "foo1",{0}\n "foo2"\n]'.format(filler)) def test_json_dump_default(self): from acme.jose.interfaces import JSONDeSerializable From 454a661d444dfc3e97db826159edc2b35ee820e7 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Fri, 9 Oct 2015 15:46:03 -0500 Subject: [PATCH 13/40] contributing.rst: fix nits pointed out by @kuba --- docs/contributing.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index a76d76cd8..b4b1619ce 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -269,14 +269,19 @@ Please: Submitting a pull request ========================= + Steps: 1. Write your code! -2. Make sure your environment is set up properly and that you're in your virtualenv (this is a **very important** step). -3. Run ``./pep8.travis.sh`` to do a cursory check of your code style. Fix any errors. +2. Make sure your environment is set up properly and that you're in your + virtualenv. You can do this by running ``./bootstrap/dev/venv.sh``. + (this is a **very important** step) +3. Run ``./pep8.travis.sh`` to do a cursory check of your code style. + Fix any errors. 4. Run ``tox -e lint`` to check for pylint errors. Fix any errors. -5. Run ``tox`` to run the unit tests. Fix any errors. -6. Run the integration tests, see `integration`_. +5. Run ``tox`` to run the entire test suite including coverage. Fix any errors. +6. If your code touches communication with an ACME server/Boulder, you + should run the integration tests, see `integration`_. 7. Submit the PR. Updating the documentation From d1ee8311379dd1c102ed54ed1ae827b527cd255f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 9 Oct 2015 16:55:34 -0700 Subject: [PATCH 14/40] Fixed version string --- letsencrypt-compatibility-test/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-compatibility-test/setup.py b/letsencrypt-compatibility-test/setup.py index 2e70fd1d7..5a54239dd 100644 --- a/letsencrypt-compatibility-test/setup.py +++ b/letsencrypt-compatibility-test/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.1.0.dev0' +version = '0.0.0.dev20151008' install_requires = [ 'letsencrypt=={0}'.format(version), From ecf82c4bcf3c3e37ab8ba5746cd3902541d6a9e3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 9 Oct 2015 16:56:28 -0700 Subject: [PATCH 15/40] Add 'strict_permissions' when creating config --- .../letsencrypt_compatibility_test/util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py index 6181da16b..816f04398 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py @@ -25,6 +25,7 @@ IP_REGEX = re.compile(r"^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$") def create_le_config(parent_dir): """Sets up LE dirs in parent_dir and returns the config dict""" config = copy.deepcopy(constants.CLI_DEFAULTS) + config["strict_permissions"] = False le_dir = os.path.join(parent_dir, "letsencrypt") config["config_dir"] = os.path.join(le_dir, "config") From 84418516c9a6e1ecbbc3387fc6bd3e9a8f60cc61 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sat, 10 Oct 2015 13:31:35 -0700 Subject: [PATCH 16/40] Limit Travis runs to master and PRs. --- .travis.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.travis.yml b/.travis.yml index 46b14fe63..3041fdd82 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,15 @@ env: - TOXENV=lint - TOXENV=cover + +# Only build pushes to the master branch, PRs, and branches beginning with +# `test-`. This reduces the number of simultaneous Travis runs, which speeds +# turnaround time on review since there is a cap of 5 simultaneous runs. +branches: + only: + - master + - /^test-.*$/ + sudo: false # containers addons: # make sure simplehttp simple verification works (custom /etc/hosts) From f96c34546e91be25429a8a1c2f99710a9539f402 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sat, 10 Oct 2015 18:33:44 -0400 Subject: [PATCH 17/40] Fixes #902 Fix for #902 If the directory does't exist, it will create the directory before proceeding. --- letsencrypt/renewer.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 1c9cddc95..953b372f5 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -146,6 +146,10 @@ def main(config=None, args=sys.argv[1:]): # take precedence over this one. config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) + # If the folder does not exist we need to create it. + if not os.path.isdir(cli_config.renewal_configs_dir): + os.makedirs(cli_config.renewal_configs_dir) + for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i if not i.endswith(".conf"): From 7a153ebf50893cc314b1536c6bb314af4ccfc817 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 11 Oct 2015 07:05:35 +0000 Subject: [PATCH 18/40] Revert "Release 0.0.0.dev20151008" This reverts commit 9e1477faa48148ad5b2c3664375a3520d589eb94. --- acme/setup.py | 2 +- letsencrypt-apache/setup.py | 2 +- letsencrypt-nginx/setup.py | 2 +- letsencrypt/__init__.py | 2 +- letshelp-letsencrypt/setup.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 36a724f97..6448b7fe9 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.0.0.dev20151008' +version = '0.1.0.dev0' install_requires = [ # load_pem_private/public_key (>=0.6) diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index ef9299b2c..626e700b2 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.0.0.dev20151008' +version = '0.1.0.dev0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index dde7243a9..a37b8222b 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.0.0.dev20151008' +version = '0.1.0.dev0' install_requires = [ 'acme=={0}'.format(version), diff --git a/letsencrypt/__init__.py b/letsencrypt/__init__.py index d7ec9c5e3..1155a5b0c 100644 --- a/letsencrypt/__init__.py +++ b/letsencrypt/__init__.py @@ -1,4 +1,4 @@ """Let's Encrypt client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '0.0.0.dev20151008' +__version__ = '0.1.0.dev0' diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index ad517c74e..a83fc8843 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.0.0.dev20151008' +version = '0.1.0.dev0' install_requires = [ 'setuptools', # pkg_resources From 5edd809161c659d82592d53bc6e48af4f49f2e1a Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 11 Oct 2015 10:51:24 +0000 Subject: [PATCH 19/40] ApacheConfigurator.is_enabled using filecmp (fixes #838). --- letsencrypt-apache/letsencrypt_apache/configurator.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f3d2b5f9a..cb81366b3 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1,5 +1,6 @@ """Apache Configuration based off of Augeas Configurator.""" # pylint: disable=too-many-lines +import filecmp import itertools import logging import os @@ -945,9 +946,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ enabled_dir = os.path.join(self.parser.root, "sites-enabled") for entry in os.listdir(enabled_dir): - if os.path.realpath(os.path.join(enabled_dir, entry)) == avail_fp: - return True - + try: + if filecmp.cmp(avail_fp, os.path.join(enabled_dir, entry)): + return True + except OSError: + pass return False def enable_site(self, vhost): From ce4120186128136376dab24715af4ed0865d56e1 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 11 Oct 2015 10:52:08 +0000 Subject: [PATCH 20/40] Require tests pass in dev release. --- tools/dev-release.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/dev-release.sh b/tools/dev-release.sh index d93a6d21f..cebe5001c 100755 --- a/tools/dev-release.sh +++ b/tools/dev-release.sh @@ -88,8 +88,7 @@ mkdir ../kgs kgs="../kgs/$version" pip freeze | tee $kgs pip install nose -# TODO: letsencrypt_apache fails due to symlink, c.f. #838 -nosetests letsencrypt $SUBPKGS || true +nosetests letsencrypt $SUBPKGS echo "New root: $root" echo "KGS is at $root/kgs" From f8daf5f09411ed9d317d310b8efc97d3f39b495e Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Oct 2015 08:53:38 -0400 Subject: [PATCH 21/40] Fixed failing lint test and explicitly created all needed folders --- letsencrypt/renewer.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 953b372f5..077af6a68 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -23,6 +23,8 @@ from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import notify from letsencrypt import storage +from letsencrypt import constants +from letsencrypt import le_util from letsencrypt.display import util as display_util from letsencrypt.plugins import disco as plugins_disco @@ -145,10 +147,16 @@ def main(config=None, args=sys.argv[1:]): # specify a config file on the command line, which, if provided, should # take precedence over this one. config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) - - # If the folder does not exist we need to create it. - if not os.path.isdir(cli_config.renewal_configs_dir): - os.makedirs(cli_config.renewal_configs_dir) + # Ensure that all of the needed folders have been created before continuing + uid = os.geteuid() + le_util.make_or_verify_dir( + cli_config.renewal_configs_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.config_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i @@ -189,4 +197,4 @@ def main(config=None, args=sys.argv[1:]): cert.update_all_links_to(cert.latest_common_version()) # TODO: restart web server (invoke IInstaller.restart() method) notify.notify("Autodeployed a cert!!!", "root", "It worked!") - # TODO: explain what happened + # TODO: explain what happened \ No newline at end of file From ef9312817e230f9d579c673685acae8547d7e5f4 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Oct 2015 11:39:55 -0400 Subject: [PATCH 22/40] Alphabetized imports and added newline at end of file --- letsencrypt/renewer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 077af6a68..81a557419 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -17,14 +17,14 @@ import zope.component from letsencrypt import account from letsencrypt import configuration +from letsencrypt import constants from letsencrypt import cli from letsencrypt import client from letsencrypt import crypto_util from letsencrypt import errors +from letsencrypt import le_util from letsencrypt import notify from letsencrypt import storage -from letsencrypt import constants -from letsencrypt import le_util from letsencrypt.display import util as display_util from letsencrypt.plugins import disco as plugins_disco @@ -197,4 +197,4 @@ def main(config=None, args=sys.argv[1:]): cert.update_all_links_to(cert.latest_common_version()) # TODO: restart web server (invoke IInstaller.restart() method) notify.notify("Autodeployed a cert!!!", "root", "It worked!") - # TODO: explain what happened \ No newline at end of file + # TODO: explain what happened From dd8c6d6548032436008f278580db6a9c278487b4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 10:20:08 -0700 Subject: [PATCH 23/40] Nginx improvements Add a server_names_hash_bucket_size directive during challenges to fix an nginx crash on restart (Fixes #922). Use fullchain instead of chain (Fixes #610). Implement OCSP stapling (Fixes #937, Fixes #931). Hide Boulder output in integration tests to make them more readable. --- .../letsencrypt_apache/configurator.py | 3 +- .../configurators/apache/common.py | 5 +- .../letsencrypt_nginx/configurator.py | 30 ++++-- letsencrypt-nginx/letsencrypt_nginx/dvsni.py | 21 ++-- letsencrypt-nginx/letsencrypt_nginx/parser.py | 10 +- .../tests/configurator_test.py | 102 ++++++++++++------ .../letsencrypt_nginx/tests/dvsni_test.py | 6 +- .../letsencrypt_nginx/tests/parser_test.py | 15 +-- .../letsencrypt_nginx/tests/util.py | 15 +++ .../tests/boulder-integration.conf.sh | 1 - letsencrypt/cli.py | 9 +- letsencrypt/client.py | 9 +- letsencrypt/interfaces.py | 4 +- letsencrypt/plugins/null.py | 3 +- letsencrypt/tests/client_test.py | 12 ++- tests/boulder-start.sh | 2 +- 16 files changed, 170 insertions(+), 77 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index f3d2b5f9a..a3c2bd186 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -163,7 +163,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): temp_install(self.mod_ssl_conf) - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, + chain_path=None, fullchain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. Currently tries to find the last directives to deploy the cert in diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py index 0d3dbb1b5..bcc66b98a 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py @@ -175,12 +175,13 @@ class Proxy(configurators_common.Proxy): else: return {"example.com"} - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, chain_path=None, + fullchain_path=None): """Installs cert""" cert_path, key_path, chain_path = self.copy_certs_and_keys( cert_path, key_path, chain_path) self._apache_configurator.deploy_cert( - domain, cert_path, key_path, chain_path) + domain, cert_path, key_path, chain_path, fullchain_path) def _is_apache_command(command): diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index a88607e58..ffca041ca 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -117,30 +117,44 @@ class NginxConfigurator(common.Plugin): temp_install(self.mod_ssl_conf) # Entry point in main.py for installing cert - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, + chain_path, fullchain_path): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. .. note:: Aborts if the vhost is missing ssl_certificate or ssl_certificate_key. - .. note:: Nginx doesn't have a cert chain directive, so the last - parameter is always ignored. It expects the cert file to have - the concatenated chain. + .. note:: Nginx doesn't have a cert chain directive. + It expects the cert file to have the concatenated chain. + However, we use the chain file as input to the + ssl_trusted_certificate directive, used for verify OCSP responses. .. note:: This doesn't save the config files! """ vhost = self.choose_vhost(domain) - directives = [['ssl_certificate', cert_path], - ['ssl_certificate_key', key_path]] + cert_directives = [['ssl_certificate', fullchain_path], + ['ssl_certificate_key', key_path]] + + # OCSP stapling was introduced in Nginx 1.3.7. If we have that version + # or greater, add config settings for it. + stapling_directives = [] + if self.version >= (1, 3, 7): + stapling_directives = [ + ['ssl_trusted_certificate', chain_path], + ['ssl_stapling', 'on'], + ['ssl_stapling_verify', 'on']] try: self.parser.add_server_directives(vhost.filep, vhost.names, - directives, True) + cert_directives, True) + self.parser.add_server_directives(vhost.filep, vhost.names, + stapling_directives, False) logger.info("Deployed Certificate to VirtualHost %s for %s", vhost.filep, vhost.names) - except errors.MisconfigurationError: + except errors.MisconfigurationError, e: + logger.debug(e) logger.warn( "Cannot find a cert or key directive in %s for %s. " "VirtualHost was not modified.", vhost.filep, vhost.names) diff --git a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py index bd9ca783f..f384082ef 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py +++ b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py @@ -90,14 +90,23 @@ class NginxDvsni(common.Dvsni): # Add the 'include' statement for the challenges if it doesn't exist # already in the main config included = False - directive = ['include', self.challenge_conf] + include_directive = ['include', self.challenge_conf] root = self.configurator.parser.loc["root"] + + bucket_directive = ['server_names_hash_bucket_size', '128'] + main = self.configurator.parser.parsed[root] - for entry in main: - if entry[0] == ['http']: - body = entry[1] - if directive not in body: - body.append(directive) + for key, value in main: + if key == ['http']: + body = value + found_bucket = False + for key, value in body: # pylint: disable=unused-variable + if key == bucket_directive[0]: + found_bucket = True + if not found_bucket: + body.insert(0, bucket_directive) + if include_directive not in body: + body.insert(0, include_directive) included = True break if not included: diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index ef87cc653..fc8ed95f1 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -476,8 +476,12 @@ def _add_directives(block, directives, replace=False): :param list directives: The new directives. """ - if replace: - for directive in directives: + for directive in directives: + if not replace: + # We insert new directives at the top of the block, mostly + # to work around https://trac.nginx.org/nginx/ticket/810 + block.insert(0, directive) + else: changed = False if len(directive) == 0: continue @@ -489,5 +493,3 @@ def _add_directives(block, directives, replace=False): raise errors.MisconfigurationError( 'LetsEncrypt expected directive for %s in the Nginx ' 'config but did not find it.' % directive[0]) - else: - block.extend(directives) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 977a65330..203f9920c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -63,11 +63,11 @@ class NginxConfiguratorTest(util.NginxTest): # pylint: disable=protected-access parsed = self.config.parser._parse_files(filep, override=True) - self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'], + self.assertEqual([[['server'], [['listen', '5001 ssl'], + ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl']]]], + ['server_name', 'example.*']]]], parsed[0]) def test_choose_vhost(self): @@ -96,18 +96,49 @@ class NginxConfiguratorTest(util.NginxTest): def test_more_info(self): self.assertTrue('nginx.conf' in self.config.more_info()) + def test_deploy_cert_stapling(self): + # Choose a version of Nginx greater than 1.3.7 so stapling code gets + # invoked. + self.config.version = (1, 9, 6) + example_conf = self.config.parser.abs_path('sites-enabled/example.com') + self.config.deploy_cert( + "www.example.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") + self.config.save() + self.config.parser.load() + generated_conf = self.config.parser.parsed[example_conf] + + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_stapling', 'on'], 2)) + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_stapling_verify', 'on'], 2)) + self.assertTrue(util.contains_at_depth(generated_conf, + ['ssl_trusted_certificate', 'example/chain.pem'], 2)) + def test_deploy_cert(self): server_conf = self.config.parser.abs_path('server.conf') nginx_conf = self.config.parser.abs_path('nginx.conf') example_conf = self.config.parser.abs_path('sites-enabled/example.com') + # Choose a version of Nginx less than 1.3.7 so stapling code doesn't get + # invoked. + self.config.version = (1, 3, 1) # Get the default SSL vhost self.config.deploy_cert( "www.example.com", - "example/cert.pem", "example/key.pem") + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") self.config.deploy_cert( "another.alias", - "/etc/nginx/cert.pem", "/etc/nginx/key.pem") + "/etc/nginx/cert.pem", + "/etc/nginx/key.pem", + "/etc/nginx/chain.pem", + "/etc/nginx/fullchain.pem") self.config.save() self.config.parser.load() @@ -119,35 +150,34 @@ class NginxConfiguratorTest(util.NginxTest): access_log = os.path.join(self.work_dir, "access.log") error_log = os.path.join(self.work_dir, "error.log") self.assertEqual([[['server'], - [['listen', '69.50.225.155:9000'], + [['include', self.config.parser.loc["ssl_options"]], + ['ssl_certificate_key', 'example/key.pem'], + ['ssl_certificate', 'example/fullchain.pem'], + ['error_log', error_log], + ['access_log', access_log], + + ['listen', '5001 ssl'], + ['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], - ['server_name', 'example.*'], - ['listen', '5001 ssl'], - ['access_log', access_log], - ['error_log', error_log], - ['ssl_certificate', 'example/cert.pem'], - ['ssl_certificate_key', 'example/key.pem'], - ['include', - self.config.parser.loc["ssl_options"]]]]], + ['server_name', 'example.*']]]], parsed_example_conf) self.assertEqual([['server_name', 'somename alias another.alias']], parsed_server_conf) - self.assertEqual([['server'], - [['listen', '8000'], - ['listen', 'somename:8080'], - ['include', 'server.conf'], - [['location', '/'], - [['root', 'html'], - ['index', 'index.html index.htm']]], - ['listen', '5001 ssl'], - ['access_log', access_log], - ['error_log', error_log], - ['ssl_certificate', '/etc/nginx/cert.pem'], - ['ssl_certificate_key', '/etc/nginx/key.pem'], - ['include', - self.config.parser.loc["ssl_options"]]]], - parsed_nginx_conf[-1][-1][-1]) + self.assertTrue(util.contains_at_depth(parsed_nginx_conf, + [['server'], + [['include', self.config.parser.loc["ssl_options"]], + ['ssl_certificate_key', '/etc/nginx/key.pem'], + ['ssl_certificate', '/etc/nginx/fullchain.pem'], + ['error_log', error_log], + ['access_log', access_log], + ['listen', '5001 ssl'], + ['listen', '8000'], + ['listen', 'somename:8080'], + ['include', 'server.conf'], + [['location', '/'], + [['root', 'html'], ['index', 'index.html index.htm']]]]], + 2)) def test_get_all_certs_keys(self): nginx_conf = self.config.parser.abs_path('nginx.conf') @@ -156,16 +186,22 @@ class NginxConfiguratorTest(util.NginxTest): # Get the default SSL vhost self.config.deploy_cert( "www.example.com", - "example/cert.pem", "example/key.pem") + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + "example/fullchain.pem") self.config.deploy_cert( "another.alias", - "/etc/nginx/cert.pem", "/etc/nginx/key.pem") + "/etc/nginx/cert.pem", + "/etc/nginx/key.pem", + "/etc/nginx/chain.pem", + "/etc/nginx/fullchain.pem") self.config.save() self.config.parser.load() self.assertEqual(set([ - ('example/cert.pem', 'example/key.pem', example_conf), - ('/etc/nginx/cert.pem', '/etc/nginx/key.pem', nginx_conf), + ('example/fullchain.pem', 'example/key.pem', example_conf), + ('/etc/nginx/fullchain.pem', '/etc/nginx/key.pem', nginx_conf), ]), self.config.get_all_certs_keys()) @mock.patch("letsencrypt_nginx.configurator.dvsni.NginxDvsni.perform") diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py index a09bebba2..9fc0a1ad7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py @@ -85,7 +85,8 @@ class DvsniPerformTest(util.NginxTest): # Make sure challenge config is included in main config http = self.sni.configurator.parser.parsed[ self.sni.configurator.parser.loc["root"]][-1] - self.assertTrue(['include', self.sni.challenge_conf] in http[1]) + self.assertTrue( + util.contains_at_depth(http, ['include', self.sni.challenge_conf], 1)) def test_perform2(self): acme_responses = [] @@ -108,7 +109,8 @@ class DvsniPerformTest(util.NginxTest): http = self.sni.configurator.parser.parsed[ self.sni.configurator.parser.loc["root"]][-1] self.assertTrue(['include', self.sni.challenge_conf] in http[1]) - self.assertTrue(['server_name', 'blah'] in http[1][-2][1]) + self.assertTrue( + util.contains_at_depth(http, ['server_name', 'blah'], 3)) self.assertEqual(len(sni_responses), 3) for i in xrange(3): diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 0af81aefe..a22d33e9c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -128,18 +128,18 @@ class NginxParserTest(util.NginxTest): r'~^(www\.)?(example|bar)\.']), [['foo', 'bar'], ['ssl_certificate', '/etc/ssl/cert.pem']]) - ssl_re = re.compile(r'foo bar;\n\s+ssl_certificate /etc/ssl/cert.pem') - self.assertEqual(1, len(re.findall(ssl_re, nginxparser.dumps( - nparser.parsed[nparser.abs_path('nginx.conf')])))) + ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem') + dump = nginxparser.dumps(nparser.parsed[nparser.abs_path('nginx.conf')]) + self.assertEqual(1, len(re.findall(ssl_re, dump))) nparser.add_server_directives(nparser.abs_path('server.conf'), set(['alias', 'another.alias', 'somename']), [['foo', 'bar'], ['ssl_certificate', '/etc/ssl/cert2.pem']]) self.assertEqual(nparser.parsed[nparser.abs_path('server.conf')], - [['server_name', 'somename alias another.alias'], + [['ssl_certificate', '/etc/ssl/cert2.pem'], ['foo', 'bar'], - ['ssl_certificate', '/etc/ssl/cert2.pem']]) + ['server_name', 'somename alias another.alias']]) def test_add_http_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) @@ -148,8 +148,9 @@ class NginxParserTest(util.NginxTest): [['listen', '80'], ['server_name', 'localhost']]] nparser.add_http_directives(filep, block) - self.assertEqual(nparser.parsed[filep][-1][0], ['http']) - self.assertEqual(nparser.parsed[filep][-1][1][-1], block) + root = nparser.parsed[filep] + self.assertTrue(util.contains_at_depth(root, ['http'], 1)) + self.assertTrue(util.contains_at_depth(root, block, 2)) def test_replace_server_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index 363944490..ad4dedfe7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -85,3 +85,18 @@ def filter_comments(tree): yield [key, values] return list(traverse(tree)) + +def contains_at_depth(haystack, needle, n): + """ + Return true if the needle is present in one of the sub-iterables in haystack + at depth n. Haystack must be an iterable. + """ + if not hasattr(haystack, '__iter__'): + return False + if n == 0: + return needle in haystack + else: + for item in haystack: + if contains_at_depth(item, needle, n - 1): + return True + return False diff --git a/letsencrypt-nginx/tests/boulder-integration.conf.sh b/letsencrypt-nginx/tests/boulder-integration.conf.sh index 006d68836..12610d895 100755 --- a/letsencrypt-nginx/tests/boulder-integration.conf.sh +++ b/letsencrypt-nginx/tests/boulder-integration.conf.sh @@ -20,7 +20,6 @@ events { } http { - server_names_hash_bucket_size 2048; # Set an array of temp and cache file options that will otherwise default to # restricted locations accessible only to root. client_body_temp_path $root/client_body; diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 64cba508d..55930e9ff 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -337,9 +337,9 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo lineage = _auth_from_domains(le_client, config, domains, plugins) - # TODO: We also need to pass the fullchain (for Nginx) le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, lineage.chain) + domains, lineage.privkey, lineage.cert, + lineage.chain, lineage.fullchain) le_client.enhance_config(domains, args.redirect) if len(lineage.available_versions("cert")) == 1: @@ -392,7 +392,8 @@ def install(args, config, plugins): args, config, authenticator=None, installer=installer) assert args.cert_path is not None # required=True in the subparser le_client.deploy_certificate( - domains, args.key_path, args.cert_path, args.chain_path) + domains, args.key_path, args.cert_path, args.chain_path, + args.fullchain_path) le_client.enhance_config(domains, args.redirect) @@ -803,6 +804,8 @@ def _paths_parser(helpful): default_cp = None if verb == "auth": default_cp = flag_default("auth_chain_path") + add("paths", "--fullchain-path", default=default_cp, + help="Accompanying path to a full certificate chain (cert plus chain).") add("paths", "--chain-path", default=default_cp, help="Accompanying path to a certificate chain.") add("paths", "--config-dir", default=flag_default("config_dir"), diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 7a78add38..123bab121 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -336,7 +336,8 @@ class Client(object): return os.path.abspath(act_cert_path), cert_chain_abspath - def deploy_certificate(self, domains, privkey_path, cert_path, chain_path): + def deploy_certificate(self, domains, privkey_path, + cert_path, chain_path, fullchain_path): """Install certificate :param list domains: list of domains to install the certificate @@ -357,8 +358,10 @@ class Client(object): # TODO: Provide a fullchain reference for installers like # nginx that want it self.installer.deploy_cert( - dom, os.path.abspath(cert_path), - os.path.abspath(privkey_path), chain_path) + domain=dom, cert_path=os.path.abspath(cert_path), + key_path=os.path.abspath(privkey_path), + chain_path=chain_path, + fullchain_path=fullchain_path) self.installer.save("Deployed Let's Encrypt Certificate") # sites may have been enabled / final cleanup diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 5e82d61aa..8bf714c88 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -241,13 +241,15 @@ class IInstaller(IPlugin): """ - def deploy_cert(domain, cert_path, key_path, chain_path=None): + def deploy_cert(domain, cert_path, key_path, chain_path, fullchain_path): """Deploy certificate. :param str domain: domain to deploy certificate file :param str cert_path: absolute path to the certificate file :param str key_path: absolute path to the private key file :param str chain_path: absolute path to the certificate chain file + :param str fullchain_path: absolute path to the certificate fullchain + file (cert plus chain) :raises .PluginError: when cert cannot be deployed diff --git a/letsencrypt/plugins/null.py b/letsencrypt/plugins/null.py index 4ba6c9d64..632fe415a 100644 --- a/letsencrypt/plugins/null.py +++ b/letsencrypt/plugins/null.py @@ -30,7 +30,8 @@ class Installer(common.Plugin): def get_all_names(self): return [] - def deploy_cert(self, domain, cert_path, key_path, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, + chain_path=None, fullchain_path=None): pass # pragma: no cover def enhance(self, domain, enhancement, options=None): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 1e63bdbb6..fddb86607 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -177,15 +177,19 @@ class ClientTest(unittest.TestCase): def test_deploy_certificate(self): self.assertRaises(errors.Error, self.client.deploy_certificate, - ["foo.bar"], "key", "cert", "chain") + ["foo.bar"], "key", "cert", "chain", "fullchain") installer = mock.MagicMock() self.client.installer = installer - self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain") + self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain", + "fullchain") installer.deploy_cert.assert_called_once_with( - "foo.bar", os.path.abspath("cert"), - os.path.abspath("key"), os.path.abspath("chain")) + cert_path=os.path.abspath("cert"), + chain_path=os.path.abspath("chain"), + domain='foo.bar', + fullchain_path='fullchain', + key_path=os.path.abspath("key")) self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index 530f9c598..8780cac7c 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -38,5 +38,5 @@ if ! go get bitbucket.org/liamstask/goose/cmd/goose ; then exit 1 fi ./test/create_db.sh -./start.py & +./start.py > /dev/null & # Hopefully start.py bootstraps before integration test is started... From 4c73db9aa1c792a831599a69fa6991a636ef501b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Sun, 11 Oct 2015 10:34:32 -0700 Subject: [PATCH 24/40] Revert "Fixed version string" This reverts commit d1ee8311379dd1c102ed54ed1ae827b527cd255f. --- letsencrypt-compatibility-test/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-compatibility-test/setup.py b/letsencrypt-compatibility-test/setup.py index 5a54239dd..2e70fd1d7 100644 --- a/letsencrypt-compatibility-test/setup.py +++ b/letsencrypt-compatibility-test/setup.py @@ -4,7 +4,7 @@ from setuptools import setup from setuptools import find_packages -version = '0.0.0.dev20151008' +version = '0.1.0.dev0' install_requires = [ 'letsencrypt=={0}'.format(version), From cd52fc02b9be44c742c88c4c2117a887c2478188 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 11:20:21 -0700 Subject: [PATCH 25/40] Add a sleep to let Nginx finish reloading. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index ffca041ca..426500eb3 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -6,6 +6,7 @@ import shutil import socket import subprocess import sys +import time import OpenSSL import zope.interface @@ -612,6 +613,10 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"): except (OSError, ValueError): logger.fatal("Nginx Restart Failed - Please Check the Configuration") sys.exit(1) + # Nginx can take a moment to recognize a newly added TLS SNI servername, so sleep + # for a second. TODO: Check for expected servername and loop until it + # appears or return an error if looping too long. + time.sleep(1) return True From f0cfd69cdcd62f3c9e97f0828357a2c31104e9d4 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 11:28:39 -0700 Subject: [PATCH 26/40] Respond to review feedback. --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 8 ++++---- letsencrypt-nginx/letsencrypt_nginx/dvsni.py | 5 ++--- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 6 ++++-- tests/boulder-start.sh | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index a3c2bd186..91aa02aef 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -164,7 +164,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): temp_install(self.mod_ssl_conf) def deploy_cert(self, domain, cert_path, key_path, - chain_path=None, fullchain_path=None): # pylint: disable=unused-argument + chain_path=None, fullchain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. Currently tries to find the last directives to deploy the cert in diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 426500eb3..8f7431a72 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -149,13 +149,13 @@ class NginxConfigurator(common.Plugin): try: self.parser.add_server_directives(vhost.filep, vhost.names, - cert_directives, True) + cert_directives, replace=True) self.parser.add_server_directives(vhost.filep, vhost.names, - stapling_directives, False) + stapling_directives, replace=False) logger.info("Deployed Certificate to VirtualHost %s for %s", vhost.filep, vhost.names) - except errors.MisconfigurationError, e: - logger.debug(e) + except errors.MisconfigurationError as error: + logger.debug(error) logger.warn( "Cannot find a cert or key directive in %s for %s. " "VirtualHost was not modified.", vhost.filep, vhost.names) diff --git a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py index f384082ef..9ac2fcd7c 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/dvsni.py +++ b/letsencrypt-nginx/letsencrypt_nginx/dvsni.py @@ -96,11 +96,10 @@ class NginxDvsni(common.Dvsni): bucket_directive = ['server_names_hash_bucket_size', '128'] main = self.configurator.parser.parsed[root] - for key, value in main: + for key, body in main: if key == ['http']: - body = value found_bucket = False - for key, value in body: # pylint: disable=unused-variable + for key, _ in body: if key == bucket_directive[0]: found_bucket = True if not found_bucket: diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index ad4dedfe7..9cfa6a1a1 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -86,12 +86,14 @@ def filter_comments(tree): return list(traverse(tree)) + def contains_at_depth(haystack, needle, n): - """ + """Is the needle in haystack at depth n? + Return true if the needle is present in one of the sub-iterables in haystack at depth n. Haystack must be an iterable. """ - if not hasattr(haystack, '__iter__'): + if not isinstance(haystack, collections.Iterable): return False if n == 0: return needle in haystack diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index 8780cac7c..530f9c598 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -38,5 +38,5 @@ if ! go get bitbucket.org/liamstask/goose/cmd/goose ; then exit 1 fi ./test/create_db.sh -./start.py > /dev/null & +./start.py & # Hopefully start.py bootstraps before integration test is started... From 06c85d6b5a88be9780f7c48b3e92f6d3b628424d Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 11:30:11 -0700 Subject: [PATCH 27/40] Fix line-wrapped function indents. --- .../configurators/apache/common.py | 2 +- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 2 +- letsencrypt/plugins/null.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py index bcc66b98a..5f183b611 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/common.py @@ -176,7 +176,7 @@ class Proxy(configurators_common.Proxy): return {"example.com"} def deploy_cert(self, domain, cert_path, key_path, chain_path=None, - fullchain_path=None): + fullchain_path=None): """Installs cert""" cert_path, key_path, chain_path = self.copy_certs_and_keys( cert_path, key_path, chain_path) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 8f7431a72..d1ab8f3d1 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -119,7 +119,7 @@ class NginxConfigurator(common.Plugin): # Entry point in main.py for installing cert def deploy_cert(self, domain, cert_path, key_path, - chain_path, fullchain_path): + chain_path, fullchain_path): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. diff --git a/letsencrypt/plugins/null.py b/letsencrypt/plugins/null.py index 632fe415a..e87537684 100644 --- a/letsencrypt/plugins/null.py +++ b/letsencrypt/plugins/null.py @@ -31,7 +31,7 @@ class Installer(common.Plugin): return [] def deploy_cert(self, domain, cert_path, key_path, - chain_path=None, fullchain_path=None): + chain_path=None, fullchain_path=None): pass # pragma: no cover def enhance(self, domain, enhancement, options=None): From f16489f762f3e0dd50f010850e9b61c7d01db4df Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 12:19:39 -0700 Subject: [PATCH 28/40] Go back to hasattr and add a test. --- .../letsencrypt_nginx/tests/parser_test.py | 12 ++++++++++-- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 4 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index a22d33e9c..40ef37e2f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -131,12 +131,14 @@ class NginxParserTest(util.NginxTest): ssl_re = re.compile(r'\n\s+ssl_certificate /etc/ssl/cert.pem') dump = nginxparser.dumps(nparser.parsed[nparser.abs_path('nginx.conf')]) self.assertEqual(1, len(re.findall(ssl_re, dump))) - nparser.add_server_directives(nparser.abs_path('server.conf'), + + server_conf = nparser.abs_path('server.conf') + nparser.add_server_directives(server_conf, set(['alias', 'another.alias', 'somename']), [['foo', 'bar'], ['ssl_certificate', '/etc/ssl/cert2.pem']]) - self.assertEqual(nparser.parsed[nparser.abs_path('server.conf')], + self.assertEqual(nparser.parsed[server_conf], [['ssl_certificate', '/etc/ssl/cert2.pem'], ['foo', 'bar'], ['server_name', 'somename alias another.alias']]) @@ -152,6 +154,12 @@ class NginxParserTest(util.NginxTest): self.assertTrue(util.contains_at_depth(root, ['http'], 1)) self.assertTrue(util.contains_at_depth(root, block, 2)) + # Check that our server block got inserted first among all server + # blocks. + http_block = filter(lambda x: x[0] == ['http'], root)[0][1] + server_blocks = filter(lambda x: x[0] == ['server'], http_block) + self.assertEqual(server_blocks[0], block) + def test_replace_server_directives(self): nparser = parser.NginxParser(self.config_path, self.ssl_options) target = set(['.example.com', 'example.*']) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index 9cfa6a1a1..953c5d367 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -93,7 +93,9 @@ def contains_at_depth(haystack, needle, n): Return true if the needle is present in one of the sub-iterables in haystack at depth n. Haystack must be an iterable. """ - if not isinstance(haystack, collections.Iterable): + # Specifically use hasattr rather than isinstance(..., collections.Iterable) + # because we want to include lists but reject strings. + if not hasattr(haystack, '__iter__'): return False if n == 0: return needle in haystack From 216e589d461b33706c0014aa49b666f6e46f1c3a Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 11 Oct 2015 12:30:02 -0700 Subject: [PATCH 29/40] Replace lambda with comprehension. --- letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 40ef37e2f..b28640d7f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -156,8 +156,8 @@ class NginxParserTest(util.NginxTest): # Check that our server block got inserted first among all server # blocks. - http_block = filter(lambda x: x[0] == ['http'], root)[0][1] - server_blocks = filter(lambda x: x[0] == ['server'], http_block) + http_block = [x for x in root if x[0] == ['http']][0][1] + server_blocks = [x for x in http_block if x[0] == ['server']] self.assertEqual(server_blocks[0], block) def test_replace_server_directives(self): From a809a059f0512159279b5dcbc100374393327e4b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Oct 2015 15:57:57 -0400 Subject: [PATCH 30/40] Don't create renewal_configs_dir and config_dir, instead just print a helpful error message and fail --- letsencrypt/renewer.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index ebd9a42d1..76922e6fd 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -169,14 +169,14 @@ def main(config=None, cli_args=sys.argv[1:]): config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) # Ensure that all of the needed folders have been created before continuing uid = os.geteuid() - le_util.make_or_verify_dir( - cli_config.renewal_configs_dir, constants.CONFIG_DIRS_MODE, uid) - le_util.make_or_verify_dir( - cli_config.config_dir, constants.CONFIG_DIRS_MODE, uid) - le_util.make_or_verify_dir( - cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) - le_util.make_or_verify_dir( - cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) + if (not os.path.isdir(cli_config.renewal_configs_dir) or + not os.path.isdir(cli_config.config_dir)): + print "Could not find config directory. Exiting. " + else: + le_util.make_or_verify_dir( + cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i From 8ec7fdd323b6e814ae54629e507ca23f0e736beb Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 11 Oct 2015 16:20:18 -0400 Subject: [PATCH 31/40] Always create the folders --- letsencrypt/renewer.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 76922e6fd..bec868483 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -172,11 +172,10 @@ def main(config=None, cli_args=sys.argv[1:]): if (not os.path.isdir(cli_config.renewal_configs_dir) or not os.path.isdir(cli_config.config_dir)): print "Could not find config directory. Exiting. " - else: - le_util.make_or_verify_dir( - cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) - le_util.make_or_verify_dir( - cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir( + cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i From 52f7a64b8442f099ea581b407ffaa9cae4a4cefb Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 11 Oct 2015 17:56:30 -0400 Subject: [PATCH 32/40] lint newline --- letsencrypt/plugins/manual.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 65d7d6876..99463c362 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -134,7 +134,6 @@ binary for temporary key/certificate generation.""".replace("\n", "") executable = "/bin/bash" else: executable = None - try: self._httpd = subprocess.Popen( command, From 90f3b26bcff975f87a26357659543b69dad2492f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Sun, 11 Oct 2015 17:29:19 -0700 Subject: [PATCH 33/40] Fixed a2enmod --- .../configurators/apache/a2enmod.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh index 0a8ade4c2..60a62407a 100755 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh @@ -1,6 +1,6 @@ #!/bin/bash # An extremely simplified version of `a2enmod` for enabling modules in the -# httpd docker image. First argument is the server_root and the second is the +# httpd docker image. First argument is the ServerRoot and the second is the # module to be enabled. confdir=$1 @@ -8,10 +8,11 @@ module=$2 echo "LoadModule ${module}_module " \ "/usr/local/apache2/modules/mod_${module}.so" >> "${confdir}/test.conf" -available_conf=$APACHE_CONFDIR"/mods-available/${module}.conf" -enabled_dir=$APACHE_CONFDIR"/mods-enabled" -enabled_conf=$enabled_dir"/"$1".conf" -if [ -e "$available_conf" -a -d "$enabled_dir" -a ! -e "$enabled_conf" ] +availbase="/mods-available/${module}.conf" +availconf=$confdir$availbase +enabldir="$confdir/mods-enabled" +enablconf="$enabldir/${module}.conf" +if [ -e $availconf -a -d $enabldir -a ! -e $enablconf ] then - ln -s "..$available_base" $enabled_conf + ln -s "..$availbase" $enablconf fi From 9c59b5089408e9e93d5436def0f747a8202ee913 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Sun, 11 Oct 2015 17:33:00 -0700 Subject: [PATCH 34/40] Reverted incorrect change from 18adec0 --- .../letsencrypt_compatibility_test/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py index 816f04398..dd4fd3b48 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py @@ -40,7 +40,7 @@ def create_le_config(parent_dir): def extract_configs(configs, parent_dir): """Extracts configs to a new dir under parent_dir and returns it""" - config_dir = os.path.join(parent_dir, "renewal") + config_dir = os.path.join(parent_dir, "configs") if os.path.isdir(configs): shutil.copytree(configs, config_dir, symlinks=True) From 67ee9bf930b95c9975920a6f3079d336bbfc5520 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 12 Oct 2015 10:00:35 -0700 Subject: [PATCH 35/40] Added 'strict_permissions' to constants.py --- .../letsencrypt_compatibility_test/util.py | 1 - letsencrypt/constants.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py index dd4fd3b48..43070cf03 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py @@ -25,7 +25,6 @@ IP_REGEX = re.compile(r"^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$") def create_le_config(parent_dir): """Sets up LE dirs in parent_dir and returns the config dict""" config = copy.deepcopy(constants.CLI_DEFAULTS) - config["strict_permissions"] = False le_dir = os.path.join(parent_dir, "letsencrypt") config["config_dir"] = os.path.join(le_dir, "config") diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 762409d25..362009ec6 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -27,6 +27,7 @@ CLI_DEFAULTS = dict( auth_cert_path="./cert.pem", auth_chain_path="./chain.pem", + strict_permissions=False, ) """Defaults for CLI flags and `.IConfig` attributes.""" From 7defdb18193500cfb287bde092a089c087887b33 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 12 Oct 2015 10:08:35 -0700 Subject: [PATCH 36/40] Updated a2enmod comments --- .../configurators/apache/a2enmod.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh index 60a62407a..4da9288a2 100755 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/a2enmod.sh @@ -1,7 +1,7 @@ #!/bin/bash # An extremely simplified version of `a2enmod` for enabling modules in the -# httpd docker image. First argument is the ServerRoot and the second is the -# module to be enabled. +# httpd docker image. First argument is the Apache ServerRoot which should be +# an absolute path. The second is the module to be enabled, such as `ssl`. confdir=$1 module=$2 From 589145686fd4b3f92b90357f39f05fcfa1900b5b Mon Sep 17 00:00:00 2001 From: David Dworken Date: Mon, 12 Oct 2015 15:02:07 -0400 Subject: [PATCH 37/40] Don't print error message and only call os.geteuid() once --- letsencrypt/renewer.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index bec868483..ea1eb5ef7 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -153,7 +153,8 @@ def main(config=None, cli_args=sys.argv[1:]): args = _create_parser().parse_args(cli_args) - le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid()) + uid = os.geteuid() + le_util.make_or_verify_dir(args.logs_dir, 0o700, uid) cli.setup_logging(args, _cli_log_handler, logfile='renewer.log') cli_config = configuration.RenewerConfiguration(args) @@ -168,10 +169,6 @@ def main(config=None, cli_args=sys.argv[1:]): # take precedence over this one. config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) # Ensure that all of the needed folders have been created before continuing - uid = os.geteuid() - if (not os.path.isdir(cli_config.renewal_configs_dir) or - not os.path.isdir(cli_config.config_dir)): - print "Could not find config directory. Exiting. " le_util.make_or_verify_dir( cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) le_util.make_or_verify_dir( From 20d7576f662999c2e53a6bd48d353fcd41a8b126 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Mon, 12 Oct 2015 15:14:13 -0400 Subject: [PATCH 38/40] Deleted duplicate line caused by #912 --- letsencrypt/renewer.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index ea1eb5ef7..b62e31bce 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -171,8 +171,6 @@ def main(config=None, cli_args=sys.argv[1:]): # Ensure that all of the needed folders have been created before continuing le_util.make_or_verify_dir( cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) - le_util.make_or_verify_dir( - cli_config.logs_dir, constants.CONFIG_DIRS_MODE, uid) for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i From d4af07a7f86d5b6f052a184ef137a6a5a819b94f Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Mon, 12 Oct 2015 16:43:22 -0400 Subject: [PATCH 39/40] PEP8 Love: E126 Fix for #945 --- letsencrypt/renewer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index b62e31bce..8d8540e6f 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -169,8 +169,8 @@ def main(config=None, cli_args=sys.argv[1:]): # take precedence over this one. config.merge(configobj.ConfigObj(cli_config.renewer_config_file)) # Ensure that all of the needed folders have been created before continuing - le_util.make_or_verify_dir( - cli_config.work_dir, constants.CONFIG_DIRS_MODE, uid) + le_util.make_or_verify_dir(cli_config.work_dir, + constants.CONFIG_DIRS_MODE, uid) for i in os.listdir(cli_config.renewal_configs_dir): print "Processing", i From c7732114cbedc915d14526f5c96a1a17e2becadb Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 13 Oct 2015 14:50:23 -0700 Subject: [PATCH 40/40] Only test CLI for nginx plugin if it is present - Fixes 919 --- letsencrypt/tests/cli_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index d0fae370d..f690e77f9 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -75,7 +75,10 @@ class CLITest(unittest.TestCase): output.truncate(0) self.assertRaises(SystemExit, self._call_stdout, ['-h', 'nginx']) out = output.getvalue() - self.assertTrue("--nginx-ctl" in out) + from letsencrypt.plugins import disco + if "nginx" in disco.PluginsRegistry.find_all(): + # may be false while building distributions without plugins + self.assertTrue("--nginx-ctl" in out) self.assertTrue("--manual-test-mode" not in out) self.assertTrue("--checkpoints" not in out) output.truncate(0)