Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.5.x] Fixed #20922 -- Allowed customizing the serializer used by co…

…ntrib.sessions

Added settings.SESSION_SERIALIZER which is the import path of a serializer
to use for sessions.

Thanks apollo13, carljm, shaib, akaariai, charettes, and dstufft for reviews.

Backport of b0ce6fe from master
  • Loading branch information...
commit 616a4d385a79d6809b618dcc8bcbda05b032d5fc 1 parent 1b23604
@timgraham timgraham authored
View
1  django/conf/global_settings.py
@@ -467,6 +467,7 @@
SESSION_EXPIRE_AT_BROWSER_CLOSE = False # Whether a user's session cookie expires when the Web browser is closed.
SESSION_ENGINE = 'django.contrib.sessions.backends.db' # The module to store session data
SESSION_FILE_PATH = None # Directory to store session files if using the file session module. If None, the backend will use a sensible default.
+SESSION_SERIALIZER = 'django.contrib.sessions.serializers.PickleSerializer' # class to serialize session data
#########
# CACHE #
View
2  django/conf/project_template/project_name/settings.py
@@ -126,6 +126,8 @@
# 'django.contrib.admindocs',
)
+SESSION_SERIALIZER = 'django.contrib.sessions.serializers.JSONSerializer'
+
# A sample logging configuration. The only tangible logging
# performed by this configuration is to send an email to
# the site admins on every HTTP 500 error when DEBUG=False.
View
17 django/contrib/messages/storage/session.py
@@ -1,4 +1,8 @@
+import json
+
from django.contrib.messages.storage.base import BaseStorage
+from django.contrib.messages.storage.cookie import MessageEncoder, MessageDecoder
+from django.utils import six
class SessionStorage(BaseStorage):
@@ -20,14 +24,23 @@ def _get(self, *args, **kwargs):
always stores everything it is given, so return True for the
all_retrieved flag.
"""
- return self.request.session.get(self.session_key), True
+ return self.deserialize_messages(self.request.session.get(self.session_key)), True
def _store(self, messages, response, *args, **kwargs):
"""
Stores a list of messages to the request's session.
"""
if messages:
- self.request.session[self.session_key] = messages
+ self.request.session[self.session_key] = self.serialize_messages(messages)
else:
self.request.session.pop(self.session_key, None)
return []
+
+ def serialize_messages(self, messages):
+ encoder = MessageEncoder(separators=(',', ':'))
+ return encoder.encode(messages)
+
+ def deserialize_messages(self, data):
+ if data and isinstance(data, six.string_types):
+ return json.loads(data, cls=MessageDecoder)
+ return data
View
1  django/contrib/messages/tests/base.py
@@ -61,6 +61,7 @@ def setUp(self):
MESSAGE_TAGS = '',
MESSAGE_STORAGE = '%s.%s' % (self.storage_class.__module__,
self.storage_class.__name__),
+ SESSION_SERIALIZER = 'django.contrib.sessions.serializers.JSONSerializer',
)
self.settings_override.enable()
View
4 django/contrib/messages/tests/session.py
@@ -10,13 +10,13 @@ def set_session_data(storage, messages):
Sets the messages into the backend request's session and remove the
backend's loaded data cache.
"""
- storage.request.session[storage.session_key] = messages
+ storage.request.session[storage.session_key] = storage.serialize_messages(messages)
if hasattr(storage, '_loaded_data'):
del storage._loaded_data
def stored_session_messages_count(storage):
- data = storage.request.session.get(storage.session_key, [])
+ data = storage.deserialize_messages(storage.request.session.get(storage.session_key, []))
return len(data)
View
25 django/contrib/sessions/backends/base.py
@@ -2,10 +2,6 @@
import base64
from datetime import datetime, timedelta
-try:
- from django.utils.six.moves import cPickle as pickle
-except ImportError:
- import pickle
import string
from django.conf import settings
@@ -15,6 +11,7 @@
from django.utils.crypto import salted_hmac
from django.utils import timezone
from django.utils.encoding import force_bytes
+from django.utils.module_loading import import_by_path
# session_key should not be case sensitive because some backends can store it
# on case insensitive file systems.
@@ -38,6 +35,7 @@ def __init__(self, session_key=None):
self._session_key = session_key
self.accessed = False
self.modified = False
+ self.serializer = import_by_path(settings.SESSION_SERIALIZER)
def __contains__(self, key):
return key in self._session
@@ -82,24 +80,25 @@ def _hash(self, value):
return salted_hmac(key_salt, value).hexdigest()
def encode(self, session_dict):
- "Returns the given session dictionary pickled and encoded as a string."
- pickled = pickle.dumps(session_dict, pickle.HIGHEST_PROTOCOL)
- hash = self._hash(pickled)
- return base64.b64encode(hash.encode() + b":" + pickled).decode('ascii')
+ "Returns the given session dictionary serialized and encoded as a string."
+ serialized = self.serializer().dumps(session_dict)
+ hash = self._hash(serialized)
+ return base64.b64encode(hash.encode() + b":" + serialized).decode('ascii')
def decode(self, session_data):
encoded_data = base64.b64decode(force_bytes(session_data))
try:
# could produce ValueError if there is no ':'
- hash, pickled = encoded_data.split(b':', 1)
- expected_hash = self._hash(pickled)
+ hash, serialized = encoded_data.split(b':', 1)
+ expected_hash = self._hash(serialized)
if not constant_time_compare(hash.decode(), expected_hash):
raise SuspiciousOperation("Session data corrupted")
else:
- return pickle.loads(pickled)
+ return self.serializer().loads(serialized)
except Exception:
- # ValueError, SuspiciousOperation, unpickling exceptions. If any of
- # these happen, just return an empty dictionary (an empty session).
+ # ValueError, SuspiciousOperation, deserialization exceptions. If
+ # any of these happen, just return an empty dictionary (an empty
+ # session).
return {}
def update(self, dict_):
View
21 django/contrib/sessions/backends/signed_cookies.py
@@ -1,26 +1,9 @@
-try:
- from django.utils.six.moves import cPickle as pickle
-except ImportError:
- import pickle
-
from django.conf import settings
from django.core import signing
from django.contrib.sessions.backends.base import SessionBase
-class PickleSerializer(object):
- """
- Simple wrapper around pickle to be used in signing.dumps and
- signing.loads.
- """
- def dumps(self, obj):
- return pickle.dumps(obj, pickle.HIGHEST_PROTOCOL)
-
- def loads(self, data):
- return pickle.loads(data)
-
-
class SessionStore(SessionBase):
def load(self):
@@ -31,7 +14,7 @@ def load(self):
"""
try:
return signing.loads(self.session_key,
- serializer=PickleSerializer,
+ serializer=self.serializer,
# This doesn't handle non-default expiry dates, see #19201
max_age=settings.SESSION_COOKIE_AGE,
salt='django.contrib.sessions.backends.signed_cookies')
@@ -91,7 +74,7 @@ def _get_session_key(self):
session_cache = getattr(self, '_session_cache', {})
return signing.dumps(session_cache, compress=True,
salt='django.contrib.sessions.backends.signed_cookies',
- serializer=PickleSerializer)
+ serializer=self.serializer)
@classmethod
def clear_expired(cls):
View
2  django/contrib/sessions/models.py
@@ -5,7 +5,7 @@
class SessionManager(models.Manager):
def encode(self, session_dict):
"""
- Returns the given session dictionary pickled and encoded as a string.
+ Returns the given session dictionary serialized and encoded as a string.
"""
return SessionStore().encode(session_dict)
View
20 django/contrib/sessions/serializers.py
@@ -0,0 +1,20 @@
+from django.core.signing import JSONSerializer as BaseJSONSerializer
+try:
+ from django.utils.six.moves import cPickle as pickle
+except ImportError:
+ import pickle
+
+
+class PickleSerializer(object):
+ """
+ Simple wrapper around pickle to be used in signing.dumps and
+ signing.loads.
+ """
+ def dumps(self, obj):
+ return pickle.dumps(obj, pickle.HIGHEST_PROTOCOL)
+
+ def loads(self, data):
+ return pickle.loads(data)
+
+
+JSONSerializer = BaseJSONSerializer
View
34 django/contrib/sessions/tests.py
@@ -273,21 +273,25 @@ def test_decode(self):
self.assertEqual(self.session.decode(encoded), data)
def test_actual_expiry(self):
- # Regression test for #19200
- old_session_key = None
- new_session_key = None
- try:
- self.session['foo'] = 'bar'
- self.session.set_expiry(-timedelta(seconds=10))
- self.session.save()
- old_session_key = self.session.session_key
- # With an expiry date in the past, the session expires instantly.
- new_session = self.backend(self.session.session_key)
- new_session_key = new_session.session_key
- self.assertNotIn('foo', new_session)
- finally:
- self.session.delete(old_session_key)
- self.session.delete(new_session_key)
+ # this doesn't work with JSONSerializer (serializing timedelta)
+ with override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer'):
+ self.session = self.backend() # reinitialize after overriding settings
+
+ # Regression test for #19200
+ old_session_key = None
+ new_session_key = None
+ try:
+ self.session['foo'] = 'bar'
+ self.session.set_expiry(-timedelta(seconds=10))
+ self.session.save()
+ old_session_key = self.session.session_key
+ # With an expiry date in the past, the session expires instantly.
+ new_session = self.backend(self.session.session_key)
+ new_session_key = new_session.session_key
+ self.assertNotIn('foo', new_session)
+ finally:
+ self.session.delete(old_session_key)
+ self.session.delete(new_session_key)
class DatabaseSessionTests(SessionTestsMixin, TestCase):
View
29 django/utils/module_loading.py
@@ -2,6 +2,35 @@
import os
import sys
+from django.core.exceptions import ImproperlyConfigured
+from django.utils import six
+from django.utils.importlib import import_module
+
+
+def import_by_path(dotted_path, error_prefix=''):
+ """
+ Import a dotted module path and return the attribute/class designated by the
+ last name in the path. Raise ImproperlyConfigured if something goes wrong.
+ """
+ try:
+ module_path, class_name = dotted_path.rsplit('.', 1)
+ except ValueError:
+ raise ImproperlyConfigured("%s%s doesn't look like a module path" % (
+ error_prefix, dotted_path))
+ try:
+ module = import_module(module_path)
+ except ImportError as e:
+ msg = '%sError importing module %s: "%s"' % (
+ error_prefix, module_path, e)
+ six.reraise(ImproperlyConfigured, ImproperlyConfigured(msg),
+ sys.exc_info()[2])
+ try:
+ attr = getattr(module, class_name)
+ except AttributeError:
+ raise ImproperlyConfigured('%sModule "%s" does not define a "%s" attribute/class' % (
+ error_prefix, module_path, class_name))
+ return attr
+
def module_has_submodule(package, module_name):
"""See if 'module' is in 'package'."""
View
28 docs/ref/settings.txt
@@ -1444,6 +1444,8 @@ Sets the minimum message level that will be recorded by the messages
framework. See the :doc:`messages documentation </ref/contrib/messages>` for
more details.
+.. setting:: MESSAGE_STORAGE
+
MESSAGE_STORAGE
---------------
@@ -1817,7 +1819,7 @@ SESSION_ENGINE
Default: ``django.contrib.sessions.backends.db``
-Controls where Django stores session data. Valid values are:
+Controls where Django stores session data. Included engines are:
* ``'django.contrib.sessions.backends.db'``
* ``'django.contrib.sessions.backends.file'``
@@ -1859,6 +1861,30 @@ Default: ``False``
Whether to save the session data on every request. See
:doc:`/topics/http/sessions`.
+.. setting:: SESSION_SERIALIZER
+
+SESSION_SERIALIZER
+------------------
+
+.. versionadded:: 1.5.3
+
+Default: ``'django.contrib.sessions.serializers.PickleSerializer'``
+
+Full import path of a serializer class to use for serializing session data.
+Included serializers are:
+
+* ``'django.contrib.sessions.serializers.PickleSerializer'``
+* ``'django.contrib.sessions.serializers.JSONSerializer'``
+
+See :ref:`session_serialization` for details, including a warning regarding
+possible remote code execution when using
+:class:`~django.contrib.sessions.serializers.PickleSerializer`.
+
+In Django 1.5.3, the default in newly created projects using
+:djadmin:`django-admin.py startproject <startproject>` is
+:class:`django.contrib.sessions.serializers.JSONSerializer`, and the global
+default will switch to this class in Django 1.6.
+
.. setting:: SHORT_DATE_FORMAT
SHORT_DATE_FORMAT
View
88 docs/topics/http/sessions.txt
@@ -124,8 +124,9 @@ and the :setting:`SECRET_KEY` setting.
.. warning::
- **If the SECRET_KEY is not kept secret, this can lead to arbitrary remote
- code execution.**
+ **If the SECRET_KEY is not kept secret and you are using the**
+ :class:`~django.contrib.sessions.serializers.PickleSerializer`, **this can
+ lead to arbitrary remote code execution.**
An attacker in possession of the :setting:`SECRET_KEY` can not only
generate falsified session data, which your site will trust, but also
@@ -252,7 +253,9 @@ You can edit it multiple times.
in 5 minutes.
* If ``value`` is a ``datetime`` or ``timedelta`` object, the
- session will expire at that specific date/time.
+ session will expire at that specific date/time. Note that ``datetime``
+ and ``timedelta`` values are only serializable if you are using the
+ :class:`~django.contrib.sessions.serializers.PickleSerializer`.
* If ``value`` is ``0``, the user's session cookie will expire
when the user's Web browser is closed.
@@ -299,6 +302,73 @@ You can edit it multiple times.
Removes expired sessions from the session store. This class method is
called by :djadmin:`clearsessions`.
+.. _session_serialization:
+
+Session serialization
+---------------------
+
+.. versionadded:: 1.5.3
+
+Before version 1.6, Django defaulted to using :mod:`pickle` to serialize
+session data before storing it in the backend. If you're using the :ref:`signed
+cookie session backend<cookie-session-backend>` and :setting:`SECRET_KEY` is
+known by an attacker, the attacker could insert a string into his session
+which, when unpickled, executes arbitrary code on the server. The technique for
+doing so is simple and easily available on the internet. Although the cookie
+session storage signs the cookie-stored data to prevent tampering, a
+:setting:`SECRET_KEY` leak immediately escalates to a remote code execution
+vulnerability.
+
+This attack can be mitigated by serializing session data using JSON rather
+than :mod:`pickle`. To facilitate this, Django 1.5.3 introduced a new setting,
+:setting:`SESSION_SERIALIZER`, to customize the session serialization format.
+For backwards compatibility, this setting defaults to
+using :class:`django.contrib.sessions.serializers.PickleSerializer` in Django
+1.5.x, but, for security hardening, defaults to
+:class:`django.contrib.sessions.serializers.JSONSerializer` in Django 1.6. If
+you upgrade and switch from pickle to JSON, sessions created before the upgrade
+will be lost. Even with the caveats described in :ref:`custom-serializers`, we
+highly recommend using JSON serialization *especially if you are using the
+cookie backend*.
+
+Bundled Serializers
+^^^^^^^^^^^^^^^^^^^
+
+.. class:: serializers.JSONSerializer
+
+ A wrapper around the JSON serializer from :mod:`django.core.signing`. Can
+ only serialize basic data types. See the :ref:`custom-serializers` section
+ for more details.
+
+.. class:: serializers.PickleSerializer
+
+ Supports arbitrary Python objects, but, as described above, can lead to a
+ remote code execution vulnerability if :setting:`SECRET_KEY` becomes known
+ by an attacker.
+
+.. _custom-serializers:
+
+Write Your Own Serializer
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Note that unlike :class:`~django.contrib.sessions.serializers.PickleSerializer`,
+the :class:`~django.contrib.sessions.serializers.JSONSerializer` cannot handle
+arbitrary Python data types. As is often the case, there is a trade-off between
+convenience and security. If you wish to store more advanced data types
+including ``datetime`` and ``Decimal`` in JSON backed sessions, you will need
+to write a custom serializer (or convert such values to a JSON serializable
+object before storign them in ``request.session``). While serializing these
+values is fairly straightforward
+(``django.core.serializers.json.DateTimeAwareJSONEncoder`` may
+be helpful), writing a decoder that can reliably get back the same thing that
+you put in is more fragile. For example, you run the risk of returning a
+``datetime`` that was actually a string that just happened to be in the same
+format chosen for ``datetime``\s).
+
+Your serializer class must implement two methods,
+``dumps(self, obj)`` and ``loads(self, data)``, to serialize and deserialize
+the dictionary of session data, respectively.
+
Session object guidelines
-------------------------
@@ -388,14 +458,15 @@ An API is available to manipulate session data outside of a view::
>>> from django.contrib.sessions.backends.db import SessionStore
>>> import datetime
>>> s = SessionStore()
- >>> s['last_login'] = datetime.datetime(2005, 8, 20, 13, 35, 10)
+ >>> # stored as seconds since epoch since datetimes are not serializable in JSON.
+ >>> s['last_login'] = 1376587691
>>> s.save()
>>> s.session_key
'2b1189a188b44ad18c35e113ac6ceead'
>>> s = SessionStore(session_key='2b1189a188b44ad18c35e113ac6ceead')
>>> s['last_login']
- datetime.datetime(2005, 8, 20, 13, 35, 0)
+ 1376587691
In order to prevent session fixation attacks, sessions keys that don't exist
are regenerated::
@@ -634,8 +705,11 @@ that is, if any of its dictionary values have been assigned or deleted.
Technical details
=================
-* The session dictionary should accept any pickleable Python object. See
- the :mod:`pickle` module for more information.
+* The session dictionary accepts any :mod:`json` serializable value when using
+ :class:`~django.contrib.sessions.serializers.JSONSerializer` or any
+ pickleable Python object when using
+ :class:`~django.contrib.sessions.serializers.PickleSerializer`. See the
+ :mod:`pickle` module for more information.
* Session data is stored in a database table named ``django_session`` .
View
40 tests/regressiontests/defer_regress/tests.py
@@ -7,6 +7,7 @@
from django.db.models import Count
from django.db.models.loading import cache
from django.test import TestCase
+from django.test.utils import override_settings
from .models import (ResolveThis, Item, RelatedItem, Child, Leaf, Proxy,
SimpleItem, Feature, ItemAndSimpleItem, OneToOneItem, SpecialFeature)
@@ -80,24 +81,6 @@ def test_basic(self):
self.assertEqual(results[0].child.name, "c1")
self.assertEqual(results[0].second_child.name, "c2")
- # Test for #12163 - Pickling error saving session with unsaved model
- # instances.
- SESSION_KEY = '2b1189a188b44ad18c35e1baac6ceead'
-
- item = Item()
- item._deferred = False
- s = SessionStore(SESSION_KEY)
- s.clear()
- s["item"] = item
- s.save()
-
- s = SessionStore(SESSION_KEY)
- s.modified = True
- s.save()
-
- i2 = s["item"]
- self.assertFalse(i2._deferred)
-
# Regression for #11936 - loading.get_models should not return deferred
# models by default.
klasses = sorted(
@@ -159,6 +142,27 @@ def test_basic(self):
]
)
+ @override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer')
+ def test_ticket_12163(self):
+ # Test for #12163 - Pickling error saving session with unsaved model
+ # instances.
+ SESSION_KEY = '2b1189a188b44ad18c35e1baac6ceead'
+
+ item = Item()
+ item._deferred = False
+ s = SessionStore(SESSION_KEY)
+ s.clear()
+ s["item"] = item
+ s.save()
+
+ s = SessionStore(SESSION_KEY)
+ s.modified = True
+ s.save()
+
+ i2 = s["item"]
+ self.assertFalse(i2._deferred)
+
+ def test_ticket_16409(self):
# Regression for #16409 - make sure defer() and only() work with annotate()
self.assertIsInstance(list(SimpleItem.objects.annotate(Count('feature')).defer('name')), list)
self.assertIsInstance(list(SimpleItem.objects.annotate(Count('feature')).only('name')), list)
View
20 tests/regressiontests/utils/module_loading.py
@@ -3,9 +3,10 @@
import imp
from zipimport import zipimporter
+from django.core.exceptions import ImproperlyConfigured
from django.utils import unittest
from django.utils.importlib import import_module
-from django.utils.module_loading import module_has_submodule
+from django.utils.module_loading import import_by_path, module_has_submodule
from django.utils._os import upath
@@ -103,6 +104,23 @@ def test_deep_loader(self):
self.assertFalse(module_has_submodule(egg_module, 'no_such_module'))
self.assertRaises(ImportError, import_module, 'egg_module.sub1.sub2.no_such_module')
+
+class ModuleImportTestCase(unittest.TestCase):
+ def test_import_by_path(self):
+ cls = import_by_path(
+ 'django.utils.module_loading.import_by_path')
+ self.assertEqual(cls, import_by_path)
+
+ # Test exceptions raised
+ for path in ('no_dots_in_path', 'unexistent.path',
+ 'tests.regressiontests.utils.unexistent'):
+ self.assertRaises(ImproperlyConfigured, import_by_path, path)
+
+ with self.assertRaises(ImproperlyConfigured) as cm:
+ import_by_path('unexistent.module.path', error_prefix="Foo")
+ self.assertTrue(str(cm.exception).startswith('Foo'))
+
+
class ProxyFinder(object):
def __init__(self):
self._cache = {}
Please sign in to comment.
Something went wrong with that request. Please try again.