Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #20922 -- Allowed customizing the serializer used by contrib.se…

…ssions

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.
  • Loading branch information...
commit b0ce6fe656873825271bacb55e55474fc346c1c6 1 parent 9d1987d
Tim Graham authored August 21, 2013
1  django/conf/global_settings.py
@@ -475,6 +475,7 @@
475 475
 SESSION_EXPIRE_AT_BROWSER_CLOSE = False                 # Whether a user's session cookie expires when the Web browser is closed.
476 476
 SESSION_ENGINE = 'django.contrib.sessions.backends.db'  # The module to store session data
477 477
 SESSION_FILE_PATH = None                                # Directory to store session files if using the file session module. If None, the backend will use a sensible default.
  478
+SESSION_SERIALIZER = 'django.contrib.sessions.serializers.JSONSerializer'  # class to serialize session data
478 479
 
479 480
 #########
480 481
 # CACHE #
17  django/contrib/messages/storage/session.py
... ...
@@ -1,4 +1,8 @@
  1
+import json
  2
+
1 3
 from django.contrib.messages.storage.base import BaseStorage
  4
+from django.contrib.messages.storage.cookie import MessageEncoder, MessageDecoder
  5
+from django.utils import six
2 6
 
3 7
 
4 8
 class SessionStorage(BaseStorage):
@@ -20,14 +24,23 @@ def _get(self, *args, **kwargs):
20 24
         always stores everything it is given, so return True for the
21 25
         all_retrieved flag.
22 26
         """
23  
-        return self.request.session.get(self.session_key), True
  27
+        return self.deserialize_messages(self.request.session.get(self.session_key)), True
24 28
 
25 29
     def _store(self, messages, response, *args, **kwargs):
26 30
         """
27 31
         Stores a list of messages to the request's session.
28 32
         """
29 33
         if messages:
30  
-            self.request.session[self.session_key] = messages
  34
+            self.request.session[self.session_key] = self.serialize_messages(messages)
31 35
         else:
32 36
             self.request.session.pop(self.session_key, None)
33 37
         return []
  38
+
  39
+    def serialize_messages(self, messages):
  40
+        encoder = MessageEncoder(separators=(',', ':'))
  41
+        return encoder.encode(messages)
  42
+
  43
+    def deserialize_messages(self, data):
  44
+        if data and isinstance(data, six.string_types):
  45
+            return json.loads(data, cls=MessageDecoder)
  46
+        return data
1  django/contrib/messages/tests/base.py
@@ -61,6 +61,7 @@ def setUp(self):
61 61
             MESSAGE_TAGS    = '',
62 62
             MESSAGE_STORAGE = '%s.%s' % (self.storage_class.__module__,
63 63
                                          self.storage_class.__name__),
  64
+            SESSION_SERIALIZER = 'django.contrib.sessions.serializers.JSONSerializer',
64 65
         )
65 66
         self.settings_override.enable()
66 67
 
4  django/contrib/messages/tests/test_session.py
@@ -11,13 +11,13 @@ def set_session_data(storage, messages):
11 11
     Sets the messages into the backend request's session and remove the
12 12
     backend's loaded data cache.
13 13
     """
14  
-    storage.request.session[storage.session_key] = messages
  14
+    storage.request.session[storage.session_key] = storage.serialize_messages(messages)
15 15
     if hasattr(storage, '_loaded_data'):
16 16
         del storage._loaded_data
17 17
 
18 18
 
19 19
 def stored_session_messages_count(storage):
20  
-    data = storage.request.session.get(storage.session_key, [])
  20
+    data = storage.deserialize_messages(storage.request.session.get(storage.session_key, []))
21 21
     return len(data)
22 22
 
23 23
 
21  django/contrib/sessions/backends/base.py
@@ -3,11 +3,6 @@
3 3
 import base64
4 4
 from datetime import datetime, timedelta
5 5
 import logging
6  
-
7  
-try:
8  
-    from django.utils.six.moves import cPickle as pickle
9  
-except ImportError:
10  
-    import pickle
11 6
 import string
12 7
 
13 8
 from django.conf import settings
