292239 - Merge PKCS #11 v2.20 header files

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Wan-Teh Chang]

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.

Comment 1

[Wan-Teh Chang]

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 2

[Robert Relyea]

Attachment: Merge in the latest 2.20 headers into NSS

Comment 3

[Wan-Teh Chang]

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.

Comment 4

[Wan-Teh Chang]

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?

Comment 5

[Robert Relyea]

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

Comment 6

[Julien Pierre]

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 ?

Comment 7

[Robert Relyea]

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.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 9

[Robert Relyea]

Attachment: Incorporate Julien and nelsons's comments

I could find no other Cryptoki references other than those I specified. If you
find any let me know;).

Comment 10

[Julien Pierre]

Attachment: replace CKR_INVALID_PARAMETERS with CKR_ARGUMENTS_BAD in the comment

Also fix typos in comments.

Comment 11

[Wan-Teh Chang]

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.

Comment 12

[Robert Relyea]

Julien, which other typo's are you talking about (to make sure I get them all).

bob

Comment 13

[Julien Pierre]

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 ?

Comment 14

[Robert Relyea]

/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

Comment 15

[Wan-Teh Chang]

Attachment: Finishing touches

I compared our headers with PKCS #11 v2.20 headers and
found one missing change.  Also have softoken report
Cryptoki v2.20.

Comment 16

[Wan-Teh Chang]

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

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 18

[Wan-Teh Chang]

Good point, Nelson.  Should we change our cryptokiVersion
back to 2.11 to be safe?