142867 - pk12util always imports CA certs into softoken

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Julien Pierre]

Even if a hardware token is specified on the pk12util command-line, CA certs are 
always imported into softoken. Since is is currently not possible to import CA 
certs into a hardware token due to a limitation in NSS APIs, we should at least 
display an message stating so rather than try and succeed importing the 
certificate into the wrong token.

Comment 1

[Wan-Teh Chang]

Assigned the bug to Bob.

Comment 2

[Wan-Teh Chang]

Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.

Comment 3

[Robert Relyea]

Attachment: Import Intermediate CA's into token from the pkcs12 file.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I see at least one problem with this patch.  The function SECOID_AddEntry is
declared in secoid.h as returning a type SECOidTab, but the actual definition 
of the function in secoid.c returns a SECStatus.  I'm surprised your compiler
didn't generate an error for this.

Comment 5

[Julien Pierre]

Nelson, following up on your comment #4, please see bug 171084 .

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 116592 [details] [diff] [review]
Import Intermediate CA's into token from the pkcs12 file.

This doesn't need C++ to catch it.  I believe many c implementations will
detect it, just not the one Bob apparently used.

Comment 7

[Robert Relyea]

There is a completely different problem. This patch is not for this bug. 

bob

Comment 8

[Robert Relyea]

This patch is for bug 196360 . I've attached an update. Note that the function
is returning OidTags, even though it's locally declared to return SECStatus. The
compilier that didn't even generate any warnings! is the Windows compiler. I'll
attach the real patch for this bug just now...

Comment 9

[Robert Relyea]

Attachment: Import Intermediate CA's into token from the pkcs12 file.

This is the correct patch for this particular bug.

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

This patch includes changes to file pk11sdr.c that do not appear to be 
related to this bug.  Am I right that that change is unrelated?  
What bug does that change apply to? 

Comment 11

[Robert Relyea]

Yes, those changes are related to bug 168398.

Comment 12

[Julien Pierre]

Comment on attachment 117089 [details] [diff] [review]
Import Intermediate CA's into token from the pkcs12 file.

Code looks good. I didn't test it because my hardware token isn't functional
right now.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

Regarding patch id=117089, This seems like it will work at intended, but in
reviewing it, I found the use of the term "locale" confusing because that 
term commonly refers to character sets and localization of strings.  
The names of the enumerated constants were meaningful to me until I read 
the comments in the declaration.

I think these variable and type names might be easier to name if they described
what CAs (if any) go into the target token.  I suggest calling the type 
SECPKCS12TargetTokenCAs, and the values

SECPKCA12TargetTokenNoCAs,
SECPKCS12TargetTokenIntermediateCAs
SECPKCS12TargetTokenAllCAs

and the function that sets this variable be SEC_PKCS12DecoderSetTargetTokenCAs.
If those names are too long, perhaps you could drop Target or Token from them.

Comment 14

[Robert Relyea]

I like nelson's names better than mine... I'll attach an updated patch (I really
didn't like locale either for much the same reasons...)

bob

Comment 15

[Robert Relyea]

Attachment: Import CAs into PKCS 12 token

Same patch as before except 1) removed code from a different bug, 2) renamed
variables as nelson suggested.

Comment 16

[Julien Pierre]

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

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 118104 [details] [diff] [review]
Import CAs into PKCS 12 token

r=nbb

Comment 18

[Robert Relyea]

Final patch checked in.