@@ -17,6 +12,7 @@
17 12
 from django.utils.crypto import salted_hmac
18 13
 from django.utils import timezone
19 14
 from django.utils.encoding import force_bytes, force_text
  15
+from django.utils.module_loading import import_by_path
20 16
 
21 17
 from django.contrib.sessions.exceptions import SuspiciousSession
22 18
 
@@ -42,6 +38,7 @@ def __init__(self, session_key=None):
42 38
         self._session_key = session_key
43 39
         self.accessed = False
44 40
         self.modified = False
  41
+        self.serializer = import_by_path(settings.SESSION_SERIALIZER)
45 42
 
46 43
     def __contains__(self, key):
47 44
         return key in self._session
@@ -86,21 +83,21 @@ def _hash(self, value):
86 83
         return salted_hmac(key_salt, value).hexdigest()
87 84
 
88 85
     def encode(self, session_dict):
89  
-        "Returns the given session dictionary pickled and encoded as a string."
90  
-        pickled = pickle.dumps(session_dict, pickle.HIGHEST_PROTOCOL)
91  
-        hash = self._hash(pickled)
92  
-        return base64.b64encode(hash.encode() + b":" + pickled).decode('ascii')
  86
+        "Returns the given session dictionary serialized and encoded as a string."
  87
+        serialized = self.serializer().dumps(session_dict)
  88
+        hash = self._hash(serialized)
  89
+        return base64.b64encode(hash.encode() + b":" + serialized).decode('ascii')
93 90
 
94 91
     def decode(self, session_data):
95 92
         encoded_data = base64.b64decode(force_bytes(session_data))
96 93
         try:
97 94
             # could produce ValueError if there is no ':'
98  
-            hash, pickled = encoded_data.split(b':', 1)
99  
-            expected_hash = self._hash(pickled)
  95
+            hash, serialized = encoded_data.split(b':', 1)
  96
+            expected_hash = self._hash(serialized)
100 97
             if not constant_time_compare(hash.decode(), expected_hash):
101 98
                 raise SuspiciousSession("Session data corrupted")
102 99
             else:
103  
-                return pickle.loads(pickled)
  100
+                return self.serializer().loads(serialized)
104 101
         except Exception as e:
105 102
             # ValueError, SuspiciousOperation, unpickling exceptions. If any of
106 103
             # these happen, just return an empty dictionary (an empty session).
21  django/contrib/sessions/backends/signed_cookies.py
... ...
@@ -1,26 +1,9 @@
1  
-try:
2  
-    from django.utils.six.moves import cPickle as pickle
3  
-except ImportError:
4  
-    import pickle
5  
-
6 1
 from django.conf import settings
7 2
 from django.core import signing
8 3
 
9 4
 from django.contrib.sessions.backends.base import SessionBase
10 5
 
11 6
 
12  
-class PickleSerializer(object):
13  
-    """
14  
-    Simple wrapper around pickle to be used in signing.dumps and
15  
-    signing.loads.
16  
-    """
17  
-    def dumps(self, obj):
18  
-        return pickle.dumps(obj, pickle.HIGHEST_PROTOCOL)
19  
-
20  
-    def loads(self, data):
21  
-        return pickle.loads(data)
22  
-
23  
-
24 7
 class SessionStore(SessionBase):
25 8
 
26 9
     def load(self):
@@ -31,7 +14,7 @@ def load(self):
31 14
         """
32 15
         try:
33 16
             return signing.loads(self.session_key,
34  
-                serializer=PickleSerializer,
  17
+                serializer=self.serializer,
35 18
                 # This doesn't handle non-default expiry dates, see #19201
36 19
                 max_age=settings.SESSION_COOKIE_AGE,
37 20
                 salt='django.contrib.sessions.backends.signed_cookies')
@@ -91,7 +74,7 @@ def _get_session_key(self):
91 74
         session_cache = getattr(self, '_session_cache', {})
92 75
         return signing.dumps(session_cache, compress=True,
93 76
             salt='django.contrib.sessions.backends.signed_cookies',
94  
-            serializer=PickleSerializer)
  77
+            serializer=self.serializer)
95 78
 
96 79
     @classmethod
97 80
     def clear_expired(cls):
2  django/contrib/sessions/models.py
@@ -5,7 +5,7 @@
5 5
 class SessionManager(models.Manager):
6 6
     def encode(self, session_dict):
7 7
         """
