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

RESOLVED FIXED in 3.11

Status

P1
normal
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: bugz, Assigned: glenbeasley)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

14 years ago
New algorithms in NSS need self-tests.
(Reporter)

Comment 1

14 years ago
Created attachment 158765 [details] [diff] [review]
implement self-test
Assignee: wchang0222 → bugz
Status: NEW → ASSIGNED
(Reporter)

Updated

14 years ago
Attachment #158765 - Flags: superreview?(wchang0222)
Attachment #158765 - Flags: review?(rrelyea0264)

Updated

14 years ago
Attachment #158765 - Flags: review?(rrelyea) → review+

Comment 2

14 years ago
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.
Attachment #158765 - Flags: superreview?(wtchang) → superreview-

Updated

14 years ago
Priority: -- → P1
Target Milestone: --- → 3.10
QA Contact: bishakhabanerjee → jason.m.reid

Updated

14 years ago
Assignee: bugz → glen.beasley
Status: ASSIGNED → NEW
Target Milestone: 3.10 → 3.11
(Assignee)

Comment 3

13 years ago
Created attachment 198159 [details] [diff] [review]
update to Ian's orginal patch
Attachment #158765 - Attachment is obsolete: true
Attachment #198159 - Flags: superreview?(wtchang)
(Assignee)

Comment 4

13 years ago
Created attachment 198162 [details] [diff] [review]
forgot to remove some debug code.
Attachment #198162 - Flags: superreview?(wtchang)
(Assignee)

Updated

13 years ago
Attachment #198159 - Attachment is obsolete: true
(Assignee)

Comment 5

13 years ago
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

13 years ago
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.
Attachment #198162 - Flags: superreview?(wtchang) → superreview+
(Assignee)

Comment 7

13 years ago
Created attachment 198585 [details] [diff] [review]
added Wan-Teh changes from comment #6
Attachment #198162 - Attachment is obsolete: true
(Assignee)

Comment 8

13 years ago
/cvsroot/mozilla/security/nss/lib/softoken/fipstest.c,v  <--  fipstest.c
new revision: 1.9; previous revision: 1.8
done
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Assignee)

Updated

13 years ago
Attachment #198159 - Flags: superreview?(wtchang)

Comment 9

13 years ago
Created attachment 198632 [details] [diff] [review]
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.
You need to log in before you can comment on or make changes to this bug.