Wrong HMAC hashes using GHC 7.6 #12

Closed
aristidb opened this Issue Dec 20, 2012 · 11 comments

Comments

Projects
None yet
3 participants

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.

aristidb referenced this issue in aristidb/aws Dec 20, 2012

Closed

api signing is broken? #48

pheaver commented Dec 20, 2012

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.

@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.

Owner

vincenthz commented Dec 20, 2012

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.

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.

Owner

vincenthz commented Dec 20, 2012

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.

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

Owner

vincenthz commented Dec 20, 2012

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.

Owner

vincenthz commented Dec 20, 2012

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.

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.

Owner

vincenthz commented Dec 21, 2012

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.

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.

vincenthz closed this Mar 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment