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

certutil usage doesn't give enough information about trust arguments

RESOLVED FIXED in 3.12.2

Status

NSS
Tools
P3
normal
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Julien Pierre, Assigned: Julien Pierre)

Tracking

3.11.8
3.12.2

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

10 years ago
The only information that certutil -H gives about trust arguments is :

   -t trustargs      Set the certificate trust attributes:
                          p      valid peer
                          P      trusted peer (implies p)
                          c      valid CA
                          T      trusted CA to issue client certs (implies c)
                          C      trusted CA to issue server certs (implies c)
                          u      user cert
                          w      send warning
                          g      make step-up cert

Nowhere is it specified that there are 3 sets of trust bits - one for SSL, another for S/MIME and one more for code signing.
(Assignee)

Updated

10 years ago
Priority: -- → P3
Target Milestone: --- → 3.12
(Assignee)

Comment 1

10 years ago
Created attachment 300821 [details] [diff] [review]
Improve usage message
Attachment #300821 - Flags: review?(nelson)
See 
<http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/cmd/lib/secutil.c&rev=1.19#2218>
Prior to revision 1.20 of nss/cmd/lib/secutil.c, certutil -L printed out 
a header line that read:

Certificate Name                                         Trust Attributes

followed by the list of nicknames and trust flags, followed by this legend:


p    Valid peer
P    Trusted peer (implies p)
c    Valid CA
T    Trusted CA to issue client certs (implies c)
C    Trusted CA to certs(only server certs for ssl) (implies c)
u    User cert
w    Send warning

Do we want to put a header and legend back into the output of certutil -L?
Comment on attachment 300821 [details] [diff] [review]
Improve usage message

r=nelson
Please consider the question in comment 2.
Attachment #300821 - Flags: review?(nelson) → review+
(Assignee)

Comment 4

10 years ago
Created attachment 301582 [details] [diff] [review]
Also add back header line in list certs

Nelson,

Thanks for the review.
Re: comment 2, I don't think we need to add the description of the trust flags back in the list certs output, since it is listed in the usage for certutil (certutil -H). However, the header line however could be helpful. I'm not sure why it was removed.
Attachment #300821 - Attachment is obsolete: true
Attachment #301582 - Flags: review?(nelson)
(Assignee)

Comment 5

10 years ago
Comment on attachment 301582 [details] [diff] [review]
Also add back header line in list certs

I figured out why the headers/footers were removed. It screws up the binary output (certutil -L -r).
Attachment #301582 - Attachment is obsolete: true
Attachment #301582 - Flags: review?(nelson)
(Assignee)

Comment 6

10 years ago
Created attachment 301605 [details] [diff] [review]
Don't print header if base64 or binary output was requested
Attachment #301605 - Flags: review?(nelson)
Comment on attachment 301605 [details] [diff] [review]
Don't print header if base64 or binary output was requested

The good news is that this patch applies cleanly, even after the patch for bug 
291384 was checked in.  The bad news is that with this patch, the line that
contains the string "(SSL, S/MIME, code signing)" is 88 columns wide (not counting
the trailing \r\n) in the output.  Please shorten that line by 10 characters.

>+        PR_fprintf(outfile, "\n%-60s %-5s\n%-60s %-5s\n\n", "Certificate Nickname",
>+               "Trust Attributes", "", "(SSL, S/MIME, code signing)");
Attachment #301605 - Flags: review?(nelson) → review-
(Assignee)

Comment 8

10 years ago
Created attachment 301821 [details] [diff] [review]
Fix indentation by adding one more line
Attachment #301605 - Attachment is obsolete: true
Attachment #301821 - Flags: review?(nelson)
Comment on attachment 301821 [details] [diff] [review]
Fix indentation by adding one more line

Sorry, but I think that breaking the names of the 3 trust flag categories onto two lines obscures the fact that this is a triplet of three values on a single line.  
Please just shorten that
header line by 10 columns.
Attachment #301821 - Flags: review?(nelson) → review-
(Assignee)

Comment 10

10 years ago
Nelson,

Attachment 301821 [details] [diff] produces the following output :

Certificate Nickname                                         Trust Attributes
                                                             (SSL, S/MIME,
                                                              code signing)


I don't think this is any more obscure in 3 lines than it would be in 2 lines.
I can't express the whole header in 2 lines of text that will fit 80 columns, without either having it look ugly and unaligned, or changing the alignment for every line output of -L, something that would also be requried if I wanted to make it fit one line - but at the cost of reducing the nickname space. And the alignment is not only in certutil.c but also in  SECU_PrintCertNickname which is in lib. I thought it might have been used by other tools in nss/cmd, though I see that it's not right now. I think changing the alignment would be fine if we were changing the number of columns, but this is only a header. It doesn't warrant reducing the length of the nickname in the output. Please reconsider my last patch.
Here's a thought.  Shorten "code signing" to either "JAR" or "XPI".
In practice, those are the only two forms of objects for which NSS code 
signing is used.
(Assignee)

Comment 12

10 years ago
Created attachment 303138 [details] [diff] [review]
As discussed over IM
Attachment #301821 - Attachment is obsolete: true
Attachment #303138 - Flags: review?(nelson)
Comment on attachment 303138 [details] [diff] [review]
As discussed over IM

I think this is a good compromise.
Attachment #303138 - Flags: review?(nelson) → review+
(Assignee)

Comment 14

10 years ago
Thanks, Nelson.

I checked this in to the trunk.

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.131; previous revision: 1.130
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
There is one problem with the checkin that was revision 1.131.

That patch should have caused those additional header lines to appear in
the output of certutil -L ONLY when it is not using the -n option.
That is, it should only affect listings of cert nicknames, not the 
commands that actually output a cert in some form.  But now, those header
files are appearing in the output of commands such as:

   certutil -d DB -L -n "Builtin Object Token:Taiwan GRCA"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 16

9 years ago
Created attachment 335820 [details] [diff] [review]
don't print headers when listing cert details
Attachment #335820 - Flags: review?(nelson)
Comment on attachment 335820 [details] [diff] [review]
don't print headers when listing cert details

looks like that will do it.
Attachment #335820 - Flags: review?(nelson) → review+
(Assignee)

Comment 18

9 years ago
Thanks for the review, Nelson. I checked in this fix to the trunk.

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