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));
 }

Reply via email to