148220 - NSS should implement FIPS PUB 198: Keyed-Hash Message Authentication Code (HMAC)

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Wan-Teh Chang]

NSS should implement FIPS PUB 198: The Keyed-Hash
Message Authentication Code (HMAC).

Comment 1

[Wan-Teh Chang]

I looked at the FIPS 198 HMAC document and the HMAC implementation
in NSS.  The algorithms are identical.

HMAC was previously specified in RFC 2104. FIPS 198 differs from
RFC 2104 only in the requirements on the lengths of the secret
keys and truncated output.

1. NSS's HMAC_Finish function does not support truncated output (the
caller can do that easily), and therefore cannot enforce FIPS 198's
requirement on truncated output length.  I don't think this is a
problem. If we need to do that, we will need to add a new function:

SECStatus
HMAC_FinishTruncate(
     HMACContext *cx,
     unsigned char *result,
     unsigned int *result_len,
     unsigned int max_result_len,
     unsigned int truncated_result_len  <== new arg
);

Alternatively, we can change the meaning of the max_result_len
argument for HMAC_Finish to mean the truncated output length
if max_result_len is less than the length of the output of the
hash algorithm.

2. We can modify NSS's HMAC_Create function to reject key
lengths that do not meet FIPS 198's requirement.

Your comments are welcome.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

If it is easy to do, please summarize here the differences in the 
requirements on key lengths and output length (truncation).  If the RFC
has no requirements, and the FIPS does, please summarize the FIPS 
requirements.  Thanks.

Comment 3

[Wan-Teh Chang]

L is the block size (in bytes) of the output of the hash function.

1. RFC 2104

[Key length] less than L bytes is strongly discouraged [...]

We recommend that the output length t be not less than half
the length of the hash output [...] and not less than
[10 bytes ...]

2. FIPS 198

The size of the key, K, shall be equal or greater than L/2 [...]

The output length, t, shall be no less than four bytes [...].
However, t shall be at least L/2 bytes [...] unless an application
or protocol makes numerous trials impractical.

Comment 4

[Wan-Teh Chang]

Two more comments on our HMAC implementation (softoken/alghmac.c).

1. The parameter B (block size, in bytes, of the input to the
hash function) is hardcoded to be 64 (the HMAC_PAD_SIZE macro).
B is 64 for SHA-1 and MD5, but seems to be 16 for MD2.  So our
HMAC code may not work with MD2.

2. Our HMAC code assumes that the block size (in bytes) of the
output of the hash function is <= 20 (the size of the hashed_secret
array in HMAC_Create), which is true for all the hash functions
currently implemented in NSS (SHA-1, MD5, and MD2).  I will add
an assertion to verify this assumption.

Comment 5

[Wan-Teh Chang]

Attachment: Proposed patch

1. Enforce FIPS 198, Sec. 3's requirement on the size
   of the key.

2. Add an assertion for the assumption our code makes
   on the size of the output of the hash function.

3. Fix the bug where we are not setting the error code
   when the result buffer is too small.

Comment 6

[Robert Relyea]

Attachment: Proposal to meet the FIPS POST integrity check.

Comment 7

[Dawn Endico]

marking this bug mozilla.org confidential until my paranoid self is 
comfortable with updating the live db by hand to blank out the attachment.

Comment 8

[Wan-Teh Chang]

I checked in my patch.

Here is a summary of our HMAC code.

1. It enforces FIPS 198, Section 3's requirement on the
size of the key.

2. It does not truncate the output.  Therefore FIPS
198, Section 4's requirement on truncated output does
not apply.

3. The algorithm is exactly as specified in FIPS 198,
Section 5.

4. It does not work with MD2 (bug 160161), which is not
a FIPS approved hash function.  So this is not an issue
for FIPS validation.

So I am marking this bug fixed.  I would still appreciate
a review of my patch.

Comment 9

[Wan-Teh Chang]

Attachment: Incremental patch: need to include secerr.h

Comment 10

[Wan-Teh Chang]

The FIPS 198 key size requirement broke the NSS QA.  I
am backing out that change and reopening this bug.

Comment 11

[Wan-Teh Chang]

