Merge pull request #77 from MSeal/secureWriteSecrets

Add secure_write function to restrict permissions on created files

jupyter_server/serverapp.py

@@ -40,6 +40,7 @@
 from jinja2 import Environment, FileSystemLoader
 
 from jupyter_server.transutils import trans, _
+from jupyter_server.utils import secure_write
 
 # Install the pyzmq ioloop. This has to be done before anything else from
 # tornado is imported.
@@ -678,7 +679,7 @@ def _default_cookie_secret_file(self):
     def _default_cookie_secret(self):
         if os.path.exists(self.cookie_secret_file):
             with io.open(self.cookie_secret_file, 'rb') as f:
-                key =  f.read()
+                key = f.read()
         else:
             key = encodebytes(os.urandom(32))
             self._write_cookie_secret_file(key)
@@ -690,18 +691,11 @@ def _write_cookie_secret_file(self, secret):
         """write my secret to my secret_file"""
         self.log.info(_("Writing notebook server cookie secret to %s"), self.cookie_secret_file)
         try:
-            with io.open(self.cookie_secret_file, 'wb') as f:
+            with secure_write(self.cookie_secret_file, True) as f:
                 f.write(secret)
         except OSError as e:
             self.log.error(_("Failed to write cookie secret to %s: %s"),
                            self.cookie_secret_file, e)
