frasertweedale / hs-jose

Haskell JOSE and JWT library
http://hackage.haskell.org/package/jose
Apache License 2.0
122 stars 46 forks source link

Parsing unprotected headers #123

Open ericpashman opened 9 months ago

ericpashman commented 9 months ago

The Crypto.JOSE.Header module provides easy-to-use parsers for the case of protected headers, but it's less easy to use the headerOptional or headerRequired parsers when protection is not required.

Unless I'm mistaken, parsing a custom header constructed in the recommended way requires first parsing it into a HeaderParam p a, then deconstructing the HeaderParam (throwing away the protection indicator) to extract the actual header value.

Here's a working example with a custom header type containing a nonce:

data MyHeader p = MyHeader
  { _myJWSHeader :: JWSHeader p
  , _myNonce :: UTCTime}

instance HasParams MyHeader where
  parseParamsFor proxy hp hu = MyHeader
    <$> parseParamsFor proxy hp hu
    <*> fmap getParam (headerRequired "nonce" hp hu:: Parser (HeaderParam () UTCTime))
    where getParam (HeaderParam _ x) = x

The difficulty is that the ProtectionIndicator type in the signature of headerRequired (or headerOptional) must unify with the one in the signature of parseParamsFor. If it's possible to make that happen polymorphically without using any type-system extensions, I gave up before figuring it out. Fixing its type to either the monomorphic () (as I did in the example) or Protection works, but (a) you must write an explicit type signature on the parser, and (b) a monomorphic parser obviously will not work for both unprotected and protected headers.

So unless I'm missing something, it seems like the module should just provide one set of parsers for explicitly protected headers (as it does), and another set for explicitly unprotected headers (as it doesn't). That would do away with both the need for explicit type signatures and for first parsing into the HeaderParam type. (If you don't know whether the header is protected or not, you would just try to parse one way, then the other.)

Another way to go about this would be instead to recommend defining custom headers wrapped in the HeaderParam type. This would require all custom headers to have the same ProtectionIndicator as the embedded JWSHeader, but it simplifies the parsing considerably. This works:

data MyHeader' p = MyHeader'
  { _myJWSHeader' :: JWSHeader p
  , _myNonce' :: HeaderParam p UTCTime}

instance HasParams MyHeader' where
  parseParamsFor proxy hp hu = MyHeader'
    <$> parseParamsFor proxy hp hu
    <*> headerRequired "nonce" hp hu
frasertweedale commented 9 months ago

Thanks for the report @ericpashman. I will review this in the coming days.

frasertweedale commented 9 months ago

@ericpashman can you please clarify the use case requirements? What fields are required, and where are they required to occur (protected header, or either header)? Does the application use compact serialisation, JSON serialisation, or either?

ericpashman commented 9 months ago

This arose again in constructing a JWT for API access. The JWT header requires a nonce, and (I think) the nonce header can be either protected or unprotected. Compact serialization.

frasertweedale commented 9 months ago

@ericpashman I think all you need to do is:

data MyHeader' p = MyHeader'
  { _myJWSHeader' :: JWSHeader p
  , _myNonce' :: HeaderParam p UTCTime}

instance HasParams MyHeader' where
  parseParamsFor proxy hp hu = MyHeader'
    <$> parseParamsFor proxy hp hu
    <*> headerRequiredProtected "nonce" hp hu

See https://hackage.haskell.org/package/jose-0.11/docs/Crypto-JOSE-Header.html#g:2 for more explanation. The protection indicator is not something you need to deal with directly when parsing attributes. You use the relevant parsing function from the list below, according to whether the parameter is optional or required, and whether it must be carried in protected header or could be carried either protected or unprotected:

For serialisation, you can force a field to appear in the protected header regardless of the user-set protection value in This is probably not as nice as it could be, but it does the job. See instance HasParams JWSHeader for example, in particular the crit field: https://github.com/frasertweedale/hs-jose/blob/e81b0e990ee1854330b3d41e033dbbc65701d17c/src/Crypto/JOSE/JWS.hs#L334-L363.

ericpashman commented 9 months ago

Yes, as I mentioned, both of the examples I constructed work. My point in providing the second example is that it is "nicer", but that it requires a type definition for user-constructed custom headers that is different from what is recommended in the documentation.

This was my (perhaps too subtle) way of suggesting that if you recommend that users define custom headers in this style, it may save them some aggravation.

ericpashman commented 7 months ago

FYI, defining custom headers in the way I described at the end of the OP does type-check, but it does not in fact work. That is, this type-checks but ultimately leads to an invalid JWS:

data MyHeader' p = MyHeader'
  { _myJWSHeader' :: JWSHeader p
  , _myNonce' :: HeaderParam p UTCTime}

instance HasParams MyHeader' where
  parseParamsFor proxy hp hu = MyHeader'
    <$> parseParamsFor proxy hp hu
    <*> headerRequired "nonce" hp hu

You can pass this header type to signJWS, and make a JWS value, but that value does not encode correctly. That is, using encodeCompact, you get something that looks like an encoded JWS, but it won't be valid.

On the one hand, yes, this is broken because I deliberately chose not to follow the instructions in the documentation about how to make a custom header type; on the other hand, it would be nice if this either (a) worked or (b) failed to type-check.

As I mentioned in the OP, defining a header type in this way would be slightly nicer (if it worked) than the way the API presently requires, because it eliminates the need to give an explicit type signature to fix the protection indicator type. Note that users must include a type variable for the protection indicator in their custom header types (because the HasParams class requires its instances to have kind * -> *), but I imagine most users will only ever use it with one particular protection indicator.