search

vincenthz / hs-cryptohash

efficient and practical cryptohashing in haskell. DEPRECATED in favor of cryptonite
Other
30 stars 24 forks source link

Wrong HMAC hashes using GHC 7.6 #12

Closed aristidb closed 11 years ago

aristidb commented 11 years ago

The following HMAC expression gives different results using GHC 7.4.2 and GHC 7.6.1:

GHC 7.4.2:

> hmac' (MacKey "12345") "abcdefg" :: SHA256
Digest "\139\ETX\164SP@\STX\CAN\151\b\200\141B\STX\206\175\219V\ESCL}\SI\245\144\150\al\238\197\ACK\195\183"

GHC 7.6.1:

> hmac' (MacKey "12345") "abcdefg" :: SHA256 
Digest "\209\194?\135\253sA\178;\251\132\&7\255:a\184\184'\199:\233#\216fa0\t\202M\209\&8\165"

I believe the behavior under GHC 7.4.2 is correct, as it allows me to sign AWS requests properly.

The cryptohash version is 0.8.1.

pheaver commented 11 years ago

I am unable to get this to type-check. GHC complains about the ambiguous phantom type 'c' in MacKey. Am I missing something obvious?

EDIT: nevermind, older versions of crytpo-api do not have the phantom type in Mackey.

aristidb commented 11 years ago

@pheaver You need to load the modules Crypto.HMAC and Crypto.Hash.SHA256. I tested both in ghci. 'c' should be resolved by the fundep in Crypto.Classes.Hash.

vincenthz commented 11 years ago

the ghc 7.6.x bug is a bug with the bytestring library: https://github.com/snoyberg/http-conduit/issues/63 Since you're not using the hmac function from cryptohash, this should be reported for crypto-api btw.

the obvious (and crappy) workaround is using -O0 for compiling crypto-api (same issue with cryptohash hmac)

btw, I would advise against using crypto-api for now too, until the performance problem have been sorted see http://tab.snarc.org/posts/haskell/2012-12-08-cryptohash_08.html with the criterion benchmarks link.

aristidb commented 11 years ago

Is the new cryptohash API affected by the same problem? Consider that I wrote the request signing code for aws before you added that. :) If it's faster I'll switch anyways (as long as there is hmac support), but it not being affected by the bug would help me prioritise that change.

vincenthz commented 11 years ago

I was hoping the problem to go away on it own with a new bytestring library not plagued by the inlining bug, but unfortunately i think the maintainers are just waiting for a new ghc release to release a new bytestring.

I'm not a big fan of turning optimisation off for the whole of cryptohash when ghc 7.6 is in use, which is the workaround i've used in tls (which has a copy of hmac too), as it will have far more performance consequences. I'm going to try to put a NOINLINE pragma for the maped bytestring to see if that could fix the problem.

aristidb commented 11 years ago

@vincenthz So uh... what is the recommended way for computing HMAC SHA256 (and SHA1) right now? I'm a bit confused by the documentation.

vincenthz commented 11 years ago

what is the version you're using ?

on 0.7.x and early 0.8.x, hmac takes a normal hash function:

hmac SHA1.hash 64 key secret
hmac SHA512.hash 128 key secret

i've forgotten hmac in the 0.8.x reshuffle, next version will have slighlty different types that will make it probably relatively similar to crypto-api's hmac.

vincenthz commented 11 years ago

btw, i've just added long overdue KATs test for hmac in the repository, could you run them just to make sure they are not passing with ghc 7.6. for some reason my installation (7.6.1) pass the tests nicely.

aristidb commented 11 years ago

I currently use 0.8.1. I like that simple type signature without any digest/phantom type complications though. :P

2012/12/20 Vincent Hanquez notifications@github.com

what is the version you're using ?

on 0.7.x and early 0.8.x, hmac takes a normal hash function:

hmac SHA1.hash 64 key secret hmac SHA512.hash 128 key secret

i've forgotten hmac in the 0.8.x reshuffle, next version will have slighlty different types that will make it probably relatively similar to crypto-api's hmac.

— Reply to this email directly or view it on GitHubhttps://github.com/vincenthz/hs-cryptohash/issues/12#issuecomment-11588582.

vincenthz commented 11 years ago

yes, maybe a good idea to not change the prototype in Crypto.MAC.HMAC and add a central hmac implementation that uses the more elaborate types. That would prevent breaking API just like what i did with all hash implementations.

If still possible, i would really appreciate to know if you're hitting the inlining bug with ghc 7.6 and hmac in cryptohash. I just can't reproduce with my installation of ghc 7.6, so it's hard for me to experiment with some better workaround fix.

aristidb commented 11 years ago

The expression

Crypto.MAC.HMAC.hmac Crypto.Hash.SHA256.hash 512 "12345" "abcdefg"

yields the same result using GHC 7.6 and 7.4 here.