Skip to content

Commit

Permalink
Merge branch 'fix-xsrf' into 4.x
Browse files Browse the repository at this point in the history
  • Loading branch information
minrk committed Dec 21, 2016
2 parents 6d3cf8b + c6485a3 commit ed9399d
Show file tree
Hide file tree
Showing 22 changed files with 266 additions and 128 deletions.
36 changes: 28 additions & 8 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,55 @@ For more detailed information, see `GitHub <https://github.com/jupyter/notebook>

Use ``pip install notebook --upgrade`` or ``conda upgrade notebook`` to
upgrade to the latest release.



.. _release-4.3.1:

4.3.1
-----

4.3.1 is a patch release with a security patch, a couple bug fixes, and improvements to the newly-released token authentication.

**Security fix**:

- CVE-2016-9971. Fix CSRF vulnerability,
where malicious forms could create untitled files and start kernels
(no remote execution or modification of existing files)
for users of certain browsers (Firefox, Internet Explorer / Edge).
All previous notebook releases are affected.

Bug fixes:

- Fix carriage return handling
- Make the font size more robust against fickle brow
- Make the font size more robust against fickle browsers
- Ignore resize events that bubbled up and didn't come from window
- Add Authorization to allowed CORS headers
- Downgrade CodeMirror to 5.16 while we figure out issues in Safari

Other improvements:

- Better docs for token-based authentication
- Further highlight token info in log output when autogenerated
- Add Authorization to allowed CORS headers

See the 4.3 milestone on GitHub for a complete list of
`issues <https://github.com/jupyter/notebook/issues?utf8=%E2%9C%93&q=is%3Aissue%20milestone%3A4.3.1%20>`__
and `pull requests <https://github.com/jupyter/notebook/pulls?utf8=%E2%9C%93&q=is%3Apr%20milestone%3A4.3.1%20>`__ involved in this release.
See the 4.3.1 milestone on GitHub for a complete list of
`issues <https://github.com/jupyter/notebook/issues?utf8=%E2%9C%93&q=is%3Aissue%20milestone%3A4.3.1>`__
and `pull requests <https://github.com/jupyter/notebook/pulls?utf8=%E2%9C%93&q=is%3Apr%20milestone%3A4.3.1>`__ involved in this release.

.. _release-4.3:

4.3
---
4.3.0
-----

4.3 is a minor release with many bug fixes and improvements.
The biggest user-facing change is the addition of token authentication,
which is enabled by default.
A token is generated and used when your browser is opened automatically,
so you shouldn't have to enter anything in the default circumstances.
If you see a login page
(e.g. by switching browsers, or launching on a new port with ``--no-browser``),
you get a login URL with the token from the command ``jupyter notebook list``,
which you can paste into your browser.


Highlights:

Expand Down Expand Up @@ -87,6 +106,7 @@ See the 4.3 milestone on GitHub for a complete list of
`issues <https://github.com/jupyter/notebook/issues?utf8=%E2%9C%93&q=is%3Aissue%20milestone%3A4.3%20>`__
and `pull requests <https://github.com/jupyter/notebook/pulls?utf8=%E2%9C%93&q=is%3Apr%20milestone%3A4.3%20>`__ involved in this release.


.. _release-4.2.3:

4.2.3
Expand Down
89 changes: 60 additions & 29 deletions notebook/auth/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def set_login_cookie(cls, handler, user_id=None):
auth_header_pat = re.compile('token\s+(.+)', re.IGNORECASE)

