326637 - Unnecessary request of PKCS11 device password while verifying certificates

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Mark Hobbs]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

If an external PKCS#11 device is installed and FireFox tries to access an non-client authenticated https site it forces the user to enter the password for the PKCS#11 device even though there is no need to do this.

I have tested this using AET's SafeSign PKCS#11 driver (several versions) using both Rainbow iKey3000 and JCOP30 smartcards both containing valid X509 certs that support SSL client authentication. The same problem exists on both Windows XP and Win98SE, both fully patched.

The problem appears to have been introduced with FireFox 1.5. It also exists in FF 1.5.0.1. and the latest release of SeaMonkey. 

FF 1.0.7 and Mozilla 1.7* do not have the same problem.

 

Reproducible: Always

Steps to Reproduce:
1. Install a PKCS#11 driver and plug-in a hardware device containing a client certificate.
2. Browse to a SSL site, say yahoo mail secure login.
3.

Actual Results:  
The Security device password will be requested from the user.

Expected Results:  
The security device password should not be requested at all as NON-client authenticated SSL can be handled by the browser directy without access to any external PKCS#11 device or associated private key/certificate.

On the Windows XP machine I also find that the PKCS#11 device password is constantly requested at approzimately ten minutes intervals. This is not the same probelm as above but could well be related as I am sure this behaviour has crept in with FF1.5.

I can't however repeat this problem on Windows 98.

Comment 1

[Wan-Teh Chang]

Bob, does this password prompt come from the new smart card
monitoring thread in Firefox 1.5?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Seems to me that the "friendly" attribute should apply to fetching CRLs
as well as to fetching certs.

Comment 3

[Robert Relyea]

Not likely. The smart card thread only send DOM events to the current window. If there isn't javascript there to redraw the page (or some other such thing).

This bug was introduced with crl processing in NSS 3.10. I believe it has not been fixed even in 3.11 (I believe there is a bug about it).

Comment 4

[Robert Relyea]

OK, I can't find the bug, so it probably fell through the cracks. (sigh).
The bug is in NSS, not PSM, so we probably should assign it to someone other than kai..

Comment 5

[Kai Engert [:KaiE:]]

over to nss component

Comment 6

[Julien Pierre]

Bob,

NSS needs to look for CRLs in PKCS#11 tokens during cert chain validation. The tokens should allow being searched without a login. What do you suggest we do ? Skip the token if not logged in ?

Comment 7

[Mark Hobbs]

As an outsider (at least from a FF code perspective) I am a little confused as to why FF is trying to "look for" CRLs on/via the PKCS#11 device. I am guessing that FF is simply trying to read individual certs on the device to gather the CRL UDP reference from any certs it may contain.

Having said that, I still don't understand what purpose it serves always refering to the PKCS#11 device when accessing an SSL site. It is possible that the required root CA (plus chain) certs may be on the card but in the vast majority of cases they will be located within the appropriate FF authorities store which should be queried first.

Finally, my tests show that FF is already quite capable of reading certs from a PKCS#11 device (despite the fact that it will try and prompt for a logon) when the logon password is withheld. This would suggest that there is no need to force a logon to implement the CRL functionality being discussed.

Comment 8

[Robert Relyea]

There are PKCS #11 devices that are used to store CRL. That being said, the bug was introduced in the way the CRL cache worked in 3.10. The fact that it's causing tokens to authenticate just to do an SSL connection is a bug.

As far as reading certificates, NSS needs to be able to 1) read the certificate, and 2) determine that the certificate is a user certificate. It does the latter by looking up the private key if it's authenticated or the public key if it's not. Since this is not required behavior in PKCS #11, you need to tell NSS that the token obeys this semantic, and that is what the PUBLICALLY_READABLE_CERTS (AKA "FRIENDLY") flag is about.

bob

Comment 9

[Julien Pierre]

Mark,

All the cert stores in NSS are PKCS#11 devices, including the softoken that manages the DB, and the root cert module that has the trusted roots. The cert chain building  code doesn't give preference to any particular PKCS#11 device, it looks in all of them, and then tries to find the best cert. The cert chain building code is invoked even without SSL client auth in order to verify the server's cert, ie. to do SSL server auth. The fact that it is searching all PKCS#11 devices is not a bug, IMO.

