diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index d4d4732abe17e..4cf2ba73c10cc 100644 --- a/django/conf/global_settings.py +++ b/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 # diff --git a/django/conf/project_template/project_name/settings.py b/django/conf/project_template/project_name/settings.py index 35117ddc61e2c..331be43833015 100644 --- a/django/conf/project_template/project_name/settings.py +++ b/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. diff --git a/django/contrib/messages/storage/session.py b/django/contrib/messages/storage/session.py index 225dfda289569..c3e293c22e97a 100644 --- a/django/contrib/messages/storage/session.py +++ b/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 diff --git a/django/contrib/messages/tests/base.py b/django/contrib/messages/tests/base.py index b3ced12773b4e..7719d662aa310 100644 --- a/django/contrib/messages/tests/base.py +++ b/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() diff --git a/django/contrib/messages/tests/session.py b/django/contrib/messages/tests/session.py index e162f49fc28ae..0d2daaa19774f 100644 --- a/django/contrib/messages/tests/session.py +++ b/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) diff --git a/django/contrib/sessions/backends/base.py b/django/contrib/sessions/backends/base.py index f79a2645004ab..44b06d6441588 100644 --- a/django/contrib/sessions/backends/base.py +++ b/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_): diff --git a/django/contrib/sessions/backends/signed_cookies.py b/django/contrib/sessions/backends/signed_cookies.py index c2b7a3123f5c7..77a6750ce4712 100644 --- a/django/contrib/sessions/backends/signed_cookies.py +++ b/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): diff --git a/django/contrib/sessions/models.py b/django/contrib/sessions/models.py index 0179c358b3ae1..3a6e31152f080 100644 --- a/django/contrib/sessions/models.py +++ b/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) diff --git a/django/contrib/sessions/serializers.py b/django/contrib/sessions/serializers.py new file mode 100644 index 0000000000000..92a31c054bbb8 --- /dev/null +++ b/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 diff --git a/django/contrib/sessions/tests.py b/django/contrib/sessions/tests.py index 8bcc505ee6fb1..829eb821072ae 100644 --- a/django/contrib/sessions/tests.py +++ b/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): diff --git a/django/utils/module_loading.py b/django/utils/module_loading.py index f8aadb34dfb22..ede585e562ba0 100644 --- a/django/utils/module_loading.py +++ b/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'.""" diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 64f269a553c5c..fc3f33904c4f4 100644 --- a/docs/ref/settings.txt +++ b/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 ` 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 ` 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 diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt index 320f26b384b6f..d430a532dfcad 100644 --- a/docs/topics/http/sessions.txt +++ b/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` 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`` . diff --git a/tests/regressiontests/defer_regress/tests.py b/tests/regressiontests/defer_regress/tests.py index d4d722035ff0b..0bd0eedf017f3 100644 --- a/tests/regressiontests/defer_regress/tests.py +++ b/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) diff --git a/tests/regressiontests/utils/module_loading.py b/tests/regressiontests/utils/module_loading.py index 3fc92b0862c07..a9e3706bb9b6f 100644 --- a/tests/regressiontests/utils/module_loading.py +++ b/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 = {}