216552 - implement reference-counted PKCS#11 sessions

Status: RESOLVED DUPLICATE of bug 283690

(NSS :: Libraries, enhancement, P1)

Description

[Ian McGreer]

This bug is in response to bug 202179.  In order to address the performance 
problems inherent in the way NSS opens and closes PKCS#11 sessions for SSL, we 
need a reference-counted session type.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

The following comment comes from bug 202179, and explains why ref counted 
sessions are needed:

I think it shows a design flaw with the present concept of a session "owner".
It shows that there may be numerous objects that reference a session, and
that the object that is the "owner" of the session must be destroyed last.
If the references are destroyed in another order, the code crashes. 

The pk11 wrapper code hides the existance and ownership of sessions from the 
caller (e.g. from libSSL).  It seems that the wrapper code that maintains this
"ownership" has been hand crafted to be correct as currently used by libSSL.
But it seems that an innocent change to libSSL that caused it to destroy 
objects in a different order would cause crashes in lower layer code.  

Similarly, one imagines that perhaps new applications that use pk11wrap 
might also experience crashes due to destroying objects in a different order
than the order that pk11wrap seems to require.  

I think the pk11wrapper layer APIs need to allow the objects to be destroyed
in any order.  I would think the reference counting, rather than the present
ownership scheme, would be necessary to accomplish that. 

Comment 2

[Wan-Teh Chang]

Moved to NSS 3.10.

Ian, what's the status of this bug?

Comment 3

[Ian McGreer]

I have been sitting on this for a while, mainly because a simple solution I
thought of turned out not to work.  I decided that something more comprehensive
was needed, along the lines of what I had implemented for Stan, see the bottom
of this file:
http://webtools.mozilla.org/bonsai/cvsblame.cgi?file=mozilla/security/nss/lib/dev/devslot.c&rev=STAN_WORK_BRANCH&root=/cvsroot

Basically, I believe there are three uses of PKCS#11 sessions in NSS:
1.  A session created for an object and used only by that object
2.  A session creates essentially for the lifetime of one operation, e.g.
generating a hash
3.  A session created for a crypto context

Uses (1) and (3) often go together -- a session is used to generate a key, and
then a context is created to do crypto with the key.  In the current
implementation, a new session is created for the context.  The goal of this
enhancement is to eliminate the need for the additional session.

I found that, instead of just reference-counting, we need a concept of an
"inheritable" session, one that can be safely passed from one object to another.
 This is essentially the same as the current "owner" concept, except that the
details of ownership should be handled by the session object, and not strewn all
 over the code.

However, in order to avoid a lot of unneccesary locking, it should be possible
to create a session that would not include the overhead required for
inheritance.  These "quick" sessions are intended for use when the lifetime of
the session is entirely within the scope of a single function, and thus thread
safety is not an issue.

When an object obtains a new session handle from C_OpenSession, it is associated
with a session object that is a "parent".  If another object then inherits the
session, it becomes a child of the parent.  All children will use the same
handle, but have different session objects.  The lifetime of session handle is
the same as the parent, thus the child objects can be safely destroyed without
the caller having to know who owns what.

I'm going to attach a rough implementation of this idea based on the Stan code.
 I've just started looking at implementing these sessions for PK11SymKeys and
PK11Contexts, but early comments are invited.

Comment 4

[Ian McGreer]

Attachment: first cut at implementation

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

I think this bug/rfe has already been fixed/implemented as the fix
for bug 283690.  Bob, do you agree?  If so, please mark this bug
as a dup of that one.

Comment 6

[Robert Relyea]

*** This bug has been marked as a duplicate of 283690 ***