94866 - DSA key gen broken on 64-bit platforms

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Ian McGreer]

DSA key generation failing using certutil on 64-bit platforms.

Comment 1

[Ian McGreer]

See the definitions of PK11Cipher and PK11Verify at:

http://lxr.mozilla.org/mozilla/source/security/nss/lib/softoken/pkcs11i.h#74

They use native types, including unsigned int.

However, the nsc_DSA_Sign_Stub and nsc_DSA_Verify_Stub functions used cryptoki
types, including CK_ULONG.  This caused data loss.

I have already checked a patch into the tip.  The reason for my haste was I
wanted to see if QA would pass.  However, it should be reviewed.  Will attach.

Comment 2

[Ian McGreer]

Attachment: patch (already checked in to tip)

Comment 3

[Wan-Teh Chang]

Good detective work, Ian!

Your patch is good.  I just have some nit-picking comments.

1. The original code uses the Hungarian naming convention.
   To follow the same spirit, the "ul" in the variable names
   should be changed to "u" (or "ui"?) because the type
   changed from CK_ULONG to unsigned int.

2. The type cast in
   PORT_Memcpy((unsigned char *)pSignature, signature.data, signature.len);
   is not necessary because the first parameter of PORT_Memcpy is a void*.

3. It is not necessary to initialize 'key' and 'context' in
   NSC_VerifyInit().  pk11_InitGeneric() should set their
   values if it returns successfully.

Comment 4

[Ian McGreer]

bugger.  tinderbox solaris 8 64-bit QA is still broken.  I can't see the logs 
from where i'm at, so I don't know why.  Perhaps it fails later on.  I was 
testing on AIX 64-bit, and the keygen worked there (didn't try the whole 
script).

1.  agreed.
2.  ditto.
3.  that was leftover from a previous guess at a fix.

I considered this a rough draft, I checked in to see if QA would pass.  
Evidently, not.

Comment 5

[Ian McGreer]

Attachment: patch based on wtc's review (same as old patch, only cleaner code)

Comment 6

[Ian McGreer]

Attachment: same patch as the last, only showing overall diff (from r1.14) to fix the bug

Comment 7

[Wan-Teh Chang]

r=wtc.

Comment 8

[Ian McGreer]

This is all in.  Key gen working on 64-bit now.  Marking fixed.