Last Call Review of draft-ietf-lake-edhoc-18
review-ietf-lake-edhoc-18-intdir-lc-eastlake-2023-01-01-00

I am an assigned INT directorate reviewer for
<draft-ietf-lake-edhoc-18.txt>. These comments were written primarily for
the benefit of the Internet Area Directors. Document editors and
shepherd(s) should treat these comments just like they would treat comments
from any other IETF contributors and resolve them along with any other Last
Call comments that have been received. For more details on the INT
Directorate, see https://datatracker.ietf.org/group/intdir/about/ <
https://datatracker.ietf.org/group/intdir/about/>.

I found this to be a relatively long, detailed, and intricate draft which
assumed knowledge of multiple other documents concerning CBOR, COSE, CWT,
and CDDL. The document specifies Ephemeral Diffie-Hellman Over COSE
(EDHOC), a lightweight authenticated key exchange protocol including
forward secrecy, identity protection, and cipher suite negotiation. I do
not see any particular INT Area concerns with this document.

Based on my review, if I were on the IESG I would ballot this document as
NO OBJECTION

*The following are issues that SHOULD be corrected before publication:*

Section 3.8, Page 21: In the last paragraph of the section, "must not" ->
"MUST NOT"

Section 3.8.1: I would recommend against the use of random padding and in
favor of deterministic padding (perhaps dependent on the padding length,
e.g., pad with bytes whose value is the padding length modulo 256). Random
padding provides a convenient covert channel and will use up some
pseudorandom number generator entropy. As long as it is going to get
strongly encrypted, there seems to be no advantage to using random padding
material.

Sections 5.2.1/5.2.2, Pages 30/31: I found it quite confusing, on first
reading, how the explanation of SUITES_I is split between these two
sections and the way "preference" is used in two different ways: as the
preference the Initiator has for different cipher suites but also as the
suite the Responder prefers among those supported by the Initiator. And the
text in 5.2.2 may be inconsistent because it says the Initiator MUST select
its most preferred cipher suite based "on what it can assume to be
supported by the Responder" but later says this SHOULD change for certain
error messages from a Responder. (There is also an extraneous close curly
brace ("}") in the last line on page 30.)
     The text for SUITES_I in Section 5.2.1 is nice and clear and almost
complete. The text on constructing SUITES_I in Section 5.2.2 seems to me to
be long, complicated, and confusing. I suggest changing the text for
SUITES_I in Section 5.2.1 to something like " - array of cipher suites
which the Initiator supports constructed as specified in Section 5.2.2".
The change the text in Section 5.2.2 for the SUITES_I bullet item to
something like:

* Construct SUITES_I as an array of cipher suites which the
Initiator supports in order of preference with the first in network byte
order being the most preferred and ending with the one selected by I for
this session. If the cipher suite most preferred by I is selected then
SUITES_I contains only that cipher suite and is encoded as an int. All
cipher suites, if any, preferred by the Initiator over the selected one
MUST be included. (See also Section 6.3.)

- The selected suite is based on what the Initiator can assume to be
supported by the Responder; however, if the Initiator previously received
from the Responder an error message with error code 2 containing SUITES_R
(see Section 6.3) indicating cipher suites supported by the Responder, then
the Initiator SHOULD select its most preferred supported cipher suite among
those (bearing in mind that error messages are not authenticated and may be
forged).

- The Initiator MUST NOT change its order of preference for cipher suites
and MUST NOT omit a cipher suite preferred to the selected one because of
previous error messages received from the Responder."


