From e8ce3a365f59b8fdb3ef00bf9e3c2da2aee702aa Mon Sep 17 00:00:00 2001 From: Todd Wolfson Date: Sun, 1 Nov 2015 18:51:55 -0600 Subject: [PATCH] Added session regenerate/destroy methods --- docs/quickstart.rst | 8 ++++-- flask/app.py | 29 ++++++++++++++++++++++ flask/sessions.py | 22 +++++++++++++++++ tests/test_basic.py | 60 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/docs/quickstart.rst b/docs/quickstart.rst index 38c14035..bb0c4adf 100644 --- a/docs/quickstart.rst +++ b/docs/quickstart.rst @@ -779,6 +779,10 @@ sessions work:: @app.route('/login', methods=['GET', 'POST']) def login(): if request.method == 'POST': + # Regenerate session to prevent session fixation + app.regenerate_session(session) + + # Handle login session['username'] = request.form['username'] return redirect(url_for('index')) return ''' @@ -790,8 +794,8 @@ sessions work:: @app.route('/logout') def logout(): - # remove the username from the session if it's there - session.pop('username', None) + # Destroy the session to prevent reuse (e.g. session fixation) + app.destroy_session(session) return redirect(url_for('index')) # set the secret key. keep this really secret: diff --git a/flask/app.py b/flask/app.py index 3d741ae9..08d6b6d9 100644 --- a/flask/app.py +++ b/flask/app.py @@ -919,6 +919,35 @@ class Flask(_PackageBoundObject): """ return self.session_interface.save_session(self, session, response) + def regenerate_session(self, session): + """Regenerate the session if it is session id based. For the default + implementation, this is a noop. Instead of overriding this + method we recommend replacing the :class:`session_interface`. + + This should be used upon login to prevent session fixation. + + https://www.owasp.org/index.php/Session_fixation + + :param session: the session to be saved (a + :class:`~werkzeug.contrib.securecookie.SecureCookie` + object) + """ + return self.session_interface.regenerate_session(self, session) + + def destroy_session(self, session): + """Destroy the session. Instead of overriding this + method we recommend replacing the :class:`session_interface`. + + This should be used upon logout to prevent session fixation. + + https://www.owasp.org/index.php/Session_fixation + + :param session: the session to be saved (a + :class:`~werkzeug.contrib.securecookie.SecureCookie` + object) + """ + return self.session_interface.destroy_session(self, session) + def make_null_session(self): """Creates a new instance of a missing session. Instead of overriding this method we recommend replacing the :class:`session_interface`. diff --git a/flask/sessions.py b/flask/sessions.py index 48fd08fa..8b2b7528 100644 --- a/flask/sessions.py +++ b/flask/sessions.py @@ -278,6 +278,18 @@ class SessionInterface(object): """ raise NotImplementedError() + def regenerate_session(self, app, session): + """This is called for regenerating sessions to prevent session fixation + attacks. + """ + raise NotImplementedError() + + def destroy_session(self, app, session): + """This is called for destroying a session's contents and underlying store + information. This is used to prevent session fixation attacks. + """ + raise NotImplementedError() + def save_session(self, app, session, response): """This is called for actual sessions returned by :meth:`open_session` at the end of the request. This is still called during a request @@ -330,6 +342,16 @@ class SecureCookieSessionInterface(SessionInterface): except BadSignature: return self.session_class() + def regenerate_session(self, app, session): + # There isn't anything related to a session id so we do nothing + pass + + def destroy_session(self, app, session): + # Empty store contents + session.clear() + + # `save_session` will take care of deleting our cookie + def save_session(self, app, session, response): domain = self.get_cookie_domain(app) path = self.get_cookie_path(app) diff --git a/tests/test_basic.py b/tests/test_basic.py index d22e9e5b..0bd8c77f 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -449,6 +449,66 @@ def test_session_cookie_setting(): run_test(expect_header=False) +def test_session_cookie_regenerate(): + app = flask.Flask(__name__) + app.testing = True + app.secret_key = 'dev key' + + @app.route('/set', methods=['POST']) + def set(): + flask.session['value'] = flask.request.form['value'] + return 'value set' + + @app.route('/get') + def get(): + return flask.session['value'] + + @app.route('/regenerate', methods=['POST']) + def regenerate(): + app.regenerate_session(flask.session) + return 'regenerated session' + + # Set/get a value in our session + c = app.test_client() + assert c.post('/set', data={'value': '42'}).data == b'value set' + assert c.get('/get').data == b'42' + + # Regenerate the session and verify the value still exists + assert c.post('/regenerate').data == b'regenerated session' + assert c.get('/get').data == b'42' + + +def test_session_cookie_destroy(): + app = flask.Flask(__name__) + app.testing = True + app.secret_key = 'dev key' + + @app.route('/set', methods=['POST']) + def set(): + flask.session['value'] = flask.request.form['value'] + return 'value set' + + @app.route('/get') + def get(): + return flask.session.get('value', '') + + @app.route('/destroy', methods=['POST']) + def destroy(): + app.destroy_session(flask.session) + return 'destroyed session' + + # Set/get a value in our session + c = app.test_client() + assert c.post('/set', data={'value': '42'}).data == b'value set' + assert c.get('/get').data == b'42' + + # Destroy the session, verify we set up the session for destruction, and verify the value no longer exists + rv = c.post('/destroy') + assert rv.data == b'destroyed session' + assert 'max-age=0' in rv.headers['Set-Cookie'].lower() + assert c.get('/get').data == b'' + + def test_flashes(): app = flask.Flask(__name__) app.secret_key = 'testkey'