[golang-dev] Re: code review 6632044: crypto/hmac: add Verify function. (issue 6632044)

https://codereview.appspot.com/6632044/diff/2002/src/pkg/crypto/hmac/hmac.go
File src/pkg/crypto/hmac/hmac.go (right):

https://codereview.appspot.com/6632044/diff/2002/src/pkg/crypto/hmac/hmac.go#newcode10
src/pkg/crypto/hmac/hmac.go:10: Receivers should be careful to use
Verify to compare MACs in order to avoid
Done.

https://codereview.appspot.com/6632044/diff/2002/src/pkg/crypto/hmac/hmac.go#newcode95
src/pkg/crypto/hmac/hmac.go:95: // Verify compares two MACs for equality
without leaking timing information.
The problem is that it's very common (almost required) to hash in
additional information to the HMAC. If the interface only supports
passing in a single slice, people will either work around it or they'll
have to alloc and copy a new slice.

https://codereview.appspot.com/6632044/

I'm currently working on a project that relies on the HMAC functions.
I have a read-verify-read-verify-... loop, which is further
complicated by the fact that certain HMAC comparisons are truncated to
some number of most significant bytes. I think a single set of signing
and verification functions is never going to fit all use cases, but a
reminder that the comparison needs to be done in constant time is very
useful.

If you want a function that does more than a simple comparison of two
byte slices, I would do something like this (what I use in my code):

// Verify compares the HMAC tag in b with the current hash value
// returned by h, truncated to t bytes. If len(b) > t, all preceding
// bytes are written to h before the comparison. h.Size() is used
// for t if t <= 0.
func Verify(b []byte, t int, h hash.Hash)

You can obviously remove t if you think that complicates the interface
beyond what is acceptable in the standard library, but truncation is a
normal part of HMAC operations (RFC 2104 section 5).

I quite like that too, but I worry that it precludes avoiding the
allocation each time when calling Sum. Without this function, users
would are verifying many MACs can reuse storage. But, since
escape-analysis isn't going to work through the interface, I think
this would have to allocate every time.


Cheers

AGL