From 1fff04ea9e95914c33c949fdfc7ddf72b7a9396b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 15 Sep 2015 18:51:24 -0700 Subject: [PATCH 01/36] Change the renewal configuration directory Fixes #732 --- letsencrypt/constants.py | 2 +- letsencrypt/tests/renewer_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 0d00f2d75..6c67ce445 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -88,7 +88,7 @@ LIVE_DIR = "live" TEMP_CHECKPOINT_DIR = "temp_checkpoint" """Temporary checkpoint directory (relative to `IConfig.work_dir`).""" -RENEWAL_CONFIGS_DIR = "configs" +RENEWAL_CONFIGS_DIR = "renewal" """Renewal configs directory, relative to `IConfig.config_dir`.""" RENEWER_CONFIG_FILENAME = "renewer.conf" diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 053f00ed8..761783d23 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -43,7 +43,7 @@ class BaseRenewableCertTest(unittest.TestCase): # TODO: maybe provide RenewerConfiguration.make_dirs? 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, "configs")) + os.makedirs(os.path.join(self.tempdir, "renewal")) config = configobj.ConfigObj() for kind in ALL_FOUR: From 03e2f043df5f2c353ad5fe787d158110323c38cd Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 16 Sep 2015 06:49:04 +0000 Subject: [PATCH 02/36] Address #726 review comments --- docs/contributing.rst | 15 +++++++-------- tests/boulder-integration.sh | 2 ++ tests/boulder-start.sh | 1 + 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 00ac509ab..f85e37c39 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -68,17 +68,16 @@ The following tools are there to help you: Integration ~~~~~~~~~~~ -First, install `Go`_ 1.5 (pick a value for GOPATH and put $GOPATH/bin in your -PATH), libtool-ltdl, mariadb-server and rabbitmq-server and then start -Boulder_, an ACME CA server:: +First, install `Go`_ 1.5, libtool-ltdl, mariadb-server and +rabbitmq-server and then start Boulder_, an ACME CA server:: ./tests/boulder-start.sh -The script will download, compile and run the executable; please be patient - -it will take some time... Once its ready, you will see ``Server running, -listening on 127.0.0.1:4000...``. Add the ``venv/bin/`` subdirectory of your -letsencrypt repo to your path, and add an ``/etc/hosts`` entry pointing -``le.wtf`` to 127.0.0.1. You may now run (in a separate terminal):: +The script will download, compile and run the executable; please be +patient - it will take some time... Once its ready, you will see +``Server running, listening on 127.0.0.1:4000...``. Add an +``/etc/hosts`` entry pointing ``le.wtf`` to 127.0.0.1. You may now +run (in a separate terminal):: ./tests/boulder-integration.sh && echo OK || echo FAIL diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 67cc4c5e9..ed877d136 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -11,6 +11,8 @@ . ./tests/integration/_common.sh export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx +export GOPATH="${GOPATH:-/tmp/go}" +export PATH="$GOPATH/bin:$PATH" common() { letsencrypt_test \ diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index e17716b54..0875c3d2f 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -2,6 +2,7 @@ # Download and run Boulder instance for integration testing export GOPATH="${GOPATH:-/tmp/go}" +export PATH="$GOPATH/bin:$PATH" # `/...` avoids `no buildable Go source files` errors, for more info # see `go help packages` From 1a2c983a9cc5c47154b2751ebcf2194ff65cbd84 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 13:13:24 -0700 Subject: [PATCH 03/36] Strict permission checking only upon request Use --strict-permissions if you're running as a privileged user on a system where non-privileged users might have write permissions to parts of the lets encrypt config or logging heirarchy. That should not normally be the case. Working toward a fix for #552 --- letsencrypt/account.py | 6 ++++-- letsencrypt/cli.py | 11 +++++++++-- letsencrypt/client.py | 3 ++- letsencrypt/crypto_util.py | 12 ++++++++++-- letsencrypt/le_util.py | 7 ++++--- letsencrypt/reverter.py | 9 ++++++--- letsencrypt/revoker.py | 6 ++++-- letsencrypt/tests/le_util_test.py | 2 +- 8 files changed, 40 insertions(+), 16 deletions(-) diff --git a/letsencrypt/account.py b/letsencrypt/account.py index e705b1484..8bee22102 100644 --- a/letsencrypt/account.py +++ b/letsencrypt/account.py @@ -129,8 +129,9 @@ class AccountFileStorage(interfaces.AccountStorage): """ def __init__(self, config): - le_util.make_or_verify_dir(config.accounts_dir, 0o700, os.geteuid()) self.config = config + le_util.make_or_verify_dir(config.accounts_dir, 0o700, os.geteuid(), + self.config.strict_permissions) def _account_dir_path(self, account_id): return os.path.join(self.config.accounts_dir, account_id) @@ -186,7 +187,8 @@ class AccountFileStorage(interfaces.AccountStorage): def save(self, account): account_dir_path = self._account_dir_path(account.id) - le_util.make_or_verify_dir(account_dir_path, 0o700, os.geteuid()) + le_util.make_or_verify_dir(account_dir_path, 0o700, os.geteuid(), + self.config.strict_permissions) try: with open(self._regr_path(account_dir_path), "w") as regr_file: regr_file.write(account.regr.json_dumps()) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d2f8ddc2d..a6ffd7df9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -659,6 +659,10 @@ def create_parser(plugins, args): "security", "-r", "--redirect", action="store_true", help="Automatically redirect all HTTP traffic to HTTPS for the newly " "authenticated vhost.") + helpful.add( + "security", "--strict-permissions", action="store_true", + help="Require that all configuration files are owned by the current " + "user; use this if your config is in /tmp/") _paths_parser(helpful) # _plugins_parsing should be the last thing to act upon the main @@ -863,15 +867,18 @@ def main(cli_args=sys.argv[1:]): parser, tweaked_cli_args = create_parser(plugins, cli_args) args = parser.parse_args(tweaked_cli_args) config = configuration.NamespaceConfig(args) + zope.component.provideUtility(config) # Setup logging ASAP, otherwise "No handlers could be found for # logger ..." TODO: this should be done before plugins discovery for directory in config.config_dir, config.work_dir: le_util.make_or_verify_dir( - directory, constants.CONFIG_DIRS_MODE, os.geteuid()) + directory, constants.CONFIG_DIRS_MODE, os.geteuid(), + "--strict-permissions" in cli_args) # TODO: logs might contain sensitive data such as contents of the # private key! #525 - le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid()) + le_util.make_or_verify_dir( + args.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args) _setup_logging(args) # do not log `args`, as it contains sensitive data (e.g. revoke --key)! diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e62d34517..60eaea5a1 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -315,7 +315,8 @@ class Client(object): """ for path in cert_path, chain_path: le_util.make_or_verify_dir( - os.path.dirname(path), 0o755, os.geteuid()) + os.path.dirname(path), 0o755, os.geteuid(), + self.config.strict_permissions) # try finally close cert_chain_abspath = None diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 71628677e..be2a84c2a 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -9,12 +9,15 @@ import logging import os import OpenSSL +import zope.component from acme import crypto_util as acme_crypto_util from acme import jose from letsencrypt import errors from letsencrypt import le_util +from letsencrypt import interfaces + logger = logging.getLogger(__name__) @@ -45,8 +48,10 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): logger.exception(err) raise err + config = zope.component.getUtility(interfaces.IConfig) # Save file - le_util.make_or_verify_dir(key_dir, 0o700, os.geteuid()) + le_util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), + config.strict_permissions) key_f, key_path = le_util.unique_file( os.path.join(key_dir, keyname), 0o600) key_f.write(key_pem) @@ -73,8 +78,11 @@ def init_save_csr(privkey, names, path, csrname="csr-letsencrypt.pem"): """ csr_pem, csr_der = make_csr(privkey.pem, names) + + config = zope.component.getUtility(interfaces.IConfig) # Save CSR - le_util.make_or_verify_dir(path, 0o755, os.geteuid()) + le_util.make_or_verify_dir(path, 0o755, os.geteuid(), + config.strict_permissions) csr_f, csr_filename = le_util.unique_file( os.path.join(path, csrname), 0o644) csr_f.write(csr_pem) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 194a80201..d7b0bea48 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -70,7 +70,7 @@ def exe_exists(exe): return False -def make_or_verify_dir(directory, mode=0o755, uid=0): +def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False): """Make sure directory exists with proper permissions. :param str directory: Path to a directory. @@ -89,9 +89,10 @@ def make_or_verify_dir(directory, mode=0o755, uid=0): os.makedirs(directory, mode) except OSError as exception: if exception.errno == errno.EEXIST: - if not check_permissions(directory, mode, uid): + if strict and not check_permissions(directory, mode, uid): raise errors.Error( - "%s exists, this client can't access it" % directory) + "%s exists, but it should be owned by user %d with" + "permissions %d" % (directory, uid, oct(mode))) else: raise diff --git a/letsencrypt/reverter.py b/letsencrypt/reverter.py index 8eed59156..d5114ae71 100644 --- a/letsencrypt/reverter.py +++ b/letsencrypt/reverter.py @@ -31,7 +31,8 @@ class Reverter(object): self.config = config le_util.make_or_verify_dir( - config.backup_dir, constants.CONFIG_DIRS_MODE, os.geteuid()) + config.backup_dir, constants.CONFIG_DIRS_MODE, os.geteuid(), + self.config.strict_permissions) def revert_temporary_config(self): """Reload users original configuration files after a temporary save. @@ -180,7 +181,8 @@ class Reverter(object): """ le_util.make_or_verify_dir( - cp_dir, constants.CONFIG_DIRS_MODE, os.geteuid()) + cp_dir, constants.CONFIG_DIRS_MODE, os.geteuid(), + self.config.strict_permissions) op_fd, existing_filepaths = self._read_and_append( os.path.join(cp_dir, "FILEPATHS")) @@ -393,7 +395,8 @@ class Reverter(object): cp_dir = self.config.in_progress_dir le_util.make_or_verify_dir( - cp_dir, constants.CONFIG_DIRS_MODE, os.geteuid()) + cp_dir, constants.CONFIG_DIRS_MODE, os.geteuid(), + self.config.strict_permissions) return cp_dir diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index e8b154012..239879542 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -54,7 +54,8 @@ class Revoker(object): self.config = config self.no_confirm = no_confirm - le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid()) + le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid(), + self.config.strict_permissions) # TODO: Find a better solution for this... self.list_path = os.path.join(config.cert_key_backup, "LIST") @@ -333,7 +334,8 @@ class Revoker(object): """ list_path = os.path.join(config.cert_key_backup, "LIST") - le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid()) + le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid(), + config.strict_permissions) cls._catalog_files( config.cert_key_backup, cert_path, key_path, list_path) diff --git a/letsencrypt/tests/le_util_test.py b/letsencrypt/tests/le_util_test.py index 98b7eb803..ed976f72d 100644 --- a/letsencrypt/tests/le_util_test.py +++ b/letsencrypt/tests/le_util_test.py @@ -92,7 +92,7 @@ class MakeOrVerifyDirTest(unittest.TestCase): def _call(self, directory, mode): from letsencrypt.le_util import make_or_verify_dir - return make_or_verify_dir(directory, mode, self.uid) + return make_or_verify_dir(directory, mode, self.uid, strict=True) def test_creates_dir_when_missing(self): path = os.path.join(self.root_path, "bar") From 9315161ef2cde571f94edff2bf9471c661fde45c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 13:20:31 -0700 Subject: [PATCH 04/36] Better documentation --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a6ffd7df9..415b08d81 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -662,7 +662,7 @@ def create_parser(plugins, args): helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " - "user; use this if your config is in /tmp/") + "user; only needed if your config is somewhere unsafe like /tmp/") _paths_parser(helpful) # _plugins_parsing should be the last thing to act upon the main From e570dac3c6ebd8ee4dd421ebe3cbc432502d5fe7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 13:21:21 -0700 Subject: [PATCH 05/36] fix type error --- letsencrypt/le_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index d7b0bea48..ffc7da190 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -92,7 +92,7 @@ def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False): if strict and not check_permissions(directory, mode, uid): raise errors.Error( "%s exists, but it should be owned by user %d with" - "permissions %d" % (directory, uid, oct(mode))) + "permissions %s" % (directory, uid, oct(mode))) else: raise From 0325c6cde6fc99b2f3e35f10f16ef345c3587c62 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 15:56:06 -0700 Subject: [PATCH 06/36] Make config singleton acquisition more robust Fixing failures in testing environments --- letsencrypt/crypto_util.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index be2a84c2a..ef66c8af2 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -9,14 +9,12 @@ import logging import os import OpenSSL -import zope.component from acme import crypto_util as acme_crypto_util from acme import jose from letsencrypt import errors from letsencrypt import le_util -from letsencrypt import interfaces @@ -48,6 +46,8 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): logger.exception(err) raise err + import zope.component + from letsencrypt import interfaces config = zope.component.getUtility(interfaces.IConfig) # Save file le_util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), @@ -78,7 +78,8 @@ def init_save_csr(privkey, names, path, csrname="csr-letsencrypt.pem"): """ csr_pem, csr_der = make_csr(privkey.pem, names) - + import zope.component + from letsencrypt import interfaces config = zope.component.getUtility(interfaces.IConfig) # Save CSR le_util.make_or_verify_dir(path, 0o755, os.geteuid(), From f450a290c35790e29d43d09aa5750f55dfbe18de Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 16:49:39 -0700 Subject: [PATCH 07/36] Ensure test cases have a config singleton --- letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py | 2 +- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 977a65330..b9512dde7 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -13,7 +13,7 @@ from letsencrypt import achallenges from letsencrypt import errors from letsencrypt_nginx.tests import util - +import zope.component class NginxConfiguratorTest(util.NginxTest): """Test a semi complex vhost configuration.""" diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index 9c8c6a5dd..1ea41b83d 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -4,6 +4,7 @@ import pkg_resources import unittest import mock +import zope.component from acme import jose @@ -60,6 +61,7 @@ def get_nginx_configurator( name="nginx", version=version) config.prepare() + zope.component.provideUtility(config) return config From d89b695be6a868d6a14bc15efe914da8cf245150 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 16:58:51 -0700 Subject: [PATCH 08/36] client and nginx configs are not the same thing... --- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index 1ea41b83d..e6fd9daf1 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -61,7 +61,13 @@ def get_nginx_configurator( name="nginx", version=version) config.prepare() - zope.component.provideUtility(config) + # also make a general client config for good measure... + namespace = mock.MagicMock( + config_dir='/tmp/config', work_dir='/tmp/foo', foo='bar', + server='https://acme-server.org:443/new') + from letsencrypt.configuration import NamespaceConfig + nsconfig = NamespaceConfig(namespace) + zope.component.provideUtility(nsconfig) return config From 630c715350f48d931bb6ea8434c37ead2dad65bf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 17:03:09 -0700 Subject: [PATCH 09/36] lintmonster --- letsencrypt/cli.py | 2 +- letsencrypt/crypto_util.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 415b08d81..ab03a576f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -675,7 +675,7 @@ def create_parser(plugins, args): # For now unfortunately this constant just needs to match the code below; # there isn't an elegant way to autogenerate it in time. -VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",\ +VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", "plugins"] diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index ef66c8af2..a0fbb9bf7 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -16,8 +16,6 @@ from acme import jose from letsencrypt import errors from letsencrypt import le_util - - logger = logging.getLogger(__name__) From 110f080de019a33b96cc5bf55820506984dc6aa0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 17:15:10 -0700 Subject: [PATCH 10/36] The renewer also needs a config singleton --- letsencrypt/renewer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 5f73a7dad..1c9cddc95 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -70,6 +70,7 @@ def renew(cert, old_version): # was an int, not a str) config.rsa_key_size = int(config.rsa_key_size) config.dvsni_port = int(config.dvsni_port) + zope.component.provideUtility(config) try: authenticator = plugins[renewalparams["authenticator"]] except KeyError: From 43a73f9a09f1199ffecdfaae102df9a464952348 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 17:15:56 -0700 Subject: [PATCH 11/36] neaten neaten --- letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py | 1 - letsencrypt/tests/configuration_test.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index b9512dde7..93b04f56f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -13,7 +13,6 @@ from letsencrypt import achallenges from letsencrypt import errors from letsencrypt_nginx.tests import util -import zope.component class NginxConfiguratorTest(util.NginxTest): """Test a semi complex vhost configuration.""" diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 498147c6d..bd5378b09 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -1,10 +1,8 @@ """Tests for letsencrypt.configuration.""" import os import unittest - import mock - class NamespaceConfigTest(unittest.TestCase): """Tests for letsencrypt.configuration.NamespaceConfig.""" From 67acebff34af3adffd80b544582a891d6febb971 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 18:43:32 -0700 Subject: [PATCH 12/36] pep8 and google style guide --- .../letsencrypt_nginx/tests/configurator_test.py | 1 + letsencrypt/crypto_util.py | 7 +++---- letsencrypt/tests/configuration_test.py | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 93b04f56f..977a65330 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -14,6 +14,7 @@ from letsencrypt import errors from letsencrypt_nginx.tests import util + class NginxConfiguratorTest(util.NginxTest): """Test a semi complex vhost configuration.""" diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index a0fbb9bf7..79cd24ed6 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -9,13 +9,16 @@ import logging import os import OpenSSL +import zope.component from acme import crypto_util as acme_crypto_util from acme import jose from letsencrypt import errors +from letsencrypt import interfaces from letsencrypt import le_util + logger = logging.getLogger(__name__) @@ -44,8 +47,6 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"): logger.exception(err) raise err - import zope.component - from letsencrypt import interfaces config = zope.component.getUtility(interfaces.IConfig) # Save file le_util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), @@ -76,8 +77,6 @@ def init_save_csr(privkey, names, path, csrname="csr-letsencrypt.pem"): """ csr_pem, csr_der = make_csr(privkey.pem, names) - import zope.component - from letsencrypt import interfaces config = zope.component.getUtility(interfaces.IConfig) # Save CSR le_util.make_or_verify_dir(path, 0o755, os.geteuid(), diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index bd5378b09..498147c6d 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -1,8 +1,10 @@ """Tests for letsencrypt.configuration.""" import os import unittest + import mock + class NamespaceConfigTest(unittest.TestCase): """Tests for letsencrypt.configuration.NamespaceConfig.""" From edbd0a77b236750a0c7df53cd8376176bdeb8df3 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 18:52:11 -0700 Subject: [PATCH 13/36] Rework config utility --- letsencrypt-nginx/letsencrypt_nginx/tests/util.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py index e6fd9daf1..363944490 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/util.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/util.py @@ -8,6 +8,8 @@ import zope.component from acme import jose +from letsencrypt import configuration + from letsencrypt.tests import test_util from letsencrypt.plugins import common @@ -56,18 +58,17 @@ def get_nginx_configurator( backup_dir=backups, temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + server="https://acme-server.org:443/new", dvsni_port=5001, ), name="nginx", version=version) config.prepare() - # also make a general client config for good measure... - namespace = mock.MagicMock( - config_dir='/tmp/config', work_dir='/tmp/foo', foo='bar', - server='https://acme-server.org:443/new') - from letsencrypt.configuration import NamespaceConfig - nsconfig = NamespaceConfig(namespace) + + # Provide general config utility. + nsconfig = configuration.NamespaceConfig(config.config) zope.component.provideUtility(nsconfig) + return config From 740f516561200a43c4720fcd7aad8346a38de77d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 19:09:04 -0700 Subject: [PATCH 14/36] Make boulder-start.sh more robust & helpful --- tests/boulder-start.sh | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index e17716b54..0af5dfb97 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -1,6 +1,23 @@ -#!/bin/sh -xe +#!/bin/bash # Download and run Boulder instance for integration testing + +# ugh, go version output is like: +# go version go1.4.2 linux/amd64 +GOVER=`go version | cut -d" " -f3 | cut -do -f2` + +# version comparison +function verlte { + [ "$1" = "`echo -e "$1\n$2" | sort -V | head -n1`" ] +} + +if ! verlte 1.5 "$GOVER" ; then + echo "We require go version 1.5 or later; you have... $GOVER" + exit 1 +fi + +set -xe + export GOPATH="${GOPATH:-/tmp/go}" # `/...` avoids `no buildable Go source files` errors, for more info @@ -8,7 +25,11 @@ export GOPATH="${GOPATH:-/tmp/go}" go get -d github.com/letsencrypt/boulder/... cd $GOPATH/src/github.com/letsencrypt/boulder # goose is needed for ./test/create_db.sh -go get bitbucket.org/liamstask/goose/cmd/goose +if ! go get bitbucket.org/liamstask/goose/cmd/goose ; then + echo Problems installing goose... perhaps rm -rf \$GOPATH \("$GOPATH"\) + echo and try again... + exit 1 +fi ./test/create_db.sh ./start.py & # Hopefully start.py bootstraps before integration test is started... From 18adec0bf24daf4d38130e1e5d8819ff55950fc8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 19:43:57 -0700 Subject: [PATCH 15/36] Fix paths in test cases --- .../letsencrypt_compatibility_test/util.py | 2 +- letsencrypt/tests/renewer_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py index 43070cf03..6181da16b 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py @@ -39,7 +39,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, "configs") + config_dir = os.path.join(parent_dir, "renewal") if os.path.isdir(configs): shutil.copytree(configs, config_dir, symlinks=True) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 761783d23..293b09537 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -49,7 +49,7 @@ class BaseRenewableCertTest(unittest.TestCase): for kind in ALL_FOUR: config[kind] = os.path.join(self.tempdir, "live", "example.org", kind + ".pem") - config.filename = os.path.join(self.tempdir, "configs", + config.filename = os.path.join(self.tempdir, "renewal", "example.org.conf") self.config = config From f2cee505f5a04917acbf9063baa8cdf9ac302fdd Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 17 Sep 2015 02:14:53 -0700 Subject: [PATCH 16/36] fix 781 --- letsencrypt-apache/letsencrypt_apache/parser.py | 2 ++ .../letsencrypt_apache/tests/complex_parsing_test.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index d7dc3c422..823d9794b 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -394,6 +394,8 @@ class ApacheParser(object): if not arg.startswith("/"): # Normpath will condense ../ arg = os.path.normpath(os.path.join(self.root, arg)) + else: + arg = os.path.normpath(arg) # Attempts to add a transform to the file if one does not already exist if os.path.isdir(arg): diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index e7bd03cc5..6ee7e1eb9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -98,6 +98,9 @@ class ComplexParserTest(util.ParserTest): def test_include_fullpath(self): self.verify_fnmatch(os.path.join(self.config_path, "test_fnmatch.conf")) + def test_include_fullpath_trailing_slash(self): + self.verify_fnmatch(self.config_path + "//") + def test_include_variable(self): self.verify_fnmatch("../complex_parsing/${fnmatch_filename}") @@ -106,5 +109,6 @@ class ComplexParserTest(util.ParserTest): self.verify_fnmatch("test_*.onf", False) + if __name__ == "__main__": unittest.main() # pragma: no cover From b5c8da21889b89b1da612b3bd09952b2f8e8e75a Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 17 Sep 2015 02:20:15 -0700 Subject: [PATCH 17/36] remove space --- .../letsencrypt_apache/tests/complex_parsing_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 6ee7e1eb9..64ecaa321 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -109,6 +109,5 @@ class ComplexParserTest(util.ParserTest): self.verify_fnmatch("test_*.onf", False) - if __name__ == "__main__": unittest.main() # pragma: no cover From 0e3eae153e9f8fcc98a295065a87fcf851ba175b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Sep 2015 12:29:42 -0700 Subject: [PATCH 18/36] Hide tracebacks, but not the ultimate error itself --- letsencrypt/cli.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index ab03a576f..a413cdf8e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -845,14 +845,17 @@ def _handle_exception(exc_type, exc_value, trace, args): if issubclass(exc_type, errors.Error): sys.exit(exc_value) - elif args is None: - sys.exit( - "An unexpected error occurred. Please see the logfile '{0}' " - "for more details.".format(logfile)) else: - sys.exit( - "An unexpected error occurred. Please see the logfiles in {0} " - "for more details.".format(args.logs_dir)) + # Tell the user a bit about what happened, without overwhelming + # them with a full traceback + msg = "An unexpected error occurred.\n" + msg += traceback.format_exception_only(exc_type,exc_value)[0] + msg += "\nPlease see the " + if args is None: + msg += "logfile '{0}' for more details.".format(logfile) + else: + msg += "logfiles in {0} for more details.".format(args.logs_dir) + sys.exit(msg) else: sys.exit("".join( traceback.format_exception(exc_type, exc_value, trace))) From 7c67df107636a6058a975b4a1392987bfdc5fa90 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Sep 2015 12:48:07 -0700 Subject: [PATCH 19/36] traceback actually provides that \n --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a413cdf8e..739385206 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -850,7 +850,7 @@ def _handle_exception(exc_type, exc_value, trace, args): # them with a full traceback msg = "An unexpected error occurred.\n" msg += traceback.format_exception_only(exc_type,exc_value)[0] - msg += "\nPlease see the " + msg += "Please see the " if args is None: msg += "logfile '{0}' for more details.".format(logfile) else: From 6c4d9e932407f0932d04f757694b54ad9da50ab2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Sep 2015 13:00:03 -0700 Subject: [PATCH 20/36] Satisfy the lintmonster --- letsencrypt/cli.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 739385206..dcd1e55b1 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -848,13 +848,13 @@ def _handle_exception(exc_type, exc_value, trace, args): else: # Tell the user a bit about what happened, without overwhelming # them with a full traceback - msg = "An unexpected error occurred.\n" - msg += traceback.format_exception_only(exc_type,exc_value)[0] - msg += "Please see the " + msg = ("An unexpected error occurred.\n" + + traceback.format_exception_only(exc_type, exc_value)[0] + + "Please see the ") if args is None: - msg += "logfile '{0}' for more details.".format(logfile) + msg += "logfile '{0}' for more details.".format(logfile) else: - msg += "logfiles in {0} for more details.".format(args.logs_dir) + msg += "logfiles in {0} for more details.".format(args.logs_dir) sys.exit(msg) else: sys.exit("".join( From 31af7b3a02c00be3bc0b15f0a629e3322756ebfd Mon Sep 17 00:00:00 2001 From: kevinlondon Date: Sun, 20 Sep 2015 14:53:45 -0700 Subject: [PATCH 21/36] Replace mktemp with mkstemp --- letsencrypt/revoker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 239879542..036309b21 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -288,7 +288,7 @@ class Revoker(object): :class:`letsencrypt.revoker.Cert` """ - list_path2 = tempfile.mktemp(".tmp", "LIST") + _, list_path2 = tempfile.mkstemp(".tmp", "LIST") idx = 0 with open(self.list_path, "rb") as orgfile: From 4ffb74d7df3b612ad49625e8b65423afd2ed95c8 Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 20 Sep 2015 19:07:55 -0400 Subject: [PATCH 22/36] Add dialog dependency and homebrew installation --- bootstrap/mac.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bootstrap/mac.sh b/bootstrap/mac.sh index a48afe11e..47450cd9e 100755 --- a/bootstrap/mac.sh +++ b/bootstrap/mac.sh @@ -1,2 +1,10 @@ #!/bin/sh +if hash brew 2>/dev/null; then + echo "Homebrew Installed" +else + echo "Homebrew Not Installed\nDownloading..." + ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" +fi + brew install augeas +brew install dialog From 7f42beda95c189ffaf457bbba0d3585f27b25a6b Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Sun, 20 Sep 2015 19:07:55 -0400 Subject: [PATCH 23/36] Add homebrew and add dialog dependency per #413 --- bootstrap/mac.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bootstrap/mac.sh b/bootstrap/mac.sh index a48afe11e..47450cd9e 100755 --- a/bootstrap/mac.sh +++ b/bootstrap/mac.sh @@ -1,2 +1,10 @@ #!/bin/sh +if hash brew 2>/dev/null; then + echo "Homebrew Installed" +else + echo "Homebrew Not Installed\nDownloading..." + ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" +fi + brew install augeas +brew install dialog From fd53b09ab53073361d1287671243d0cae4315020 Mon Sep 17 00:00:00 2001 From: Brandon Kreisel Date: Mon, 21 Sep 2015 06:58:35 -0400 Subject: [PATCH 24/36] Remove homebrew existing message --- bootstrap/mac.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bootstrap/mac.sh b/bootstrap/mac.sh index 47450cd9e..6779188a7 100755 --- a/bootstrap/mac.sh +++ b/bootstrap/mac.sh @@ -1,7 +1,5 @@ #!/bin/sh -if hash brew 2>/dev/null; then - echo "Homebrew Installed" -else +if ! hash brew 2>/dev/null; then echo "Homebrew Not Installed\nDownloading..." ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)" fi From ff9f12aea606448a5705486c4630cca3a299defc Mon Sep 17 00:00:00 2001 From: kevinlondon Date: Mon, 21 Sep 2015 08:05:55 -0700 Subject: [PATCH 25/36] Use the file handle provided by mkstemp for opening the file. --- letsencrypt/revoker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 036309b21..7fe1bd106 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -288,12 +288,12 @@ class Revoker(object): :class:`letsencrypt.revoker.Cert` """ - _, list_path2 = tempfile.mkstemp(".tmp", "LIST") + newfile_handle, list_path2 = tempfile.mkstemp(".tmp", "LIST") idx = 0 with open(self.list_path, "rb") as orgfile: csvreader = csv.reader(orgfile) - with open(list_path2, "wb") as newfile: + with os.fdopen(newfile_handle, "wb") as newfile: csvwriter = csv.writer(newfile) for row in csvreader: @@ -308,7 +308,7 @@ class Revoker(object): "Did not find all cert_list items to remove from LIST") shutil.copy2(list_path2, self.list_path) - os.remove(list_path2) + newfile.close() def _row_to_backup(self, row): """Convenience function From d4fa0363e320d7e68350eef20d952ef0074b5b88 Mon Sep 17 00:00:00 2001 From: kevinlondon Date: Mon, 21 Sep 2015 09:25:27 -0700 Subject: [PATCH 26/36] Removed the temp file again, not closing a closed file. --- letsencrypt/revoker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 7fe1bd106..32c6f003d 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -308,7 +308,7 @@ class Revoker(object): "Did not find all cert_list items to remove from LIST") shutil.copy2(list_path2, self.list_path) - newfile.close() + os.remove(list_path2) def _row_to_backup(self, row): """Convenience function From 8009993b52b9c13a8bb1fd42b84894e478f94e53 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 21 Sep 2015 14:50:00 -0700 Subject: [PATCH 27/36] Removed hardcoded signature --- letsencrypt/plugins/manual_test.py | 33 +++--------------------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 2d7c3e1e4..ce4ec22f9 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -46,12 +46,9 @@ class ManualAuthenticatorTest(unittest.TestCase): self.assertEqual([], self.auth.perform([])) @mock.patch("letsencrypt.plugins.manual.sys.stdout") - @mock.patch("letsencrypt.plugins.manual.os.urandom") @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") @mock.patch("__builtin__.raw_input") - def test_perform(self, mock_raw_input, mock_verify, mock_urandom, - mock_stdout): - mock_urandom.side_effect = nonrandom_urandom + def test_perform(self, mock_raw_input, mock_verify, mock_stdout): mock_verify.return_value = True resp = challenges.SimpleHTTPResponse(tls=False) @@ -61,27 +58,8 @@ class ManualAuthenticatorTest(unittest.TestCase): self.achalls[0].challb.chall, "foo.com", KEY.public_key(), 4430) message = mock_stdout.write.mock_calls[0][1][0] - self.assertEqual(message, """\ -Make sure your web server displays the following content at -http://foo.com/.well-known/acme-challenge/ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ before continuing: - -{"header": {"alg": "RS256", "jwk": {"e": "AQAB", "kty": "RSA", "n": "rHVztFHtH92ucFJD_N_HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2-6QWE30cWgdmJS86ObRz6lUTor4R0T-3C5Q"}}, "payload": "eyJ0bHMiOiBmYWxzZSwgInRva2VuIjogIlpYWmhSM2htUVVSek5uQlRVbUl5VEVGMk9VbGFaakUzUkhRemFuVjRSMG9yVUVOME9USjNjaXR2UVEiLCAidHlwZSI6ICJzaW1wbGVIdHRwIn0", "signature": "jFPJFC-2eRyBw7Sl0wyEBhsdvRZtKk8hc6HykEPAiofZlIwdIu76u2xHqMVZWSZdpxwMNUnnawTEAqgMWFydMA"} - -Content-Type header MUST be set to application/jose+json. - -If you don\'t have HTTP server configured, you can run the following -command on the target server (as root): - -mkdir -p /tmp/letsencrypt/public_html/.well-known/acme-challenge -cd /tmp/letsencrypt/public_html -echo -n \'{"header": {"alg": "RS256", "jwk": {"e": "AQAB", "kty": "RSA", "n": "rHVztFHtH92ucFJD_N_HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2-6QWE30cWgdmJS86ObRz6lUTor4R0T-3C5Q"}}, "payload": "eyJ0bHMiOiBmYWxzZSwgInRva2VuIjogIlpYWmhSM2htUVVSek5uQlRVbUl5VEVGMk9VbGFaakUzUkhRemFuVjRSMG9yVUVOME9USjNjaXR2UVEiLCAidHlwZSI6ICJzaW1wbGVIdHRwIn0", "signature": "jFPJFC-2eRyBw7Sl0wyEBhsdvRZtKk8hc6HykEPAiofZlIwdIu76u2xHqMVZWSZdpxwMNUnnawTEAqgMWFydMA"}\' > .well-known/acme-challenge/ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ -# run only once per server: -$(command -v python2 || command -v python2.7 || command -v python2.6) -c \\ -"import BaseHTTPServer, SimpleHTTPServer; \\ -SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {\'\': \'application/jose+json\'}; \\ -s = BaseHTTPServer.HTTPServer((\'\', 4430), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ -s.serve_forever()" \n""") - #self.assertTrue(validation in message) + token = self.achalls[0].challb.chall.encode("token") + self.assertTrue(token in message) mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) @@ -130,10 +108,5 @@ s.serve_forever()" \n""") mock_killpg.assert_called_once_with(1234, signal.SIGTERM) -def nonrandom_urandom(num_bytes): - """Returns a string of length num_bytes""" - return "x" * num_bytes - - if __name__ == "__main__": unittest.main() # pragma: no cover From 65dd4c668c31cc9cdb6d82c05d24798254f2de8d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 21 Sep 2015 14:55:12 -0700 Subject: [PATCH 28/36] Increased letsencrypt and letsencrypt-nginx cover minimums --- tox.cover.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.cover.sh b/tox.cover.sh index 6f8a5697b..edfd9b81a 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -16,13 +16,13 @@ fi cover () { if [ "$1" = "letsencrypt" ]; then - min=96 + min=97 elif [ "$1" = "acme" ]; then min=100 elif [ "$1" = "letsencrypt_apache" ]; then min=100 elif [ "$1" = "letsencrypt_nginx" ]; then - min=96 + min=97 elif [ "$1" = "letshelp_letsencrypt" ]; then min=100 else From f482b5bd5373f0bb2d0db7afb05a5069f09d53c4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 21 Sep 2015 15:08:11 -0700 Subject: [PATCH 29/36] Removed unnecessary .challb --- letsencrypt/plugins/manual_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index ce4ec22f9..6b9359db1 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -58,8 +58,7 @@ class ManualAuthenticatorTest(unittest.TestCase): self.achalls[0].challb.chall, "foo.com", KEY.public_key(), 4430) message = mock_stdout.write.mock_calls[0][1][0] - token = self.achalls[0].challb.chall.encode("token") - self.assertTrue(token in message) + self.assertTrue(self.achalls[0].chall.encode("token") in message) mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) From 5b080b6056856db15121d1db4ce84e5be2f704b4 Mon Sep 17 00:00:00 2001 From: yan Date: Mon, 21 Sep 2015 15:33:40 -0700 Subject: [PATCH 30/36] Update Dockerfile-dev and instructions. --- Dockerfile-dev | 6 +++++- docs/contributing.rst | 5 ++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Dockerfile-dev b/Dockerfile-dev index 835b3a7cc..2fe1a818d 100644 --- a/Dockerfile-dev +++ b/Dockerfile-dev @@ -32,7 +32,7 @@ RUN /opt/letsencrypt/src/ubuntu.sh && \ # the above is not likely to change, so by putting it further up the # Dockerfile we make sure we cache as much as possible -COPY setup.py README.rst CHANGES.rst MANIFEST.in requirements.txt EULA linter_plugin.py tox.cover.sh tox.ini /opt/letsencrypt/src/ +COPY setup.py README.rst CHANGES.rst MANIFEST.in requirements.txt EULA linter_plugin.py tox.cover.sh tox.ini pep8.travis.sh .pep8 .pylintrc /opt/letsencrypt/src/ # all above files are necessary for setup.py, however, package source # code directory has to be copied separately to a subdirectory... @@ -46,6 +46,8 @@ COPY letsencrypt /opt/letsencrypt/src/letsencrypt/ COPY acme /opt/letsencrypt/src/acme/ COPY letsencrypt-apache /opt/letsencrypt/src/letsencrypt-apache/ COPY letsencrypt-nginx /opt/letsencrypt/src/letsencrypt-nginx/ +COPY letshelp-letsencrypt /opt/letsencrypt/src/letshelp-letsencrypt/ +COPY letsencrypt-compatibility-test /opt/letsencrypt/src/letsencrypt-compatibility-test/ COPY tests /opt/letsencrypt/src/tests/ RUN virtualenv --no-site-packages -p python2 /opt/letsencrypt/venv && \ @@ -55,6 +57,8 @@ RUN virtualenv --no-site-packages -p python2 /opt/letsencrypt/venv && \ -e /opt/letsencrypt/src \ -e /opt/letsencrypt/src/letsencrypt-apache \ -e /opt/letsencrypt/src/letsencrypt-nginx \ + -e /opt/letsencrypt/src/letshelp-letsencrypt \ + -e /opt/letsencrypt/src/letsencrypt-compatibility-test \ -e /opt/letsencrypt/src[dev,docs,testing] # install in editable mode (-e) to save space: it's not possible to diff --git a/docs/contributing.rst b/docs/contributing.rst index f85e37c39..c6443e3b2 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -129,9 +129,8 @@ Docker OSX users will probably find it easiest to set up a Docker container for development. Let's Encrypt comes with a Dockerfile (``Dockerfile-dev``) -for doing so. To use Docker on OSX, install boot2docker using the -instructions at https://docs.docker.com/installation/mac/ and start it -from the command line (``boot2docker init``). +for doing so. To use Docker on OSX, install and setup docker-machine using the +instructions at https://docs.docker.com/installation/mac/. To build the development Docker image:: From cf08fe799ae48606c8aded138397c0288f5c65e0 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Mon, 21 Sep 2015 16:57:28 -0700 Subject: [PATCH 31/36] fix #799 --- letsencrypt/cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index dcd1e55b1..e4787a849 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -172,6 +172,8 @@ def _find_duplicative_certs(domains, config, renew_config): identical_names_cert, subset_names_cert = None, None configs_dir = renew_config.renewal_configs_dir + le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) + cli_config = configuration.RenewerConfiguration(config) for renewal_file in os.listdir(configs_dir): try: From 6e4faac9c07297beeb56c83a5239eea3fed767a2 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:15:11 -0700 Subject: [PATCH 32/36] Allow single/double quotes around Include dirs --- letsencrypt-apache/letsencrypt_apache/parser.py | 3 +++ .../letsencrypt_apache/tests/complex_parsing_test.py | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 823d9794b..e70d22d4e 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -390,6 +390,9 @@ class ApacheParser(object): # logger.error("Error: Invalid regexp characters in %s", arg) # return [] + # Remove beginning and ending quotes + arg = arg.strip("'\"") + # Standardize the include argument based on server root if not arg.startswith("/"): # Normpath will condense ../ diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 64ecaa321..a373b9766 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -78,7 +78,7 @@ class ComplexParserTest(util.ParserTest): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - + # def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser @@ -89,6 +89,7 @@ class ComplexParserTest(util.ParserTest): else: self.assertFalse(self.parser.find_dir("FNMATCH_DIRECTIVE")) + # NOTE: Only run one test per function otherwise you will have inf recursion def test_include(self): self.verify_fnmatch("test_fnmatch.?onf") @@ -101,6 +102,12 @@ class ComplexParserTest(util.ParserTest): def test_include_fullpath_trailing_slash(self): self.verify_fnmatch(self.config_path + "//") + def test_include_single_quotes(self): + self.verify_fnmatch("'" + self.config_path + "'") + + def test_include_double_quotes(self): + self.verify_fnmatch('"' + self.config_path + '"') + def test_include_variable(self): self.verify_fnmatch("../complex_parsing/${fnmatch_filename}") From 19d65c3e2f919ddc14cfc1bd34dc9b29a5c725fd Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:56:26 -0700 Subject: [PATCH 33/36] Add variable quote parsing --- letsencrypt-apache/letsencrypt_apache/parser.py | 8 ++++++++ .../letsencrypt_apache/tests/complex_parsing_test.py | 7 +++++++ .../tests/testdata/complex_parsing/apache2.conf | 2 ++ .../tests/testdata/complex_parsing/test_variables.conf | 1 + 4 files changed, 18 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index e70d22d4e..0ba438e65 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -315,6 +315,14 @@ class ApacheParser(object): """ value = self.aug.get(match) + + # No need to strip quotes for variables, as apache2ctl already does this + # but we do need to strip quotes for all normal arguments. + + # Note: normal argument may be a quoted variable + # e.g. strip now, not later + value = value.strip("'\"") + variables = ApacheParser.arg_var_interpreter.findall(value) for var in variables: diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index a373b9766..37c0208c1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -32,6 +32,7 @@ class ComplexParserTest(util.ParserTest): "COMPLEX": "", "tls_port": "1234", "fnmatch_filename": "test_fnmatch.conf", + "tls_port_str": "1234" } ) @@ -49,6 +50,12 @@ class ComplexParserTest(util.ParserTest): self.assertEqual(len(matches), 1) self.assertEqual(self.parser.get_arg(matches[0]), "1234") + def test_basic_variable_parsing_quotes(self): + matches = self.parser.find_dir("TestVariablePortStr") + + self.assertEqual(len(matches), 1) + self.assertEqual(self.parser.get_arg(matches[0]), "1234") + def test_invalid_variable_parsing(self): del self.parser.variables["tls_port"] diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf index 26bf47263..14cf95f9e 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/apache2.conf @@ -46,6 +46,8 @@ IncludeOptional sites-enabled/*.conf Define COMPLEX Define tls_port 1234 +Define tls_port_str "1234" + Define fnmatch_filename test_fnmatch.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf index a38191837..1a9edff74 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/complex_parsing/test_variables.conf @@ -1,4 +1,5 @@ TestVariablePort ${tls_port} +TestVariablePortStr "${tls_port_str}" LoadModule status_module modules/mod_status.so From 202b21f260cecbb9354e03831497ac4c38134bed Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 08:58:02 -0700 Subject: [PATCH 34/36] Remove extra # --- .../letsencrypt_apache/tests/complex_parsing_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 37c0208c1..bb0ff6af9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -85,7 +85,7 @@ class ComplexParserTest(util.ParserTest): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - # + def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser From e922a82277b52203ebfc74787e28bbd605d7d9e7 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 09:06:53 -0700 Subject: [PATCH 35/36] letsencrypt-apache/ --- letsencrypt-apache/letsencrypt_apache/parser.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 0ba438e65..0a3643064 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -241,6 +241,10 @@ class ApacheParser(object): Directives should be in the form of a case insensitive regex currently .. todo:: arg should probably be a list + .. todo:: arg search currently only supports direct matching. It does + not handle the case of variables or quoted arguments. This should + be adapted to use a generic search for the directive and then do a + case-insensitive self.get_arg filter Note: Augeas is inherently case sensitive while Apache is case insensitive. Augeas 1.0 allows case insensitive regexes like From d4d71a73a55990d127dfb7d531f634b8b4219116 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 22 Sep 2015 09:16:49 -0700 Subject: [PATCH 36/36] Remove trailing whitespace --- .../letsencrypt_apache/tests/complex_parsing_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index bb0ff6af9..7099c388f 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -85,7 +85,7 @@ class ComplexParserTest(util.ParserTest): # This is in an IfDefine self.assertTrue("ssl_module" in self.parser.modules) self.assertTrue("mod_ssl.c" in self.parser.modules) - + def verify_fnmatch(self, arg, hit=True): """Test if Include was correctly parsed.""" from letsencrypt_apache import parser