259135 - power-up self-tests needed for SHA-256,384,512 and AES

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Ian McGreer]

New algorithms in NSS need self-tests.

Comment 1

[Ian McGreer]

Attachment: implement self-test

Comment 2

[Wan-Teh Chang]

Comment on attachment 158765 [details] [diff] [review]
implement self-test

Ian, Bob, I believe the following PORT_Memcmp call
reads beyond the end of the aes_ecb256_known_ciphertext,
which has length zero.	In fact, this code is a syntax
error (at least under Microsoft Visual C++ compiler).

>Index: fipstest.c

>+#define FIPS_AES_ENCRYPT_LENGTH                 16  /* 128-bits */

>+    /* AES Known Ciphertext (256-bits). */
>+    static const PRUint8 aes_ecb256_known_ciphertext[] = { };

>+    const PRUint8 *aes_ecb_known_ciphertext =
>+	( aes_key_size == 16) ? aes_ecb128_known_ciphertext :
>+	( aes_key_size == 24) ? aes_ecb192_known_ciphertext :
>+	                        aes_ecb256_known_ciphertext;


>+    if( ( aes_status != SECSuccess ) ||
>+        ( aes_bytes_encrypted != FIPS_AES_ENCRYPT_LENGTH ) ||
>+        ( PORT_Memcmp( aes_computed_ciphertext, aes_ecb_known_ciphertext,
>+                       FIPS_AES_ENCRYPT_LENGTH ) != 0 ) )
>+        return( CKR_DEVICE_ERROR );

Also, this function should check that aes_key_size is 16,
24, or 32.

Comment 3

[glen beasley]

Attachment: update to Ian's orginal patch

Comment 4

[glen beasley]

Attachment: forgot to remove some debug code.

Comment 5

[glen beasley]

The changes to Ian's original fix were:

For the AES tests I added some defines, FIPS_AES_128_BLOCK_SIZE,
FIPS_AES_128_KEY_SIZE, FIPS_AES_192_KEY_SIZE, and FIPS_AES_256_KEY_SIZE.

I added a check for the AES key size per Wan-Teh Comment #2 request.

I corrected the known cipher text for aes_cbc192_known_ciphertext,
and added the known cipher text for aes_ecb256_known_ciphertext and
aes_cbc256_known_ciphertext.

Fixed typo for old style "pk11_fips" to "sftk_fips"
other than that Ian's changes for SHA1, SHA256, SHA384,
and SHA512 were not modified.

This enhancement adds:
Hashing tests for SHA1, SHA256, SHA384, and SHA512
AES-ECB, and AES-CBC single round Encrypt/Decrypt tests
for AES 128, 192, and 256 Key sizes.

HMAC self tests still need to be added, but this will
be done in bug 298522

Comment 6

[Wan-Teh Chang]

Comment on attachment 198162 [details] [diff] [review]
forgot to remove some debug code.

r=wtc.	Please make the following changes before you check in.

>+#define FIPS_AES_128_BLOCK_SIZE			16  /* 128-bits */

Rename FIPS_AES_128_BLOCK_SIZE as FIPS_AES_BLOCK_SIZE
because AES's block size is 128 bits.  Note that the underlying
algorithm (Rijndael) can use a block size up to 256 bits,
but in AES the block size is fixed at 128 bits.

>+/* FIPS preprocessor directives for digests.                    */
>+#define FIPS_KNOWN_HASH_MESSAGE_LENGTH          64  /* 512-bits */

Change "digests" to "message digests" or "hashes" in the comment.

>+    /* AES-CBC Known Initialization Vector (up to 256-bits). */
>+    static const PRUint8 aes_cbc_known_initialization_vector[] = 
>+	{ "SecurityytiruceSSecurityytiruceS" };

Change "up to 256-bits" to "128-bits" and change
"SecurityytiruceSSecurityytiruceS" to
"SecurityytiruceS".  The initialization vector is the same
size as the input block, which is 128 bits for AES (see
my comment above about AES vs. Rijndael block sizes).

>+    /* AES Known Ciphertext (128-bits). */
>+    static const PRUint8 aes_ecb128_known_ciphertext[] = {

Change "128-bits" to "128-bit key".

>+    /* AES Known Ciphertext (192-bits). */
>+    static const PRUint8 aes_ecb192_known_ciphertext[] = {

Change "192-bits" to "192-bit key".

>+    /* AES Known Ciphertext (256-bits). */
>+    static const PRUint8 aes_ecb256_known_ciphertext[] = {

Change "256-bits" to "256-bit key".

>+    /******************************************************/
>+    /* AES-ECB Single-Round Known Answer Encryption Test: */
>+    /******************************************************/
>+
>+    aes_context = AES_CreateContext( aes_known_key, NULL, NSS_AES, PR_TRUE,
>+                                     aes_key_size, 16 );

Change 16 to FIPS_AES_BLOCK_SIZE.

>+    /******************************************************/
>+    /* AES-CBC Single-Round Known Answer Decryption Test. */
>+    /******************************************************/
>+
>+    aes_context = AES_CreateContext( aes_known_key,
>+                                     aes_cbc_known_initialization_vector,
>+                                     NSS_AES_CBC, PR_FALSE, aes_key_size, 16 );

Change 16 to FIPS_AES_BLOCK_SIZE.

>     /* SHA-1 Power-Up SelfTest(s). */
>-    rv = sftk_fips_SHA1_PowerUpSelfTest();
>+    rv = sftk_fips_SHA_PowerUpSelfTest();

Change "SHA-1" to "SHA" or "SHA-X" in the comment.

Comment 7

[glen beasley]

Attachment: added Wan-Teh changes from comment #6

Comment 8

[glen beasley]

/cvsroot/mozilla/security/nss/lib/softoken/fipstest.c,v  <--  fipstest.c
new revision: 1.9; previous revision: 1.8
done

Comment 9

[Wan-Teh Chang]

Attachment: Additional patch

This patch has a minor comment fix and also a fix
for build breakage.  Some C compilers still require
that variable declarations precede the code.  I have
checked in this patch.