lake-wg / edhoc

Ephemeral Diffie-Hellman Over COSE (EDHOC)
Other
6 stars 12 forks source link

Lars Eggert's review #417

Closed gselander closed 1 year ago

gselander commented 1 year ago

https://mailarchive.ietf.org/arch/msg/lake/BxQZLgOSX7_jZs3gZaTEzfXgqC8/


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?

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?

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

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

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

IANA

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


COMMENT:

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:

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:

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

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.

emanjon commented 1 year ago

https://media.defense.gov/2021/Sep/27/2002862527/-1/-1/0/CNSS%20WORKSHEET.PDF

If we think DoD will also remove the webpage we could refer to Wikipedia.

https://en.wikipedia.org/wiki/Commercial_National_Security_Algorithm_Suite

I think we should change "CNSA" to "CNSA 1.0" in the body.

https://www.iacr.org/cryptodb/archive/2003/CRYPTO/1495/1495.pdf https://link.springer.com/chapter/10.1007/978-3-540-45146-4_24

If we want the long version we probably have to ask Hugo to make it available somewhere.

larseggert commented 1 year ago

Please see if there is an archive.org link you could use (for all broken links).

gselander commented 1 year ago

Please see if there is an archive.org link

Did you mean arxiv.org? Unfortunately, SIGMA isn't there.

gselander commented 1 year ago

But it seems the short version of the paper covers all referrals in the draft so we can use the iacr URL for example.

larseggert commented 1 year ago

No, I mean https://archive.org/

Like

You just need to check if those are the correct versions.

gselander commented 1 year ago

Ok, thanks. I did a workaround, but good to know we can fall back to this.