Some possible short term tweaks and work items for PKCS#11 spec

[Stef Walter <stefw@redhat.com>]

Hi all,

So I looked over the spec again to refresh myself on the issues that I
ran into when developing PKCS#11 stuff. Since these are rather tedious,
I figured I'd send this around in advance of our discussion this evening.

In many cases the things that were confusing have been clarified well in
the latest revision of the spec, such as the principle that objects are
fully formed, and others.

Below are the some of the issues that seem to be more doable on the
short term.  There are bigger things not included here, such as
introspection, events, and trust storage. In addition the work on
isolating multiple callers may well need more than what's included here
to be solved correctly.

Cheers,

Stef

EXTRACTION ATTACKS

 * 7: Says: "We note that none of the attacks just described can
   compromise keys marked “sensitive,” since a key that is sensitive
   will always remain sensitive. Similarly, a key that is unextractable
   cannot be modified to be extractable."

   There are a number of vulnerabilities in the spec with regards to
   extracting keys that should not be extractable. These can be
   theoretically fixed by tightening up the spec (although in practice
   a test suite would be needed).

   See: http://www.lsv.ens-cachan.fr/Publis/PAPERS/PDF/BCFS-ccs10.pdf

MULTIPLE CALLERS

 * 11: C_Login: Why is there a limitation that CKU_SO can only log in
   if no other read-only sessions exist? This seems arbitrary, and is
   particularly awkward when there are multiple callers of PKCS#11 in a
   single process. Does Solaris enforce this? They use PKCS#11 as their
   standard crypto interface, and the chance of multiple callers within
   a process is high.

   Code written to enumerate objects might open sessions read-only
   sessions if write access is unecessary. However this would cause a
   concurrent login as CKU_SO to fail.

   If the intent is to prevent using CKU_SO when any other callers are
   present then we should change it so that CKU_SO fails if any other
   sessions are currently open.

 * 6.7.7: We should explicitly note that calling C_Initialize is not
   thread safe. We do mention it should only be called once, but good
   to make this clearer.

   Although obviously this is a particularly tough requirement, and
   one that would be nice to change in in a future version, because of
   multiple callers. We wrap modules to handle this in p11-kit. How
   does Solaris deal with it?

   Fixing this, so C_Initialize is thread safe, and coordinated
   between multiple callers (ie: support multiple C_Initialize calls
   which must be matched with equal C_Finalize calls before actual
   finalization takes place) seems like it would be more of a long
   term thing.

 * 11: C_CloseSession and C_CloseAllSessions: We need to make clear that
   any assumptions about the login state after calling one of these
   functions is inherently racy. Another thread (or uncoordinated
   caller) could open a session which races with the close call.

   We should recommend against using C_CloseAllSessions if there is
   any possibility at all that another caller (perhaps an uncoordinated
   caller in a shared library) may be using the token.

CONSTANTS

 * 10.6.2: Missing constants for CKA_CERTIFICATE_CATEGORY. And perhaps
   also CKA_JAVA_MIDP_SECURITY_DOMAIN.

 * PKCS#11 uses (CK_ULONG)-1 all over the place. We should have a
   constant like CK_INVALID or something like that.

 * 11: C_GetAttributeValue: This is not true: "Note that any attribute
   whose value is an array of attributes is identifiable by virtue of
   the attribute type having the CKF_ARRAY_ATTRIBUTE bit set."
   CK_ALLOWED_MECHANISMS also has this bit set, which is unfortunate
   because the template attributes really are different beasts than any
   other attribute, and it's worth having a bit to identify them.

 * Appendix A: All of the constants in Appendix A should have 'UL'
   after them. This makes a difference when applications use these
   constants in places where they do not get cast, such as passing them
   to varargs functions. Seen crashers in the wild because of this.

BEHAVIOR

 * When you try to create a session object with CKA_PRIVATE from a R/O
   session or SO session: Should fail with CKR_USER_NOT_LOGGED_IN, not
   CKR_TEMPLATE_INCONSISTENT.

 * CK_C_INITIALIZE_ARGS.pReserved is often used to for a module to
   accept an initialization string. Should we define a flag for this
   usage and bless it?

   This is relevant to this part: "For this version of Cryptoki, the
   value of pReserved thereby obtained must be NULL_PTR; if it’s not,
   then C_Initialize should return with the value CKR_ARGUMENTS_BAD".

 * We should define first attribute wins in section 10.1.1 with a note
   towards compatibility.

 * 11: C_Logout: This is hard to implement: "any of the application’s
   handles to private objects become invalid (even if a user is later
   logged back into the token, those handles remain invalid)".

 * 11: GetObjectSize: Should be able to return
   CK_UNAVAILABLE_INFORMATION especially if that was indicated in token
   info.

 * 11: C_FindObjectsInit: We should specify that a empty set of results
   should be returned when C_FindObjectsInit is used with attributes
   that the module doesn't know about.

CLARIFICATIONS and FORMATTING

 * 10.2: Table 15 is awkward to use, and this may have an effect on
   compliance with the various attribute footnotes. Suggest placing
   appropriate footnotes under each attribute table if possible.

 * 6.3: If an attacker can run/change the application or its libraries,
   then all bets are off. Not sure if this is worth mentioning as a
   benefit of static linking.

 * We should note that object handles and slot ids can and often will
   change between instances of an application, or once C_Finalize has
   been called. Various PKCS#11 callers take it for granted that these
   won't change.

 * 8.1: We should mention that on Linux (and other Unixes?) that (while
   possible) no such byte-alignment is done. This was my very first
   PKCS#11 crasher :)

 * The fact that CKA_SERIAL_NUMBER is the encoded DER TLV (and not just
   the contents) has had confusion, may be worth providing an example
   or something like: \x02\x02\x99\x88

 * 10.6.2: "If the attribute is not present then the type defaults to
   SHA-1" should say "CKA_SHA_1" instead.

 * 10.9: We should clarify that the PIN provided to a
   CKU_CONTEXT_SPECIFIC login may be the user pin, or may be a key
   specific PIN. No assumption should be made.

 * 11: C_FindObjectsInit: We should clarify that this clause applies to
   empty/default attributes as well: "The matching criterion is an
   exact byte-for-byte match with all attributes in the template."

 * 12.2: Seems like this should say *not impact*: "Whenever possible,
   changing the value of an object attribute should impact the
   behavior of active operations in other applications or threads."

 * 12.3: Refers to CKR_TEMPLATE_INVALID. Is this new?