mirror of
https://github.com/certbot/certbot.git
synced 2026-08-03 03:32:13 +02:00
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
This commit is contained in:
@@ -129,8 +129,9 @@ class AccountFileStorage(interfaces.AccountStorage):
|
|||||||
|
|
||||||
"""
|
"""
|
||||||
def __init__(self, config):
|
def __init__(self, config):
|
||||||
le_util.make_or_verify_dir(config.accounts_dir, 0o700, os.geteuid())
|
|
||||||
self.config = config
|
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):
|
def _account_dir_path(self, account_id):
|
||||||
return os.path.join(self.config.accounts_dir, account_id)
|
return os.path.join(self.config.accounts_dir, account_id)
|
||||||
@@ -186,7 +187,8 @@ class AccountFileStorage(interfaces.AccountStorage):
|
|||||||
|
|
||||||
def save(self, account):
|
def save(self, account):
|
||||||
account_dir_path = self._account_dir_path(account.id)
|
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:
|
try:
|
||||||
with open(self._regr_path(account_dir_path), "w") as regr_file:
|
with open(self._regr_path(account_dir_path), "w") as regr_file:
|
||||||
regr_file.write(account.regr.json_dumps())
|
regr_file.write(account.regr.json_dumps())
|
||||||
|
|||||||
+9
-2
@@ -659,6 +659,10 @@ def create_parser(plugins, args):
|
|||||||
"security", "-r", "--redirect", action="store_true",
|
"security", "-r", "--redirect", action="store_true",
|
||||||
help="Automatically redirect all HTTP traffic to HTTPS for the newly "
|
help="Automatically redirect all HTTP traffic to HTTPS for the newly "
|
||||||
"authenticated vhost.")
|
"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)
|
_paths_parser(helpful)
|
||||||
# _plugins_parsing should be the last thing to act upon the main
|
# _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)
|
parser, tweaked_cli_args = create_parser(plugins, cli_args)
|
||||||
args = parser.parse_args(tweaked_cli_args)
|
args = parser.parse_args(tweaked_cli_args)
|
||||||
config = configuration.NamespaceConfig(args)
|
config = configuration.NamespaceConfig(args)
|
||||||
|
zope.component.provideUtility(config)
|
||||||
|
|
||||||
# Setup logging ASAP, otherwise "No handlers could be found for
|
# Setup logging ASAP, otherwise "No handlers could be found for
|
||||||
# logger ..." TODO: this should be done before plugins discovery
|
# logger ..." TODO: this should be done before plugins discovery
|
||||||
for directory in config.config_dir, config.work_dir:
|
for directory in config.config_dir, config.work_dir:
|
||||||
le_util.make_or_verify_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
|
# TODO: logs might contain sensitive data such as contents of the
|
||||||
# private key! #525
|
# 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)
|
_setup_logging(args)
|
||||||
|
|
||||||
# do not log `args`, as it contains sensitive data (e.g. revoke --key)!
|
# do not log `args`, as it contains sensitive data (e.g. revoke --key)!
|
||||||
|
|||||||
@@ -315,7 +315,8 @@ class Client(object):
|
|||||||
"""
|
"""
|
||||||
for path in cert_path, chain_path:
|
for path in cert_path, chain_path:
|
||||||
le_util.make_or_verify_dir(
|
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
|
# try finally close
|
||||||
cert_chain_abspath = None
|
cert_chain_abspath = None
|
||||||
|
|||||||
@@ -9,12 +9,15 @@ import logging
|
|||||||
import os
|
import os
|
||||||
|
|
||||||
import OpenSSL
|
import OpenSSL
|
||||||
|
import zope.component
|
||||||
|
|
||||||
from acme import crypto_util as acme_crypto_util
|
from acme import crypto_util as acme_crypto_util
|
||||||
from acme import jose
|
from acme import jose
|
||||||
|
|
||||||
from letsencrypt import errors
|
from letsencrypt import errors
|
||||||
from letsencrypt import le_util
|
from letsencrypt import le_util
|
||||||
|
from letsencrypt import interfaces
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@@ -45,8 +48,10 @@ def init_save_key(key_size, key_dir, keyname="key-letsencrypt.pem"):
|
|||||||
logger.exception(err)
|
logger.exception(err)
|
||||||
raise err
|
raise err
|
||||||
|
|
||||||
|
config = zope.component.getUtility(interfaces.IConfig)
|
||||||
# Save file
|
# 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(
|
key_f, key_path = le_util.unique_file(
|
||||||
os.path.join(key_dir, keyname), 0o600)
|
os.path.join(key_dir, keyname), 0o600)
|
||||||
key_f.write(key_pem)
|
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)
|
csr_pem, csr_der = make_csr(privkey.pem, names)
|
||||||
|
|
||||||
|
|
||||||
|
config = zope.component.getUtility(interfaces.IConfig)
|
||||||
# Save CSR
|
# 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(
|
csr_f, csr_filename = le_util.unique_file(
|
||||||
os.path.join(path, csrname), 0o644)
|
os.path.join(path, csrname), 0o644)
|
||||||
csr_f.write(csr_pem)
|
csr_f.write(csr_pem)
|
||||||
|
|||||||
@@ -70,7 +70,7 @@ def exe_exists(exe):
|
|||||||
return False
|
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.
|
"""Make sure directory exists with proper permissions.
|
||||||
|
|
||||||
:param str directory: Path to a directory.
|
: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)
|
os.makedirs(directory, mode)
|
||||||
except OSError as exception:
|
except OSError as exception:
|
||||||
if exception.errno == errno.EEXIST:
|
if exception.errno == errno.EEXIST:
|
||||||
if not check_permissions(directory, mode, uid):
|
if strict and not check_permissions(directory, mode, uid):
|
||||||
raise errors.Error(
|
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:
|
else:
|
||||||
raise
|
raise
|
||||||
|
|
||||||
|
|||||||
@@ -31,7 +31,8 @@ class Reverter(object):
|
|||||||
self.config = config
|
self.config = config
|
||||||
|
|
||||||
le_util.make_or_verify_dir(
|
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):
|
def revert_temporary_config(self):
|
||||||
"""Reload users original configuration files after a temporary save.
|
"""Reload users original configuration files after a temporary save.
|
||||||
@@ -180,7 +181,8 @@ class Reverter(object):
|
|||||||
|
|
||||||
"""
|
"""
|
||||||
le_util.make_or_verify_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)
|
||||||
|
|
||||||
op_fd, existing_filepaths = self._read_and_append(
|
op_fd, existing_filepaths = self._read_and_append(
|
||||||
os.path.join(cp_dir, "FILEPATHS"))
|
os.path.join(cp_dir, "FILEPATHS"))
|
||||||
@@ -393,7 +395,8 @@ class Reverter(object):
|
|||||||
cp_dir = self.config.in_progress_dir
|
cp_dir = self.config.in_progress_dir
|
||||||
|
|
||||||
le_util.make_or_verify_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
|
return cp_dir
|
||||||
|
|
||||||
|
|||||||
@@ -54,7 +54,8 @@ class Revoker(object):
|
|||||||
self.config = config
|
self.config = config
|
||||||
self.no_confirm = no_confirm
|
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...
|
# TODO: Find a better solution for this...
|
||||||
self.list_path = os.path.join(config.cert_key_backup, "LIST")
|
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")
|
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(
|
cls._catalog_files(
|
||||||
config.cert_key_backup, cert_path, key_path, list_path)
|
config.cert_key_backup, cert_path, key_path, list_path)
|
||||||
|
|||||||
@@ -92,7 +92,7 @@ class MakeOrVerifyDirTest(unittest.TestCase):
|
|||||||
|
|
||||||
def _call(self, directory, mode):
|
def _call(self, directory, mode):
|
||||||
from letsencrypt.le_util import make_or_verify_dir
|
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):
|
def test_creates_dir_when_missing(self):
|
||||||
path = os.path.join(self.root_path, "bar")
|
path = os.path.join(self.root_path, "bar")
|
||||||
|
|||||||
Reference in New Issue
Block a user