Skip to content

Added secure_write to function for cookie / token saves #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions jupyter_server/serverapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -1512,7 +1506,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"),
Expand Down
72 changes: 70 additions & 2 deletions jupyter_server/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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)
70 changes: 70 additions & 0 deletions jupyter_server/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import stat
import sys
from distutils.version import LooseVersion
from contextlib import contextmanager

try:
from inspect import isawaitable
Expand Down Expand Up @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down