@classmethod
def get_user_token(cls, handler):
def get_token(cls, handler):
"""Get the user token from a request
Default:
Expand All @@ -117,14 +117,29 @@ def get_user_token(cls, handler):
@classmethod
def should_check_origin(cls, handler):
"""Should the Handler check for CORS origin validation?
Origin check should be skipped for token-authenticated requests.
Returns:
- True, if Handler must check for valid CORS origin.
- False, if Handler should skip origin check since requests are token-authenticated.
"""
return not cls.is_token_authenticated(handler)

@classmethod
def is_token_authenticated(cls, handler):
"""Returns True if handler has been token authenticated. Otherwise, False.
Login with a token is used to signal certain things, such as:
- permit access to REST API
- xsrf protection
- skip origin-checks for scripts
"""
if getattr(handler, '_user_id', None) is None:
# ensure get_user has been called, so we know if we're token-authenticated
handler.get_current_user()
token_authenticated = getattr(handler, '_token_authenticated', False)
return not token_authenticated
return getattr(handler, '_token_authenticated', False)

@classmethod
def get_user(cls, handler):
Expand All @@ -136,40 +151,56 @@ def get_user(cls, handler):
# called on LoginHandler itself.
if getattr(handler, '_user_id', None):
return handler._user_id
user_id = handler.get_secure_cookie(handler.cookie_name)
if not user_id:
user_id = cls.get_user_token(handler)
if user_id is None:
user_id = handler.get_secure_cookie(handler.cookie_name)
else:
cls.set_login_cookie(handler, user_id)
# Record that the current request has been authenticated with a token.
# Used in is_token_authenticated above.
handler._token_authenticated = True
if user_id is None:
# prevent extra Invalid cookie sig warnings:
handler.clear_login_cookie()
token = handler.token
if not token and not handler.login_available:
if not handler.login_available:
# Completely insecure! No authentication at all.
# No need to warn here, though; validate_security will have already done that.
return 'anonymous'
if token:
# check login token from URL argument or Authorization header
user_token = cls.get_user_token(handler)
one_time_token = handler.one_time_token
authenticated = False
if user_token == token:
# token-authenticated, set the login cookie
handler.log.info("Accepting token-authenticated connection from %s", handler.request.remote_ip)
authenticated = True
elif one_time_token and user_token == one_time_token:
# one-time-token-authenticated, only allow this token once
handler.settings.pop('one_time_token', None)
handler.log.info("Accepting one-time-token-authenticated connection from %s", handler.request.remote_ip)
authenticated = True
if authenticated:
user_id = uuid.uuid4().hex
cls.set_login_cookie(handler, user_id)
# Record that we've been authenticated with a token.
# Used in should_check_origin above.
handler._token_authenticated = True
user_id = 'anonymous'

# cache value for future retrievals on the same request
handler._user_id = user_id
return user_id

@classmethod
def get_user_token(cls, handler):
"""Identify the user based on a token in the URL or Authorization header
Returns:
- uuid if authenticated
- None if not
"""
token = handler.token
if not token:
return
# check login token from URL argument or Authorization header
user_token = cls.get_token(handler)
one_time_token = handler.one_time_token
authenticated = False
if user_token == token:
# token-authenticated, set the login cookie
handler.log.debug("Accepting token-authenticated connection from %s", handler.request.remote_ip)
authenticated = True
elif one_time_token and user_token == one_time_token:
# one-time-token-authenticated, only allow this token once
handler.settings.pop('one_time_token', None)
handler.log.info("Accepting one-time-token-authenticated connection from %s", handler.request.remote_ip)
authenticated = True

if authenticated:
return uuid.uuid4().hex
else:
return None


@classmethod
def validate_security(cls, app, ssl_options=None):
Expand Down
30 changes: 26 additions & 4 deletions notebook/base/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def log():

class AuthenticatedHandler(web.RequestHandler):
"""A RequestHandler with an authenticated user."""

@property
def content_security_policy(self):
"""The default Content-Security-Policy header
Expand Down Expand Up @@ -95,6 +95,13 @@ def skip_check_origin(self):
return False
return not self.login_handler.should_check_origin(self)

@property
def token_authenticated(self):
"""Have I been authenticated with a token?"""
if self.login_handler is None or not hasattr(self.login_handler, 'is_token_authenticated'):
return False
return self.login_handler.is_token_authenticated(self)

@property
def cookie_name(self):
default_cookie_name = non_alphanum.sub('-', 'username-{}'.format(
Expand Down Expand Up @@ -285,8 +292,12 @@ def check_origin(self, origin_to_satisfy_tornado=""):
host = self.request.headers.get("Host")
origin = self.request.headers.get("Origin")

# If no header is provided, assume it comes from a script/curl.
# We are only concerned with cross-site browser stuff here.
# If no header is provided, let the request through.
# Origin can be None for:
# - same-origin (IE, Firefox)
# - Cross-site POST form (IE, Firefox)
# - Scripts
# The cross-site POST (XSRF) case is handled by tornado's xsrf_token
if origin is None or host is None:
return True

Expand All @@ -310,7 +321,15 @@ def check_origin(self, origin_to_satisfy_tornado=""):
self.request.path, origin, host,
)
return allow


def check_xsrf_cookie(self):
"""Bypass xsrf cookie checks when token-authenticated"""
if self.token_authenticated or self.settings.get('disable_check_xsrf', False):
# Token-authenticated requests do not need additional XSRF-check
# Servers without authentication are vulnerable to XSRF
return
return super(IPythonHandler, self).check_xsrf_cookie()

#---------------------------------------------------------------
# template rendering
#---------------------------------------------------------------
Expand Down Expand Up @@ -338,6 +357,9 @@ def template_namespace(self):
contents_js_source=self.contents_js_source,
version_hash=self.version_hash,
ignore_minified_js=self.ignore_minified_js,
xsrf_form_html=self.xsrf_form_html,
token=self.token,
xsrf_token=self.xsrf_token.decode('utf8'),
**self.jinja_template_vars
)

Expand Down
16 changes: 7 additions & 9 deletions notebook/nbconvert/tests/test_nbconvert_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@

