search

frasertweedale / hs-jose

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

Perform key sanity check when verifying signatures #46

Closed frasertweedale closed 2 years ago

frasertweedale commented 7 years ago

Currently if a key is too small, the verification fails with InvalidSignature instead of KeySizeTooSmall. Add a key sanity check step before attempting to validate the signature so that the appropriate error gets returned to the caller.

Thanks to @begriffs for reporting.

frasertweedale commented 7 years ago

@begriffs have a look at: https://github.com/frasertweedale/hs-jose/commit/59ca5e656370e5b7812faadf5234aa9b0724e782

Note that this step is not automatically performed during verification for a couple of reasons:

  1. An offending JWK in a JWK store might not actually be used for a particular verification; checking all JWKs in a JWKStore is too aggressive and may result in spurious verification failures.

  2. Passing the per-signature verification failures up to the caller is... quite awkward, to say the least. A single signature may be checked against multiple keys. If the all fail, we'd need to pass all the errors up, presumably with a mapping of each error the JWK that caused it, then let the user sort it out.

Instead of presenting the above problems to library users, I've decided to just implement the checkJWK sanity check function. Library users can call it themselves for each JWK that intend to use for verification, and can fail / report / omit offending keys according to their needs. It is an optional extra step. it can also aid debugging (e.g. could be used for post facto diagnostics).

Let me know what you think of the approach and its usability.

Cheers!

begriffs commented 7 years ago

Thanks for the helper function. I will try it out and see how it goes!

frasertweedale commented 7 years ago

@begriffs did you have any feedback re checkJWK ?

begriffs commented 7 years ago

Thanks, I haven't gotten around to using this function, but I filed an issue in my project so that I or someone else will use it to improve error messages for users.

frasertweedale commented 7 years ago

@begriffs righto, I'll merge it soon.

ecthiender commented 6 years ago

Why can't verifyClaims function verify the size of the key and fail with KeySizeTooSmall ?

Much better UX for library users IMHO.

frasertweedale commented 6 years ago

@ecthiender it's because there can be multiple keys that could be used to verify some signature, and there is not yet a way to propagate complete information about the verification results for particular keys.

I'll think about how the API could be enhanced to provide that. If you have ideas, please share.