diff --git a/CHANGES b/CHANGES index de6d13ec..a7c97cd7 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,10 @@ Version 0.10 Release date to be decided. +- Changed default cookie serialization format from pickle to JSON to + limit the impact an attacker can do if the secret key leaks. See + :ref:`upgrading-to-010` for more information. + Version 0.9 ----------- diff --git a/docs/api.rst b/docs/api.rst index 8a7b5ce0..316c76ab 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -215,6 +215,9 @@ implementation that Flask is using. .. autoclass:: SecureCookieSessionInterface :members: +.. autoclass:: SecureCookieSession + :members: + .. autoclass:: NullSession :members: diff --git a/docs/upgrading.rst b/docs/upgrading.rst index 7226d60e..c295fb1c 100644 --- a/docs/upgrading.rst +++ b/docs/upgrading.rst @@ -19,6 +19,20 @@ installation, make sure to pass it the ``-U`` parameter:: $ easy_install -U Flask +.. _upgrading-to-010: + +Version 0.10 +------------ + +The biggest change going from 0.9 to 0.10 is that the cookie serialization +format changed from pickle to a specialized JSON format. This change has +been done in order to avoid the damage an attacker can do if the secret +key is leaked. When you upgrade you will notice two major changes: all +sessions that were issued before the upgrade are invalidated and you can +only store a limited amount of types in the session. + +TODO: add external module for session upgrading + Version 0.9 ----------- diff --git a/flask/__init__.py b/flask/__init__.py index 196eb033..ac1a3a00 100644 --- a/flask/__init__.py +++ b/flask/__init__.py @@ -20,7 +20,7 @@ from jinja2 import Markup, escape from .app import Flask, Request, Response from .config import Config -from .helpers import url_for, jsonify, json_available, flash, \ +from .helpers import url_for, jsonify, flash, \ send_file, send_from_directory, get_flashed_messages, \ get_template_attribute, make_response, safe_join, \ stream_with_context @@ -37,8 +37,8 @@ from .signals import signals_available, template_rendered, request_started, \ request_finished, got_request_exception, request_tearing_down # only import json if it's available -if json_available: - from .helpers import json +from .helpers import json # backwards compat, goes away in 1.0 from .sessions import SecureCookieSession as Session +json_available = True diff --git a/flask/helpers.py b/flask/helpers.py index 7e20c97d..b2d34a71 100644 --- a/flask/helpers.py +++ b/flask/helpers.py @@ -23,21 +23,9 @@ from werkzeug.routing import BuildError from werkzeug.urls import url_quote from functools import update_wrapper -# try to load the best simplejson implementation available. If JSON -# is not installed, we add a failing class. -json_available = True -json = None -try: - import simplejson as json -except ImportError: - try: - import json - except ImportError: - try: - # Google Appengine offers simplejson via django - from django.utils import simplejson as json - except ImportError: - json_available = False +# Use the same json implementation as itsdangerous on which we +# depend anyways. +from itsdangerous import simplejson as json from werkzeug.datastructures import Headers @@ -55,19 +43,10 @@ from .globals import session, _request_ctx_stack, _app_ctx_stack, \ current_app, request -def _assert_have_json(): - """Helper function that fails if JSON is unavailable.""" - if not json_available: - raise RuntimeError('simplejson not installed') - - # figure out if simplejson escapes slashes. This behavior was changed # from one version to another without reason. -if not json_available or '\\/' not in json.dumps('/'): - +if '\\/' not in json.dumps('/'): def _tojson_filter(*args, **kwargs): - if __debug__: - _assert_have_json() return json.dumps(*args, **kwargs).replace('/', '\\/') else: _tojson_filter = json.dumps @@ -192,8 +171,6 @@ def jsonify(*args, **kwargs): .. versionadded:: 0.2 """ - if __debug__: - _assert_have_json() return current_app.response_class(json.dumps(dict(*args, **kwargs), indent=None if request.is_xhr else 2), mimetype='application/json') diff --git a/flask/sessions.py b/flask/sessions.py index 2795bb1f..ba0b0ed7 100644 --- a/flask/sessions.py +++ b/flask/sessions.py @@ -10,8 +10,14 @@ :license: BSD, see LICENSE for more details. """ +import hashlib from datetime import datetime -from werkzeug.contrib.securecookie import SecureCookie +from werkzeug.http import http_date, parse_date +from werkzeug.datastructures import CallbackDict +from .helpers import json +from . import Markup + +from itsdangerous import URLSafeTimedSerializer, BadSignature class SessionMixin(object): @@ -41,11 +47,53 @@ class SessionMixin(object): modified = True -class SecureCookieSession(SecureCookie, SessionMixin): - """Expands the session with support for switching between permanent - and non-permanent sessions. +class TaggedJSONSerializer(object): + """A customized JSON serializer that supports a few extra types that + we take for granted when serializing (tuples, markup objects, datetime). """ + def dumps(self, value): + def _tag(value): + if isinstance(value, tuple): + return {' t': [_tag(x) for x in value]} + elif callable(getattr(value, '__html__', None)): + return {' m': unicode(value.__html__())} + elif isinstance(value, list): + return [_tag(x) for x in value] + elif isinstance(value, datetime): + return {' d': http_date(value)} + elif isinstance(value, dict): + return dict((k, _tag(v)) for k, v in value.iteritems()) + return value + return json.dumps(_tag(value), separators=(',', ':')) + + def loads(self, value): + def object_hook(obj): + if len(obj) != 1: + return obj + the_key, the_value = obj.iteritems().next() + if the_key == ' t': + return tuple(the_value) + elif the_key == ' m': + return Markup(the_value) + elif the_key == ' d': + return parse_date(the_value) + return obj + return json.loads(value, object_hook=object_hook) + + +session_json_serializer = TaggedJSONSerializer() + + +class SecureCookieSession(CallbackDict, SessionMixin): + """Baseclass for sessions based on signed cookies.""" + + def __init__(self, initial=None): + def on_update(self): + self.modified = True + CallbackDict.__init__(self, initial, on_update) + self.modified = False + class NullSession(SecureCookieSession): """Class used to generate nicer error messages if sessions are not @@ -98,6 +146,13 @@ class SessionInterface(object): #: this type. null_session_class = NullSession + #: A flag that indicates if the session interface is pickle based. + #: This can be used by flask extensions to make a decision in regards + #: to how to deal with the session object. + #: + #: .. versionadded:: 0.10 + pickle_based = False + def make_null_session(self, app): """Creates a null session which acts as a replacement object if the real session support could not be loaded due to a configuration @@ -178,28 +233,60 @@ class SessionInterface(object): class SecureCookieSessionInterface(SessionInterface): - """The cookie session interface that uses the Werkzeug securecookie - as client side session backend. + """The default session interface that stores sessions in signed cookies + through the :mod:`itsdangerous` module. """ + #: the salt that should be applied on top of the secret key for the + #: signing of cookie based sessions. + salt = 'cookie-session' + #: the hash function to use for the signature. The default is sha1 + digest_method = staticmethod(hashlib.sha1) + #: the name of the itsdangerous supported key derivation. The default + #: is hmac. + key_derivation = 'hmac' + #: A python serializer for the payload. The default is a compact + #: JSON derived serializer with support for some extra Python types + #: such as datetime objects or tuples. + serializer = session_json_serializer session_class = SecureCookieSession + def get_signing_serializer(self, app): + if not app.secret_key: + return None + signer_kwargs = dict( + key_derivation=self.key_derivation, + digest_method=self.digest_method + ) + return URLSafeTimedSerializer(app.secret_key, salt=self.salt, + serializer=self.serializer, + signer_kwargs=signer_kwargs) + def open_session(self, app, request): - key = app.secret_key - if key is not None: - return self.session_class.load_cookie(request, - app.session_cookie_name, - secret_key=key) + s = self.get_signing_serializer(app) + if s is None: + return None + val = request.cookies.get(app.session_cookie_name) + if not val: + return self.session_class() + max_age = app.permanent_session_lifetime.total_seconds() + try: + data = s.loads(val, max_age=max_age) + return self.session_class(data) + except BadSignature: + return self.session_class() def save_session(self, app, session, response): - expires = self.get_expiration_time(app, session) domain = self.get_cookie_domain(app) path = self.get_cookie_path(app) + if not session: + if session.modified: + response.delete_cookie(app.session_cookie_name, + domain=domain, path=path) + return httponly = self.get_cookie_httponly(app) secure = self.get_cookie_secure(app) - if session.modified and not session: - response.delete_cookie(app.session_cookie_name, path=path, - domain=domain) - else: - session.save_cookie(response, app.session_cookie_name, path=path, - expires=expires, httponly=httponly, - secure=secure, domain=domain) + expires = self.get_expiration_time(app, session) + val = self.get_signing_serializer(app).dumps(dict(session)) + response.set_cookie(app.session_cookie_name, val, + expires=expires, httponly=httponly, + domain=domain, path=path, secure=secure) diff --git a/flask/testsuite/basic.py b/flask/testsuite/basic.py index 388b5a8e..3d758b3a 100644 --- a/flask/testsuite/basic.py +++ b/flask/testsuite/basic.py @@ -13,6 +13,7 @@ from __future__ import with_statement import re import flask +import pickle import unittest from datetime import datetime from threading import Thread @@ -297,6 +298,31 @@ class BasicFunctionalityTestCase(FlaskTestCase): self.assert_equal(c.get('/').data, 'None') self.assert_equal(c.get('/').data, '42') + def test_session_special_types(self): + app = flask.Flask(__name__) + app.secret_key = 'development-key' + app.testing = True + now = datetime.utcnow().replace(microsecond=0) + + @app.after_request + def modify_session(response): + flask.session['m'] = flask.Markup('Hello!') + flask.session['dt'] = now + flask.session['t'] = (1, 2, 3) + return response + + @app.route('/') + def dump_session_contents(): + return pickle.dumps(dict(flask.session)) + + c = app.test_client() + c.get('/') + rv = pickle.loads(c.get('/').data) + self.assert_equal(rv['m'], flask.Markup('Hello!')) + self.assert_equal(type(rv['m']), flask.Markup) + self.assert_equal(rv['dt'], now) + self.assert_equal(rv['t'], (1, 2, 3)) + def test_flashes(self): app = flask.Flask(__name__) app.secret_key = 'testkey' diff --git a/flask/wrappers.py b/flask/wrappers.py index 3ee718ff..060ee25b 100644 --- a/flask/wrappers.py +++ b/flask/wrappers.py @@ -14,7 +14,7 @@ from werkzeug.utils import cached_property from .exceptions import JSONBadRequest from .debughelpers import attach_enctype_error_multidict -from .helpers import json, _assert_have_json +from .helpers import json from .globals import _request_ctx_stack @@ -95,8 +95,6 @@ class Request(RequestBase): This requires Python 2.6 or an installed version of simplejson. """ - if __debug__: - _assert_have_json() if self.mimetype == 'application/json': request_charset = self.mimetype_params.get('charset') try: diff --git a/setup.py b/setup.py index 1d0761fe..4a9182b9 100644 --- a/setup.py +++ b/setup.py @@ -91,7 +91,8 @@ setup( platforms='any', install_requires=[ 'Werkzeug>=0.7', - 'Jinja2>=2.4' + 'Jinja2>=2.4', + 'itsdangerous>=0.17' ], classifiers=[ 'Development Status :: 4 - Beta',