Re: [Lake] Lars Eggert's Discuss on draft-ietf-lake-edhoc-20: (with DISCUSS and COMMENT)

[Göran Selander <[e-mail]>]

Hi Lars,

Thanks for your review. I tracked it as github issue #417<https://github.com/lake-wg/edhoc/issues/417> and started PR #425<https://github.com/lake-wg/edhoc/pull/425>. Please see responses to your comments inline below.

 Göran

From: Lars Eggert via Datatracker <[e-mail]>
Date: Tuesday, 22 August 2023 at 10:24
To: The IESG <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Lars Eggert's Discuss on draft-ietf-lake-edhoc-20: (with DISCUSS and COMMENT)
Lars Eggert has entered the following ballot position for
draft-ietf-lake-edhoc-20: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-lake-edhoc/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

# 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).

## Discuss

### Section 3.4, paragraph 6
```
     *  denial-of-service protection,
```
No IETF transport protocol provides DDoS protection. If this is an
actual requirement, how will it be provided?

[GS] Agree “protection” is not the right word here. DTLS uses “DoS prevention” for a similar property (Section 5 in RFC 9147) but “prevention” is perhaps even stronger than “protection”? We actually think “mitigation” is a good term to use here, which is synonym to “alleviation”/”diminution”/etc. I made a general replace to “mitigation” as placeholder pending an agreement on this point.

### Section 8, paragraph 3
```
     An implementation MAY support only a single method.  None of the
     methods are mandatory-to-implement.
```
How is interoperability guaranteed without at least a single
mandatory-to-implement method?

[GS] This has been discussed in the WG, in short it is the application profile (Section 3.9) which makes implementations interoperable. Considering the large variety of IoT applications and the potential constrainedness of target deployments it is not always feasible to implement other methods than those which are actually used. For example, for the most constrained devices it may only be possible to support method 3, whereas more traditional certificate based deployments would typically be based on signature public keys and method 0. The method is just one out of several parameters that needs to be specified in the application profile to achieve interoperability, including cipher suite, credential identification, etc.

### Section 9.7, paragraph 1
```
     state, perform cryptographic operations, and amplify messages.  To
     mitigate such attacks, an implementation SHOULD rely on lower layer
     mechanisms.  For instance, when EDHOC is transferred as an exchange
     of CoAP messages, the CoAP server can use the Echo option defined in
     [RFC9175] which forces the CoAP client to demonstrate reachability at
     its apparent network address.  To avoid an additional roundtrip the
     Initiator can reduce the amplification factor by padding message_1,
     i.e., using EAD_1, see Section 3.8.1.
```
While the Echo option prevents some resource exhaustion aspects of
spoofing, it does not prevent DDoS by actual CoAP clients. Likewise,
while limiting amplification reduces the impact of a DDoS attack by
actual clients, it does not prevent it. It is hence incorrect to say
that these attacks are mitigated by COAP. (They also wouldn't be
mitigated by any other IETF transport protocol.)

[GS] Good points, I added one paragraph to this section:
”Note that while the Echo option mitigates some resource exhaustion aspects of spoofing, it does not protect against a distributed denial-of-service attack made by real, potentially compromised, clients. Similarly, limiting amplification only reduces the impact, which still may be significant because of a large number of clients engaged in the attack.”
As above, I kept the term “mitigation” as in “to make (something) less severe, harmful, or painful” (Britannica) or “to cause to become less harsh or hostile” (Merriam-Webster). IMHO it seems to me to be an appropriate term to use. Happy to stand corrected.

### "A.2.", paragraph 1
```
     duplication.  CoAP can also perform fragmentation and protect against
     denial-of-service attacks.  The underlying CoAP transport should be
```
Per above, COAP does not protect against DDoS.
[GS] Replaced “protect against” with “mitigate certain”.

### "A.2.", paragraph 6
```
     To protect against denial-of-service attacks, the CoAP server MAY
     respond to the first POST request with a 4.01 (Unauthorized)
     containing an Echo option [RFC9175].  This forces the Initiator to
```
Per above, this mitigates some aspects of spoofing, but does not
protect against DDoS.
[GS] Replaced “protect against” with “mitigate certain”

### IANA

