Bugzilla Bug 313196: HMAC code should not use a fixed hash input block size

of 64 bytes, which is wrong for SHA-384 and SHA-512.  This requires adding
the hash input block size to the SECHashObject structure. r=relyea,nelsonb
Modified Files:
	cryptohi/hasht.h cryptohi/sechash.c freebl/alghmac.c
	freebl/blapit.h freebl/rawhash.c

5 files changed, 60 insertions, 23 deletions

diff --git a/security/nss/lib/cryptohi/hasht.h b/security/nss/lib/cryptohi/hasht.h
index 2d2c6b0ca..6df38b49a 100644
--- a/security/nss/lib/cryptohi/hasht.h
+++ b/security/nss/lib/cryptohi/hasht.h
@@ -72,13 +72,15 @@ typedef enum {
  * Structure to hold hash computation info and routines
  */
 struct SECHashObjectStr {
-    unsigned int length;
+    unsigned int length;  /* hash output length (in bytes) */
     void * (*create)(void);
     void * (*clone)(void *);
     void (*destroy)(void *, PRBool);
     void (*begin)(void *);
     void (*update)(void *, const unsigned char *, unsigned int);
     void (*end)(void *, unsigned char *, unsigned int *, unsigned int);
+    unsigned int blocklength;  /* hash input block size (in bytes) */
+    HASH_HashType type;
 };
 
 struct HASHContextStr {
diff --git a/security/nss/lib/cryptohi/sechash.c b/security/nss/lib/cryptohi/sechash.c
index e977dd5f2..eebe23d95 100644
--- a/security/nss/lib/cryptohi/sechash.c
+++ b/security/nss/lib/cryptohi/sechash.c
@@ -114,7 +114,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) null_hash_begin,
     (void (*)(void *, const unsigned char *, unsigned int)) null_hash_update,
     (void (*)(void *, unsigned char *, unsigned int *,
-	      unsigned int)) null_hash_end
+	      unsigned int)) null_hash_end,
+    0,
+    HASH_AlgNULL
   },
   { MD2_LENGTH,
     (void * (*)(void)) md2_NewContext,
@@ -123,7 +125,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    MD2_BLOCK_LENGTH,
+    HASH_AlgMD2
   },
   { MD5_LENGTH,
     (void * (*)(void)) md5_NewContext,
@@ -132,7 +136,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    MD5_BLOCK_LENGTH,
+    HASH_AlgMD5
   },
   { SHA1_LENGTH,
     (void * (*)(void)) sha1_NewContext,
@@ -141,7 +147,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    SHA1_BLOCK_LENGTH,
+    HASH_AlgSHA1
   },
   { SHA256_LENGTH,
     (void * (*)(void)) sha256_NewContext,
@@ -150,7 +158,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    SHA256_BLOCK_LENGTH,
+    HASH_AlgSHA256
   },
   { SHA384_LENGTH,
     (void * (*)(void)) sha384_NewContext,
@@ -159,7 +169,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    SHA384_BLOCK_LENGTH,
+    HASH_AlgSHA384
   },
   { SHA512_LENGTH,
     (void * (*)(void)) sha512_NewContext,
@@ -168,7 +180,9 @@ const SECHashObject SECHashObjects[] = {
     (void (*)(void *)) PK11_DigestBegin,
     (void (*)(void *, const unsigned char *, unsigned int)) PK11_DigestOp,
     (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) 
-							PK11_DigestFinal
+							PK11_DigestFinal,
+    SHA512_BLOCK_LENGTH,
+    HASH_AlgSHA512
   },
 };
 
diff --git a/security/nss/lib/freebl/alghmac.c b/security/nss/lib/freebl/alghmac.c
index 5f09548e8..796cb990c 100644
--- a/security/nss/lib/freebl/alghmac.c
+++ b/security/nss/lib/freebl/alghmac.c
@@ -39,7 +39,7 @@
 #include "alghmac.h"
 #include "secerr.h"
 
