auth0 / node-jwa

JSON Web Algorithms
http://tools.ietf.org/id/draft-ietf-jose-json-web-algorithms-08.html
MIT License
98 stars 42 forks source link

Signature is generated differently on Node v4 and Node v6 #19

Open Pierre-Gilles opened 7 years ago

Pierre-Gilles commented 7 years ago

Hello,

I'm trying to switch a project from Node v4 to Node v6, but I'm facing a little problem.

We are using node-jsonwebtoken, which use this library to sign jwt, and it seems that signatures are not generated the same way on node v4 and v6.

I've investigated the issue, I ended up here and found where it happens.

My secret and my payload look like this :

const crypto = require('crypto');
const secret = crypto.createHash('sha256').update('secret').digest('binary');

const payload = { 
     uid: 'test',
     iat: 1455988418,
     iss: 'test' 
};

Then, if I generate a signature :

const jwa = require('jwa');
const algo = jwa('HS256');
const sig = algo.sign(payload, secret);

console.log(sig);
// Node v4 => "_zPq9vDP4_Ve0mTVTF_9H3NRkluQhoR4yAg8X4yqR8Q"
// Node v6 => "hk9bpxID-HOmvNpJUy7x80KqT5JP8tb_BoAJLYVIYsE"

After reading the code of this library, seems that the problem is coming from this line => https://github.com/brianloveswords/node-jwa/blob/master/index.js#L35

The signature is generated like this :

var sig = (hmac.update(thing), hmac.digest('base64'));

I went back to the crypto library, and found that crypto default encoding for digest has changed between node 4 and 6 ( https://github.com/nodejs/node/issues/6813#issuecomment-219753580 )

I tried to change in the lib hmac.update(thing) to hmac.update(thing, 'binary') but it changes nothing.

By the way, the secret generated is still the same between Node 4 and Node 6.

Do you have any idea of what is happening ?

Thanks a lot for this library, and for your help :)

Have a nice day.

omsmith commented 7 years ago

Thanks for the report @Pierre-Gilles. The same encoding change effects the secret as well, which is why your edit didn't correct the issue.

Shouldn't be hard to fix up (have it locally), just want to be able to do some performance testing and consider ecosystem impact (it may break workflows of users who have only used the library in Node 5+).

Pierre-Gilles commented 7 years ago

Thanks for your quick answer.

I completely understand, the goal is not to break other things !

Let me know when you have more news on that.

Pierre-Gilles commented 7 years ago

Hello,

I'm getting back to you on the issue, do you have the time to test the performance & the ecosystem impact of this modification?

We are still stuck on Node 4 for the moment and really need to change before Node 6 is becoming LTS.

Thank you very much for your time and your help.

Pierre-Gilles commented 7 years ago

Hi!

I'm really sorry to insist, do you have any changes I can do on my side so we can switch to a more recent version of Node?

Like a way of verifying signature of both Node v4/v6 tokens so we can upgrade without disconnecting all our logged users?

Thank you very much for your time, I totally understand if you don't have the time to push an update on this module, it's just that we need to upgrade to Node v6, and if there is a way to do it on our side without impacting the whole community, that would be great :)

Have a nice day,