Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Python 3 + PyCrypto: crypt.verify_signed_jwt_with_certs raises "Invalid token signature" due to string/bytes bug #201

Closed
thomasboyt opened this issue Jun 23, 2015 · 12 comments

Comments

@thomasboyt
Copy link

I was getting a mysterious "Invalid token signature" error trying to validate Google Sign-In-generated tokens using this method on Python 3.4.0 with PyCrypto. After confirming with jwt.io and the tokeninfo endpoint that the token was, indeed, valid, I dove into the method source. After realizing that all exceptions were swallowed in the PyCrypto wrapper's verify method, I found out that the call:

SHA256.new(message)

was raising:

TypeError: Unicode-objects must be encoded before hashing

This is because message is a string, and not bytes. This isn't surprising when you see where message is initially set:
https://github.com/google/oauth2client/blob/c5eb9f14d46d89fb33c4409a0d32fad4b919e136/oauth2client/crypt.py#L413.

I applied a quick fix to that line:

signed = ('%s.%s' % (segments[0], segments[1])).encode('utf-8')

And the method validated my token correctly.

In addition to this fix, I'd recommend removing that try/except block - I'm not sure why you'd want to swallow exceptions there, other than to make it harder for poor developers like me to debug issues ;)

thomasboyt pushed a commit to thomasboyt/oauth2client that referenced this issue Jun 23, 2015
@dhermes
Copy link
Contributor

dhermes commented Jun 23, 2015

@thomasboyt Can you send a code snippet that invoked this failure? Also, what version of the library are you using:

import oauth2client
print oauth2client.__version__

@thomasboyt
Copy link
Author

>>> print(oauth2client.__version__)
1.4.11

The code snippet invoking this failure was essentially identical to the one on https://developers.google.com/identity/sign-in/web/backend-auth#using-a-google-api-client-library

@dhermes
Copy link
Contributor

dhermes commented Jun 23, 2015

It seems the relevant bits here then are:

from oauth2client import client, crypt
idinfo = client.verify_id_token(token, CLIENT_ID)

along with the fact that you are using PyCrypto (instead of pyOpenSSL) and Python 3.

@dhermes
Copy link
Contributor

dhermes commented Jun 24, 2015

I am verifiying the failure in https://gist.github.com/dhermes/f77b972cbd5934b5f403 (with an expired token for my personal GMail account). My hunch is that the problem is with '%s.%s' when signing the first two segments of the JWT.

@dhermes
Copy link
Contributor

dhermes commented Jun 24, 2015

I tested on Py2.7 and Py3.4 with OpenSSL and PyCrypto. Of the four possible cases, it only fails on Python 3.4 with PyCrypto.

@dhermes
Copy link
Contributor

dhermes commented Jun 24, 2015

With the most recent revision of the gist (making sure signed part was bytes, as suggested), all four scenarios are working.

dhermes added a commit to dhermes/oauth2client that referenced this issue Jun 24, 2015
Also
- Removing bare `except:` statements in both the PyCrypto and
  OpenSSL verifiers (in the `verify` method).
- Catching the only exception possible in the OpenSSL verifier
  (it is `OpenSSL.crypto.Error`).
- Converting the signature to bytes (if not already) in the
  OpenSSL verifier.
- Adding a test with both a unicode and bytes signature for
  each verifier.

Fixes googleapis#201.
@dhermes
Copy link
Contributor

dhermes commented Jun 24, 2015

@thomasboyt Thanks a lot for this. I saw your tweets and tried to help out a little bit.

Question, why are you using the special combination of PyCrypto and Python 3.4? Typically we see people using PyCrypto without OpenSSL when they are on App Engine. But Python 3.4 means this isn't the case.

dhermes added a commit to dhermes/oauth2client that referenced this issue Jul 2, 2015
Also
- Removing bare `except:` statements in both the PyCrypto and
  OpenSSL verifiers (in the `verify` method).
- Catching the only exception possible in the OpenSSL verifier
  (it is `OpenSSL.crypto.Error`).
- Converting the signature to bytes (if not already) in the
  OpenSSL verifier.
- Adding a test with both a unicode and bytes signature for
  each verifier.

Fixes googleapis#201.
@dhermes
Copy link
Contributor

dhermes commented Jul 13, 2015

@thomasboyt Sorry it took so long to get this fixed. I'm not sure when the next release will be cut.

Care to offer why you are using the special combination of PyCrypto and Python 3.4?

@thomasboyt
Copy link
Author

Thanks for the PR; I've got a hotfixed fork I'm using but will switch back once the next release is cut :)

As for why PyCrypto: I'm using http://www.paramiko.org/ in my codebase and that depends on PyCrypto, so it's what I had installed by default. I could use PyOpenSSL if it were a requirement, but I figured that since it supported either, I'd just stick with PyCrypto.

@dhermes
Copy link
Contributor

dhermes commented Jul 13, 2015

Thanks for the info! We certainly want to support all code-paths, Py3.4+PyCrypto just seemed to be a rare one.

@PAStheLoD
Copy link

It could be rare, but we also happened to spend quite a few hours figuring this out on Py3.4 with PyCrypto. :)

@dhermes
Copy link
Contributor

dhermes commented Jul 14, 2015

Sorry for the trouble. Thanks for the report!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants