Closed Bug 259135 Opened 20 years ago Closed 19 years ago

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

Categories

(NSS :: Libraries, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugz, Assigned: glenbeasley)

Details

Attachments

(2 files, 3 obsolete files)

New algorithms in NSS need self-tests.
Attached patch implement self-test (obsolete) — Splinter Review
Assignee: wchang0222 → bugz
Status: NEW → ASSIGNED
Attachment #158765 - Flags: superreview?(wchang0222)
Attachment #158765 - Flags: review?(rrelyea0264)
Attachment #158765 - Flags: review?(rrelyea) → review+
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-
Priority: -- → P1
Target Milestone: --- → 3.10
QA Contact: bishakhabanerjee → jason.m.reid
Assignee: bugz → glen.beasley
Status: ASSIGNED → NEW
Target Milestone: 3.10 → 3.11
Attached patch update to Ian's orginal patch (obsolete) — Splinter Review
Attachment #158765 - Attachment is obsolete: true
Attachment #198159 - Flags: superreview?(wtchang)
Attachment #198162 - Flags: superreview?(wtchang)
Attachment #198159 - Attachment is obsolete: true
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 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+
Attachment #198162 - Attachment is obsolete: true
/cvsroot/mozilla/security/nss/lib/softoken/fipstest.c,v <-- fipstest.c new revision: 1.9; previous revision: 1.8 done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Attachment #198159 - Flags: superreview?(wtchang)
Attached patch Additional patchSplinter Review
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.

Attachment

General

Creator:
Created:
Updated:
Size: