Closed Bug 301554 Opened 16 years ago Closed 16 years ago

SSL/TLS Client Authentication with 3rd party PKCS#11 module failes in case there is an unrecognized token in addition to the token that ought to be used.

Categories

(NSS :: Libraries, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.10.2

People

(Reporter: markus.heintel, Assigned: rrelyea)

Details

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9) Gecko/20050711 

System setup:
- 3rd party PKCS#11 module successfully installed in Firefox
- 2 smart card readers attached to the system
- reader 1 holding a smart card which is not supported by the 3rd party PKCS#11 
module
- reader 2 holding a smart card that works with the 3rd party PKCS#11 module


The following happens:
-- After calling C_GetSlotList() Firefox calls C_GetSlotInfo(). Both slots 
report CKF_TOKEN_PRESENT (which is OK from our point of view, since 
CKF_TOKEN_PRESENT does not indicate whether the token is supported or not)
-- Firefox calls C_GetTokenInfo() for both readers:
---> For reader 1 the call returns with CKR_TOKEN_NOT_RECOGNIZED
---> For reader 2 the call returns successfully
-- Nevertheless Firefox continues playing around with reader 1 (e.g. calls to 
C_FindObjectsInit(hSession = 0) which naturally return with CKR_OBJECT_HANDLE 
invalid)
-- Firefox also continues going on with the client authentication using reader 
2, but at some point it stops.

It is especially interesting that in case we change the 3rd party PKCS#11 
module in a way that there is a short delay (about 2 secs.) before returning 
CKR_TOKEN_NOT_RECOGNIZED the client authentication succeeds.

In case it would help I can send the complete PKCS#11 transcript of both cases 
working and non working (i.e. with and without the delay).

Reproducible: Always

Steps to Reproduce:
please see above.
->NSS i think...
Assignee: nobody → wtchang
Component: Security → Libraries
Product: Firefox → NSS
QA Contact: firefox → jason.m.reid
Markus: if you have the PKCS #11 transcripts handy, please
attach them to this bug report.
NSS has a function called within_token_delay_period:

http://lxr.mozilla.org/security/ident?i=within_token_delay_period

which the nssSlot_IsTokenPresent function uses to prevent
polling for token presence too often.

http://lxr.mozilla.org/security/ident?i=nssSlot_IsTokenPresent

The current token presence polling delay time is 1 second.
This is why your 2 second delay makes a difference.  So the
investigation of this bug should begin with these two functions.
PKCS#11 transcript for two tokens. One is not recognized which seems to prevent
Firefox from a successfull SSL/TLS client auth with the other token.
Transcript for a successfull SSL/TLS client authentication with two tokens. One
of the tokens is not recognized. The only difference in the PKCS#11 library
compared two the other log (pkcs11_firefox2_m2.log) is that there is now a
sleep(2 sec.) before CKR_TOKEN_NOT_RECOGNIZED is returned. However, firefox
seems two behave quite different now.
Attachment #191945 - Attachment mime type: text/html → text/plain
I think there are 2 issues here.

The first is whether or not the 3rd party token should show 'present' for an
unrecognized token (I believe the answer is no). The weirdness related to this I
attribute to the token itself (the 2 second delay etc). The problem here is
without the ability to create a session object, NSS has no way of knowing when a
token change has happenned.

The second is that NSS is failing the second client auth. Other then a token out
and out crashing, a misbehaving token should not interfere with the normal
operation of other tokens.
I'll take a look at the symptoms anyway. There is probably some very bad
interaction with the stan code that is causing this. Note the large number of
calls with invalid sessions.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee: wtchang → rrelyea
Status: ASSIGNED → NEW
I think it is OK for PKCS#11 library to report a token as present even if it is 
not recognized. The PKCS#11 standard is not very explicit about this, it just 
states that CKF_TOKEN_PRESENT is set in case a token is present.

At least an application could tell from that information that a specific slot 
is occupied by an unsupported token. The return code CKR_TOKEN_NOT_RECOGNIZED 
for C_GetTokenInfo() would not make sense to me in case unrecognized tokens 
would never be shown by C_GetSlotInfo() and C_GetSlotList(). Should 
C_GetTokenInfo() return CKR_TOKEN_NOT_PRESENT then (even if the slot is 
occupied by an unrecognized token)? Unfortunately I do not remember whether 
this has been discussed on the Cryptoki mailing list before.

