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.
Created attachment 300821 [details] [diff] [review] Improve usage message
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.
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.
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).
Created attachment 301605 [details] [diff] [review] Don't print header if base64 or binary output was requested
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)");
Created attachment 301821 [details] [diff] [review] Fix indentation by adding one more line
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.
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.
Created attachment 303138 [details] [diff] [review] As discussed over IM
Comment on attachment 303138 [details] [diff] [review] As discussed over IM I think this is a good compromise.
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
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"
Created attachment 335820 [details] [diff] [review] don't print headers when listing cert details
Comment on attachment 335820 [details] [diff] [review] don't print headers when listing cert details looks like that will do it.
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