Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change to use JSONSerializer for SignedCookieSessionFactory #3413

Merged
merged 2 commits into from
Nov 4, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ unreleased
Features
--------

- Changed the default ``serializer`` on
``pyramid.session.SignedCookieSessionFactory`` to use
``pyramid.session.JSONSerializer`` instead of
``pyramid.session.PickleSerializer``. Read
"Changes to ISession in Pyramid 2.0" in the "Sessions" chapter of the
documentation for more information about why this change was made.
See https://github.com/Pylons/pyramid/pull/3413

Bug Fixes
---------

Expand Down Expand Up @@ -52,6 +60,14 @@ Backward Incompatibilities
lead to remove code execution if the secret key is compromised.
See https://github.com/Pylons/pyramid/pull/3412

- Changed the default ``serializer`` on
``pyramid.session.SignedCookieSessionFactory`` to use
``pyramid.session.JSONSerializer`` instead of
``pyramid.session.PickleSerializer``. Read
"Changes to ISession in Pyramid 2.0" in the "Sessions" chapter of the
documentation for more information about why this change was made.
See https://github.com/Pylons/pyramid/pull/3413

Documentation Changes
---------------------

4 changes: 2 additions & 2 deletions docs/api/session.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

.. autofunction:: BaseCookieSessionFactory

.. autoclass:: PickleSerializer

.. autoclass:: JSONSerializer

.. autoclass:: PickleSerializer

30 changes: 12 additions & 18 deletions docs/narr/sessions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Using the Default Session Factory
In order to use sessions, you must set up a :term:`session factory` during your
:app:`Pyramid` configuration.

A very basic, insecure sample session factory implementation is provided in the
A very basic session factory implementation is provided in the
:app:`Pyramid` core. It uses a cookie to store session information. This
implementation has the following limitations:

Expand All @@ -38,8 +38,8 @@ implementation has the following limitations:
of the session is fewer than 4000. This is suitable only for very small data
sets.

It is digitally signed, however, and thus its data cannot easily be tampered
with.
It is digitally signed, however, and thus a client cannot easily tamper with
the content without compromising the secret key.
mmerickel marked this conversation as resolved.
Show resolved Hide resolved
mmerickel marked this conversation as resolved.
Show resolved Hide resolved

You can configure this session factory in your :app:`Pyramid` application by
using the :meth:`pyramid.config.Configurator.set_session_factory` method.
Expand Down Expand Up @@ -70,25 +70,20 @@ using the :meth:`pyramid.config.Configurator.set_session_factory` method.

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.

.. index::
triple: pickle deprecation; JSON-serializable; ISession interface

.. _pickle_session_deprecation:

Upcoming Changes to ISession in Pyramid 2.0
-------------------------------------------
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.
In :app:`Pyramid` 2.0 the :class:`pyramid.interfaces.ISession` interface was changed to require that session implementations only need to support JSON-serializable data types.
This is a stricter contract than the previous 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.
Previously, if a client-side session implementation was compromised, it left 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:
Expand Down Expand Up @@ -178,11 +173,10 @@ object are in the :class:`pyramid.interfaces.ISession` documentation.

Some gotchas:

- Keys and values of session data must be *pickleable*. This means, typically,
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. Please also see :ref:`pickle_session_deprecation`.
- Keys and values of session data must be JSON-serializable.
This means, typically, 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 JSON-serializable, an error will be raised when the 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
Expand Down
24 changes: 15 additions & 9 deletions src/pyramid/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -1084,21 +1084,27 @@ class ISession(IDict):
""" An interface representing a session (a web session object,
usually accessed via ``request.session``.

Keys and values of a session must be pickleable.
Keys and values of a session must be JSON-serializable.

.. 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.
In :app:`Pyramid` 2.0 the session was changed to 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 was made.

.. versionchanged:: 1.9

Sessions are no longer required to implement ``get_csrf_token`` and
``new_csrf_token``. CSRF token support was moved to the pluggable
:class:`pyramid.interfaces.ICSRFStoragePolicy` configuration hook.
Sessions are no longer required to implement ``get_csrf_token`` and
``new_csrf_token``. CSRF token support was moved to the pluggable
:class:`pyramid.interfaces.ICSRFStoragePolicy` configuration hook.

.. versionchanged:: 2.0

Sessions now need to be JSON-serializable. This is more strict than
the previous requirement of pickleable objects.

"""

Expand Down
27 changes: 10 additions & 17 deletions src/pyramid/session.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import binascii
import os
import time
import warnings

from zope.deprecation import deprecated
from zope.interface import implementer
Expand Down Expand Up @@ -350,8 +349,6 @@ def SignedCookieSessionFactory(
serializer=None,
):
"""
.. versionadded:: 1.5

Configure a :term:`session factory` which will provide signed
cookie-based sessions. The return value of this
function is a :term:`session factory`, which may be provided as
Expand Down Expand Up @@ -441,33 +438,29 @@ def SignedCookieSessionFactory(
method should accept bytes and return a Python object. The ``dumps``
method should accept a Python object and return bytes. A ``ValueError``
should be raised for malformed inputs. If a serializer is not passed,
the :class:`pyramid.session.PickleSerializer` serializer will be used.
the :class:`pyramid.session.JSONSerializer` serializer will be used.

.. warning::

In :app:`Pyramid` 2.0 the default ``serializer`` option will change to
In :app:`Pyramid` 2.0 the default ``serializer`` option changed to
use :class:`pyramid.session.JSONSerializer`. See
:ref:`pickle_session_deprecation` for more information about why this
change is being made.
change was made.

.. versionadded: 1.5a3

.. versionchanged: 1.10

Added the ``samesite`` option and made the default ``Lax``.
Added the ``samesite`` option and made the default ``Lax``.

.. versionchanged: 2.0

Changed the default ``serializer`` to be an instance of
:class:`pyramid.session.JSONSerializer`.

"""
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,
)
serializer = JSONSerializer()

signed_serializer = SignedSerializer(
secret, salt, hashalg, serializer=serializer
Expand Down
22 changes: 2 additions & 20 deletions tests/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,10 @@ def _serialize(self, value, salt=b'pyramid.session.', hashalg='sha512'):
import base64
import hashlib
import hmac
import pickle
import json

digestmod = lambda: hashlib.new(hashalg)
cstruct = pickle.dumps(value, pickle.HIGHEST_PROTOCOL)
cstruct = json.dumps(value).encode('utf-8')
sig = hmac.new(salt + b'secret', cstruct, digestmod).digest()
return base64.urlsafe_b64encode(sig + cstruct).rstrip(b'=')

Expand Down Expand Up @@ -505,24 +505,6 @@ def test_very_long_key(self):
self.assertEqual(result, None)
self.assertTrue('Set-Cookie' in dict(response.headerlist))

def test_bad_pickle(self):
import base64
import hashlib
import hmac

digestmod = lambda: hashlib.new('sha512')
# generated from dumping an object that cannot be found anymore, eg:
# class Foo: pass
# print(pickle.dumps(Foo()))
cstruct = b'(i__main__\nFoo\np0\n(dp1\nb.'
sig = hmac.new(b'pyramid.session.secret', cstruct, digestmod).digest()
cookieval = base64.urlsafe_b64encode(sig + cstruct).rstrip(b'=')

request = testing.DummyRequest()
request.cookies['session'] = cookieval
session = self._makeOne(request, secret='secret')
self.assertEqual(session, {})


class Test_manage_accessed(unittest.TestCase):
def _makeOne(self, wrapped):
Expand Down