Ephemeral Diffie-Hellman Over COSE (EDHOC)
draft-ietf-lake-edhoc-23

# Internet AD comments for draft-ietf-lake-edhoc-20
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S6.2

* I'm not sure about "MUST be ... written in English".  Does it not suffice
  to say "MUST be a human-readable string" and "SHOULD be English", since the
  exact written language is not protocol-critical?

### Appendix B

* Do any of the Certicom patents as notified to SECG:

      https://www.secg.org/certicom_patent_letter_SECG.pdf

  apply the text in this section?  If so, has the working group been made
  aware and collectively agreed to proceed?

  I just found this linked off the https://www.secg.org/ site and it seemed
  to mention "point compression", but I'm not really qualified to make any
  reasonable assessment about whether readers of this I-D should be made
  aware of this stuff or not.

## Nits

### S3.3.2

* Figure 5 could make it more clear that the CBOR encoding values are
  in hex (right?)

### S3.9

* "because of wrong credential"

  Either "because of wrong credentials" or maybe "because of a wrong
  credential" might read better.

### S5.2.2, S5.3.2, S5.4.2, S5.5.2

* I found the use of "Processing" in the section heading to somewhat
  misleading.  I don't think of the sender as "processing" a message that
  it's going to send.  Perhaps, "Preparation" or "Composition" might be more
  natural?

### Appendix D.5

* You might be asked by the Editor about alternative language for
  "man-in-the-middle" (e.g., "on-path attacker" or something).

thank you to Mališa Vučinić for the illuminating and helpful shepherd writeup, and to the authors for the well-written document.

One nit (and it is the smallest of nits) is that the backslash ("\") character that occurs in Figure 17 seemed odd.

Thank you to Radia Perlman for the SECDIR review.

Thank you for addressing my DISCUSS and COMMENT feedback.

Other than supporting other ADs positions, I have nothing to add (it is far outside my areas of expertise).

Thanks for working on this specification. Thanks to Michael Scharf his valuable TSVART review and nice to those addressed.

I would like to have responses on the following points as I believe clarity would help this specification - 

   - It appeared to me that reliable transport is preferred while EDHOC messages are transmitted, however, this is not clearly mentioned. I think if this is the case then it should be clear in this specification.  

   - I also like section 3.4, however, it is not clear to me if the list provided, is a "must to meet" criteria for any transport or fulfilling any subset of features is good enough. If the later then this specification should describe how the missing criteria should be fulfilled or ignore or describe the impact. 

For the similar reason, I am also supporting Lars's discuss on clarification required for DoS protection by the transport.

Thank you for the work put into this document as it is outside of my technical expertise, I am trusting the SEC ADs for their reviews, I have detected nothing related to INT area while browsing the I-D.

Special thanks to Mališa Vučinić for the shepherd's detailed write-up including the WG consensus and some justification of the intended status. 

Thanks also to Donald Eastlake for his last call int-dir review, I have seen that the authors had engaged with the reviewer.

Regards,

-éric

# GEN AD review of draft-ietf-lake-edhoc-20

CC @larseggert

Thanks to Christer Holmberg for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/tvJRHUSdUtXJpishMcOd0KwR4O0).

## Comments

### Section 3.4, paragraph 5
```
     *  flow control,
```
But not congestion control?

### Section 10.2, paragraph 17
```
     | -20 to 23      | Standards Action with Expert Review |
```
Why still Expert Review if this already requires a Standards Action?
(Same comment for other registry ranges with this policy.)

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term `master`; alternatives might be `active`, `central`, `initiator`,
   `leader`, `main`, `orchestrator`, `parent`, `primary`, `server`
 * Term `man`; alternatives might be `individual`, `people`, `person`
 * Term `dummy`; alternatives might be `placeholder`, `sample`, `stand-in`,
   `substitute`
 * Term `native`; alternatives might be `built-in`, `fundamental`, `ingrained`,
   `intrinsic`, `original`

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Outdated references

Document references `draft-selander-lake-authz-02`, but `-03` is the latest
available revision.

Document references `draft-ietf-core-oscore-key-update-04`, but `-05` is the
latest available revision.

Document references `draft-ietf-teep-architecture`, but that has been published
as `RFC9397`.

Document references `draft-ietf-cose-cbor-encoded-cert-05`, but `-06` is the
latest available revision.

