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

NEW

Status

defect
6 years ago
2 years ago

People

(Reporter: kaie, Assigned: elio.maldonado.batiz)

Tracking

3.14.2
3.15.2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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.
Assignee: nobody → emaldona
Attachment #724584 - Flags: review?(rrelyea)
(Assignee)

Updated

6 years ago
Target Milestone: --- → 3.15

Comment 2

6 years ago
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
Attachment #724584 - Flags: review?(rrelyea) → review-
(Assignee)

Comment 4

6 years ago
Attachment #724584 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #738056 - Attachment description: interdiff relative to the previous one fot convenience → interdiff relative to the previous one for convenience

Comment 5

6 years ago
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).
Attachment #738054 - Flags: review?(rrelyea) → review+
(Assignee)

Comment 6

6 years ago
certutil part pushed: https://hg.mozilla.org/projects/nss/rev/d22ffa0c1f79
(Assignee)

Comment 7

6 years ago
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
(Assignee)

Updated

6 years ago
Target Milestone: 3.15 → 3.15.1
(Assignee)

Updated

6 years ago
Target Milestone: 3.15.1 → 3.15.2
(Assignee)

Comment 8

6 years ago
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
(Reporter)

Comment 10

6 years ago
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
Attachment #775397 - Flags: review?(kaie) → review-
(Assignee)

Comment 11

6 years ago
Attachment #775397 - Attachment is obsolete: true
Attachment #778190 - Flags: review?(kaie)
(Reporter)

Comment 12

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

r=kaie
Attachment #778190 - Flags: review?(kaie) → review+
(Assignee)

Comment 13

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

Comment 14

6 years ago
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

6 years ago
Also checked modutil and pk12util as those are commands I am familiar with - looks good.
(Assignee)

Comment 16

6 years ago
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

6 years ago
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.
(Assignee)

Updated

5 years ago
Depends on: 989558, 1007126, 1038526, 1038728
You need to log in before you can comment on or make changes to this bug.