-#define HMAC_PAD_SIZE 64
+#define HMAC_PAD_SIZE HASH_BLOCK_LENGTH_MAX
 
 struct HMACContextStr {
     void *hash;
@@ -86,7 +86,7 @@ HMAC_Init( HMACContext * cx, const SECHashObject *hash_obj,
     if (cx->hash == NULL)
 	goto loser;
 
-    if (secret_len > HMAC_PAD_SIZE) {
+    if (secret_len > cx->hashobj->blocklength) {
 	cx->hashobj->begin( cx->hash);
 	cx->hashobj->update(cx->hash, secret, secret_len);
 	PORT_Assert(cx->hashobj->length <= sizeof hashed_secret);
@@ -99,8 +99,8 @@ HMAC_Init( HMACContext * cx, const SECHashObject *hash_obj,
 	secret = (const unsigned char *)&hashed_secret[0];
     }
 
-    PORT_Memset(cx->ipad, 0x36, sizeof cx->ipad);
-    PORT_Memset(cx->opad, 0x5c, sizeof cx->opad);
+    PORT_Memset(cx->ipad, 0x36, cx->hashobj->blocklength);
+    PORT_Memset(cx->opad, 0x5c, cx->hashobj->blocklength);
 
     /* fold secret into padding */
     for (i = 0; i < secret_len; i++) {
@@ -139,7 +139,7 @@ HMAC_Begin(HMACContext *cx)
 {
     /* start inner hash */
     cx->hashobj->begin(cx->hash);
-    cx->hashobj->update(cx->hash, cx->ipad, sizeof(cx->ipad));
+    cx->hashobj->update(cx->hash, cx->ipad, cx->hashobj->blocklength);
 }
 
 void
@@ -162,7 +162,7 @@ HMAC_Finish(HMACContext *cx, unsigned char *result, unsigned int *result_len,
 	return SECFailure;
 
     cx->hashobj->begin(cx->hash);
-    cx->hashobj->update(cx->hash, cx->opad, sizeof(cx->opad));
+    cx->hashobj->update(cx->hash, cx->opad, cx->hashobj->blocklength);
     cx->hashobj->update(cx->hash, result, *result_len);
     cx->hashobj->end(cx->hash, result, result_len, max_result_len);
     return SECSuccess;
@@ -182,8 +182,8 @@ HMAC_Clone(HMACContext *cx)
     newcx->hash = cx->hashobj->clone(cx->hash);
     if (newcx->hash == NULL)
 	goto loser;
-    PORT_Memcpy(newcx->ipad, cx->ipad, sizeof(cx->ipad));
-    PORT_Memcpy(newcx->opad, cx->opad, sizeof(cx->opad));
+    PORT_Memcpy(newcx->ipad, cx->ipad, cx->hashobj->blocklength);
+    PORT_Memcpy(newcx->opad, cx->opad, cx->hashobj->blocklength);
     return newcx;
 
 loser:
diff --git a/security/nss/lib/freebl/blapit.h b/security/nss/lib/freebl/blapit.h
index a0f4e364f..2400ce325 100644
--- a/security/nss/lib/freebl/blapit.h
+++ b/security/nss/lib/freebl/blapit.h
@@ -95,9 +95,13 @@
  * Input block size for each hash algorithm.
  */
 
+#define MD2_BLOCK_LENGTH 	 64 	/* bytes */
+#define MD5_BLOCK_LENGTH 	 64 	/* bytes */
+#define SHA1_BLOCK_LENGTH 	 64 	/* bytes */
 #define SHA256_BLOCK_LENGTH 	 64 	/* bytes */
 #define SHA384_BLOCK_LENGTH 	128 	/* bytes */
 #define SHA512_BLOCK_LENGTH 	128 	/* bytes */
+#define HASH_BLOCK_LENGTH_MAX 	SHA512_BLOCK_LENGTH
 
 #define AES_KEY_WRAP_IV_BYTES    8
 #define AES_KEY_WRAP_BLOCK_SIZE  8  /* bytes */
diff --git a/security/nss/lib/freebl/rawhash.c b/security/nss/lib/freebl/rawhash.c
index f16ed0caf..a3efe9b30 100644
--- a/security/nss/lib/freebl/rawhash.c
+++ b/security/nss/lib/freebl/rawhash.c
@@ -84,7 +84,9 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *)) null_hash_begin,
     (void (*)(void *, const unsigned char *, unsigned int)) null_hash_update,
     (void (*)(void *, unsigned char *, unsigned int *,
-	      unsigned int)) null_hash_end
+	      unsigned int)) null_hash_end,
+    0,
+    HASH_AlgNULL
   },
   { MD2_LENGTH,
     (void * (*)(void)) MD2_NewContext,
@@ -92,7 +94,9 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) MD2_DestroyContext,
     (void (*)(void *)) MD2_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) MD2_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) MD2_End
+    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) MD2_End,
+    MD2_BLOCK_LENGTH,
+    HASH_AlgMD2
   },
   { MD5_LENGTH,
     (void * (*)(void)) MD5_NewContext,
@@ -100,7 +104,9 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) MD5_DestroyContext,
     (void (*)(void *)) MD5_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) MD5_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) MD5_End
+    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) MD5_End,
+    MD5_BLOCK_LENGTH,
+    HASH_AlgMD5
   },
   { SHA1_LENGTH,
     (void * (*)(void)) SHA1_NewContext,
@@ -108,7 +114,9 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) SHA1_DestroyContext,
     (void (*)(void *)) SHA1_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) SHA1_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) SHA1_End
+    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) SHA1_End,
+    SHA1_BLOCK_LENGTH,
+    HASH_AlgSHA1
   },
   { SHA256_LENGTH,
     (void * (*)(void)) SHA256_NewContext,
@@ -116,7 +124,10 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) SHA256_DestroyContext,
     (void (*)(void *)) SHA256_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) SHA256_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) SHA256_End
+    (void (*)(void *, unsigned char *, unsigned int *,
+	      unsigned int)) SHA256_End,
+    SHA256_BLOCK_LENGTH,
+    HASH_AlgSHA256
   },
   { SHA384_LENGTH,
     (void * (*)(void)) SHA384_NewContext,
@@ -124,7 +135,10 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) SHA384_DestroyContext,
     (void (*)(void *)) SHA384_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) SHA384_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) SHA384_End
+    (void (*)(void *, unsigned char *, unsigned int *,
+	      unsigned int)) SHA384_End,
+    SHA384_BLOCK_LENGTH,
+    HASH_AlgSHA384
   },
   { SHA512_LENGTH,
     (void * (*)(void)) SHA512_NewContext,
@@ -132,7 +146,10 @@ const SECHashObject SECRawHashObjects[] = {
     (void (*)(void *, PRBool)) SHA512_DestroyContext,
     (void (*)(void *)) SHA512_Begin,
     (void (*)(void *, const unsigned char *, unsigned int)) SHA512_Update,
-    (void (*)(void *, unsigned char *, unsigned int *, unsigned int)) SHA512_End
+    (void (*)(void *, unsigned char *, unsigned int *,
+	      unsigned int)) SHA512_End,
+    SHA512_BLOCK_LENGTH,
+    HASH_AlgSHA512
   },
 };