144605 - CAC card problem (PSM bug 143181)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Stephane Saux]

This is the NSS bug for the PSM bug 143181
I'd like this to be fixed for 3.5.

Comment 1

[Wan-Teh Chang]

Assigned the bug to Bob.  Target 3.5, priority P1.

Comment 2

[Robert Relyea]

Ian this is the bug we talked about, the problem description is in bug 143181 .

The basic problem seems to be this:
The card gets reinserted, and the certs are found in the cache, but since the
card is not logged in yet, the certs do not show up as user certs. Then when we
do log in, the certs don't get updated.

bob

Comment 3

[Ian McGreer]

Attachment: proposed patch (refresh certs after login)

This has the downside of occuring after every login.  But logins don't happen
that often, and the update is merely a matter of looking at all the certs in
the cache.

It would be better to have something like

if ("certs are friendly" and "keys are not visible until login") then update

since this is a specific case.	But we don't have any way of getting the second
boolean, that I know of.

Comment 4

[Robert Relyea]

Attachment: refresh token cache on login.

This is basically Ian's patch with a call to only do the update if the
'Publically Readable Certs' or the 'friendly' bit is turn on (the only case we
are having the issue). This patch is against the 3.5 BRANCH. I have verified
that it does fix the problem. I'll check it into the tip just now... 

wtc could you review the patch and make sure it makes it into 3.5 when the
correct approval's happen?

bob

Comment 5

[Wan-Teh Chang]

Comment on attachment 84692 [details] [diff] [review]
refresh token cache on login.

r=wtc.

Comment 6

[Stephane Saux]

Great, this is fantastic.

I've started using the "If it has not been used after [] minutes or longer"
feature for password timeout in the browser, setting it to one minute.  I'll be
able to tell whether there's any problem with doing this after every login.

Comment 7

[Wan-Teh Chang]

The fix has been checked into the tip of NSS,
NSS_3_5_BRANCH, and NSS_CLIENT_TAG (used by the
Mozilla trunk build).

Stephane, shadow, please verify with today's
Mozilla trunk build.  Thanks.

Comment 8

[bill]

I just ran into the problem again..after prompting me for the card's PIN (and
presenting it), I got an error dialog telling me that the cert couldn't be
found.  But I was able to immediately click "send" again and it went through.

maybe there's a timeout that's not waiting long enough?

Comment 9

[Stephane Saux]

I'm reopening this bug. I still can't find the cert once I remove and reinsert
the card.
This is using 2002052808 build on the trunk.

Comment 10

[bill]

I'm getting this error even when I leave the card firmly in its slot.  it
prompts me for the card PIN, but it just complains that it can't find the cert.

Comment 11

[Ian McGreer]

From reading the comments, it looks like the bug itself is fixed, but there is
still a timing issue.  Is that correct?  Or does the cert never become available?

Comment 12

[bill]

after playing with the new build all day I notice that after a "while" MachV
can't send signed email because it says it can't find the signing cert.

If I manually go back through the cert picker and choose my desired signing cert
(stored on a card), the signing and sending operation goes through.  This is
100% reproducable but I don't know when it's going to fail.

I retrack my previous statement about timing.

Comment 13

[Stephane Saux]

Ian, my experience with yesterday's build (which I assume must have the fix, but
I'm not 100% certain) is that the behavior is exactly the same with or without
the fix.
If I remove my card, I have to shut down the browser before I'm able to reuse
the card.

Comment 14

[Stephane Saux]

I've also seen the behavior Bill describes in comment #12, although I wasn't
aware of the trick about reselecting the cert.

Comment 15

[bill]

One more comment about this patch:  when a card is removed and re-inserted, it
used to be that client-auth would always fail until you explicitly logged into
the card using the device manager.

With this patch installed I no longer have to "pre login" to the smartcard, Mach
V correctly prompts for the card password when it's required.

But there still seems to be problems when it needs the cert to sign an email.

Comment 16

[Ian McGreer]

I just remembered something that may be relevant to this bug.  Bob implemented a
function that checks to see if a token is present.  This can sometimes be an
expensive operation, so Bob made it such that the last check is good for 10
seconds (harcoded value).  That is, the actual device is only polled every 10
seconds.

That may explain some of the timing problems mentioned with insertion/removal. 
Maybe we should try reducing the period to see if that helps.

Comment 17

[Robert Relyea]

Attachment: flush the certs when we detect a token change.

The last issue was a problem with the cached PKCS #11 handle in the cert
structure. When we remove and insert the same token, we still need to flush the
cert cache because all the instance entries have the wrong handle for the cert
(PKCS #11 devices are allowed to create new token handles for the 'same' object
between insertions and removals of the token. Active Card does this). This
would cause the certs to fail to find the keys (because it can't do a match
object because the handle doesn't point to a valid cert object, so it can't
find the CKA_ID value to look the key up by).

Comment 18

[Wan-Teh Chang]

Ian, could you review Bob's patch?  Thanks.

Comment 19

[Ian McGreer]

Comment on attachment 87586 [details] [diff] [review]
flush the certs when we detect a token change.


looks good to me.

Comment 20

[Wan-Teh Chang]

Attachment: Patch for NSS_3_5_BRANCH

Note the intentionally misspelled "Nofify".  The
function name "nssToken_NotifyCertsNotVisible" is
misspelled on the NSS_3_5_BRANCH.

Comment 21

[Wan-Teh Chang]

Bob's fix has been checked into the tip, NSS_3_5_BRANCH,
and NSS_CLIENT_TAG of NSS.

Stephane, shadow, please verify the fix with next Monday's
(6/17) Mozilla trunk daily build.

Comment 22

[Stephane Saux]

adding adt1.0.0 as per 06/17 4pm ADT meeting.

Comment 23

[Stephane Saux]

Tested the 2002061712 trunk build. Card removal and re-insertion works very well.


Bill, can you test as well?

I'll take may card tonight.

Tested: sending a signed email after removal/re-insertion.
        Client-auth sites.

Comment 24

[bill]

This patch seems to work fine but performance is quite slow compared to the same
operation in Netscape 4.79.

I've confirmed that I can:
- remove card, reinsert card
- start Mozilla
- compose a signed email
- press "send"
- correctly get prompted for smartcard password to sign email
- correctly get prompted to pick cert to sign email
- correctly get prompted to pick cert to auth to SMTP server
- correctly get prompted to pick cert to auth to IMAP server (save to "Sent" folder)

I then did the following test in both Netscape 4.79 and 2002061712-trunk build
of mozilla:
- quit all browsers
- remove smartcard, reinsert smartcard
- start browser
- go to an SSL site (e.g. "https://certificates.netscape.com") to make sure that
SSL engine is pre-loaded and won't skew timing measurements
- go to an SSL client-auth site (e.g. "aka.netscape.com") to time how long it
takes for me to access the webpage

In N4.79 it took the browser a total of 10 seconds to prompt me for the
smartcard password, pick the cert, do the client-auth, and present the web page

In mozilla it took the browser 20 seconds to prompt me for the smartcard
password, another 10 seconds to pick the correct cert, and another 5 seconds to
do the client and load the webpage.  Total time: 35 seconds in mozilla.

These tests are not scientific but were done on the same machine.

This patch puts the smartcard functionality back in, but we need to address the
performance.

Comment 25

[Stephane Saux]

Confirming that after coming in this morning and inserting my card, it worked
perfectly.

I'll work with ADT to patch the branch.

Comment 26

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking in.

Comment 27

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 28

[Wan-Teh Chang]

Fix checked into the MOZILLA_1_0_BRANCH.

Stephane, please verify with tomorrow's (6/19) branch build.