This document seems to have unresolved IANA issues. Holding a DISCUSS for IANA,
so we can determine next steps during the telechat.

[GS] Resolved now.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------


## Comments

### Section 3.4, paragraph 5
```
     *  flow control,
```
But not congestion control?
[GS] CoAP, which is the default transport mechanism, provides flow control and congestion control, so those are indeed properties we expect from the transport. We added “congestion control” to the list. As discussed in the response to Martin Duke’s review, the security protocol strictly speaking does not require anything from transport but will not perform very well without suitable properties depending on the connection.


### 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.)

[GS] Ongoing thread on mailing list. With current last mail from Carsten no change will be made.

### 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`
[GS] ”master” used in the terms OSCORE Master Secret/Salt which is the terminology used in RFC 8613.

 * Term `man`; alternatives might be `individual`, `people`, `person`
[GS] “man-in-the-middle” replaced with “active on-path”

 * Term `dummy`; alternatives might be `placeholder`, `sample`, `stand-in`,
   `substitute`
[GS] removed “dummy” as it was redundant

 * Term `native`; alternatives might be `built-in`, `fundamental`, `ingrained`,
   `intrinsic`, `original`
[GS] “native” only used in the change log which will be removed before publication.

## 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://protect2.fireeye.com/v1/url?k=31323334-501cfaf3-313273af-454445554331-2ff81d81b9b4007d&q=1&e=014883c7-9ca0-4d1a-a260-33fd1fa98485&u=https%3A%2F%2Fgithub.com%2Flarseggert%2Fietf-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.
[GS] Latest version will be fixed in the next version.

### URLs

These URLs in the document did not return content:

 * https://apps.nsa.gov/iaarchive/programs/iad-initiatives/cnsa-suite.cfm
[GS] Changed to Wikipedia. Clarified the reference refer to CNSA 1.0.
https://en.wikipedia.org/wiki/Commercial_National_Security_Algorithm_Suite

 * https://webee.technion.ac.il/~hugo/sigma-pdf.pdf

[GS] Changed to IACR, short version of the paper, which covers the use in the document.
https://www.iacr.org/cryptodb/archive/2003/CRYPTO/1495/1495.pdf

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

 * https://protect2.fireeye.com/v1/url?k=31323334-501cfaf3-313273af-454445554331-e494bd73e348ae79&q=1&e=014883c7-9ca0-4d1a-a260-33fd1fa98485&u=http%3A%2F%2Fcbor.me%2F


[GS] Yes cbor.me now supports https

https://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".

[GS] Comma doesn’t make sense here.


#### 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".

[GS] Done


#### 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.
[GS] Done


#### 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.
[GS] Done


#### 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".
[GS] Done


#### 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.
[GS] Done


#### 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".
[GS] Done


#### 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.
[GS] Done


#### 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.
[GS] Done


#### 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,"?
[GS] Done


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


#### "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".
[GS] Done


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


#### "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".
[GS] Done


#### "A.2.1.", paragraph 8
```
ing. What verifications are needed depend on the deployment, in particular th
                                   ^^^^^^
```
Did you mean "to depend"?
[GS] Replaced with “Which verifications that are needed depend on . . .“


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

[GS] Done

#### "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.
[GS] Not done, change log will be removed.



## 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://protect2.fireeye.com/v1/url?k=31323334-501cfaf3-313273af-454445554331-29a4b48826f5565a&q=1&e=014883c7-9ca0-4d1a-a260-33fd1fa98485&u=https%3A%2F%2Fgithub.com%2Fmnot%2Fietf-comments%2Fblob%2Fmain%2Fformat.md
[ICT]: https://protect2.fireeye.com/v1/url?k=31323334-501cfaf3-313273af-454445554331-0580e87653a1b8a9&q=1&e=014883c7-9ca0-4d1a-a260-33fd1fa98485&u=https%3A%2F%2Fgithub.com%2Fmnot%2Fietf-comments
[IRT]: https://protect2.fireeye.com/v1/url?k=31323334-501cfaf3-313273af-454445554331-2ff81d81b9b4007d&q=1&e=014883c7-9ca0-4d1a-a260-33fd1fa98485&u=https%3A%2F%2Fgithub.com%2Flarseggert%2Fietf-reviewtool