tekul / jose-jwt

Haskell implementation of JOSE/JWT standards
BSD 3-Clause "New" or "Revised" License
35 stars 22 forks source link

Add support for creating ec signed JWS, + fix benchmark #11

Closed phyrex1an closed 7 years ago

tekul commented 8 years ago

Thanks a lot for the PR. I'm particularly pleased that you seem likely to have solved an issue which had been bothering me (and others) for some time (https://github.com/fpco/stackage/issues/1125). The SHA384 issue in cryptonite is the most likely explanation for the intermittent test failures. Did you actually find that after updating the benchmarks? Just goes to show that you can never have too many tests.

Regarding the EC code, I deliberately left out any functionality which used any code with the timing warnings, which I'd prefer to avoid in the library. Is this something you have an immediate need for? I've also asked @vincenthz for his opinion on how we can improve things in cryptonite itself.

phyrex1an commented 8 years ago

Yeah, the benchmark failed deterministically with cryptonite <= 0.12. I wasn't aware of fpco/stackage#1125. The symptoms looks very similar save the non-determinism. In the benchmarks the buffer overflow manifests as an out of memory error but can be turned into a segfault by some modifications of the code, neither error will yield any useful output when running stack bench and I guess the same applies to stack test.

I figured the timing side channels might be the reason for ec signing being unsupported. I don't have any further need for it so I'll close this PR and make a new one for just the benchmark fix instead.

tekul commented 8 years ago

It's OK, I can pull in the commit from here - no need for another issue. Also I'd like to add EC support so perhaps the discussion can be continued here. I already downloaded your branch and reproduced the failure in the benchmark suite. I'd guess the intermittency is due to the fact that the main tests only run the SHA384 code once or twice whereas the benchmark will do multiple runs.