From 1a2c983a9cc5c47154b2751ebcf2194ff65cbd84 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 16 Sep 2015 13:13:24 -0700 Subject: [PATCH] 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")