Closed Bug 292239 Opened 15 years ago Closed 15 years ago
Merge PKCS #11 v2
.20 header files
We need to merge the latest PKCS #11 v2.20 header files. They can be downloaded from http://www.rsasecurity.com/rsalabs/node.asp?id=2133.
QA Contact: bishakhabanerjee → jason.m.reid
Another thing we need to do is to change pInfo->cryptokiVersion.minor from 11 to 20 in lib/softoken/pkcs11.c. I am surprised that none of the PKCS #11 headers define macros for its major and minor versions.
Comment on attachment 190989 [details] [diff] [review] Merge in the latest 2.20 headers into NSS r=wtc. Just a nit: > * The Initial Developer of the Original Code is >- * Netscape Communications Corporation. >+ * RSA Security INC. "Inc." looks nicer than "INC." We also need to change pInfo->cryptokiVersion.minor from 11 to 20 in lib/softoken/pkcs11.c.
Attachment #190989 - Flags: review?(wtchang) → review+
Comment on attachment 190989 [details] [diff] [review] Merge in the latest 2.20 headers into NSS More questions: there are four PKCS #11 headers: cryptoki.h pkcs11.h pkcs11f.h pkcs11t.h We don't have cryptoki.h in lib/softoken. Why? This patch doesn't change pkcs11.h. Is that file not changed in PKCS #11 v2.20?
cryptoki.h is a sample of the platform dependent defines. We use NSPR to construct the platform dependent defines and stick them in our version of pkcs11.h You are correct, there were not changes to pkcs11.h. Most of the changes that happen from release to release is in pkcs11t.h
I reviewed the patch, but even after applying it, our pkcs11t.h still has several differences with the one from RSA . Many are in comments - "Cryptoki" vs "PKCS#11", but some others are not, such as : 1) #define CK_INVALID_SESSION 0 Is a session of zero officially defined in the spec as being invalid ? If so, we should push it into the standard header file. We have the followin ones that are not in the official header files also : #define CKO_KG_PARAMETERS 0x00000006 #define CKK_INVALID_KEY_TYPE 0xffffffff #define CKF_EC_FP 0x00100000 #define CKR_KEY_PARAMS_INVALID 0x0000006B And lastly, we have : CK_CHAR_PTR *LibraryParameters; in CK_C_INITIALIZE_ARGS . A developer at Sun recently asked me about how to initialize libsoftokn3.so, and I told him about the string to pass in the LibraryParameters field in CK_C_INITIALIZE_ARGS, only to learn this field didn't exist in the official spec ;) . The official headers only have pReserved. We are effectively using LibraryParameters in place of pReserved, yet we still have another pReserved afterwards. I think we should do something to clarify the situation in our header file, at least a comment. And perhaps we should remove the pReserved field after LibraryParameters, and use a union for LibraryParameters and pReserved ?
RE Cryptoki versus PKCS #11 - I should have changed all occurances except for the following: 1) Places in the copywrite. 2) The use of cryptoki in the API itself (one occurance: cryptokiVersion). The changes were made long ago because of worries about cryptoki potentially being someone's trademark. I would prefer to eventually loose these otherwise gratuitous change from the released version of the header. Re:CK_INVALID_SESSION Sigh, yes, "0" is an invalid session according to the PKCS #11 spec. The "official" define was CK_INVALID_HANDLE. Ideally CK_INVALID_SESSION should have been defined in one of our private header files (as CK_INVALID_HANDLE). I maintained them since older programs (in fact softoken itself) requires the old names. CKO_KG_PARAM is a depricated version of CKO_DOMAIN_PARAM CKF_EC_FP is a depricated version of CKF_EC_F_P #define CKK_INVALID_KEY_TYPE 0xffffffff is not official and #define CKR_KEY_PARAMS_INVALID 0x0000006B is not official. The latter should have beend defined in the vendor space and added to pkcs11n.h I agree we should add some comments on the initialization arguments, I'll create another patch for that.
IMO, all the symbols that we define, that are not part of the standard headers, should be grouped together at the bottom of the file, should be preceeded by a substantial block comment explaining that they are extensions to the standard headers, and perhaps should be ifdeffed. Any symbols that are non-standard synonyms of other standard symbols should be defined in terms of the standard symbols, not in terms of the value of the standard symbols. E.g. #define CKO_KG_PARAMETERS CKO_DOMAIN_PARAM #define CKF_EC_FP CKF_EC_F_P is much better than #define CKO_KG_PARAMETERS 0x00000006 #define CKF_EC_FP 0x00100000
I could find no other Cryptoki references other than those I specified. If you find any let me know;).
Also fix typos in comments.
Attachment #191253 - Flags: superreview?(julien.pierre.bugs) → superreview-
I suggest that we also check in the diffs between the official PKCS #11 headers and our PKCS #11 headers as a patch file. This will make it easier to upgrade to new versions of those headers in the future.
Julien, which other typo's are you talking about (to make sure I get them all). bob
Bob, I emailed you the typos. They can be found by diffing the last two patches currently attached ("incorporate Julien and Nelson's comments" and "replace CKR_INVALID_PARAMETERS ..."). Wan-Teh, Where do you suggest we check the official vs NSS headers diff to ? The softoken directory ?
Attachment #191284 - Flags: review?(rrelyea) → review+
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11t.h,v <-- pkcs11t.h new revision: 1.15; previous revision: 1.14 done Checking in pkcs11f.h; /cvsroot/mozilla/security/nss/lib/softoken/pkcs11f.h,v <-- pkcs11f.h new revision: 1.6; previous revision: 1.5 done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
I compared our headers with PKCS #11 v2.20 headers and found one missing change. Also have softoken report Cryptoki v2.20.
Attachment #192555 - Flags: review?(rrelyea)
Attachment #192555 - Flags: review?(rrelyea) → review+
Comment on attachment 192555 [details] [diff] [review] Finishing touches Checked in on the NSS trunk (NSS 3.11). Checking in pkcs11f.h; /cvsroot/mozilla/security/nss/lib/softoken/pkcs11f.h,v <-- pkcs11f.h new revision: 1.7; previous revision: 1.6 done Checking in pkcs11.c; /cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v <-- pkcs11.c new revision: 1.110; previous revision: 1.109 done
IINM, there are certain object attributes defined in v2.20 that are not yet in NSS. Can we claim that NSS softoken is v2.20 while they remain unimplemented? The trust attribute is one of them.
Good point, Nelson. Should we change our cryptokiVersion back to 2.11 to be safe?
You need to log in before you can comment on or make changes to this bug.