class NbconvertAPI(object):
"""Wrapper for nbconvert API calls."""
def __init__(self, base_url):
self.base_url = base_url
def __init__(self, request):
self.request = request

def _req(self, verb, path, body=None, params=None):
response = requests.request(verb,
url_path_join(self.base_url, 'nbconvert', path),
response = self.request(verb,
url_path_join('nbconvert', path),
data=body, params=params,
)
response.raise_for_status()
Expand Down Expand Up @@ -69,7 +69,7 @@ def setUp(self):
encoding='utf-8') as f:
write(nb, f, version=4)

self.nbconvert_api = NbconvertAPI(self.base_url())
self.nbconvert_api = NbconvertAPI(self.request)

def tearDown(self):
nbdir = self.notebook_dir.name
Expand Down Expand Up @@ -109,8 +109,7 @@ def test_from_file_zip(self):

@onlyif_cmds_exist('pandoc')
def test_from_post(self):
nbmodel_url = url_path_join(self.base_url(), 'api/contents/foo/testnb.ipynb')
nbmodel = requests.get(nbmodel_url).json()
nbmodel = self.request('GET', 'api/contents/foo/testnb.ipynb').json()

r = self.nbconvert_api.from_post(format='html', nbmodel=nbmodel)
self.assertEqual(r.status_code, 200)
Expand All @@ -124,8 +123,7 @@ def test_from_post(self):

@onlyif_cmds_exist('pandoc')
def test_from_post_zip(self):
nbmodel_url = url_path_join(self.base_url(), 'api/contents/foo/testnb.ipynb')
nbmodel = requests.get(nbmodel_url).json()
nbmodel = self.request('GET', 'api/contents/foo/testnb.ipynb').json()

r = self.nbconvert_api.from_post(format='latex', nbmodel=nbmodel)
self.assertIn(u'application/zip', r.headers['Content-Type'])
Expand Down
18 changes: 18 additions & 0 deletions notebook/notebookapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ def init_settings(self, ipython_app, kernel_manager, contents_manager,
login_handler_class=ipython_app.login_handler_class,
logout_handler_class=ipython_app.logout_handler_class,
password=ipython_app.password,
xsrf_cookies=True,
disable_check_xsrf=ipython_app.disable_check_xsrf,

# managers
kernel_manager=kernel_manager,
Expand Down Expand Up @@ -559,6 +561,22 @@ def _token_changed(self, name, old, new):
"""
)

disable_check_xsrf = Bool(False, config=True,
help="""Disable cross-site-request-forgery protection
Jupyter notebook 4.3.1 introduces protection from cross-site request forgeries,
requiring API requests to either:
- originate from pages served by this server (validated with XSRF cookie and token), or
- authenticate with a token
Some anonymous compute resources still desire the ability to run code,
completely without authentication.
These services can disable all authentication and security checks,
with the full knowledge of what that implies.
"""
)

open_browser = Bool(True, config=True,
help="""Whether to open in a browser after starting.
The specific browser used is platform dependent and
Expand Down
10 changes: 5 additions & 5 deletions notebook/services/config/tests/test_config_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

class ConfigAPI(object):
"""Wrapper for notebook API calls."""
def __init__(self, base_url):
self.base_url = base_url
def __init__(self, request):
self.request = request

def _req(self, verb, section, body=None):
response = requests.request(verb,
url_path_join(self.base_url, 'api/config', section),
response = self.request(verb,
url_path_join('api/config', section),
data=body,
)
response.raise_for_status()
Expand All @@ -34,7 +34,7 @@ def modify(self, section, values):
class APITest(NotebookTestBase):
"""Test the config web service API"""
def setUp(self):
self.config_api = ConfigAPI(self.base_url())
self.config_api = ConfigAPI(self.request)

def test_create_retrieve_config(self):
sample = {'foo': 'bar', 'baz': 73}
Expand Down
10 changes: 5 additions & 5 deletions notebook/services/contents/tests/test_contents_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ def dirs_only(dir_model):

class API(object):
"""Wrapper for contents API calls."""
def __init__(self, base_url):
self.base_url = base_url
def __init__(self, request):
self.request = request

def _req(self, verb, path, body=None, params=None):
response = requests.request(verb,
url_path_join(self.base_url, 'api/contents', path),
response = self.request(verb,
url_path_join('api/contents', path),
data=body, params=params,
)
response.raise_for_status()
Expand Down Expand Up @@ -209,7 +209,7 @@ def setUp(self):
blob = self._blob_for_name(name)
self.make_blob(u'{}/{}.blob'.format(d, name), blob)

self.api = API(self.base_url())
self.api = API(self.request)

def tearDown(self):
for dname in (list(self.top_level_dirs) + self.hidden_dirs):
Expand Down
Loading

0 comments on commit ed9399d

Please sign in to comment.