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

[Alissa Cooper <[e-mail]>]

Dale, thanks for your review. Russ, thanks for your response. I entered a No Objection ballot.

Alissa


> On Jul 18, 2019, at 10:19 AM, Russ Housley <[e-mail]> wrote:
> 
> 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
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art