Re: [Gen-art] Genart last call review of draft-ietf-lamps-cms-hash-sig-08

[Russ Housley <[e-mail]>]

Dale:

Thank you for the careful review.

> Summary:
> 
>       This draft is in great shape and ready for publication as a
>       proposed standard RFC, with only a few editorial nits.

Good to hear.

> Nits/editorial comments: 
> 
> 2.2.  Leighton-Micali Signature (LMS)
> 
>   The [HASHSIG] specification supports five tree sizes:
> 
>      LMS_SHA256_M32_H5;
>      LMS_SHA256_M32_H10;
>      LMS_SHA256_M32_H15;
>      LMS_SHA256_M32_H20; and
>      LMS_SHA256_M32_H25.
> 
> This text seems redundant with the description in the preceding
> paragraph.

True.  The intent was to provide the identifiers for the five tree sizes.

Perhaps the two paragraphs should be merged, with the sentence before the list saying:

   ... As a result, the [HASHSIG] specification supports
   five tree sizes; they are identified as:

>   The LMS public key is the string consists of four elements: the
> 
> Perhaps "An LMS public key consists of ...".

Yes, that reads better.

>      u32str(lms_algorithm_type) || u32str(otstype) || I || T[1]
> 
> The notation "T[1]" seems to be undefined (although the intended value
> is described clearly in the preceding paragraph).

Good catch.  How about:

   ... and the m-byte string associated with the root
   node of the tree (T[1]).

> 2.3.  Leighton-Micali One-time Signature Algorithm (LM-OTS)
> 
>      n -  The number of bytes associated with the hash function.
>           [HASHSIG] supports only SHA-256 [SHS], with n=32.
> 
> "associated" seems to me to be vague.  Perhaps "The length in bytes of
> the output of the hash function."

Okay.  How about:

   n -  The length in bytes of the hash function output. ...

>      ls - The number of left-shift bits used in the checksum function,
>           which is defined in Section 4.4 of [HASHSIG].
> 
> "The number of left-shift bits" is not quite right.  Perhaps "The
> number of bits of left-shifting used in ..." or "The amount/size of
> the left-shift used in ...".

These words were taken directly from Section 4.1 of RFC 8554.  That said, I think you are right that it could be more clear.  How about:

   ls - The number of bits that are left-shifted in the final step of
        the checksum function, which is defined in Section 4.4
        of [HASHSIG].

> 5.  Signed-data Conventions
> 
> This paragraph has to be a number of minor wording issues, which I
> have described interline:
> 
>   As specified in [CMS], the digital signature is produced from the
>   message digest and the signer's private key.  The signature is
>   computed over different value depending on whether signed attributes
> 
> s/value/values/

Fixed.

>   are absent or present.  When signed attributes are absent, the
>   HSS/LMS signature is computed over the content.  When signed
> 
> It might help the reader to put a paragraph break before "When signed
> attributes are present..."

Okay.  Done.

>   attributes are present, a hash is computed over the content using the
>   same hash function that is used in the HSS/LMS tree, and then a
>   message-digest attribute is constructed with the resulting hash
> 
> I would replace "with" with "containing" or "whose value is"

How about:

   ... a message-digest attribute is constructed to contain the
   resulting hash value, and ...

>   value, and then DER encode the set of signed attributes, which MUST
> 
> For parallelism, this clause should start with a subject and a passive
> verb.  Perhaps "the DER encoding is constructed of ...".

How about:

   ... and then the result of DER encoding the set of signed
   attributes, which ...

> 
>   include a content-type attribute and a message-digest attribute, and
> 
> It might be clearer if the clause "which MUST ... attribute" was put
> in parentheses.

Okay.  There are a lot of commas in this sentence.

>   then the HSS/LMS signature is computed over the output of the DER-
>   encode operation.  In summary:
> 
> You can probably change "the output of the DER-encode operation" with
> "the DER encoding".

How about:

   ... then the HSS/LMS signature is computed over the
   DER-encoded output.

> The paragraph contains four clauses joined by three successive "and
> then".  You probably want to change that, perhaps breaking it out as a
> numbered/bulleted list.  (What does the Editor recommend?)

I think the text is accurate.  I wil wait for the RFC Editor to propose a different format if they want to do so.

> And in this computation:
> 
>      IF (signed attributes are absent)
>      THEN HSS_LMS_Sign(content)
>      ELSE message-digest attribute = Hash(content);
> 
> I think you want to add a hyphen:
> s/message-digest attribute/message-digest-attribute/

No.  This is the way that the attributes are talked about in RFC 5652.
(See the indented paragraphs on Page 15 of RFC 5652 as an example.)

> 
>           HSS_LMS_Sign(DER(SignedAttributes))


Thanks again for the careful review.

Russ