Bob,

Certainly there were changes made in NSS 3.10 to the CRL cache code. But IMO it was a bug in pre-3.10 if it didn't search all the PKCS#11 tokens for CRLs. If the problem is that we are forcing a login with unfriendly tokens, I don't think this is an NSS bug. If we are forcing a login with friendly tokens, then we need to fix it.

Comment 10

[Mark Hobbs]

(In reply to comment #9)
> Mark,
> 
> All the cert stores in NSS are PKCS#11 devices, including the softoken that
> manages the DB, and the root cert module that has the trusted roots. The cert
> chain building  code doesn't give preference to any particular PKCS#11 device,
> it looks in all of them, and then tries to find the best cert. The cert chain
> building code is invoked even without SSL client auth in order to verify the
> server's cert, ie. to do SSL server auth. The fact that it is searching all
> PKCS#11 devices is not a bug, IMO.
> 
> Bob,
> 
> Certainly there were changes made in NSS 3.10 to the CRL cache code. But IMO it
> was a bug in pre-3.10 if it didn't search all the PKCS#11 tokens for CRLs. If
> the problem is that we are forcing a login with unfriendly tokens, I don't
> think this is an NSS bug. If we are forcing a login with friendly tokens, then
> we need to fix it.
> 

Thanks Bob and Julien.
It all makes a little more sense now. I'll try to confirm that the PKCS#11 middleware I am using supports the "friendly" flag. However, I suspect it does as FF can actually read the certs from the smartcard even if I cancel the "logon" prompt.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Last night I replaced the NSS shared libs that my mozilla mail client is using
with the lastest trunk builds.  That was the ONLY change I made.
Now, using IMAPS is excruciatingly painful.  O M G!

Every time I go to read a different email message from the IMAPS server, I get
4 prompts for the master password, and one more prompt for the IMAPS server's
password.  If I have the patience to click CANCEL through all 5 prompts, then
eventually I do get the email message I want.  But after just one day of this,
I'm ready to throw this email client out the window.

I think we have to treat this as a regression in 3.11 and fix as a P1 bug!
Any takers?

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Julien, please take a look at this.

Comment 13

[Julien Pierre]

Nelson,

Did you have a smartcard in your environment as the reporter for this bug did ?
I'm not sure this is the same bug. Also, you don't specify which mozilla client you used, which makes it hard to reproduce your issue.

Comment 14

[Robert Relyea]

Ok, I've tracked this one other time before.... let's put it in the bug for posterity.

The difference between 3.9 and 3.10 is not that 3.9 was not searching all the present tokens, but that 3.10 is authenticating before doing the search.

In 3.9 the crl's are searched using PK11_FindCrlByName(). It traversed the stan cache of objects using nssTrustDomain_FindCRLsBySubject() and does not authenticate to any tokens.

In 3.10, the code was changed to call a new pk11_RetrieveCrls() with Julien added new in 3.10. This function is necessary because it returns several crls matching the criteria rather than just one. Instead of using nssTrustDomain_FindCRLsBySubject, however, it uses pk11_TraverseAllSlots, which *does* authenticate.


Soooo...


I can see 2 possible ways to fix this:

1) The expedient way is change pk11_TraverseAllSlots to take a flag indicating whether or not to authenticate. There are currently 4 callers of this private function (there used to be more, but they have steadily been replaced by the stan calls. 1 is here, the other is in PK11_LookupCrls, and the other two are in cert code. The latter 2 use pk11_TraverseAllSlots as a quick way to authenticate all unfriendly tokens.

2) I believe the more correct fix is to recode pk11_RetrieveCrls to use the same 
nssTrustDomain_FindCRLsBySubject as PK11_FindCrlByName(). At some point in the future we can examine if we can purge pk11_TraverseAllSlots from PK11_LookupCrls, in which case we should rename the function to something like "pk11_AuthenticateUnfriendlyTokens()" to avoid this kind of issue in the future (and to start removing the number of traversing functions by one inside the code).

bob

Comment 15

[Julien Pierre]

Bob,

I don't think 1) is going to work. Even if the traverse function only authenticates optionally, there is the problem of where it's going to get the value passed from all the way from the PSM stack. I would rather not go that route.

