From c3188340e841633924e8ab7a055c1df0dffed9c1 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 16 Sep 2018 11:06:05 -0500 Subject: [PATCH 1/5] deprecate pickleable sessions, recommend json --- CHANGES.rst | 11 +++++++ docs/api/session.rst | 2 ++ docs/narr/sessions.rst | 72 +++++++++++++++++++++++++++++++----------- pyramid/interfaces.py | 8 +++++ pyramid/session.py | 30 ++++++++++++++++-- 5 files changed, 101 insertions(+), 22 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d0dbbe5c0e..92e1e43137 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -57,6 +57,11 @@ Features - Add support for Python 3.7. Add testing on Python 3.8 with allowed failures. See https://github.com/Pylons/pyramid/pull/3333 +- Added ``pyramid.session.JSONSerializer``. See "Upcoming Changes to ISession + in Pyramid 2.0" in the "Sessions" chapter of the documentation for more + information about this feature. + See https://github.com/Pylons/pyramid/pull/3353 + Bug Fixes --------- @@ -79,6 +84,12 @@ Bug Fixes Deprecations ------------ +- The ``pyramid.intefaces.ISession`` interface will move to require + json-serializable objects in Pyramid 2.0. See + "Upcoming Changes to ISession in Pyramid 2.0" in the "Sessions" chapter + of the documentation for more information about this change. + See https://github.com/Pylons/pyramid/pull/3353 + Backward Incompatibilities -------------------------- diff --git a/docs/api/session.rst b/docs/api/session.rst index 53bae7c523..e0d2db726b 100644 --- a/docs/api/session.rst +++ b/docs/api/session.rst @@ -17,3 +17,5 @@ .. autoclass:: PickleSerializer + .. autoclass:: JSONSerializer + diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 2d80b1a63d..17e8291a0b 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -59,25 +59,59 @@ using the :meth:`pyramid.config.Configurator.set_session_factory` method. By default the :func:`~pyramid.session.SignedCookieSessionFactory` implementation contains the following security concerns: - - Session data is *unencrypted*. You should not use it when you keep - sensitive information in the session object, as the information can be - easily read by both users of your application and third parties who have - access to your users' network traffic. - - - If you use this sessioning implementation, and you inadvertently create a - cross-site scripting vulnerability in your application, because the - session data is stored unencrypted in a cookie, it will also be easier for - evildoers to obtain the current user's cross-site scripting token. - - - The default serialization method, while replaceable with something like - JSON, is implemented using pickle which can lead to remote code execution - if your secret key is compromised. - - In short, use a different session factory implementation (preferably one - which keeps session data on the server) for anything but the most basic of - applications where "session security doesn't matter", you are sure your - application has no cross-site scripting vulnerabilities, and you are confident - your secret key will not be exposed. + - Session data is *unencrypted* (but it is signed / authenticated). + + This means an attacker cannot change the session data, but they can view it. + You should not use it when you keep sensitive information in the session object, as the information can be easily read by both users of your application and third parties who have access to your users' network traffic. + + At the very least, use TLS and set ``secure=True`` to avoid arbitrary users on the network from viewing the session contents. + + - If you use this sessioning implementation, and you inadvertently create a cross-site scripting vulnerability in your application, because the session data is stored unencrypted in a cookie, it will also be easier for evildoers to obtain the current user's cross-site scripting token. + + Set ``httponly=True`` to mitigate this vulnerability by hiding the cookie from client-side JavaScript. + + - The default serialization method, while replaceable with something like JSON, is implemented using pickle which can lead to remote code execution if your secret key is compromised. + + To mitigate this, set ``serializer=pyramid.session.JSONSerializer()`` to use :class:`pyramid.session.JSONSerializer`. This option will be the default in :app:`Pyramid` 2.0. + See :ref:`pickle_session_deprecation` for more information about this change. + + In short, use a different session factory implementation (preferably one which keeps session data on the server) for anything but the most basic of applications where "session security doesn't matter", you are sure your application has no cross-site scripting vulnerabilities, and you are confident your secret key will not be exposed. + +.. _pickle_session_deprecation: + +Upcoming Changes to ISession in Pyramid 2.0 +------------------------------------------- + +In :app:`Pyramid` 2.0 the :class:`pyramid.interfaces.ISession` interface will be changing to require that session implementations only need to support json-serializable data types. +This is a stricter contract than the current requirement that all objects be pickleable and it is being done for security purposes. +This is a backward-incompatible change. +Currently, if a client-side session implementation is compromised, it leaves the application vulnerable to remote code execution attacks using specially-crafted sessions that execute code when deserialized. + +For users with compatibility concerns, it's possible to craft a serializer that can handle both formats until you are satisfied that clients have had time to reasonably upgrade. +Remember that sessions should be short-lived and thus the number of clients affected should be small (no longer than an auth token, at a maximum). An example serializer: + +.. code-block:: python + :linenos: + + from pyramid.session import JSONSerializer + from pyramid.session import PickleSerializer + + class JSONSerializerWithPickleFallback(object): + def __init__(self): + self.json = JSONSerializer() + self.pickle = PickleSerializer() + + def dumps(self, value): + # maybe catch serialization errors here and keep using pickle + # while finding spots in your app that are not storing + # json-serializable objects, falling back to pickle + return self.json.dumps(value) + + def loads(self, value): + try: + return self.json.loads(value) + except ValueError: + return self.pickle.loads(value) .. index:: single: session object diff --git a/pyramid/interfaces.py b/pyramid/interfaces.py index bedfb60b37..551ab701e2 100644 --- a/pyramid/interfaces.py +++ b/pyramid/interfaces.py @@ -960,6 +960,14 @@ class ISession(IDict): Keys and values of a session must be pickleable. + .. warning:: + + In :app:`Pyramid` 2.0 the session will only be required to support + types that can be serialized using JSON. It's recommended to switch any + session implementations to support only JSON and to only store primitive + types in sessions. See :ref:`pickle_session_deprecation` for more + information about why this change is being made. + .. versionchanged:: 1.9 Sessions are no longer required to implement ``get_csrf_token`` and diff --git a/pyramid/session.py b/pyramid/session.py index 97039a4044..3caf4181a9 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -4,11 +4,15 @@ import hmac import os import time +import warnings from zope.deprecation import deprecated from zope.interface import implementer -from webob.cookies import SignedSerializer +from webob.cookies import ( + JSONSerializer, + SignedSerializer, +) from pyramid.compat import ( pickle, @@ -131,6 +135,10 @@ def dumps(self, appstruct): """Accept a Python object and return bytes.""" return pickle.dumps(appstruct, self.protocol) + +JSONSerializer = JSONSerializer # api + + def BaseCookieSessionFactory( serializer, cookie_name='session', @@ -145,8 +153,6 @@ def BaseCookieSessionFactory( set_on_exception=True, ): """ - .. versionadded:: 1.5 - Configure a :term:`session factory` which will provide cookie-based sessions. The return value of this function is a :term:`session factory`, which may be provided as the ``session_factory`` argument of a @@ -508,6 +514,7 @@ def dumps(self, appstruct): 'so existing user session data will be destroyed if you switch to it.' ) + def SignedCookieSessionFactory( secret, cookie_name='session', @@ -618,14 +625,31 @@ def SignedCookieSessionFactory( should be raised for malformed inputs. If a serializer is not passed, the :class:`pyramid.session.PickleSerializer` serializer will be used. + .. warning:: + + In :app:`Pyramid` 2.0 the default ``serializer`` option will change to + use :class:`pyramid.session.JSONSerializer`. See + :ref:`pickle_session_deprecation` for more information about why this + change is being made. + .. versionadded: 1.5a3 .. versionchanged: 1.10 Added the ``samesite`` option and made the default ``Lax``. + """ if serializer is None: serializer = PickleSerializer() + warnings.warn( + 'The default pickle serializer is deprecated as of Pyramid 1.9 ' + 'and it will be changed to use pyramid.session.JSONSerializer in ' + 'version 2.0. Explicitly set the serializer to avoid future ' + 'incompatibilities. See "Upcoming Changes to ISession in ' + 'Pyramid 2.0" for more information about this change.', + DeprecationWarning, + stacklevel=1, + ) signed_serializer = SignedSerializer( secret, From ba5ca651c2cba9e45c80e0fb0ed6c6408ea3e042 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 16 Sep 2018 11:35:49 -0500 Subject: [PATCH 2/5] deprecate signed_serialize and signed_deserialize --- CHANGES.rst | 9 +++++++++ docs/api/session.rst | 6 ------ pyramid/session.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 92e1e43137..97a38591c0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -90,6 +90,15 @@ Deprecations of the documentation for more information about this change. See https://github.com/Pylons/pyramid/pull/3353 +- The ``pyramid.session.signed_serialize`` and + ``pyramid.session.signed_deserialize`` functions will be removed in Pyramid + 2.0, along with the removal of + ``pyramid.session.UnencryptedCookieSessionFactoryConfig`` which was + deprecated in Pyramid 1.5. Please switch to using the + ``SignedCookieSessionFactory``, copying the code, or another session + implementation if you're still using these features. + See https://github.com/Pylons/pyramid/pull/3353 + Backward Incompatibilities -------------------------- diff --git a/docs/api/session.rst b/docs/api/session.rst index e0d2db726b..d0cb112ece 100644 --- a/docs/api/session.rst +++ b/docs/api/session.rst @@ -5,14 +5,8 @@ .. automodule:: pyramid.session - .. autofunction:: signed_serialize - - .. autofunction:: signed_deserialize - .. autofunction:: SignedCookieSessionFactory - .. autofunction:: UnencryptedCookieSessionFactoryConfig - .. autofunction:: BaseCookieSessionFactory .. autoclass:: PickleSerializer diff --git a/pyramid/session.py b/pyramid/session.py index 3caf4181a9..b953fa1848 100644 --- a/pyramid/session.py +++ b/pyramid/session.py @@ -64,6 +64,14 @@ def signed_serialize(data, secret): cookieval = signed_serialize({'a':1}, 'secret') response.set_cookie('signed_cookie', cookieval) + + .. deprecated:: 1.10 + + This function will be removed in :app:`Pyramid` 2.0. It is using + pickle-based serialization, which is considered vulnerable to remote + code execution attacks and will no longer be used by the default + session factories at that time. + """ pickled = pickle.dumps(data, pickle.HIGHEST_PROTOCOL) try: @@ -74,6 +82,13 @@ def signed_serialize(data, secret): sig = hmac.new(secret, pickled, hashlib.sha1).hexdigest() return sig + native_(base64.b64encode(pickled)) +deprecated( + 'signed_serialize', + 'This function will be removed in Pyramid 2.0. It is using pickle-based ' + 'serialization, which is considered vulnerable to remote code execution ' + 'attacks.', +) + def signed_deserialize(serialized, secret, hmac=hmac): """ Deserialize the value returned from ``signed_serialize``. If the value cannot be deserialized for any reason, a @@ -86,6 +101,13 @@ def signed_deserialize(serialized, secret, hmac=hmac): cookieval = request.cookies['signed_cookie'] data = signed_deserialize(cookieval, 'secret') + + .. deprecated:: 1.10 + + This function will be removed in :app:`Pyramid` 2.0. It is using + pickle-based serialization, which is considered vulnerable to remote + code execution attacks and will no longer be used by the default + session factories at that time. """ # hmac parameterized only for unit tests try: @@ -109,6 +131,13 @@ def signed_deserialize(serialized, secret, hmac=hmac): return pickle.loads(pickled) +deprecated( + 'signed_deserialize', + 'This function will be removed in Pyramid 2.0. It is using pickle-based ' + 'serialization, which is considered vulnerable to remote code execution ' + 'attacks.', +) + class PickleSerializer(object): """ A serializer that uses the pickle protocol to dump Python From 38bbea331f9c485d40892a17674272a8876a55a1 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 16 Sep 2018 15:43:43 -0500 Subject: [PATCH 3/5] tweak some docs --- CHANGES.rst | 2 +- docs/narr/sessions.rst | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 97a38591c0..54b8beba4b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -85,7 +85,7 @@ Deprecations ------------ - The ``pyramid.intefaces.ISession`` interface will move to require - json-serializable objects in Pyramid 2.0. See + JSON-serializable objects in Pyramid 2.0. See "Upcoming Changes to ISession in Pyramid 2.0" in the "Sessions" chapter of the documentation for more information about this change. See https://github.com/Pylons/pyramid/pull/3353 diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 17e8291a0b..971b4502d1 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -79,10 +79,13 @@ using the :meth:`pyramid.config.Configurator.set_session_factory` method. .. _pickle_session_deprecation: +.. index:: + triple: pickle deprecation; JSON-serializable; ISession interface + Upcoming Changes to ISession in Pyramid 2.0 ------------------------------------------- -In :app:`Pyramid` 2.0 the :class:`pyramid.interfaces.ISession` interface will be changing to require that session implementations only need to support json-serializable data types. +In :app:`Pyramid` 2.0 the :class:`pyramid.interfaces.ISession` interface will be changing to require that session implementations only need to support JSON-serializable data types. This is a stricter contract than the current requirement that all objects be pickleable and it is being done for security purposes. This is a backward-incompatible change. Currently, if a client-side session implementation is compromised, it leaves the application vulnerable to remote code execution attacks using specially-crafted sessions that execute code when deserialized. @@ -104,7 +107,7 @@ Remember that sessions should be short-lived and thus the number of clients affe def dumps(self, value): # maybe catch serialization errors here and keep using pickle # while finding spots in your app that are not storing - # json-serializable objects, falling back to pickle + # JSON-serializable objects, falling back to pickle return self.json.dumps(value) def loads(self, value): @@ -173,7 +176,7 @@ Some gotchas: that they are instances of basic types of objects, such as strings, lists, dictionaries, tuples, integers, etc. If you place an object in a session data key or value that is not pickleable, an error will be raised when the - session is serialized. + session is serialized. Please also see :ref:`pickle_session_deprecation`. - If you place a mutable value (for example, a list or a dictionary) in a session object, and you subsequently mutate that value, you must call the From 07207637818049d27abb90792d48d7ed8fdd2340 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Sun, 16 Sep 2018 22:45:05 -0500 Subject: [PATCH 4/5] ref after index apparently --- docs/narr/sessions.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index 971b4502d1..d4d3c1074c 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -77,11 +77,11 @@ using the :meth:`pyramid.config.Configurator.set_session_factory` method. In short, use a different session factory implementation (preferably one which keeps session data on the server) for anything but the most basic of applications where "session security doesn't matter", you are sure your application has no cross-site scripting vulnerabilities, and you are confident your secret key will not be exposed. -.. _pickle_session_deprecation: - .. index:: triple: pickle deprecation; JSON-serializable; ISession interface +.. _pickle_session_deprecation: + Upcoming Changes to ISession in Pyramid 2.0 ------------------------------------------- From 97ee7f3aa8af74a01e51c0c14fda1c0a5a490663 Mon Sep 17 00:00:00 2001 From: Michael Merickel Date: Tue, 25 Sep 2018 15:49:23 -0500 Subject: [PATCH 5/5] show how to use the serializer --- docs/narr/sessions.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/narr/sessions.rst b/docs/narr/sessions.rst index d4d3c1074c..ded7e87e33 100644 --- a/docs/narr/sessions.rst +++ b/docs/narr/sessions.rst @@ -98,6 +98,7 @@ Remember that sessions should be short-lived and thus the number of clients affe from pyramid.session import JSONSerializer from pyramid.session import PickleSerializer + from pyramid.session import SignedCookieSessionFactory class JSONSerializerWithPickleFallback(object): def __init__(self): @@ -116,6 +117,11 @@ Remember that sessions should be short-lived and thus the number of clients affe except ValueError: return self.pickle.loads(value) + # somewhere in your configuration code + serializer = JSONSerializerWithPickleFallback() + session_factory = SignedCookieSessionFactory(..., serializer=serializer) + config.set_session_factory(session_factory) + .. index:: single: session object