Set tight permissions for key files

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
 ----
 

simp_le.py

@@ -21,15 +21,19 @@
 import abc
 import argparse
 import collections
+import contextlib
 import datetime
 import doctest
 import hashlib
 import errno
 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,20 +556,18 @@ 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()
         if data.chain is None:
             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()

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-----