I think we should at least be able to limit the authentication to 1 time for all devices for now. IMO it's always a bug to authenticate over and over and again to the same tokens if the user didn't explicitly log out.

Comment 16

[Robert Relyea]

No, in option 1) only the 2 callers would choose. The 2 cert functions would pass PR_TRUE for authentication (since their whole purpose is calling the function is to authenticate all the 'unfriendly' tokens), pk11_RetrieveCrls() would call it with PR_FALSE (since that is the reason for this bug/regression), and we can have a long debate on whether PK11_LookupCrls passes PR_TRUE or PR_FALSE, but the conservative thing to do is to pass PR_TRUE only because that was the semantics it had before 3.9.

We can make option 1 moot by just implementing option 2 (which is my preference).

bob

Comment 17

[Kai Engert [:KaiE:]]

Any objections to Bob's proposal 2) in comment 14?
A fix to this bug would be important for us.

Comment 18

[Nelson Bolyard (seldom reads bugmail)]

Is this a P1 for RedHat?

Comment 19

[Julien Pierre]

Bob,

The CRL code needs to find all possible CRLs. If I understand correctly from your comments, pk11_RetrieveCrls / pk11_TraverseAllSlots currently log in to all tokens, whether  friendly or unfriendly.

We should only eliminate the login to the friendly tokens, which is unnecessary. If any unfriendly tokens exist however, we still need to login in order for the CRL search to be complete.

I looked at pk11_TraverseAllSlots . It calls pk11_AuthenticateUnfriendly . That doesn't sound like it should authenticate any friendly tokens. The code for that function is very simple and simply calls PK11_Authenticate if !PK11_IsFriendly .

So, if we are seeing a forced login on friendly tokens, I think that can only be happening because PK11_IsFriendly is returning the wrong value - ie. it thinks the token is unfriendly, even though it is. I don't know why this problem would be specific to the CRL search. It seems that it should affect certificate search equally.

Comment 20

[Julien Pierre]

I reproduced the behavior on Solaris with NSS 3.11 by adding the Solaris crypto framework (/usr/lib/libpkcs11.so) to PSM . I visited an SSL web site, and got prompted for the "Sun MetaSlot" token password. The relevant part of the stack from pstack was as follows :

 fa413ced PK11_DoPassword (9b8e010, 0, a08a100) + 79
 fa41382a pk11_AuthenticateUnfriendly (9b8e010, 0, a08a100) + 62
 fa425bd9 pk11_TraverseAllSlots (fa425af0, fcbfb7c0, a08a100) + 55
 fa42226b pk11_RetrieveCrls (fcbfb810, b8251f0, a08a100) + a7

The fact that pk11_AuthenticateUnfriendly tries to authenticate implies that PK11_IsFriendly() returned PR_FALSE for this token . I don't know why that's the case. I never get prompted for the password for the NSS softoken. Also, if I remove libpkcs11.so, I don't get prompted.

I never get a stack where we are trying to authenticate to the unfriendly while looking for certificates. Maybe this is because the proper certificate is found in the NSS softoken or the built-ins first, and so the code doesn't search the other token ? This doesn't help for CRLs, since in most cases no CRLs are available, and currently the CRL cache tries to find the most appropriate one, and thus needs to look at all of them.

Comment 21

[Julien Pierre]

I removed the built-ins module and restarted mozilla. I still didn't get an authentication request for the Solaris module when looking for certs - only for CRLs. The root cert could have been in the unfriendly Solaris module, but NSS would never have looked for it there ! It seems like we have a separate bug there.

I then deleted the Solaris module and re-added it with modutil, adding the FRIENDLY mechanism. Then, I no longer got prompted by NSS for the token password during the CRL search.

The search and login logic in the CRL code / pk11wrap code is correct.
The only problem is that NSS isn't automatically able to figure out that the third-party token is friendly or not. A manual setting of the mechanism with modutil was required.

Comment 22

[Robert Relyea]

Julien,

We should not try to log into any tokens to search for CRLs. The previous code did not. I don't think it's an issue to not log into 'unfriendly' tokens. CRL storage in tokens is an NSS specific extension, so requiring tokens that use the extension to be inherently friendly (whether or not the bit is set) is not an unreasonable restriction. Requiring every unfriendly token to authenticate on an SSL connection is an unreasonable restriction, and is causing *this* regression.

