auth0 / node-jws

JSON Web Signatures
http://self-issued.info/docs/draft-ietf-jose-json-web-signature.html
MIT License
707 stars 107 forks source link

Add 'JWS JSON Serialization Overview' according to RFC7515 #32

Open dol opened 9 years ago

dol commented 9 years ago

The header informations can be separated in protected and not protected parts. Defined in RFC7515 Section 3.2.

omsmith commented 9 years ago

What would you hope an implementation would look like?

dol commented 9 years ago

The RFC states it as 'protected' JSON property.

The obviously one would be:

const signature = jws.sign({
  header: { alg: 'HS256' },
  protected: { secret: 'my secret'},
  payload: 'h. jon benjamin',
  secret: 'has a van',
});

Verify can stay the same.

The 'decode' part is somehow tricky. If the JSON object contains the protected header field the obviously return would be a 'protected' field alongside with the 'header' field. This leads to a big security issue.

const signature = jws.sign({
  header: { alg: 'HS256', is_admin: 'false' },
  payload: 'h. jon benjamin',
  secret: 'has a van',
});

const parts = jws.decode(signature);
// Value of is_admin = false
const is_admin = parts.header.is_admin;

const signature2 = jws.sign({
  header: { alg: 'HS256', is_admin: 'true' },
  protected: { is_admin: 'false' }
  payload: 'h. jon benjamin',
  secret: 'has a van',
});

const parts2 = jws.decode(signature2);
// Value of is_admin2 = true
const is_admin2 = parts.header.is_admin;

The only way to mitigate this security risk is to add a BC break and renaming the header property to protected_header or something similar. I just realised that the RFC is not well defined for this issue. They should have named the values differently. Or not mixing header to be sign and not signed in to different formats.

omsmith commented 9 years ago

@dol Well, during serialization they're separate, but the RFC describes them being merged into a "JOSE Header", at which point duplicates across the header parts is a "MUST NOT".

So, a returned header could just be the "jose header". It would be nice to differentiate between fields that came protected/unprotected for sure (although, in your specific example, is_admin should really probably be part of the payload, as would be the case for claims in a JWT, no?)

michaelabuckley commented 9 years ago

+1. I'm posting signed messages to another endpoint. It's much nicer to debug when the main message is available in plaintext. I can also use application/json as mime-type.