Document references `draft-ietf-core-oscore-edhoc-07`, but `-08` is the latest
available revision.

### URLs

These URLs in the document did not return content:

 * https://apps.nsa.gov/iaarchive/programs/iad-initiatives/cnsa-suite.cfm
 * https://webee.technion.ac.il/~hugo/sigma-pdf.pdf

These URLs in the document can probably be converted to HTTPS:

 * http://cbor.me/

### Grammar/style

#### Section 3.5.1, paragraph 2
```
n used by Initiator or Responder. Similarly for CRED_I, see Section 5.4.2. Th
                                  ^^^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Similarly".

#### Section 4.1.1.3, paragraph 5
```
 used for two different purposes. However an application can re-derive the s
                                  ^^^^^^^
```
A comma may be missing after the conjunctive/linking adverb "However".

#### Section 4.1.2, paragraph 1
```
al times as long as it is done in a secure way. For example, in most encrypt
                               ^^^^^^^^^^^^^^^
```
Consider replacing this phrase with the adverb "securely" to avoid wordiness.

#### Section 5.3.2, paragraph 17
```
s is similar to waiting for an acknowledgement (ACK) in a transport protocol.
                               ^^^^^^^^^^^^^^^
```
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

#### Section 6, paragraph 11
```
s 8 and 9, and prefers suite 8, so therefore selects suite 8 in the second m
                                ^^^^^^^^^^^^
```
Consider using "so" or "therefore".

#### Section 6.3.1, paragraph 3
```
 multiple times due to missing acknowledgement on the CoAP messaging layer.
                               ^^^^^^^^^^^^^^^
```
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

#### Section 9.1, paragraph 6
```
e use of authenticated encryption. Hence the message authenticating function
                                   ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".

#### Section 9.5, paragraph 1
```
rves such as Ed25519 and Ed448 can mapped to and from short-Weierstrass form
                                   ^^^^^^
```
The modal verb "can" requires the verb's base form.

#### Section 9.8, paragraph 1
```
H_3, TH_4) does not make use of a so called running hash. This is a design c
                                  ^^^^^^^^^
```
The expression "so-called" is usually spelled with a hyphen.

#### Section 11.2, paragraph 11
```
sage flow" (see Appendix A.2.2). By default we assume the forward message flo
                                 ^^^^^^^^^^
```
Did you mean: "By default,"?

#### "A.1.", paragraph 4
```
t representation trivially avoids so called "benign malleability" attacks whe
                                  ^^^^^^^^^
```
The expression "so-called" is usually spelled with a hyphen.

#### "A.1.", paragraph 6
```
yte 0x02 (i.e., M = 0x02 || X). * If a uncompressed y-coordinate is required,
                                     ^
```
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

#### "A.2.", paragraph 2
```
he major type. CBOR supports several different types of data items, in addit
                             ^^^^^^^^^^^^^^^^^
```
Consider using "several".

#### "A.2.", paragraph 7
```
 CDDL C.2. CDDL Definitions This sections compiles the CDDL definitions for e
                                 ^^^^^^^^
```
Consider using the singular form after the singular determiner "This".

#### "A.2.1.", paragraph 8
```
ing. What verifications are needed depend on the deployment, in particular th
                                   ^^^^^^
```
Did you mean "to depend"?

#### "D.3.", paragraph 1
```
cation message is successful, then the the Initiator transitions from COMPLET
                                   ^^^^^^^
```
Possible typo: you repeated a word.

#### "Appendix E.", paragraph 7
```
 with example state machine o Acknowledgements o Language improvements by na
                              ^^^^^^^^^^^^^^^^
```
Do not mix variants of the same word ("acknowledgement" and "acknowledgment")
within a single text.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Thanks to Michael Scharf for the TSVART review.

I appreciate the clear statement of transport requirements in S3.4.

You state that deduplication is a requirement of the transport, but then in S7 you state that some transports don't actually meet the requirement, and what to do in that case. So it really isn't a requirement, is it? Just a nice to have! It would be good to state that in 3.4.

Is it really necessary for the transport to handle reordering? IIUC this protocol consists of up to 4 atomic messages that fit in a datagram, and each one is input to the next. How could there be reordered messages in this context? (Similarly, I fail to see how a transport could support ordering but not deduplication -- perhaps I'm not imaginative enough).