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)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11
People
(Reporter: bugz, Assigned: glenbeasley)
Details
Attachments
(2 files, 3 obsolete files)
16.34 KB,
patch
|
Details | Diff | Splinter Review | |
3.30 KB,
patch
|
Details | Diff | Splinter Review |
New algorithms in NSS need self-tests.
Reporter | ||
Comment 1•20 years ago
|
||
Assignee: wchang0222 → bugz
Status: NEW → ASSIGNED
Reporter | ||
Updated•20 years ago
|
Attachment #158765 -
Flags: superreview?(wchang0222)
Attachment #158765 -
Flags: review?(rrelyea0264)
Updated•20 years ago
|
Attachment #158765 -
Flags: review?(rrelyea) → review+
Comment 2•20 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•20 years ago
|
Priority: -- → P1
Target Milestone: --- → 3.10
Updated•20 years ago
|
QA Contact: bishakhabanerjee → jason.m.reid
Updated•20 years ago
|
Assignee: bugz → glen.beasley
Status: ASSIGNED → NEW
Target Milestone: 3.10 → 3.11
Assignee | ||
Comment 3•19 years ago
|
||
Attachment #158765 -
Attachment is obsolete: true
Attachment #198159 -
Flags: superreview?(wtchang)
Assignee | ||
Comment 4•19 years ago
|
||
Attachment #198162 -
Flags: superreview?(wtchang)
Assignee | ||
Updated•19 years ago
|
Attachment #198159 -
Attachment is obsolete: true
Assignee | ||
Comment 5•19 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•19 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•19 years ago
|
||
Attachment #198162 -
Attachment is obsolete: true
Assignee | ||
Comment 8•19 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
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Attachment #198159 -
Flags: superreview?(wtchang)
Comment 9•19 years ago
|
||
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.
Description
•