certutil -L -h token doesn't report token authentication failure

RESOLVED FIXED in 3.11.10

Status

NSS
Tools
P2
normal
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Julien Pierre, Assigned: Julien Pierre)

Tracking

trunk
3.11.10

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

4.48 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
I was running

certutil -d . -L -h "Sun Metaslot"

The following (roughly) code sequence is executed by certutil is :
NSS_Initialize
PK11_Authenticate(metaslot, ..)
PK11_ListCertsInSlot(metaslot)

The PK11_Authenticate in my case call failed due to bug 396999 . But certutil ignored the error and tried to list the certs in the slot anyway, and ended up reporting "problem printing certificate nicknames" .

I will attach a fix for this.
(Assignee)

Updated

10 years ago
OS: SunOS → All
Priority: -- → P2
Target Milestone: --- → 3.11.10
(Assignee)

Comment 1

10 years ago
Created attachment 301828 [details] [diff] [review]
Report PK11_Authenticate failures

I found more than one such silent authentication error case, so I fixed them all in this patch.
Attachment #301828 - Flags: superreview?(rrelyea)
Attachment #301828 - Flags: review?(nelson)

Comment 2

10 years ago
Comment on attachment 301828 [details] [diff] [review]
Report PK11_Authenticate failures

r+ 

optional enhancement:

Add the token name to the SECU_PrintError...

SECU_PrintError(progName, "could not authenticate to token or database name %s.", PK11_GetTokenName(slot));
Attachment #301828 - Flags: superreview?(rrelyea) → superreview+
(Assignee)

Comment 3

10 years ago
Created attachment 302050 [details] [diff] [review]
Add Bob's suggestion

As checked in to the trunk.

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.128; previous revision: 1.127
done

Nelson, please review for the branch.
Attachment #301828 - Attachment is obsolete: true
Attachment #302050 - Flags: review?(nelson)
Attachment #301828 - Flags: review?(nelson)
Attachment #302050 - Flags: review?(nelson) → review+
(Assignee)

Comment 4

10 years ago
Thanks, Nelson. As it turns out, this patch doesn't apply directly to the branch because your first patch for bug 291384 isn't on the branch yet. I'll wait until that bug is resolved to check in my patch.
(Assignee)

Updated

10 years ago
Depends on: 291384
(Assignee)

Comment 5

10 years ago
I checked in the patch to NSS_3_11_BRANCH :

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.97.2.13; previous revision: 1.97.2.12
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.