Closed Bug 836477 Opened 11 years ago Closed 4 months ago

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

Categories

(NSS :: Documentation, defect)

3.14.2
defect

Tracking

(Not tracked)

RESOLVED WONTFIX
3.15.2

People

(Reporter: KaiE, Unassigned)

References

Details

Attachments

(3 files, 2 obsolete files)

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.
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)
Target Milestone: --- → 3.15
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-
Attachment #738056 - Attachment description: interdiff relative to the previous one fot convenience → interdiff relative to the previous one for convenience
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+
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
Target Milestone: 3.15 → 3.15.1
Target Milestone: 3.15.1 → 3.15.2
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 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-
Attachment #775397 - Attachment is obsolete: true
Attachment #778190 - Flags: review?(kaie)
Comment on attachment 778190 [details] [diff] [review]
fix mistakes that Kai pointed out

r=kaie
Attachment #778190 - Flags: review?(kaie) → review+
Committed and pushed changes to the tip
https://hg.mozilla.org/projects/nss/rev/89f4a4b47761
Regenerated html and and man pages.
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
Also checked modutil and pk12util as those are commands I am familiar with - looks good.
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
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.
Depends on: 989558, 1007126, 1038526, 1038728

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

Assignee: elio.maldonado.batiz → nobody
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.