I now realize this may have been ignored simply because the clock on the sending machine was horribly off and many people sort mail by date. So... Should this go in? Am I missing something?
On Thu, Apr 30, 2015 at 06:03:23PM -0400, Jean-Philippe Ouellet wrote: > The intermediate values calculated in hmac_sha1 as part of > pkcs5_pbkdf2 are not zeroed afterwards, so we leak a single-hashed > version of the key on the stack in tk[]. > > Also, the correct RFC defining this is > RFC 2104 - HMAC: Keyed-Hashing for Message Authentication > not > RFC 2202 - Test Cases for HMAC-MD5 and HMAC-SHA-1 > > From RFC 2104, section 4, paragraph 2: > "We stress that the stored intermediate values need to > be treated and protected the same as secret keys." > So it's not just best-practice dictating that these should be > bzeroed. > > Here is a patch for the same code in libutil and libsa. > > Index: lib/libutil/pkcs5_pbkdf2.c > =================================================================== > RCS file: /cvs/src/lib/libutil/pkcs5_pbkdf2.c,v > retrieving revision 1.9 > diff -u -p -d -r1.9 pkcs5_pbkdf2.c > --- lib/libutil/pkcs5_pbkdf2.c 5 Feb 2015 12:59:57 -0000 1.9 > +++ lib/libutil/pkcs5_pbkdf2.c 10 Jun 2015 19:59:09 -0000 > @@ -28,7 +28,7 @@ > #define MINIMUM(a,b) (((a) < (b)) ? (a) : (b)) > > /* > - * HMAC-SHA-1 (from RFC 2202). > + * HMAC-SHA-1 (from RFC 2104). > */ > static void > hmac_sha1(const u_int8_t *text, size_t text_len, const u_int8_t *key, > @@ -67,6 +67,10 @@ hmac_sha1(const u_int8_t *text, size_t t > SHA1Update(&ctx, k_pad, SHA1_BLOCK_LENGTH); > SHA1Update(&ctx, digest, SHA1_DIGEST_LENGTH); > SHA1Final(digest, &ctx); > + > + explicit_bzero(&ctx, sizeof(ctx)); > + explicit_bzero(k_pad, sizeof(k_pad)); > + explicit_bzero(tk, sizeof(tk)); > } > > /* > Index: sys/lib/libsa/hmac_sha1.c > =================================================================== > RCS file: /cvs/src/sys/lib/libsa/hmac_sha1.c,v > retrieving revision 1.1 > diff -u -p -d -r1.1 hmac_sha1.c > --- sys/lib/libsa/hmac_sha1.c 9 Oct 2012 12:36:50 -0000 1.1 > +++ sys/lib/libsa/hmac_sha1.c 10 Jun 2015 19:58:39 -0000 > @@ -23,7 +23,7 @@ > #include "hmac_sha1.h" > > /* > - * HMAC-SHA-1 (from RFC 2202). > + * HMAC-SHA-1 (from RFC 2104). > */ > void > hmac_sha1(const u_int8_t *text, size_t text_len, const u_int8_t *key, > @@ -62,4 +62,8 @@ hmac_sha1(const u_int8_t *text, size_t t > SHA1Update(&ctx, k_pad, SHA1_BLOCK_LENGTH); > SHA1Update(&ctx, digest, SHA1_DIGEST_LENGTH); > SHA1Final(digest, &ctx); > + > + explicit_bzero(&ctx, sizeof(ctx)); > + explicit_bzero(k_pad, sizeof(k_pad)); > + explicit_bzero(tk, sizeof(tk)); > } >
