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

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

Merged
merged 1 commit into from
Nov 12, 2018

Conversation

bkabrda
Copy link
Contributor

@bkabrda bkabrda commented Oct 26, 2018

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: pallets/flask#2952

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

@bkabrda
Copy link
Contributor Author

bkabrda commented Oct 26, 2018

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
Copy link
Owner

Thanks!

@puiterwijk puiterwijk merged commit 0ee31ff into puiterwijk:master Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants