Note: There are a few cases of duplicates in user autocompletion which are being worked on.

certutil fails with list everything in the hardware token

RESOLVED FIXED in 3.4.3

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: thomask, Assigned: Ian McGreer)

Tracking

3.4.2
3.4.3
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
In pki3hack.c

cache_token_cert(NSSCertificate *c, void *arg)
{
    NSSToken *token = (NSSToken *)arg;
    NSSTrustDomain *td = STAN_GetDefaultTrustDomain();
    NSSCertificate *cp = nssCertificate_AddRef(c);
    if (nssList_Count(token->certList) > NSSTOKEN_MAX_LOCAL_CERTS) {
        nssToken_DestroyCertList(token, PR_TRUE);
       /* terminate the traversal */
       return PR_FAILURE;
    }

When NSS is used with certutil to list certificates in hardware
token, certutil fails to list everything when there is more than
NSSTOKEN_MAX_LOCAL_CERTS.

it is because the local cert got destroyed.

Comment 1

15 years ago
Assigned the bug to Ian.
Assignee: wtc → ian.mcgreer
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

15 years ago
Created attachment 99053 [details] [diff] [review]
patch for 3.4 branch


This cert cache code was replaced in NSS 3.5+, so this patch is only for the
3.4 branch.

Comment 3

15 years ago
Comment on attachment 99053 [details] [diff] [review]
patch for 3.4 branch

r=wtc.	Bob, could you review this patch too?
Attachment #99053 - Flags: review+
(Reporter)

Comment 4

15 years ago
Applied the patch to my tree, and it core dumped:

(/tools/ns/workshop-6.2/bin/../WS6U2/bin/sparcv9/dbx) where
current thread: t@1
=>[1] nssList_Count(list = (nil)), line 310 in "list.c"
  [2] cache_token_cert(c = 0x3436c0, arg = 0x2ceaa8), line 115 in "pki3hack.c"
  [3] retrieve_cert(t = 0x2ceaa8, session = 0x2ceaf0, h = 2181039210U, arg =
0xffbef7f8), line 573 in "devobject.c"
  [4] traverse_objects_by_template(tok = 0x2ceaa8, sessionOpt = (nil),
obj_template = 0xffbef76c, otsize = 2U, callback = 0x179118 =
&`certutil`devobject.c`retrieve_cert(struct NSSTokenStr *t, struct nssSessionStr
*session, CK_OBJECT_HANDLE h, void *arg), arg = 0xffbef7f8), line 267 in
"devobject.c"
  [5] nssToken_TraverseCertificates(token = 0x2ceaa8, sessionOpt = (nil), search
= 0xffbef7f8), line 610 in "devobject.c"
  [6] nssToken_LoadCerts(token = 0x2ceaa8), line 269 in "pki3hack.c"
  [7] STAN_LoadDefaultNSS3TrustDomain(), line 378 in "pki3hack.c"
  [8] nss_Init(configdir = 0x2a7160 ".", certPrefix = 0x28d4a8 "", keyPrefix =
0x28d4a8 "", secmodName = 0x262210 "secmod.db", readOnly = 1, noCertDB = 0,
noModDB = 0, forceOpen = 0, noRootInit = 0), line 454 in "nssinit.c"
  [9] NSS_Initialize(configdir = 0x2a7160 ".", certPrefix = 0x28d4a8 "",
keyPrefix = 0x28d4a8 "", secmodName = 0x262210 "secmod.db", flags = 1U), line
512 in "nssinit.c"
  [10] main(argc = 6, argv = 0xffbefb44), line 2650 in "certutil.c"
(/tools/ns/workshop-6.2/bin/../WS6U2/bin/sparcv9/dbx)


Then, I put 

static PRStatus
cache_token_cert(NSSCertificate *c, void *arg)
{
    NSSToken *token = (NSSToken *)arg;
    NSSTrustDomain *td = STAN_GetDefaultTrustDomain();
    NSSCertificate *cp = nssCertificate_AddRef(c);
 +   if (token->certList == 0) 
 +     return PR_FAILURE;
    if (nssList_Count(token->certList) > NSSTOKEN_MAX_LOCAL_CERTS) {
        nssToken_DestroyCertList(token, PR_TRUE);

Then, it core dumped in


(/tools/ns/workshop-6.2/bin/../WS6U2/bin/sparcv9/dbx) where
current thread: t@1
=>[1] nssList_Clone(list = (nil)), line 337 in "list.c"
  [2] nssList_CreateIterator(list = (nil)), line 364 in "list.c"
  [3] nssToken_UpdateTrustForCerts(token = 0x2cead8), line 301 in "pki3hack.c"
  [4] pk11_CheckPassword(slot = 0x321828, pw = 0x2fdc08 "userpin"), line 681 in
"pk11slot.c"
  [5] PK11_DoPassword(slot = 0x321828, loadCerts = 1, wincx = 0xffbefa8c), line
1158 in "pk11slot.c"
  [6] PK11_Authenticate(slot = 0x321828, loadCerts = 1, wincx = 0xffbefa8c),
line 853 in "pk11slot.c"
  [7] listCerts(handle = 0x2ce820, name = (nil), slot = 0x321828, raw = 0, ascii
= 0, outfile = 0x2abc70, pwarg = 0xffbefa8c), line 646 in "certutil.c"
  [8] ListCerts(handle = 0x2ce820, name = (nil), slot = 0x321828, raw = 0, ascii
= 0, outfile = 0x2abc70, pwdata = 0xffbefa8c), line 703 in "certutil.c"
  [9] main(argc = 6, argv = 0xffbefb44), line 2680 in "certutil.c"
(/tools/ns/workshop-6.2/bin/../WS6U2/bin/sparcv9/dbx)
(Assignee)

Comment 5

15 years ago
Thomas-

Did you do a clean rebuild?  Dependencies don't work at all in NSS 3.4, and
certutil is linked statically.
(Assignee)

Comment 6

15 years ago
Nevermind, I see the problem now.  The hardware token is not friendly, correct?
 (that is, you must login to see the certs)
(Assignee)

Comment 7

15 years ago
The second crash you reported is due to the fact that some code is assuming that
all hardware token certs are cached.

I could fix that piece of code, but I'm worried about how many other parts of
the code make similar assumptions.  I believe it has not been a problem to date
because we haven't tested with a device that has more than 10 certs (the current
maximum).

So, another less risky way to fix this bug is to remove the maximum, so that
hardware certs are always cached.  Provided we aren't faced with a hardware
token that has millions of certs, that shouldn't be a problem.
(Assignee)

Comment 8

15 years ago
Created attachment 99086 [details] [diff] [review]
alternative patch - remove the maximum


This patch completely removes the limit of cached token certs.
(Reporter)

Comment 9

15 years ago
Yes, I agree that removes the limit is a less risky fix. I did try to
change the limit to 100, and it fixed the problem. Thanks for your
help Ian!
(Assignee)

Updated

15 years ago
Attachment #99053 - Attachment is obsolete: true
(Assignee)

Comment 10

15 years ago
Comment on attachment 99086 [details] [diff] [review]
alternative patch - remove the maximum


patch checked in, reviewed by wtc over IRC
Attachment #99086 - Flags: review+
(Assignee)

Comment 11

15 years ago
marking fixed per comment #9 (leaving milestone open, should be 3.4.3)
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Priority: -- → P1
Resolution: --- → FIXED

Comment 12

15 years ago
The definition of the NSSTOKEN_MAX_LOCAL_CERTS macro
should also be removed.

Thanks, Ian!
Whiteboard: [3.4.3]

Comment 13

15 years ago
Created attachment 99139 [details] [diff] [review]
Remove the unused macro definition

I've checked in this patch on the NSS_3_4_BRANCH.

Updated

15 years ago
Whiteboard: [3.4.3]
Target Milestone: --- → 3.4.3

Comment 14

15 years ago
We should test 3.6 with a token with more than 10 certs. I would rather fix the
problems we find there than remove the limit.

bob
You need to log in before you can comment on or make changes to this bug.