NSS QA failed in the SSL Cipher Coverate tests.
The ciphers that were broken were
TLS RSA EXPORT WITH RC4 40 MD5
TLS RSA EXPORT WITH RC2 CBC 40 MD5
TLS RSA EXPORT WITH DES CBC SHA (new)
TLS RSA EXPORT WITH RC4 56 SHA (new)

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Why this patch broke NSS QA:

TLS uses HMAC as part of the "Pseudo Random Function" by which it expands
the small secrets produced for export ciphersuites (e.g. 40 bit secrets)
into the larger keys needed by some crypto algorithms (e.g. RC4).  
These small secrets are smaller than the FIPS 198 minimum secret length
requirement.  So, it is not possible to use a FIPS 198 compliant HMAC 
implementation to implement TLS's export cipher suites.  

The call stack of the failure was:

[1] HMAC_Create(0xfee0a8c4, 0xffbed240, 3), line 69 in "alghmac.c"
[2] pk11_P_hash(HASH_AlgMD5, 0xffbed110, ...), line 4149 in "pkcs11c.c"
[3] pk11_PRF(0xffbed1f4, "client write key", ...), line 4224 in "pkcs11c.c"
[4] NSC_DeriveKey(16U, ...), line 4777 in "pkcs11c.c"
[5] pk11_DeriveWithTemplate(...), line 2521 in "pk11skey.c"
[6] PK11_Derive(...), line 2403 in "pk11skey.c"
[7] ssl3_GenerateSessionKeys(ss = 0x72868, ...), line 2102 in "ssl3con.c"
[8] ssl3_InitPendingCipherSpec(ss = 0x72868, ...), line 1098 in "ssl3con.c"
[9] sendRSAClientKeyExchange(ss = 0x72868, ...), line 3241 in "ssl3con.c"
[10] ssl3_SendClientKeyExchange(ss = 0x72868), line 3834 in "ssl3con.c"
...

Comment 13

[Wan-Teh Chang]

Attachment: Combination of previous patches

Comment 14

[Wan-Teh Chang]

Attachment: Patch 2: implement FIPS 198 conformance partially

This patch implements FIPS 198 conformance in alghmac.c and
pkcs11c.c.  lowpbe.c is still callling HMAC_Create in
non-FIPS mode.

The private function HMAC_Finish does not allow truncated
output, so the internal use of HMAC in the softoken
satisfies FIPS 198's requirement on truncated output
trvially.  However, the PKCS #11 CKM_XXX_HMAC_GENERAL
mechanisms allow truncated output. I enforce the FIPS
requirement in pk11_doHMACInit (a common point in the
code paths of these mechanisms).

Review invited.

Comment 15

[Robert Relyea]

Comment on attachment 93741 [details] [diff] [review]
Combination of previous patches

This patch is good. (bob)

Comment 16

[Robert Relyea]

Comment on attachment 93743 [details] [diff] [review]
Patch 2: implement FIPS 198 conformance partially

The HMAC_Create calls in lowpbe should pass PR_TRUE for isFips. Rationale: We
know that these functions should always satisfy the FIPS requirement anyway. If
we pass PR_TRUE then we don't have to extend the evaluation to proving that the
truncation is either not happening or is within the FIPS limits. Since we are
confident that this is the case there should be no impact to the code to
setting these values to PR_TRUE.

bob

Comment 17

[Wan-Teh Chang]

Attachment: Patch 2 v2: implements FIPS 198 conformance fully

On Bob's suggestion I am passing PR_TRUE for the
isFIPS argument to HMAC_Create in lowpbe.c.  This
means we have full conformance to FIPS 198.

Comment 18

[Robert Relyea]

Comment on attachment 94400 [details] [diff] [review]
Patch 2 v2: implements FIPS 198 conformance fully

Looks good.

bob

Comment 19

[Wan-Teh Chang]

Attachment: Incremental patch: remove unused isFIPS field from HMACContext

Nelson pointed out that the isFIPS field in HMACContext
is unused.  So I removed it.

I originally added the isFIPS field in preparation for
supporting truncated output.  But the field turns out to
be unnecessary because of our policy to only allow
truncated output at the PKCS #11 mechanism level.
That is, the internal HMAC_Finish function does not
truncate output, so the FIPS requirement doesn't apply.

Comment 20

[Wan-Teh Chang]

All the patches have been checked in.  The softoken should
fully conform to FIPS 198 while in FIPS mode now.