8  
-        Returns the given session dictionary pickled and encoded as a string.
  8
+        Returns the given session dictionary serialized and encoded as a string.
9 9
         """
10 10
         return SessionStore().encode(session_dict)
11 11
 
20  django/contrib/sessions/serializers.py
... ...
@@ -0,0 +1,20 @@
  1
+from django.core.signing import JSONSerializer as BaseJSONSerializer
  2
+try:
  3
+    from django.utils.six.moves import cPickle as pickle
  4
+except ImportError:
  5
+    import pickle
  6
+
  7
+
  8
+class PickleSerializer(object):
  9
+    """
  10
+    Simple wrapper around pickle to be used in signing.dumps and
  11
+    signing.loads.
  12
+    """
  13
+    def dumps(self, obj):
  14
+        return pickle.dumps(obj, pickle.HIGHEST_PROTOCOL)
  15
+
  16
+    def loads(self, data):
  17
+        return pickle.loads(data)
  18
+
  19
+
  20
+JSONSerializer = BaseJSONSerializer
34  django/contrib/sessions/tests.py
@@ -285,21 +285,25 @@ def test_decode_failure_logged_to_security(self):
285 285
 
286 286
 
287 287
     def test_actual_expiry(self):
288  
-        # Regression test for #19200
289  
-        old_session_key = None
290  
-        new_session_key = None
291  
-        try:
292  
-            self.session['foo'] = 'bar'
293  
-            self.session.set_expiry(-timedelta(seconds=10))
294  
-            self.session.save()
295  
-            old_session_key = self.session.session_key
296  
-            # With an expiry date in the past, the session expires instantly.
297  
-            new_session = self.backend(self.session.session_key)
298  
-            new_session_key = new_session.session_key
299  
-            self.assertNotIn('foo', new_session)
300  
-        finally:
301  
-            self.session.delete(old_session_key)
302  
-            self.session.delete(new_session_key)
  288
+        # this doesn't work with JSONSerializer (serializing timedelta)
  289
+        with override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer'):
  290
+            self.session = self.backend()  # reinitialize after overriding settings
  291
+
  292
+            # Regression test for #19200
  293
+            old_session_key = None
  294
+            new_session_key = None
  295
+            try:
  296
+                self.session['foo'] = 'bar'
  297
+                self.session.set_expiry(-timedelta(seconds=10))
  298
+                self.session.save()
  299
+                old_session_key = self.session.session_key
  300
+                # With an expiry date in the past, the session expires instantly.
  301
+                new_session = self.backend(self.session.session_key)
  302
+                new_session_key = new_session.session_key
  303
+                self.assertNotIn('foo', new_session)
  304
+            finally:
  305
+                self.session.delete(old_session_key)
  306
+                self.session.delete(new_session_key)
303 307
 
304 308
 
305 309
 class DatabaseSessionTests(SessionTestsMixin, TestCase):
24  docs/ref/settings.txt
@@ -2403,7 +2403,7 @@ SESSION_ENGINE
2403 2403
 
2404 2404
 Default: ``django.contrib.sessions.backends.db``
2405 2405
 
2406  
-Controls where Django stores session data. Valid values are:
  2406
+Controls where Django stores session data. Included engines are:
2407 2407
 
2408 2408
 * ``'django.contrib.sessions.backends.db'``
2409 2409
 * ``'django.contrib.sessions.backends.file'``
@@ -2446,6 +2446,28 @@ Whether to save the session data on every request. If this is ``False``
2446 2446
 (default), then the session data will only be saved if it has been modified --
2447 2447
 that is, if any of its dictionary values have been assigned or deleted.
2448 2448
 
  2449
+.. setting:: SESSION_SERIALIZER
  2450
+
  2451
+SESSION_SERIALIZER
  2452
+------------------
  2453
+
  2454
+Default: ``'django.contrib.sessions.serializers.JSONSerializer'``
  2455
+
  2456
+.. versionchanged:: 1.6
  2457
+
  2458
+    The default switched from
  2459
+    :class:`~django.contrib.sessions.serializers.PickleSerializer` to
  2460
+    :class:`~django.contrib.sessions.serializers.JSONSerializer` in Django 1.6.
  2461
+
  2462
+Full import path of a serializer class to use for serializing session data.
  2463
+Included serializers are:
  2464
+
  2465
+* ``'django.contrib.sessions.serializers.PickleSerializer'``
  2466
+* ``'django.contrib.sessions.serializers.JSONSerializer'``
  2467
+
  2468
+See :ref:`session_serialization` for details, including a warning regarding
  2469
+possible remote code execution when using
  2470
+:class:`~django.contrib.sessions.serializers.PickleSerializer`.
2449 2471
 
2450 2472
 Sites
2451 2473
 =====
23  docs/releases/1.6.txt
@@ -727,6 +727,29 @@ the ``name`` argument so it doesn't conflict with the new url::
727 727
 You can remove this url pattern after your app has been deployed with Django
728 728
 1.6 for :setting:`PASSWORD_RESET_TIMEOUT_DAYS`.
729 729
 
  730
+Default session serialization switched to JSON
  731
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  732
+
  733
+Historically, :mod:`django.contrib.sessions` used :mod:`pickle` to serialize
  734
+session data before storing it in the backend. If you're using the :ref:`signed
  735
+cookie session backend<cookie-session-backend>` and :setting:`SECRET_KEY` is
  736
+known by an attacker, the attacker could insert a string into his session
  737
+which, when unpickled, executes arbitrary code on the server. The technique for
  738
+doing so is simple and easily available on the internet. Although the cookie
  739
+session storage signs the cookie-stored data to prevent tampering, a
  740
+:setting:`SECRET_KEY` leak immediately escalates to a remote code execution
  741
+vulnerability.
  742
+
  743
+This attack can be mitigated by serializing session data using JSON rather
  744
+than :mod:`pickle`. To facilitate this, Django 1.5.3 introduced a new setting,
  745
+:setting:`SESSION_SERIALIZER`, to customize the session serialization format.
  746
+For backwards compatibility, this setting defaulted to using :mod:`pickle`
  747
+in Django 1.5.3, but we've changed the default to JSON in 1.6. If you upgrade
  748
+and switch from pickle to JSON, sessions created before the upgrade will be
  749
+lost. While JSON serialization does not support all Python objects like
  750
+:mod:`pickle` does, we highly recommend using JSON-serialized sessions. See the
  751
+:ref:`session_serialization` documentation for more details.
  752
+
730 753
 Miscellaneous
731 754
 ~~~~~~~~~~~~~
732 755
 
87  docs/topics/http/sessions.txt
@@ -128,8 +128,9 @@ and the :setting:`SECRET_KEY` setting.
128 128
 
129 129
 .. warning::
130 130
 
131  
-    **If the SECRET_KEY is not kept secret, this can lead to arbitrary remote
132  
-    code execution.**
  131
+    **If the SECRET_KEY is not kept secret and you are using the**
  132
+    :class:`~django.contrib.sessions.serializers.PickleSerializer`, **this can
  133
+    lead to arbitrary remote code execution.**
133 134
 
134 135
     An attacker in possession of the :setting:`SECRET_KEY` can not only
135 136
     generate falsified session data, which your site will trust, but also
@@ -256,7 +257,9 @@ You can edit it multiple times.
256 257
         in 5 minutes.
257 258
 
258 259
       * If ``value`` is a ``datetime`` or ``timedelta`` object, the
259  
-        session will expire at that specific date/time.
  260
+        session will expire at that specific date/time. Note that ``datetime``
  261
+        and ``timedelta`` values are only serializable if you are using the
  262
+        :class:`~django.contrib.sessions.serializers.PickleSerializer`.
260 263
 
261 264
       * If ``value`` is ``0``, the user's session cookie will expire
262 265
         when the user's Web browser is closed.
@@ -301,6 +304,72 @@ You can edit it multiple times.
301 304
       Removes expired sessions from the session store. This class method is
302 305
       called by :djadmin:`clearsessions`.
303 306
 
  307
+.. _session_serialization:
  308
+
  309
+Session serialization
  310
+---------------------
  311
+
  312
+.. versionchanged:: 1.6
  313
+
  314
+Before version 1.6, Django defaulted to using :mod:`pickle` to serialize
  315
+session data before storing it in the backend. If you're using the :ref:`signed
  316
+cookie session backend<cookie-session-backend>` and :setting:`SECRET_KEY` is
  317
+known by an attacker, the attacker could insert a string into his session
  318
+which, when unpickled, executes arbitrary code on the server. The technique for
  319
+doing so is simple and easily available on the internet. Although the cookie
  320
+session storage signs the cookie-stored data to prevent tampering, a
  321
+:setting:`SECRET_KEY` leak immediately escalates to a remote code execution
  322
+vulnerability.
  323
+
  324
+This attack can be mitigated by serializing session data using JSON rather
  325
+than :mod:`pickle`. To facilitate this, Django 1.5.3 introduced a new setting,
  326
+:setting:`SESSION_SERIALIZER`, to customize the session serialization format.
  327
+For backwards compatibility, this setting defaults to
  328
+using :class:`django.contrib.sessions.serializers.PickleSerializer` in
  329
+Django 1.5.x, but, for security hardening, defaults to
  330
+:class:`django.contrib.sessions.serializers.JSONSerializer` in Django 1.6.
  331
+Even with the caveats described in :ref:`custom-serializers`, we highly
  332
+recommend sticking with JSON serialization *especially if you are using the
  333
+cookie backend*.
  334
+
  335
+Bundled Serializers
  336
+^^^^^^^^^^^^^^^^^^^
  337
+
  338
+.. class:: serializers.JSONSerializer
  339
+
  340
+    A wrapper around the JSON serializer from :mod:`django.core.signing`. Can
  341
+    only serialize basic data types. See the :ref:`custom-serializers` section
  342
+    for more details.
  343
+
  344
+.. class:: serializers.PickleSerializer
  345
+
  346
+    Supports arbitrary Python objects, but, as described above, can lead to a
  347
+    remote code execution vulnerability if :setting:`SECRET_KEY` becomes known
  348
+    by an attacker.
  349
+
  350
+.. _custom-serializers:
  351
+
  352
+Write Your Own Serializer
  353
+^^^^^^^^^^^^^^^^^^^^^^^^^
  354
+
  355
+Note that unlike :class:`~django.contrib.sessions.serializers.PickleSerializer`,
  356
+the :class:`~django.contrib.sessions.serializers.JSONSerializer` cannot handle
  357
+arbitrary Python data types. As is often the case, there is a trade-off between
  358
+convenience and security. If you wish to store more advanced data types
  359
+including ``datetime`` and ``Decimal`` in JSON backed sessions, you will need
  360
+to write a custom serializer (or convert such values to a JSON serializable
  361
+object before storing them in ``request.session``). While serializing these
  362
+values is fairly straightforward
  363
+(``django.core.serializers.json.DateTimeAwareJSONEncoder`` may be helpful),
  364
+writing a decoder that can reliably get back the same thing that you put in is
  365
+more fragile. For example, you run the risk of returning a ``datetime`` that
  366
+was actually a string that just happened to be in the same format chosen for
  367
+``datetime``\s).
  368
+
  369
+Your serializer class must implement two methods,
  370
+``dumps(self, obj)`` and ``loads(self, data)``, to serialize and deserialize
  371
+the dictionary of session data, respectively.
  372
+
304 373
 Session object guidelines
305 374
 -------------------------
306 375
 
@@ -390,14 +459,15 @@ An API is available to manipulate session data outside of a view::
390 459
     >>> from django.contrib.sessions.backends.db import SessionStore
391 460
     >>> import datetime
392 461
     >>> s = SessionStore()
393  
-    >>> s['last_login'] = datetime.datetime(2005, 8, 20, 13, 35, 10)
  462
+    >>> # stored as seconds since epoch since datetimes are not serializable in JSON.
  463
+    >>> s['last_login'] = 1376587691
394 464
     >>> s.save()
395 465
     >>> s.session_key
396 466
     '2b1189a188b44ad18c35e113ac6ceead'
397 467
 
398 468
     >>> s = SessionStore(session_key='2b1189a188b44ad18c35e113ac6ceead')
399 469
     >>> s['last_login']
400  
-    datetime.datetime(2005, 8, 20, 13, 35, 0)
  470
+    1376587691
401 471
 
402 472
 In order to prevent session fixation attacks, sessions keys that don't exist
403 473
 are regenerated::
@@ -543,8 +613,11 @@ behavior:
543 613
 Technical details
544 614
 =================
545 615
 
546  
-* The session dictionary should accept any pickleable Python object. See
547  
-  the :mod:`pickle` module for more information.
  616
+* The session dictionary accepts any :mod:`json` serializable value when using
  617
+  :class:`~django.contrib.sessions.serializers.JSONSerializer` or any
  618
+  pickleable Python object when using
  619
+  :class:`~django.contrib.sessions.serializers.PickleSerializer`. See the
  620
+  :mod:`pickle` module for more information.
548 621
 
549 622
 * Session data is stored in a database table named ``django_session`` .
550 623
 
40  tests/defer_regress/tests.py
@@ -7,6 +7,7 @@
7 7
 from django.db.models import Count
8 8
 from django.db.models.loading import cache
9 9
 from django.test import TestCase
  10
+from django.test.utils import override_settings
10 11
 
11 12
 from .models import (
12 13
     ResolveThis, Item, RelatedItem, Child, Leaf, Proxy, SimpleItem, Feature,
@@ -83,24 +84,6 @@ def test_basic(self):
83 84
         self.assertEqual(results[0].child.name, "c1")
84 85
         self.assertEqual(results[0].second_child.name, "c2")
85 86
 
86  
-        # Test for #12163 - Pickling error saving session with unsaved model
87  
-        # instances.
88  
-        SESSION_KEY = '2b1189a188b44ad18c35e1baac6ceead'
89  
-
90  
-        item = Item()
91  
-        item._deferred = False
92  
-        s = SessionStore(SESSION_KEY)
93  
-        s.clear()
94  
-        s["item"] = item
95  
-        s.save()
96  
-
97  
-        s = SessionStore(SESSION_KEY)
98  
-        s.modified = True
99  
-        s.save()
100  
-
101  
-        i2 = s["item"]
102  
-        self.assertFalse(i2._deferred)
103  
-
104 87
         # Regression for #16409 - make sure defer() and only() work with annotate()
105 88
         self.assertIsInstance(
106 89
             list(SimpleItem.objects.annotate(Count('feature')).defer('name')),
@@ -147,6 +130,27 @@ def test_ticket_11936(self):
147 130
                 cache.get_app("defer_regress"), include_deferred=True))
148 131
         )
149 132
 
  133
+    @override_settings(SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer')
  134
+    def test_ticket_12163(self):
  135
+        # Test for #12163 - Pickling error saving session with unsaved model
  136
+        # instances.
  137
+        SESSION_KEY = '2b1189a188b44ad18c35e1baac6ceead'
  138
+
  139
+        item = Item()
  140
+        item._deferred = False
  141
+        s = SessionStore(SESSION_KEY)
  142
+        s.clear()
  143
+        s["item"] = item
  144
+        s.save()
  145
+
  146
+        s = SessionStore(SESSION_KEY)
  147
+        s.modified = True
  148
+        s.save()
  149
+
  150
+        i2 = s["item"]
  151
+        self.assertFalse(i2._deferred)
  152
+
  153
+    def test_ticket_16409(self):
150 154
         # Regression for #16409 - make sure defer() and only() work with annotate()
151 155
         self.assertIsInstance(
152 156
             list(SimpleItem.objects.annotate(Count('feature')).defer('name')),

0 notes on commit b0ce6fe

Please sign in to comment.
Something went wrong with that request. Please try again.