-        try:
-            os.chmod(self.cookie_secret_file, 0o600)
-        except OSError:
-            self.log.warning(
-                _("Could not set permissions on %s"),
-                self.cookie_secret_file
-            )
 
     token = Unicode('<generated>',
         help=_("""Token used for authenticating first-time connections to the server.
@@ -1519,7 +1513,7 @@ def server_info(self):
     def write_server_info_file(self):
         """Write the result of server_info() to the JSON file info_file."""
         try:
-            with open(self.info_file, 'w') as f:
+            with secure_write(self.info_file) as f:
                 json.dump(self.server_info(), f, indent=2, sort_keys=True)
         except OSError as e:
             self.log.error(_("Failed to write server-info to %s: %s"),

jupyter_server/tests/test_utils.py

@@ -5,14 +5,18 @@
 
 import ctypes
 import os
+import re
+import stat
+import shutil
+import tempfile
 
 import nose.tools as nt
 
 from traitlets.tests.utils import check_help_all_output
-from jupyter_server.utils import url_escape, url_unescape, is_hidden, is_file_hidden
+from jupyter_server.utils import url_escape, url_unescape, is_hidden, is_file_hidden, secure_write
 from ipython_genutils.py3compat import cast_unicode
 from ipython_genutils.tempdir import TemporaryDirectory
-from ipython_genutils.testing.decorators import skip_if_not_win32
+from ipython_genutils.testing.decorators import skip_if_not_win32, skip_win32
 
 
 def test_help_output():
@@ -91,3 +95,67 @@ def test_is_hidden_win32():
         assert is_hidden(subdir1, root)
         assert is_file_hidden(subdir1)
 
+@skip_if_not_win32
+def test_secure_write_win32():
+    def fetch_win32_permissions(filename):
+        '''Extracts file permissions on windows using icacls'''
+        role_permissions = {}
+        for index, line in enumerate(os.popen("icacls %s" % filename).read().splitlines()):
+            if index == 0:
+                line = line.split(filename)[-1].strip().lower()
+            match = re.match(r'\s*([^:]+):\(([^\)]*)\)', line)
+            if match:
+                usergroup, permissions = match.groups()
+                usergroup = usergroup.lower().split('\\')[-1]
+                permissions = set(p.lower() for p in permissions.split(','))
+                role_permissions[usergroup] = permissions
+            elif not line.strip():
+                break
+        return role_permissions
+
+    def check_user_only_permissions(fname):
+        # Windows has it's own permissions ACL patterns
+        import win32api
+        username = win32api.GetUserName().lower()
+        permissions = fetch_win32_permissions(fname)
+        print(permissions) # for easier debugging
+        nt.assert_true(username in permissions)
+        nt.assert_equal(permissions[username], set(['r', 'w']))
+        nt.assert_true('administrators' in permissions)
+        nt.assert_equal(permissions['administrators'], set(['f']))
+        nt.assert_true('everyone' not in permissions)
+        nt.assert_equal(len(permissions), 2)
+
+    directory = tempfile.mkdtemp()
+    fname = os.path.join(directory, 'check_perms')
+    try:
+        with secure_write(fname) as f:
+            f.write('test 1')
+        check_user_only_permissions(fname)
+        with open(fname, 'r') as f:
+            nt.assert_equal(f.read(), 'test 1')
+    finally:
+        shutil.rmtree(directory)
+
+@skip_win32
+def test_secure_write_unix():
+    directory = tempfile.mkdtemp()
+    fname = os.path.join(directory, 'check_perms')
+    try:
+        with secure_write(fname) as f:
+            f.write('test 1')
+        mode = os.stat(fname).st_mode
+        nt.assert_equal('0600', oct(stat.S_IMODE(mode)).replace('0o', '0'))
+        with open(fname, 'r') as f:
+            nt.assert_equal(f.read(), 'test 1')
+
+        # Try changing file permissions ahead of time
+        os.chmod(fname, 0o755)
+        with secure_write(fname) as f:
+            f.write('test 2')
+        mode = os.stat(fname).st_mode
+        nt.assert_equal('0600', oct(stat.S_IMODE(mode)).replace('0o', '0'))
+        with open(fname, 'r') as f:
+            nt.assert_equal(f.read(), 'test 2')
+    finally:
+        shutil.rmtree(directory)

jupyter_server/utils.py

@@ -11,6 +11,7 @@
 import stat
 import sys
 from distutils.version import LooseVersion
+from contextlib import contextmanager
 
 try:
     from inspect import isawaitable
@@ -233,6 +234,75 @@ def is_hidden(abs_path, abs_root=''):
 
     return False
 
+# TODO: Move to jupyter_core
+def win32_restrict_file_to_user(fname):
+    """Secure a windows file to read-only access for the user.
+    Follows guidance from win32 library creator:
+    http://timgolden.me.uk/python/win32_how_do_i/add-security-to-a-file.html
+
+    This method should be executed against an already generated file which
+    has no secrets written to it yet.
+
+    Parameters
+    ----------
+
+    fname : unicode
+        The path to the file to secure
+    """
+    import win32api
+    import win32security
+    import ntsecuritycon as con
+
+    # everyone, _domain, _type = win32security.LookupAccountName("", "Everyone")
+    admins, _domain, _type = win32security.LookupAccountName("", "Administrators")
+    user, _domain, _type = win32security.LookupAccountName("", win32api.GetUserName())
+
+    sd = win32security.GetFileSecurity(fname, win32security.DACL_SECURITY_INFORMATION)
+
+    dacl = win32security.ACL()
+    # dacl.AddAccessAllowedAce(win32security.ACL_REVISION, con.FILE_ALL_ACCESS, everyone)
+    dacl.AddAccessAllowedAce(win32security.ACL_REVISION, con.FILE_GENERIC_READ | con.FILE_GENERIC_WRITE, user)
+    dacl.AddAccessAllowedAce(win32security.ACL_REVISION, con.FILE_ALL_ACCESS, admins)
+
+    sd.SetSecurityDescriptorDacl(1, dacl, 0)
+    win32security.SetFileSecurity(fname, win32security.DACL_SECURITY_INFORMATION, sd)
+
+# TODO: Move to jupyter_core
+@contextmanager
+def secure_write(fname, binary=False):
+    """Opens a file in the most restricted pattern available for
+    writing content. This limits the file mode to `600` and yields
+    the resulting opened filed handle.
+
+    Parameters
+    ----------
+
+    fname : unicode
+        The path to the file to write
+    """
+    mode = 'wb' if binary else 'w'
+    open_flag = os.O_CREAT | os.O_WRONLY | os.O_TRUNC
+    try:
+        os.remove(fname)
+    except (IOError, OSError):
+        # Skip any issues with the file not existing
+        pass
+
+    if os.name == 'nt':
+        # Python on windows does not respect the group and public bits for chmod, so we need
+        # to take additional steps to secure the contents.
+        # Touch file pre-emptively to avoid editing permissions in open files in Windows
+        fd = os.open(fname, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600)
+        os.close(fd)
+        open_flag = os.O_WRONLY | os.O_TRUNC
+        win32_restrict_file_to_user(fname)
+
+    with os.fdopen(os.open(fname, open_flag, 0o600), mode) as f:
+        if os.name != 'nt':
+            # Enforce that the file got the requested permissions before writing
+            assert '0600' == oct(stat.S_IMODE(os.stat(fname).st_mode)).replace('0o', '0')
+        yield f
+
 def samefile_simple(path, other_path):
     """
     Fill in for os.path.samefile when it is unavailable (Windows+py2).

setup.py

@@ -86,7 +86,8 @@
         'ipykernel', # bless IPython kernel for now
         'Send2Trash',
         'terminado>=0.8.1',
-        'prometheus_client'
+        'prometheus_client',
+        "pywin32>=1.0 ; sys_platform == 'win32'"
     ],
     extras_require = {
         ':python_version == "2.7"': ['ipaddress'],