Other API's to get CRL values through NSS do not require this authentication, we should be consistent.

bob

Comment 23

[Julien Pierre]

Re: comment 22,

I agree with you. The 2 known modules that store CRLs to this day are both friendly. So we don't need to authenticate to find the CRLs.
My preference is your option 1 fix, because it is a lot simpler. Option 2 means rewriting code to the Stan API instead of the pk11_traverse API. I don't think one is much better than the other since we still have PK11_LookupCrls, which uses traverse. As you point out, it currently authenticates. I think changing that behavior not to authenticate is OK - we need to be consistent if we decide a CRL search doesn't need a login. I don't think binary compatibility will be a problem.

I am still disturbed by the fact that no token login occurs during the cert chain building if an unfriendly token is present. Certs in PKCS#11 are not an NSS extension, and we can't assume the modules that token certs to be all friendly. I will investigate and file a separate bug about that.

Comment 24

[Julien Pierre]

There is one caveat about the friendliness. Is our softoken supposed to be friendly even in FIPS140-2 mode ? I verified that it currently was. But I don't know if that is correct behavior. Is it ?

If not, then we would still need to authenticate to it to find CRLs while softoken is in FIPS mode.

Comment 25

[Robert Relyea]

The softoken is always friendly not matter what mode it is in.

The most common reason for a token not be be friendly is not that we can read certs, but that we can't we can't determine that a given certificate is a user certificate (has an associated private key). AFAIK the only completely unfriendly token was the FORTEZZA token.

bob

Comment 26

[Robert Relyea]

Attachment: Implement option 1.

Comment 27

[Kai Engert [:KaiE:]]

Comment on attachment 223315 [details] [diff] [review]
Implement option 1.

r=kengert

Comment 28

[Julien Pierre]

Comment on attachment 223315 [details] [diff] [review]
Implement option 1.

Actually there is another instance of a call to pk11_TraverseAllSlots in lib/certhigh, so this patch is incomplete - it will break the build as-is.

Comment 29

[Julien Pierre]

Attachment: complete patch

Comment 30

[Robert Relyea]

Comment on attachment 224643 [details] [diff] [review]
complete patch

r+=relyea

Thanks Julien

Comment 31

[Julien Pierre]

Checked in to the tip :

Checking in lib/certhigh/certhigh.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certhigh.c,v  <--  certhigh.c
new revision: 1.36; previous revision: 1.35
done
Checking in lib/pk11wrap/pk11cert.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v  <--  pk11cert.c
new revision: 1.151; previous revision: 1.150
done
Checking in lib/pk11wrap/pk11nobj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11nobj.c,v  <--  pk11nobj.c
new revision: 1.7; previous revision: 1.6
done
Checking in lib/pk11wrap/pk11obj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11obj.c,v  <--  pk11obj.c
new revision: 1.13; previous revision: 1.12
done
Checking in lib/pk11wrap/pk11priv.h;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11priv.h,v  <--  pk11priv.h
new revision: 1.6; previous revision: 1.5
done

And NSS_3_11_BRANCH :

Checking in lib/certhigh/certhigh.c;
/cvsroot/mozilla/security/nss/lib/certhigh/certhigh.c,v  <--  certhigh.c
new revision: 1.34.2.2; previous revision: 1.34.2.1
done
Checking in lib/pk11wrap/pk11cert.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cert.c,v  <--  pk11cert.c
new revision: 1.143.2.7; previous revision: 1.143.2.6
done
Checking in lib/pk11wrap/pk11nobj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11nobj.c,v  <--  pk11nobj.c
new revision: 1.6.2.1; previous revision: 1.6
done
Checking in lib/pk11wrap/pk11obj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11obj.c,v  <--  pk11obj.c
new revision: 1.11.2.2; previous revision: 1.11.2.1
done
Checking in lib/pk11wrap/pk11priv.h;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11priv.h,v  <--  pk11priv.h
new revision: 1.5.2.1; previous revision: 1.5
done

Comment 32

[Julien Pierre]

*** Bug 211523 has been marked as a duplicate of this bug. ***

Comment 33

[Kai Engert [:KaiE:]]

*** Bug 323278 has been marked as a duplicate of this bug. ***