Therefore, I think an application should be prepared to handle 
CKR_TOKEN_NOT_RECOGNIZED.
OK, I don't have any tokens the the described behavior to test. This patch was
found by code inspection. It basically detects unfriendly modules who say the
slot is 'present' but can't do anything about the token in the slot anyway.
This patch will cause IsPresent to return false in this case. It needs to be
tested.

If everything works, I would still like to reproduce the problem where a bad
token is causing a good token to fail in client auth. It's probably some loop
looking up the certs which gets a nasty failure from one slot and aborts the
loop, but I'd like to track down where it's at.

In the meantime, Markus, would you like to try this patch out and see if it
solves your problem? If you can't build firefox yourself, I can supply you with
new NSS libraries.

bob
Of course I'd like to try your fix. I would prefer you sending me the new NSS 
libraries. 
Of course I'd like to try your fix. I would prefer you sending me the new NSS 
libraries. 
Fix works in our environment. The problem did not occur any longer on a system 
where we could reliably reproduce the problem without the fix applied.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment on attachment 193881 [details] [diff] [review]
Clear the 'Present flag if slot fails to refresh'

please review. needed for mozilla, so definately for 3.11, probably for 3.10.2
as well.

thanks.
Attachment #193881 - Flags: superreview?(nelson)
Attachment #193881 - Flags: review?(wtchang)
adding nelson to cc list since I asked for a review... setting target to 3.10.2


Target Milestone: --- → 3.10.2
BTW I believe wtc reopened this because the patch is not yet: 1) in the NSS
tree, nor 2) in the mozilla tree. Thanks for the test Markus.

bob
Comment on attachment 193881 [details] [diff] [review]
Clear the 'Present flag if slot fails to refresh'

r=wtc.	I have some suggested changes to the new comment.

>+	/* the token has been removed, and reinserted, (or the slot contains
>+         * a token it doesn't recognize. invalidate all the old
>+	 * information we had on this token, if we can't refresh, set
>+	 * the present state to off */

Please do something about the opening parenthesis that doesn't
have a matching closing parenthesis.  You can just remove the '('.

I think "clear the 'present' flag" is easier to understand than
"set the present state to off".

Do you know what the pre-existing "XXX" comment means?

Earlier in the function, we have:

#ifdef PURE_STAN_CODE
    } else if (!slot->token) {
	/* token was not present at boot time, is now */
	slot->token = nssToken_Create(slot->slotID, slot);
	return (slot->token != NULL);
#endif

Should we also clear the 'present' flag if we return PR_FALSE
there (i.e., if the nssToken_Create call failed)?  (I know
PURE_STAN_CODE is dead code.)
Attachment #193881 - Flags: review?(wtchang) → review+
Comment on attachment 193881 [details] [diff] [review]
Clear the 'Present flag if slot fails to refresh'

r=nelson
Without a lot more study of this stan code, I am unable to judge whether this
tiny change will fix the problems described in this bug, or not, but it seems
like a correct change. I do not believe it will be detrimental.
Attachment #193881 - Flags: superreview?(nelson) → review+
Comment on attachment 193881 [details] [diff] [review]
Clear the 'Present flag if slot fails to refresh'

Bob, I am not sure if this bug fix is critical enough for
NSS 3.10.2, but I am confident that it has a very low risk.
For inclusion in NSS 3.10.2, please check this patch in today
because we will build NSS 3.10.2 Beta 1 tomorrow morning.
I checked in the fix on the NSS_3_10_BRANCH (NSS 3.10.2) and
NSS trunk (NSS 3.11) for Bob.  I took the liberty of making
the two comment changes I suggested.  Bob, feel free to edit
the comment as you see fit.
Attachment #193881 - Attachment is obsolete: true
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Thanks Wan-Teh... I wondered why I have having such a hard time finding this
bug. so I could get the fix checked in;).

bob
Comment 21 was evidently attached to the wrong bug.  I marked it private to 
reduce confusion.
You need to log in before you can comment on or make changes to this bug.