brianloveswords / python-jws

python implementation of JSON Web Signatures
57 stars 35 forks source link

Verify the input JSON, don't re-gen it #12

Closed tgs closed 9 years ago

tgs commented 10 years ago

Python dict keys are in an undefined order, which means json.dumps() doesn't reliably produce the same string. So in JWT, it's best to verify the input JSON, rather than decoding it, re-encoding it, and verifying it. Otherwise you get spurious verification failures.

tgs commented 10 years ago

If you want to fix the verify() function itself, you could change jws.utils.decode to use an OrderedDict, as in http://stackoverflow.com/questions/6921699/can-i-get-json-to-load-into-an-ordereddict-in-python

The problem is that that introduces non-stdlib dependencies in Python <2.7

socketpair commented 10 years ago

HAY! the problem is not here. Suppose, json was generated in NodeJS. So, order of fields may not be the same as in python. So, we should never generate json representation from dicts when verifying something. So, verify(head, payload, encoded_signature) should NEVER be called with dicts as first arguments. Next, you should remove is_json=False feature. Next, please implement to_jwt() and from_jwt() as library functions, not as examples. For example, this is to_jws() from nodejs:

The simplest, clear algorythim (https://github.com/hokaccha/node-jwt-simple/blob/master/lib/jwt.js):

var segments = [];
segments.push(base64urlEncode(JSON.stringify(header)));
segments.push(base64urlEncode(JSON.stringify(payload)));
segments.push(sign(segments.join('.'), key, signingMethod, signingType));
return segments.join('.');
tgs commented 10 years ago

Hi @socketpair - after I reported this bug I switched to PyJWT, and that's been going well for me.

socketpair commented 10 years ago

OK, why not to close this project? I mean write in readme, that this project is obsolete, please use PyJWT... ?