diff --git a/README.rst b/README.rst index d9c3967..6fdf967 100644 --- a/README.rst +++ b/README.rst @@ -71,6 +71,21 @@ Installation ./venv.sh . venv/bin/activate +File permissions +---------------- +Newly created key files are only accessible by ``simp_le`` user (mode +600) while certificates are restricted by the umask setting. + +Should you need more relaxed permissions, either create an empty file +first or modify the permission afterwards. ``simp_le`` will not modify +permissions for existing files. Servers like Apache and nginx typically +start as root and as such they do not need such a workaround. + +It is recommended to run ``simp_le`` as a different unprivileged user, +dedicated for certificate management. Only this user will then be able +to read the account and private key. Certificates are public +information, so these can be world-readable. + Help ---- diff --git a/simp_le.py b/simp_le.py index 3ba76e7..3b07221 100755 --- a/simp_le.py +++ b/simp_le.py @@ -21,6 +21,7 @@ import abc import argparse import collections +import contextlib import datetime import doctest import hashlib @@ -28,8 +29,11 @@ import logging import os import shlex +import shutil +import stat import subprocess import sys +import tempfile import time import traceback import unittest @@ -78,6 +82,25 @@ class TestCase(unittest.TestCase): """simp_le unit test case.""" +@contextlib.contextmanager +def temp_umask(umask): + """Context manager that temporarily sets the process umask.""" + oldmask = os.umask(umask) + try: + yield + finally: + os.umask(oldmask) + + +def open_sensitive(filename, mode): + """Opens a sensitive file for writing.""" + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + # Windows-only + if 'b' in mode and hasattr(os, "O_BINARY"): + flags |= os.O_BINARY # pylint: disable=no-member + return os.fdopen(os.open(filename, flags, 0o600), mode) + + def gen_pkey(bits): """Generate a private key. @@ -243,8 +266,9 @@ class IOPlugin(object): - for `cert`: certificate, an instance of `OpenSSL.crypto.X509` - for `chain`: certificate chain, a list of `OpenSSL.crypto.X509` instances """ + Data.__new__.__defaults__ = (None,) * 4 - EMPTY_DATA = Data(account_key=None, key=None, cert=None, chain=None) + EMPTY_DATA = Data() def __init__(self, path, **dummy_kwargs): self.path = path @@ -318,6 +342,10 @@ def load(self): # previously return self.EMPTY_DATA raise + if not content: + # Empty files may exist when their parent directory is not + # writable, or when the file mode needs to be overridden. + return self.EMPTY_DATA return self.load_from_content(content) @abc.abstractmethod @@ -330,11 +358,13 @@ def load_from_content(self, content): """ raise NotImplementedError() - def save_to_file(self, data): + def save_to_file(self, data, sensitive=False): """Save data to file.""" - logger.info('Saving %s', self.path) + logger.info('Saving %s (%ssensitive)', self.path, + '' if sensitive else 'not ') + open_func = open_sensitive if sensitive else open try: - with open(self.path, self.WRITE_MODE) as persist_file: + with open_func(self.path, self.WRITE_MODE) as persist_file: persist_file.write(data) except OSError as error: logging.exception(error) @@ -364,14 +394,14 @@ class AccountKey(FileIOPlugin, JWKIOPlugin): WRITE_MODE = 'w' def persisted(self): - return self.Data(account_key=True, key=False, cert=False, chain=False) + return self.Data(account_key=True) def load_from_content(self, content): - return self.Data(account_key=self.load_jwk(content), key=None, - cert=None, chain=None) + return self.Data(account_key=self.load_jwk(content)) def save(self, data): - return self.save_to_file(self.dump_jwk(data.account_key)) + return self.save_to_file(self.dump_jwk(data.account_key), + sensitive=True) class OpenSSLIOPlugin(IOPlugin): # pylint: disable=abstract-method @@ -508,12 +538,12 @@ class ChainFile(FileIOPlugin, OpenSSLIOPlugin): _SEP = b'\n\n' # TODO: do all webservers like this? def persisted(self): - return self.Data(account_key=False, key=False, cert=False, chain=True) + return self.Data(chain=True) def load_from_content(self, output): chain = [self.load_cert(cert_data) for cert_data in output.split(self._SEP)] - return self.Data(account_key=None, key=None, cert=None, chain=chain) + return self.Data(chain=chain) def save(self, data): return self.save_to_file(self._SEP.join( @@ -526,7 +556,7 @@ class FullChainFile(ChainFile): """Full chain file plugin.""" def persisted(self): - return self.Data(account_key=False, key=False, cert=True, chain=True) + return self.Data(cert=True, chain=True) def load(self): data = super(FullChainFile, self).load() @@ -534,12 +564,10 @@ def load(self): cert, chain = None, None else: cert, chain = data.chain[0], data.chain[1:] - return self.Data(account_key=data.account_key, key=data.key, - cert=cert, chain=chain) + return self.Data(cert=cert, chain=chain) def save(self, data): return super(FullChainFile, self).save(self.Data( - account_key=data.account_key, key=data.key, cert=None, chain=([data.cert] + data.chain))) @@ -549,14 +577,13 @@ class KeyFile(FileIOPlugin, OpenSSLIOPlugin): """Private key file plugin.""" def persisted(self): - return self.Data(account_key=False, key=True, cert=False, chain=False) + return self.Data(key=True) def load_from_content(self, output): - return self.Data(account_key=None, key=self.load_key(output), - cert=None, chain=None) + return self.Data(key=self.load_key(output)) def save(self, data): - return self.save_to_file(self.dump_key(data.key)) + return self.save_to_file(self.dump_key(data.key), sensitive=True) @IOPlugin.register(path='cert.der', typ=OpenSSL.crypto.FILETYPE_ASN1) @@ -565,11 +592,10 @@ class CertFile(FileIOPlugin, OpenSSLIOPlugin): """Certificate file plugin.""" def persisted(self): - return self.Data(account_key=False, key=False, cert=True, chain=False) + return self.Data(cert=True) def load_from_content(self, output): - return self.Data(account_key=None, key=None, - cert=self.load_cert(output), chain=None) + return self.Data(cert=self.load_cert(output)) def save(self, data): return self.save_to_file(self.dump_cert(data.cert)) @@ -848,8 +874,7 @@ def test(args): def check_plugins_persist_all(ioplugins): """Do plugins cover all components (key/cert/chain)?""" - persisted = IOPlugin.Data( - account_key=False, key=False, cert=False, chain=False) + persisted = IOPlugin.Data() for plugin_name in ioplugins: persisted = IOPlugin.Data(*componentwise_or( persisted, IOPlugin.registered[plugin_name].persisted())) @@ -1183,5 +1208,26 @@ def test_error_exit_codes(self, dummy_stderr): assert False +class KeyFileTest(TestCase): + """Integration tests for plugin interface.""" + + # this is unittest suite | pylint: disable=missing-docstring + + def setUp(self): + self.root = tempfile.mkdtemp() + self.key_path = os.path.join(self.root, 'key.pem') + self.key_data = IOPlugin.Data(key=ComparablePKey(gen_pkey(1024))) + + def tearDown(self): + shutil.rmtree(self.root) + + def test_keyfile(self): + """Test whether newly saved key files are not world-readable.""" + keyfile = KeyFile(path=self.key_path) + with temp_umask(0o022): + keyfile.save(self.key_data) + self.assertEqual(os.stat(self.key_path).st_mode & stat.S_IROTH, 0) + + if __name__ == '__main__': main() diff --git a/testdata/rsa2048_key.pem b/testdata/rsa2048_key.pem new file mode 100644 index 0000000..33efd34 --- /dev/null +++ b/testdata/rsa2048_key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEA8HwZMHeImB/iM8/n8CTCR4KeYQB2gLGO3v8xLms+PWH3Zbxc +dVtEn25Y34scIh+iOuEXBcSBalBddLHKBGVN3nCfmpupoLm52xgRG44q9OWODpg4 +FSi4afqVw2agMx0RHi0v3GVcdpqB83UW42kK1ESZHUuq7mxLg8u3IMYZFm6Amsf+ +YQjBbDNn8NczJOFhsExP2EdM5ykgM1Om8aqTqqPMgPub68/r4Sym+BjLnvRq5Qtz +h/jCfOBIIpAwg3lj7l8OyE3kkD3ALtuiuminNUqLHEkUaLq/Xiv8V8mvnrhG7h3Q ++L1Xc707P0dz5YM5XxTMhmUE1cae/lQ0KbNrpwIDAQABAoIBAAiDXCDrGlrIRimv +YnaN1pLRfOnSKl/D6VrbjdIm2b0yip9/W4aMBJHgRiUjt4s9s3CCJ1585lftIGHR +KWWecHM/aWb/u7GE4Z9v6qsfDUY+GhlKKjIVjvGxfTu9lk446TI4R0l2DR/luFP2 +ASlrvoZlJ0ZyN0rZapLv0zvFx32Tukd+3rcMmXfHl7aRGMZG1YTKNmBJ4d9iJ6cP +HG3fgSzLQMPLNO/20MzbXdREG5FNQtwaMuFnIcVbtMCvc/71lQQEfANMLCUweEed +YWGOjgDeh+731nJsopel+2TSTgnf5VhcFrgChZZdqeKvP+HbXjTE2VkWo7BrzoM7 +xICYBwECgYEA/ZF/JOjZfwIMUdikv8vldJzRMdFryl4NJWnh4NeThNOEpUcpxnyK +wyMnnQaGJa51u9EEnzl0sZ2h2ODjD6KFpz6fkWaVRq5SWalVPAoKZGaoPZV3IUOI +8Tm0xkXho+A/FUUEcxCLME+3V9EdPfHaVRJOrbfDyxvNhsj4w9F0aAkCgYEA8sp7 +XTrolOknJGv4Qt1w6gcm5+cMtLaRfi8ZHPHujl2x9eWE8/s2818az7jc0Xr/G4HQ +NeU+3Es4BblEckSHmhUZhx26cZgkLSIIDofEtaEc6u8CyWfxsWvn3l4T3kMdeSLC +9UoLk59AH2tkMIh8vzV8LSisLJa341lMdgryQi8CgYAlJKr7PSCe+i3Tz2hSsAts +iYwbQBIKErzaPihYRzvUuSc1DreP26535y5mUg5UdrnISVXj/Qaa/fw3SLn6EFSD +qyi0o9I6CE8H00YpBU+AZYk/fCV3Oe1VaJ6SbKog1zhmZTXBpSq+aO7ybi9aY5MX +4xajW8fSeMAifk3yYTwsAQKBgErcEcOCOVpItU/uloKPYpRWFjHktK83p46fmP+q +vOJak1d9KExOBfhuN4caucNBSE1D7l3fzE0CSEjDgg41gRYKMW/Ow8DopybfWlqY +lBdokNEDVvmgug35dmnC2h9q1DiYdkJJTV57+Lp3U1H/k28lX59Q7h1lb1eDHic7 +YszzAoGBAOx05dhOiYbzAJSTQu3oBHFn4mTYIqCcDO6cQrEJwPKAq7mAhT0yOk9N +CrqRV/1aes665829cyTwcAZl6nqbzHv5XjX5+g6vmooCb4oCkq49rumHjoQdrX8D +RR5b+Spkc1jo4rctCcExzSkgo+K5N3oBVYznecje7O7Z0/qiJE/8 +-----END RSA PRIVATE KEY-----