836477 - Complete the initial review of the docbook documentation for NSS command line tools

Status: RESOLVED WONTFIX

(NSS :: Documentation, defect)

Description

[Kai Engert [:KaiE:]]

We have added early versions of documentation for some of the command line utilities to NSS.

They haven't yet been fully reviewed.
We welcome review comments to get them enhanced, clarified and potential mistakes removes.

The intention of this bug is to:
- collect feedback and patches
- check in patches
- once we believe the review has been sufficient,
  to remove the "work in progress" from the documentation,
  and mark this bug fixed.

Comment 1

[Elio Maldonado]

Attachment: Fixes certutil.xml source file for man pages in patch form

Addresses some of Bob's review comments from 
https://bugzilla.redhat.com/show_bug.cgi?id=606020#c8
I still have to regenegerate all of the examples for one.

Comment 2

[Robert Relyea]

Comment on attachment 724584 [details] [diff] [review]
Fixes certutil.xml source file for man pages in patch form

r-

Most of the changes are find and beneficial. There are a couple that seem problemantic, however:

        <term>-k rsa|dsa|ec|all</term>
-        <listitem><para>Specify the type of a key. The valid options are RSA, DSA, ECC, or all. The default value is rsa. Specifying the type of key can avoid mistakes caused by duplicate nicknames.</para></listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term>-k key-type-or-id</term>
         <listitem>
-          <para>Specify the type or specific ID of a key. </para>
-          <para>
-           The valid key type options are RSA, DSA, ECC, or all. The default 
-           value is rsa. Specifying the type of key can avoid mistakes caused by
-           duplicate nicknames. Giving a key type generates a new key pair; 
-           giving the ID of an existing key reuses that key pair (which is 
-           required to renew certificates).
-          </para>
+          <para>Specify the type or specific ID of a key.</para>
           <para>
            The valid key type options are RSA, DSA, ECC, or all. The default 
            value is rsa. Specifying the type of key can avoid mistakes caused by
            duplicate nicknames. Giving a key type generates a new key pair; 
            giving the ID of an existing key reuses that key pair (which is 
            required to renew certificates).
           </para>

Looks like it's trying to reduce some verbage, except it now reads like the parameters for -k when specifying key type should be RSA, DSA, ECC or all, when in fact they should be rsa, dsa, ec or all. Note that the command only accepts lower case, and it takes ECC keys as 'ec' not 'ecc'.

-------------------------------------------------------------
-<programlisting>$ certutil -K -d sql:/home/my/sharednssdb
+<programlisting>$ certutil -K -d sql:nssdb


Several places you changed an explicit path into an implicit patch. I much prefer your changes to -d sql:$HOME/nssdb. Ideally they should be sql:/etc/pki/nssdb, but that's only applicable to Red hat, so sql:$HOME/nssdb is acceptable.

bob

Comment 3

[Elio Maldonado]

Attachment: Fixes for certutil.xml source file for man pages in patch form : V2

Comment 4

[Elio Maldonado]

Attachment: interdiff relative to the previous one for convenience

Comment 5

[Robert Relyea]

Comment on attachment 738054 [details] [diff] [review]
Fixes for certutil.xml source file for man pages in patch form : V2

r+, but you missed one issue...

-           The valid key type options are RSA, DSA, ECC, or all. The default 
+           The valid key type options are rsa, dsa, ecc, or all. The default 

ecc should be ec (see my previous comment).

Comment 6

[Elio Maldonado]

certutil part pushed: https://hg.mozilla.org/projects/nss/rev/d22ffa0c1f79

Comment 7

[Elio Maldonado]

As part of the certutil review Bob requested that the Authors and License sections be updated and that these changes be also propagated to all the tools pages. I have done that. The change set pushed: https://hg.mozilla.org/projects/nss/rev/beb69b96db71

Comment 8

[Elio Maldonado]

Downstream testing shows that we are missing man page command line options descriptions for:
certutil: --email, extNC, --keyAttrFlags, keyOpt{On|Off}
cmsutil: -b, -f, -H, -k, -V
crlutil: -W, -Z

Comment 9

[Elio Maldonado]

Attachment: Supply missing option descriptions for certutil, cmsutil, and crlutil

Comment 10

[Kai Engert [:KaiE:]]

Comment on attachment 775397 [details] [diff] [review]
Supply missing option descriptions for certutil, cmsutil, and crlutil

Review of attachment 775397 [details] [diff] [review]:
-----------------------------------------------------------------

Please fix the minor issues that I identified.

::: doc/certutil.xml
@@ +645,5 @@
> +
> +      <varlistentry>
> +        <term>--keyAttrFlags attrflags</term>
> +        <listitem><para>
> +PKCS #11 key Attributes. Comma separated list of key attribute attribute flags, selected from the following list of choices: {token | session} {public | private} {sensitive | insensitive} {modifiable | unmodifiable} {extractable | unextractable}</para></listitem>

remove duplicate word "attribute"

@@ +820,5 @@
>  XKYFwPMJjWCihVku6bw/ihZfuMHhxK22Nue6inNQ6eDu7WmrqL8z3iUrQwxs+WiF
>  ob2rb8XRVVJkzXdXxlk4uo3UtNvw8sAz7sWD71qxKaIHU5q49zijfg==
>  -----END CERTIFICATE-----
>  </programlisting>
> +<para>For a humam-readable display</para>

human with a trailing N (not m)

::: doc/cmsutil.xml
@@ +135,5 @@
> +
> +      <varlistentry>
> +        <term>-H hash</term>
> +        <listitem>
> +          <para>Use hash (default:SHA1).</para>

maybe "Use specified hash algorithm" ?

::: doc/crlutil.xml
@@ +275,5 @@
> +
> +      <varlistentry>
> +        <term>-Z algorithm</term>
> +        <listitem>
> +          <para>Spefiy the hash algorithm to use for signing the CRL.</para>

Fix: Specify

Comment 11

[Elio Maldonado]

Attachment: fix mistakes that Kai pointed out

Comment 12

[Kai Engert [:KaiE:]]

Comment on attachment 778190 [details] [diff] [review]
fix mistakes that Kai pointed out

r=kaie

Comment 13

[Elio Maldonado]

Committed and pushed changes to the tip
https://hg.mozilla.org/projects/nss/rev/89f4a4b47761
Regenerated html and and man pages.

Comment 14

[Rich Megginson]

certutil:
-t trustargs
 The attribute codes for the categories are separated by commas, and the entire set of attributes enclosed by quotation marks. For example:

-t "TCu,Cu,Tuw"

"w" is not one of the trust flags listed under trustargs

Comment 15

[Rich Megginson]

Also checked modutil and pk12util as those are commands I am familiar with - looks good.

Comment 16

[Elio Maldonado]

I'll drop that spurious 'w' and while I'm at it I will also fix the documented maximum key size in bits which, as Bob pointed out, was increased from 8192 to 16384. Thank you Rich for the reviews. - Elio

Comment 17

[Paul Wouters]

It is not entirely clear to me if modutil -fips survives a reboot. I think it must because it seems to modify the nss database. if so, than I'm curious what library call to use to detect this separately from the fips=1 kernel argument.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.