Subject: [COSE] Review of draft-ietf-cose-hash-algs-01

From: ivaylo petrov <[email-masked]>
Date: Wed, 11 Sep 2019 10:09:43 +0200

[ Posting as an individual ]

Dear all,

This is my review of draft-ietf-cose-hash-algs. Overall I believe the
document is nearly ready with few nits and some minor editorial changes
that I believe should be addressed.

Nit: First page
* `Contributing to this document` paragraph for me should either be removed
or a note to the RFC editor should be left, so that it can be removed right
before publication.

==
Nit: section 1:
* s/include add/include/

   itself provide any strong security service.  Additionally, an

   application is going to be better off defining this type of structure

   so that it can include add any additional data that needs to be

   hashed, as well as methods of obtaining the data.
* s/for the a/for a/

  content that will be signed.  Doing indirect signing allows for the a

   signature to be validated without first downloading all of the

   content associated with the signature.  This capability can be of
* s/grater/greater/

content associated with the signature.  This capability can be of

   even grater greater importance in a constrained environment as not all
of the

   content signed may be needed by the device.

==

Nit: section 2:
* s/reduce strength/reduce the strength/ (section 2)

   them.  Applications should also make sure that the ability to change

   hash functions is part of the base design as cryptographic advances
   are sure to reduce the strength reduce strength of a hash function.
* s/include creation/include the creation/

   looking at the possible security issues.  Cryptographic purposes

   would include the creation of signatures or the use of hashes for

   indirect signatures.  These functions may still be usable for non-
* s/setup/set up/

[COSE] did not provide a default structure for holding a hash value

   not only because no separate hash algorithms were defined, but

   because how the structure is set up is frequently application

   specific.  There are four fields that are often included as part of a

   hash structure:

* s/that are to be/that is to be/

   *  Additional data, this can be something as simple as a random value

      to make finding hash collisions slightly harder as the value

      handed to the application cannot have been selected to have a

      collision, or as complicated as a set of processing instructions

      that is are to be used with the object that is pointed to.

* Split the line into multiple ones if possible as currently the txt format
is not good.

    COSE_Hash_V = ( 1 : int / tstr, # Algorithm identifier 2 : bstr, # Hash
value 3 : tstr ?, # Location of object hashed 4 : any ?   # object
containing other details and things - prefixed to the object to be hashed )

==
Nit: Section 3

* Please consider adding a note to the RFC editor to replace "[This
document]" in Table 1 with the actual RFC number. The same applies for
Table 2 and Table 3.

* s/algorithms has/algorithms have/

   issues.  While the SHA-3 hash algorithms have has been published since

   that time, the SHA-2 algorithms are still broadly used.

* what the minimum/what is the minimum/

   for shorter lengths.  Applications can specify what is the minimum

   length for a hash function for the protocol.  A validator can infer

==

Section 6
* I could not see where I-D.ietf-cose-rfc8152bis-struct is referenced in
the document. I believe it is not needed and can be removed from this
section.

==

Best regards,
Ivaylo

---

From: Jim Schaad <[email-masked]>
Date: Wed, 11 Sep 2019 13:36:34 -0700

From: ivaylo petrov <[email-masked]> 
Sent: Wednesday, September 11, 2019 1:10 AM
To: [email-masked]
Cc: cose <[email-masked]>
Subject: Review of draft-ietf-cose-hash-algs-01

Dear all,

This is my review of draft-ietf-cose-hash-algs. Overall I believe the document is nearly ready with few nits and some minor editorial changes that I believe should be addressed.

Nit: First page

* `Contributing to this document` paragraph for me should either be removed or a note to the RFC editor should be left, so that it can be removed right before publication.

[JLS] This note exists in the XML, but I added the new V3 tag for this

==

Nit: section 1:

* s/include add/include/

[JLS] done

* s/for the a/for a/

[JLS] fixed

* s/grater/greater/

   even grater greater importance in a constrained environment as not all of the

[JLS] fixed

==

Nit: section 2:

[JLS] Fixed

[JLS] Fixed

* s/setup/set up/

   hash structure:

[JLS] Perm Merriam-Webster, setup is correct.

[JLS] Changed to “that are used”

* Split the line into multiple ones if possible as currently the txt format is not good.

    COSE_Hash_V = ( 1 : int / tstr, # Algorithm identifier 2 : bstr, # Hash value 3 : tstr ?, # Location of object hashed 4 : any ?   # object containing other details and things - prefixed to the object to be hashed )

[JLS] How odd, this is not the way it renders on my laptop.  It is possible that I fixed this, it is possible that this was a rendering problem with XML2RFC which has been fixed.

==

Nit: Section 3

* Please consider adding a note to the RFC editor to replace "[This document]" in Table 1 with the actual RFC number. The same applies for Table 2 and Table 3.

[JLS]  This is well understood behavior.  I don’t think the comment is needed.  Not to mention that I would catch this during Auth48 if they did not do it.

[JLS] Fixed

[JLS] Changed to “Applications can specify a minimum length for any hash function.”

==

Section 6

* I could not see where I-D.ietf-cose-rfc8152bis-struct is referenced in the document. I believe it is not needed and can be removed from this section.

[JLS] There is currently a reference in the Abstract, this reference is not “legal” and will be removed by the RFC Editor, but having it there makes life simpler in some respects at this time.    It also ensures that the tools will complain and thus the text will get updated to the correct RFC number.

Jim

==

Best regards,
Ivaylo