HMAC-SHA1 implementation for Mac based on CommonCrypto

HMAC-SHA1 implementation for Mac based on CommonCrypto;
allow Windows HMAC-SHA1 to use keys longer than 16 bytes.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=1724

[Mark Mentovai, 2008-09-04 00:05:38]

OK, I don't know if we actually want this, since it pretty much hinges on the
OpenSSL decision.  I do want to get this out of my tree even if it doesn't go
into the repository and I don't mind forcing the issue.

If we're not going to use OpenSSL, we could bring in our own SHA-1
implementation from somewhere and use our own HMAC function.  HMAC is pretty
easy once you have the digest code.  Maybe that's what we'll eventually do. 
Note that we already have base/md5.cc.

cc Amanda and Evan so we can all talk about what makes sense for Mac and Linux
here.  HMAC-SHA1 is only used by safe browsing, so it's not incredibly critical
right this moment.

[Evan Martin, 2008-09-04 00:11:45]

I believe the conclusion was to use NSS on Linux and the native mac stuff on
mac.  Is that not correct?

[wtc, 2008-09-04 02:44:00]

I did some web searches for the system crypto library on
Mac OS X.  I found libCdsaCrypt.  You can find sample
code at
http://developer.apple.com/samplecode/CryptoSample/index.html

I also found low-level HMAC functions in libSystem:
http://developer.apple.com/documentation/Darwin/Reference/ManPages/man3/CCHma...

These functions are low-level in that they take the secret
key directly as an array of bytes rather than an opaque key
handle.  So I suspect that libCdsaCrypt is the preferred
method.  But these Common Crypto functions may be easier to
use.  They seem to be newly added in Leopard.

In any case, do NOT copy HMAC code into the Chromium source
tree.

[Mark Mentovai, 2008-09-04 14:39:39]

To Wan-Teh for review.

I haven't tested this on Windows yet, I'll do that before checking in.

[Mark Mentovai, 2008-09-04 16:49:43]

I tested this on Windows and updated it so that hmac_win.cc is able to handle
keys longer than 16 bytes.  Windows 2000 was the limiting factor in this case,
but apparently the code already failed on Windows 2000, and we aren't overly
concerned with supporting that version in any event.

[wtc, 2008-09-04 17:30:35]

I agree with the use of CommonCrypto.  It seems to be
newly available in Leopard:
http://developer.apple.com/leopard/overview/osfoundations.html

Do we only support Leopard?

http://codereview.chromium.org/218/diff/21/242
File base/hmac.h (right):

http://codereview.chromium.org/218/diff/21/242#newcode11
Line 11: #include "base/basictypes.h"
Just curious: in general we should include the system
headers before our own headers.  But we need to make
an exception to get the definition of the OS_xxx macro.
Is "base/basictypes.h" the header that defines OS_xxx?
Or should we use a smaller header such as
"build/build_config.h"?

http://codereview.chromium.org/218/diff/21/248
File base/hmac_mac.cc (right):

http://codereview.chromium.org/218/diff/21/248#newcode40
Line 40: DCHECK(digest_length >= algorithm_digest_length);
We should return false if this is false.  Otherwise
it's a buffer overflow.

http://codereview.chromium.org/218/diff/21/245
File base/hmac_win.cc (right):

http://codereview.chromium.org/218/diff/21/245#newcode68
Line 68: // TODO(paulg): Bug: http://b/1084719, 'ImportKey' will not currently
work on
We should take care of this TODO now.  I'll see what needs
to be done here.

http://codereview.chromium.org/218/diff/21/245#newcode73
Line 73: NULL, CRYPT_IPSEC_HMAC_KEY, &hkey_)) {
Nit: MSDN example code passes 0 instead of NULL here:
http://msdn.microsoft.com/en-us/library/aa380207(VS.85).aspx

I believe it's because HCRYPTKEY is not a pointer.  It is
defined as ULONG_PTR.

[wtc, 2008-09-04 18:10:28]

http://codereview.chromium.org/218/diff/21/245
File base/hmac_win.cc (right):

http://codereview.chromium.org/218/diff/21/245#newcode66
Line 66: // CRYPT_IPSEC_HMAC_KEY allows keys longer than 16 bytes, but is not
Let's replace this block comment with something like this:

This code doesn't work on Win2k because PLAINTEXTKEYBLOB
and CRYPT_IPSEC_HMAC_KEY are not supported on Windows 2000.
PLAINTEXTKEYBLOB allows the import of an unencrypted key.
(On Win2k we need to use a cumbersome exponent-of-one key
procedure: http://support.microsoft.com/kb/228786/en-us.)
CRYPT_IPSEC_HMAC_KEY allows keys longer than 16 bytes.

I will close bug http://b/1084719.

[wtc, 2008-09-04 18:40:42]

LGTM.

I also looked at Patch Set 7 even though the comments
below were added to Patch Set 6.

http://codereview.chromium.org/218/diff/263/51
File base/hmac_mac.cc (right):

http://codereview.chromium.org/218/diff/263/51#newcode41
Line 41: DCHECK(false);
WOuld NOTREACHED() be better?

http://codereview.chromium.org/218/diff/263/48
File base/hmac_win.cc (right):

http://codereview.chromium.org/218/diff/263/48#newcode56
Line 56: sizeof(KeyBlob));
I'm not sure if we need to do std::max here.  It'd be
useful only if key_length is <= 0.