Figures 11 and 12, Page 42: It looks like these figures have gigantic
captions of 9 or 10 lines. Is that what you intended? I think that Figure
captions need to be like Section names, usually one line although
occasionally two lines is OK. While I do not think it is required for this
document, sometimes relatively long RFCs with many figures have a Table of
Figures after the Table of Contents (see RFC 6325 for an example); that
wouldn't work with a 9-line caption. I suggest changing these to short
captions (maybe "Cipher Suite Negotiation Example 1" and "Cipher Suite
Negotiation Example 2" although that isn't very specific or creative). Then
move the current caption text to a paragraph referencing the Figure.

Section 8.8, Page 51: "a random seed must" -> "a random seed MUST"

Section 8.8, Page 53: There should be a definition or a reference for
"Trusted Execution Environment"

Section 9: GENERAL IANA CONSIDERATIONS PROBLEMS
     If additional values can be specified in future documents, which is
true for these registries you are creating, there must be a "Reference"
column saying where that value was assigned and its meaning specified.
     In almost all cases there should be a Reserved value that can be used
as some sort of escape if a future extension needs to be defined.
     I notice that all of these requests to create registries and assign
code points are written as if they have already been done but a spot check
of several seems to indicate that these have not yet been done by IANA. It
is true that an Internet Draft should generally be written so that it reads
correctly when published as an RFC. However, I think the IANA
Considerations are somewhat of an exception. Because code points are
frequently assigned in advance of the publication or approval of a
protocol, to avoid confusion between those that have and have not yet been
done, the usual wording is to say that IANA is "requested" to do something
unless it has actually already been done. So I would re-word all of these
as requests (except for any that have actually been done).
     I would also add text between the 9. and 9.1. headers, something like
"This Section gives IANA Considerations and, unless otherwise noted,
conforms with [RFC8126]." Then you can pull out all the [RFC8126]
references in the Section.

Section 9.1, Page 54:  It is unreasonable to expect IANA to know what a
"uint" is or what its range is. It is best to be as explicit and exact as
practical in the IANA Considerations so a complete table is better than
just a list of a few values. I suggest replacing this Section with
something like

IANA is requested to create a new registry under the new registry group
"Ephemeral Diffie-Hellman Over COSE (DHOC)" as follows:

Registry Name:  EHOC Exporter Label
Assignment Policy:  Expert Review
Reference:  [this document]

 Label     Description                   Reference
------    ----------------------------   ---------
     0    Derived OSCORE Master Secret   [this document]
     1    Derived OSCORE Master Salt     [this document]
2-65534   unassigned
65535     Reserved                       [this document]

Sections 9.2, 9.3, 9.4, 9.5, and 9.6: All of these should be changed in a
way analogous to that indicated above.

Section 9.11, Page 60: In first sentence
OLD
   "Expert Review"
NEW
   "Expert Review" or "Specification Required"

Section 9.11, Page 60:
OLD
   *  Specifications are recommended.  When specifications are not
      provided,
NEW
   *  Even for "Expert Review" specifications are recommended.  When
specifications are not
      provided for a request where Expert Review is the assignment policy,

Appendix B: I assume "ceil" is the ceiling function but this is not
specified anywhere. Suggest adding to the end of the first sentence in
which it is used "where ceil(x) is the smallest integer not less than x".

*The following are minor wording issues with the document:*

Section 1.1, Page 4:
OLD
   This specification emphasizes the possibility to reference rather
   than to transport credentials in order to reduce message overhead,
   but the latter is also supported.
NEW
   This specification emphasizes the possibility of referencing rather
   than transporting credentials in order to reduce message overhead,
   but the latter is also supported.

Section 1.3: Claims to talk about the remainder of the document but says
nothing about Appendices B through J. (I do not think it needs to mention
K. (Actually, when I have a "changes" appendix like K I usually label it as
"Appendix Z" to make it clear it is not part of the sequence of any other
appendices.) )

Section 3.5.3, Pages 16/17, in the two bulleted items:
"for the Initiator to retrieve" -> "the Initiator retrieving"
"for the Responder to retrieve" -> "the Responder retrieving"

Section 1.2, Page 5, this is just a bit too verbose for my taste:
OLD
   Compared to the DTLS 1.3 handshake [RFC9147] with ECDHE and
   connection ID, the EDHOC message size when transferred in CoAP can
   be less than 1/6 when RPK authentication is used, see
   [I-D.ietf-lwig-security-protocol-comparison].
NEW
   When RPK authentication is used, the EDHOC message size transferred
   in CoAP can be less than 1/6 that of a DTLS 1.3 handshake [RFC9147]
   with ECDHE and connection ID
   [I-D.ietf-lwig-security-protocol-comparison].

Section 3.9, Page 23: "incompliance" -> "noncompliance" (2 occurrences)

Section 5.1, Pages 29/30: I found the last two sentences of the last
paragraph of Section 5.1 a bit confusing and wordy. Suggest the following:
OLD
   This assumes that message duplication due to re-transmissions is
   handled by the transport protocol, see Section 3.4.  The case when
   the transport does not support message deduplication is addressed in
   Appendix G.
NEW
   Message deduplication MUST be done by the transport protocol (see
   Section 3.4) or as described in Appendix G.

Section 7, Page 43: In the next to last sentence "supporting one or both of
these is no essential difference" does not read well. Suggest replacing it
with "supporting one or both of these is not significantly different".

Section 8.2, Page 48: In the last sentence:
OLD
         it is recommended to
   use a freshly generated authentication key as identity in each
   initial TOFU exchange.
NEW
   using a freshly generated authentication key as identity in each
   initial TOFU exchange is RECOMMENDED.

Section 8.3, Pages 48/49:
OLD
   it is RECOMMENDED to use the
   same hash algorithm as in the cipher suite but with as much
   truncation as possible, i.e., when the EDHOC hash algorithm is
   SHA-256 it is RECOMMENDED to use SHA-256/64 in x5t and c5t.
NEW
   using the
   same hash algorithm as in the cipher suite, but with as much
   truncation as possible, is RECOMMENDED.  That is, when the EDHOC hash
algorithm is
   SHA-256, using SHA-256/64 in x5t and c5t is RECOMMENDED.

Section 8.8, Page 54:
OLD
      It is RECOMMENDED to discontinue the protocol if
   the received EDHOC message is not deterministic CBOR.
NEW
      Discontinuing the protocol if
   the received EDHOC message is not deterministic CBOR is RECOMMENDED.

Section 9.6: The table in this section should probably have a figure number
and caption since other tables in this document do.

Section 9.11, Page 60: "approving point assignment" -> "approving code
point assignment"

Appendix C, Page 75: "implementors to get used to" -> "implementors get
used to"

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 d3e3e3@gmail.com