puiterwijk / flask-oidc

OpenID Connect support for Flask
BSD 2-Clause "Simplified" License
154 stars 217 forks source link

Don't fail on bad signature of id token cookie, e.g. if session hashing mechanism changes #67

Closed bkabrda closed 5 years ago

bkabrda commented 5 years ago

A traceback caused by the change in session hashing mechanism in itsdangerous makes Pagure (which uses flask-oidc) return 500:

[Fri Oct 26 08:44:10.541892 2018] [:error] [pid 1563] Traceback (most recent call last):
[Fri Oct 26 08:44:10.541895 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask/app.py", line 2292, in wsgi_app
[Fri Oct 26 08:44:10.541896 2018] [:error] [pid 1563]     response = self.full_dispatch_request()
[Fri Oct 26 08:44:10.541898 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask/app.py", line 1815, in full_dispatch_request
[Fri Oct 26 08:44:10.541899 2018] [:error] [pid 1563]     rv = self.handle_user_exception(e)
[Fri Oct 26 08:44:10.541900 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask/app.py", line 1718, in handle_user_exception
[Fri Oct 26 08:44:10.541902 2018] [:error] [pid 1563]     reraise(exc_type, exc_value, tb)
[Fri Oct 26 08:44:10.541904 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask/app.py", line 1811, in full_dispatch_request
[Fri Oct 26 08:44:10.541905 2018] [:error] [pid 1563]     rv = self.preprocess_request()
[Fri Oct 26 08:44:10.541906 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask/app.py", line 2087, in preprocess_request
[Fri Oct 26 08:44:10.541908 2018] [:error] [pid 1563]     rv = func()
[Fri Oct 26 08:44:10.541909 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask_oidc/__init__.py", line 406, in _before_request
[Fri Oct 26 08:44:10.541910 2018] [:error] [pid 1563]     self.authenticate_or_redirect()
[Fri Oct 26 08:44:10.541911 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask_oidc/__init__.py", line 426, in authenticate_or_redirect
[Fri Oct 26 08:44:10.541912 2018] [:error] [pid 1563]     id_token = self._get_cookie_id_token()
[Fri Oct 26 08:44:10.541913 2018] [:error] [pid 1563]   File "/usr/lib/python2.7/site-packages/flask_oidc/__init__.py", line 352, in _get_cookie_id_token
[Fri Oct 26 08:44:10.541915 2018] [:error] [pid 1563]     return self.cookie_serializer.loads(id_token_cookie)
[Fri Oct 26 08:44:10.541916 2018] [:error] [pid 1563]   File "/usr/lib64/python2.7/site-packages/itsdangerous/jws.py", line 187, in loads
[Fri Oct 26 08:44:10.541917 2018] [:error] [pid 1563]     self, s, salt, return_header=True
[Fri Oct 26 08:44:10.541918 2018] [:error] [pid 1563]   File "/usr/lib64/python2.7/site-packages/itsdangerous/jws.py", line 143, in loads
[Fri Oct 26 08:44:10.541919 2018] [:error] [pid 1563]     self.make_signer(salt, self.algorithm).unsign(want_bytes(s)),
[Fri Oct 26 08:44:10.541920 2018] [:error] [pid 1563]   File "/usr/lib64/python2.7/site-packages/itsdangerous/signer.py", line 175, in unsign
[Fri Oct 26 08:44:10.541921 2018] [:error] [pid 1563]     raise BadSignature("Signature %r does not match" % sig, payload=value)
[Fri Oct 26 08:44:10.541922 2018] [:error] [pid 1563] BadSignature: Signature '<signature edited out>' does not match

I've started getting this problem after itsdangerous updated from 0.24 to 1.0.0, seems like I'm not the only one: https://github.com/pallets/flask/issues/2952

This fix should prevent such failures in the future by logging the user out when BadSignature is detected.

bkabrda commented 5 years ago

FWIW, the Travis failure seems related to a non-python-2.6 syntax in one of the underlying dependencies:

  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/rsa/prime.py", line 134
    return number in {2, 3, 5, 7}
puiterwijk commented 5 years ago

Thanks!