From 4a8af93b5bd92c02a2502cdc7281a9e7b90ac780 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 13 Dec 2016 14:57:57 +0100 Subject: [PATCH 01/11] enable tornado xsrf cookie --- notebook/base/handlers.py | 19 +++++++++++++++++-- notebook/notebookapp.py | 1 + notebook/templates/login.html | 1 + 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/notebook/base/handlers.py b/notebook/base/handlers.py index 5ef320b13..88c882ecc 100755 --- a/notebook/base/handlers.py +++ b/notebook/base/handlers.py @@ -288,8 +288,12 @@ class IPythonHandler(AuthenticatedHandler): 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, allow it. + # 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 @@ -340,6 +344,8 @@ class IPythonHandler(AuthenticatedHandler): sys_info=sys_info, 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, **self.jinja_template_vars ) @@ -403,6 +409,15 @@ class APIHandler(IPythonHandler): raise web.HTTPError(404) return super(APIHandler, self).prepare() + def check_xsrf_cookie(self): + """Check non-empty body on POST for XSRF + + instead of checking the cookie for forms. + """ + if self.request.method.upper() == 'POST' and not self.request.body: + # Require non-empty POST body for XSRF + raise web.HTTPError(400, "POST requests must have a JSON body. If no content is needed, use '{}'.") + @property def content_security_policy(self): csp = '; '.join([ diff --git a/notebook/notebookapp.py b/notebook/notebookapp.py index 7998c14d4..484ba7e2e 100755 --- a/notebook/notebookapp.py +++ b/notebook/notebookapp.py @@ -225,6 +225,7 @@ class NotebookWebApplication(web.Application): login_handler_class=jupyter_app.login_handler_class, logout_handler_class=jupyter_app.logout_handler_class, password=jupyter_app.password, + xsrf_cookies=True, # managers kernel_manager=kernel_manager, diff --git a/notebook/templates/login.html b/notebook/templates/login.html index df394db32..61aafaf17 100644 --- a/notebook/templates/login.html +++ b/notebook/templates/login.html @@ -22,6 +22,7 @@
From 70e79a0ad69f02d46abed57e007ce5ef7f79319b Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 13 Dec 2016 15:54:41 +0100 Subject: [PATCH 02/11] add token_authenticated property indicates if a token is used for authentication, in which case xsrf checks should be skipped. --- notebook/auth/login.py | 78 +++++++++++++++++++++++++-------------- notebook/base/handlers.py | 30 +++++++++------ 2 files changed, 69 insertions(+), 39 deletions(-) diff --git a/notebook/auth/login.py b/notebook/auth/login.py index ee260b7da..3511058ef 100644 --- a/notebook/auth/login.py +++ b/notebook/auth/login.py @@ -97,7 +97,7 @@ class LoginHandler(IPythonHandler): 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: @@ -120,11 +120,22 @@ class LoginHandler(IPythonHandler): Origin check should be skipped for token-authenticated requests. """ + return not cls.is_token_authenticated(handler) + + @classmethod + def is_token_authenticated(cls, handler): + """Check if the handler has been authenticated by a token. + + This 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): @@ -136,40 +147,51 @@ class LoginHandler(IPythonHandler): # 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 we've been authenticated with a token. + # Used in should_check_origin 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""" + 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): diff --git a/notebook/base/handlers.py b/notebook/base/handlers.py index 88c882ecc..7edc4099d 100755 --- a/notebook/base/handlers.py +++ b/notebook/base/handlers.py @@ -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 @@ -95,6 +95,13 @@ class AuthenticatedHandler(web.RequestHandler): 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( @@ -317,7 +324,15 @@ class IPythonHandler(AuthenticatedHandler): self.request.path, origin, host, ) return allow - + + def check_xsrf_cookie(self): + """Bypass xsrf checks when token-authenticated""" + if self.token_authenticated: + # 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 #--------------------------------------------------------------- @@ -346,6 +361,8 @@ class IPythonHandler(AuthenticatedHandler): 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, **self.jinja_template_vars ) @@ -409,15 +426,6 @@ class APIHandler(IPythonHandler): raise web.HTTPError(404) return super(APIHandler, self).prepare() - def check_xsrf_cookie(self): - """Check non-empty body on POST for XSRF - - instead of checking the cookie for forms. - """ - if self.request.method.upper() == 'POST' and not self.request.body: - # Require non-empty POST body for XSRF - raise web.HTTPError(400, "POST requests must have a JSON body. If no content is needed, use '{}'.") - @property def content_security_policy(self): csp = '; '.join([ From 9478a6b82be4a21663c6ae2d85eb35722eab18b6 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 13 Dec 2016 16:58:58 +0100 Subject: [PATCH 03/11] use tornado xsrf token in API - Cookie-authenticated API requests must use set X-XSRFToken header - add utils.ajax for making ajax requests, adding xsrf header from default location --- notebook/base/handlers.py | 2 +- notebook/static/base/js/utils.js | 32 +++++++++++++++++--- notebook/static/services/kernels/kernel.js | 12 ++++---- notebook/static/services/sessions/session.js | 10 +++--- notebook/static/tree/js/notebooklist.js | 2 +- notebook/static/tree/js/sessionlist.js | 2 +- notebook/static/tree/js/terminallist.js | 6 ++-- notebook/templates/page.html | 9 +++++- 8 files changed, 53 insertions(+), 22 deletions(-) diff --git a/notebook/base/handlers.py b/notebook/base/handlers.py index 7edc4099d..1dd376438 100755 --- a/notebook/base/handlers.py +++ b/notebook/base/handlers.py @@ -362,7 +362,7 @@ class IPythonHandler(AuthenticatedHandler): ignore_minified_js=self.ignore_minified_js, xsrf_form_html=self.xsrf_form_html, token=self.token, - xsrf_token=self.xsrf_token, + xsrf_token=self.xsrf_token.decode('utf8'), **self.jinja_template_vars ) diff --git a/notebook/static/base/js/utils.js b/notebook/static/base/js/utils.js index 46911233b..1089d40c6 100644 --- a/notebook/static/base/js/utils.js +++ b/notebook/static/base/js/utils.js @@ -603,7 +603,7 @@ define([ var to_absolute_cursor_pos = function (cm, cursor) { console.warn('`utils.to_absolute_cursor_pos(cm, pos)` is deprecated. Use `cm.indexFromPos(cursor)`'); - return cm.indexFromPos(cusrsor); + return cm.indexFromPos(cursor); }; var from_absolute_cursor_pos = function (cm, cursor_pos) { @@ -752,6 +752,29 @@ define([ return wrapped_error; }; + var ajax = function (url, settings) { + // like $.ajax, but ensure Authorization header is set + settings = _add_auth_header(settings); + return $.ajax(url, settings); + }; + + var _add_auth_header = function (settings) { + /** + * Adds auth header to jquery ajax settings + */ + settings = settings || {}; + if (!settings.headers) { + settings.headers = {}; + } + if (!settings.headers.Authorization) { + var xsrf_token = get_body_data('xsrfToken'); + if (xsrf_token) { + settings.headers['X-XSRFToken'] = xsrf_token; + } + } + return settings; + }; + var promising_ajax = function(url, settings) { /** * Like $.ajax, but returning an ES6 promise. success and error settings @@ -766,7 +789,7 @@ define([ log_ajax_error(jqXHR, status, error); reject(wrap_ajax_error(jqXHR, status, error)); }; - $.ajax(url, settings); + ajax(url, settings); }); }; @@ -1010,10 +1033,11 @@ define([ is_or_has : is_or_has, is_focused : is_focused, mergeopt: mergeopt, - ajax_error_msg : ajax_error_msg, - log_ajax_error : log_ajax_error, requireCodeMirrorMode : requireCodeMirrorMode, XHR_ERROR : XHR_ERROR, + ajax : ajax, + ajax_error_msg : ajax_error_msg, + log_ajax_error : log_ajax_error, wrap_ajax_error : wrap_ajax_error, promising_ajax : promising_ajax, WrappedError: WrappedError, diff --git a/notebook/static/services/kernels/kernel.js b/notebook/static/services/kernels/kernel.js index 1b609dd94..a5942d87d 100644 --- a/notebook/static/services/kernels/kernel.js +++ b/notebook/static/services/kernels/kernel.js @@ -152,7 +152,7 @@ define([ * @param {function} [error] - functon executed on ajax error */ Kernel.prototype.list = function (success, error) { - $.ajax(this.kernel_service_url, { + utils.ajax(this.kernel_service_url, { processData: false, cache: false, type: "GET", @@ -194,7 +194,7 @@ define([ } }; - $.ajax(url, { + utils.ajax(url, { processData: false, cache: false, type: "POST", @@ -218,7 +218,7 @@ define([ * @param {function} [error] - functon executed on ajax error */ Kernel.prototype.get_info = function (success, error) { - $.ajax(this.kernel_url, { + utils.ajax(this.kernel_url, { processData: false, cache: false, type: "GET", @@ -244,7 +244,7 @@ define([ Kernel.prototype.kill = function (success, error) { this.events.trigger('kernel_killed.Kernel', {kernel: this}); this._kernel_dead(); - $.ajax(this.kernel_url, { + utils.ajax(this.kernel_url, { processData: false, cache: false, type: "DELETE", @@ -278,7 +278,7 @@ define([ }; var url = utils.url_path_join(this.kernel_url, 'interrupt'); - $.ajax(url, { + utils.ajax(url, { processData: false, cache: false, type: "POST", @@ -323,7 +323,7 @@ define([ }; var url = utils.url_path_join(this.kernel_url, 'restart'); - $.ajax(url, { + utils.ajax(url, { processData: false, cache: false, type: "POST", diff --git a/notebook/static/services/sessions/session.js b/notebook/static/services/sessions/session.js index 4224c0ec1..ade904f1f 100644 --- a/notebook/static/services/sessions/session.js +++ b/notebook/static/services/sessions/session.js @@ -76,7 +76,7 @@ define([ * @param {function} [error] - functon executed on ajax error */ Session.prototype.list = function (success, error) { - $.ajax(this.session_service_url, { + utils.ajax(this.session_service_url, { processData: false, cache: false, type: "GET", @@ -117,7 +117,7 @@ define([ } }; - $.ajax(this.session_service_url, { + utils.ajax(this.session_service_url, { processData: false, cache: false, type: "POST", @@ -139,7 +139,7 @@ define([ * @param {function} [error] - functon executed on ajax error */ Session.prototype.get_info = function (success, error) { - $.ajax(this.session_url, { + utils.ajax(this.session_url, { processData: false, cache: false, type: "GET", @@ -165,7 +165,7 @@ define([ this.notebook_model.path = path; } - $.ajax(this.session_url, { + utils.ajax(this.session_url, { processData: false, cache: false, type: "PATCH", @@ -192,7 +192,7 @@ define([ this.kernel._kernel_dead(); } - $.ajax(this.session_url, { + utils.ajax(this.session_url, { processData: false, cache: false, type: "DELETE", diff --git a/notebook/static/tree/js/notebooklist.js b/notebook/static/tree/js/notebooklist.js index 4019e435a..dfe08b3e5 100644 --- a/notebook/static/tree/js/notebooklist.js +++ b/notebook/static/tree/js/notebooklist.js @@ -734,7 +734,7 @@ define([ 'api/sessions', encodeURIComponent(session.id) ); - $.ajax(url, settings); + utils.ajax(url, settings); } }; diff --git a/notebook/static/tree/js/sessionlist.js b/notebook/static/tree/js/sessionlist.js index 001eb9f04..3c664193b 100644 --- a/notebook/static/tree/js/sessionlist.js +++ b/notebook/static/tree/js/sessionlist.js @@ -62,7 +62,7 @@ define([ error : utils.log_ajax_error, }; var url = utils.url_path_join(this.base_url, 'api/sessions'); - $.ajax(url, settings); + utils.ajax(url, settings); }; SesssionList.prototype.sessions_loaded = function(data){ diff --git a/notebook/static/tree/js/terminallist.js b/notebook/static/tree/js/terminallist.js index c8e8fcf1c..975aad906 100644 --- a/notebook/static/tree/js/terminallist.js +++ b/notebook/static/tree/js/terminallist.js @@ -60,12 +60,12 @@ define([ this.base_url, 'api/terminals' ); - $.ajax(url, settings); + utils.ajax(url, settings); }; TerminalList.prototype.load_terminals = function() { var url = utils.url_path_join(this.base_url, 'api/terminals'); - $.ajax(url, { + utils.ajax(url, { type: "GET", cache: false, dataType: "json", @@ -113,7 +113,7 @@ define([ }; var url = utils.url_path_join(that.base_url, 'api/terminals', utils.encode_uri_components(name)); - $.ajax(url, settings); + utils.ajax(url, settings); return false; }); item.find(".item_buttons").text("").append(shutdown_button); diff --git a/notebook/templates/page.html b/notebook/templates/page.html index e96aac227..9da14665d 100644 --- a/notebook/templates/page.html +++ b/notebook/templates/page.html @@ -197,7 +197,14 @@ - +