Re: [Lake] Review of draft-ietf-lake-edhoc-12

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

Hi Marco,

Thanks for the review. It is recorded as github issue #192 and a proposed update to the draft is in PR #199.

Comments inline. Please let us know if you have further comments on the updates made or if it is OK to merge #199 and close #192.


From: Lake <[e-mail]> on behalf of Marco Tiloca <marco.tiloca=[e-mail]>
Date: Wednesday, 3 November 2021 at 17:26
To: [e-mail] <[e-mail]>
Subject: [Lake] Review of draft-ietf-lake-edhoc-12
Hi all,

As promised, please find below my review of EDHOC v -12.

Best,
/Marco

=============

[Section 1.1]

* "This specification focuses on referencing instead of transporting
credentials to reduce message overhead."

    It is possible to transport credentials both by reference and by
value. What suggests the case of transport by reference to be the
"focus"? Perhaps you mean that it is possible and preferable/recommended
to transport credentials by reference?
[GS] According to the requirements document draft-ietf-lake-reqs-04, the initial focus as of June 2020 was on RPK (by reference and value) and certificate by reference. Since then several participants have requested the draft to be more explicit about how to use ID_CRED_x for transport of credentials, which just follows from the use of COSE. I rephrased as:
NEW
This specification emphasizes the possibility to reference rather than to transport credentials in order to reduce message overhead, but the latter is also supported.


* "future algorithms and credentials targeting IoT"

    This probably means "future algorithms and identity credential types
targeting IoT".

[GS] The term "identity credential" is not previously used in the draft so I settled for "credential types".


* s/compromise of the long-term keys/compromise of the long-term
identity keys

[GS] The term "identity key" is not used elsewhere in the document. The term "long-term key" is used throughout the document including the security considerations and is a common term used in protocol analysis which is what this text talks about. So I think it makes sense to not change here. The alternative would be to talk about "private authentication key" which is also used in the document, but more in the context of identities.



[Section 1.3]

* s/the number of bytes in EDHOC + CoAP can be/the EDHOC message size in
bytes when transferred in CoAP can be

[GS] Changed to
NEW
the EDHOC message size when transferred in CoAP can be


[Section 1.5]

* "and CDDL [RFC8610]. The Concise Data Definition Language (CDDL) is
used to express CBOR data structures [RFC8949]"

    can be rephrased as:

    "and the Concise Data Definition Language (CDDL) [RFC8610], which is
used to express CBOR data structures [RFC8949]"

[GS] Harmonizing with previous references, and removed the reference to CBOR which is already in this paragraph, I changed to:
NEW
"and the Concise Data Definition Language (CDDL, [RFC8610]), which is used to express CBOR data structures."



[Section 2]

* "Verification of a common preferred cipher suite"

    Shouldn't this say: "Verification of a commonly supported cipher
suite which is most preferred by the Initiator" ?

[GS] Changed to
NEW
"Verification of the selected cipher suite"



[Section 3.2]

* In Figure 4, the second and third column can rather have labels
"Initiator Authentication Key" and "Responder Authentication Key".

[GS] Done. Also replaced first column label "Value" with "Method Type Value".

[Section 3.3]

* "or in a subsequent application protocol"

    can be expanded as:

    "or of an application/security context in a subsequent application
protocol"

[GS] Changed to
NEW
or in subsequent applications of EDHOC

[Section 3.4.1]

* "EDHOC transports that do not inherently provide correlation across
all messages of an exchange"

    can be rephrased as:

    "Transports that do not inherently provide correlation across all
EDHOC messages of an exchange"

[GS] Done



[Section 3.5.1]

* "The EDHOC implementation or the application must enforce information
about ..."

    can be rephrased as:

    "The EDHOC implementation or the application must take a decision on
allowing or not a connection based on information about ..."

[GS] Changed to
NEW
The EDHOC implementation or the application must decide on allowing a connection or not depending on the intended endpoint


* s/if it is allowed to communicate with/if it is allowed to communicate
with those

[GS] Rephrased to harmonize with PKI text. Note that further changes are expected to 3.5 following Stephen's review comment about restructuring.

[Section 4.3]

* The context can for example be the empty (zero-length) sequence or a
single CBOR byte string.

    Isn't the context supposed to be just a single CBOR byte string? See
how it is defined in Section 4.2 when introducing Expand.

[GS]  Changed to:

NEW
The context can for example be the empty CBOR byte string.

* s/where H() is the hash function/where H() is the EDHOC hash algorithm

[GS] Done


[Section 5.1]

* s/a second time for EDHOC processing/a second time for EDHOC
processing within the same ongoing session

[GS] Fixed.

[Section 5.2.3]

* "If an error message is sent, the session MUST be discontinued."

    Is this sentence actually required? Regardless sending an error
message or not, the session is discontinued if any processing step
fails, which is the reason for possibly sending an error message. Or are
there situations where an error message is sent even if no processing
step fails?

[GS] This sentence does not talk about failed processing, it is a general implication stating the necessity of discontinuing the session if an error message is sent (for whatever reason), in contrast to the converse statement in the preceding sentence which states that there is no necessity of sending an error message if the session is discontinued. I opened issue #208 for potential further discussion on the topic.

[Section 5.3.1]

* "G_Y, the ephemeral public key of the Responder, and ..."

    Just to avoid any risk to interpret this as the concatenation of
three elements, I suggest to rephrase as:

    "G_Y (i.e., the ephemeral public key of the Responder) and ..."
[GS] Done.


[Section 5.3.2]

* s/H() is the hash function in/H() is the EDHOC hash algorithm in
[GS] Done


* "is the 'signature' field of a COSE_Sign1 object as defined in Section
4.4 of [I-D.ietf-cose-rfc8152bis-struct] using the signature algorithm ..."

    should be rephrased as:

    "is the 'signature' field of a COSE_Sign1 object as defined in
Section 4.2 of [I-D.ietf-cose-rfc8152bis-struct], computed as defined in
Section 4.4 of [I-D.ietf-cose-rfc8152bis-struct] by using the signature
algorithm ..."

[GS] I think it suffices to refer to how it is computed, which in turn references how it is defined:
NEW
is the 'signature' field of a COSE_Sign1 object, computed as specified in Section 4.4 of {{I-D.ietf-cose-rfc8152bis-struct}} using the signature algorithm

[Section 5.3.3]

* "If an error message is sent, the session MUST be discontinued."

    Is this sentence actually required? Regardless sending an error
message or not, the session is discontinued if any processing step
fails, which is the reason for possibly sending an error message. Or are
there situations where an error message is sent even if no processing
step fails?

[GS] Same as above



[Section 5.4.2]

* s/H() is the hash function in/H() is the EDHOC hash algorithm in

[GS] Done


* "is the 'signature' field of a COSE_Sign1 object as defined in Section
4.4 of [I-D.ietf-cose-rfc8152bis-struct] using the signature algorithm ..."

    should be rephrased as:

    "is the 'signature' field of a COSE_Sign1 object as defined in
Section 4.4 of [I-D.ietf-cose-rfc8152bis-struct], computed as defined in
Section 4.4 of [I-D.ietf-cose-rfc8152bis-struct] by using the signature
algorithm ..."

[GS] Same as above


* s/as defined in Section 5.3 of [I-D.ietf-cose-rfc8152bis-struct]/as
defined in Sections 5.2 and 5.3 of [I-D.ietf-cose-rfc8152bis-struct]

[GS] Done

[Section 5.4.3]

* s/or the prepended C_I/or the prepended C_R

[GS] Done


* s/as defined in Section 5.3 of [I-D.ietf-cose-rfc8152bis-struct]/as
defined in Sections 5.2 and 5.3 of [I-D.ietf-cose-rfc8152bis-struct]

[GS] Done


* "If an error message is sent, the session MUST be discontinued."

    Is this sentence actually required? Regardless sending an error
message or not, the session is discontinued if any processing step
fails, which is the reason for possibly sending an error message. Or are
there situations where an error message is sent even if no processing
step fails?

[GS] Same as above

* s/no other party than the Responder/no other party than the Initiator
[GS] Done

[Section 5.5]

* "In deployments where no protected application message is sent from
the Responder to the Initiator, the Responder MUST send message_4."

    Consider to rephrase as:

    "In deployments where no protected application message is sent from
the Responder to the Initiator, message_4 MUST be supported and the
Responder MUST send message_4"

[GS] Changed to ", message_4 MUST be supported and MUST be used" to apply to I as well as R.

"Responder MUST send message_4" is still strange, would about I? Maybe just "message_4 MUST be used"?




[Section 5.5.2]

* s/as defined in Section 5.3 of [I-D.ietf-cose-rfc8152bis-struct]/as
defined in Sections 5.2 and 5.3 of [I-D.ietf-cose-rfc8152bis-struct]

[GS] Done



[Section 5.5.3]

* s/as defined in Section 5.3 of [I-D.ietf-cose-rfc8152bis-struct]/as
defined in Sections 5.2 and 5.3 of [I-D.ietf-cose-rfc8152bis-struct]

[GS] Done


* "If an error message is sent, the session MUST be discontinued."

    Is this sentence actually required? Regardless sending an error
message or not, the session is discontinued if any processing step
fails, which is the reason for possibly sending an error message. Or are
there situations where an error message is sent even if no processing
step fails?

[GS] See above



[Section 6]

* s/error SHALL be/An error message SHALL be

[GS] This change is not done, note that "error" refers to the name of the CBOR sequence defined below.


[Section 6.3]

* "Error code 2 MUST only be used in a response to message_1 ..."

    can be rephrased as:

    "Error code 2 MUST only be used when replying to message_1" ,
avoiding to use words like "request" and "response".

[GS] Done

[Section 6.3.2]

* s/the Responder shall only accept message_1 if/the Responder SHALL
accept message_1 only if

[GS] Done

* The last two paragraph are not further comments on the examples, but
are rather related to the cipher suite negotiation. I think they better
fit as last paragraphs of Section 6.3.1.

[GS] Done


[Section 8]

* "Compromise of PRK_4x3m leads to compromise of all exported keying
material derived after the last invocation of the EDHOC-KeyUpdate function."

    I suggest to expand as follows:

    "Compromise of PRK_4x3m leads to compromise of all exported keying
material derived from that key through the EDHOC-Exporter function. If
the EDHOC-KeyUpdate function has been used to renew PRK_4x3m, the
compromise is limited to the exported keying material derived from the
PRK_4x3m installed after the last invocation of the EDHOC-KeyUpdate
function."
[GS] Changed to
NEW
Compromise of PRK_4x3m leads to compromise of all keying material derived with the EDHOC-Exporter since the last invocation (if any) of the EDHOC-KeyUpdate function.


[Section 8.6]

* The way the first paragraph starts is probably a remnant of when CoAP
was still part of the document body rather than in Appendix A. Also, as
later discussed in Appendix A.3, the Echo exchange is started by a CoAP
server, regardless if it acts exactly as Responder. I suggest to
rephrase the paragraph as follows.

    "EDHOC itself does not provide countermeasures against
Denial-of-Service attacks. In particular, by sending a number of new or
replayed message_1 an attacker may cause the Responder to allocate
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
[I-D.ietf-core-echo-request-tag], which forces the CoAP client to
demonstrate its reachability at its apparent network address."

[GS] Done



[Section 8.7]

* "... but intended to simplify ..."

    Since "security context" is mentioned in the following sentences, it
is better to explicitly mention that they are referring to the
application protocol and not to EDHOC anymore.

[GS] I didn't make this proposed change because "security context" is not limited to OSCORE, so this sentence applies also to EDHOC. If you still find this misleading, please explain in what way it is so.

[Appendix A.3]

* "EDHOC message_2 or the EDHOC error message is sent from the server to
the client in the payload of a 2.04 (Changed) response. EDHOC message_3
or the EDHOC error message is sent from the client to the server's
resource in the payload of a POST request. If needed, an EDHOC error
message is sent from the server to the client in the payload of a 2.04
(Changed) response."

    This text should also be a remnant of old versions. When using EDHOC
for OSCORE, EDHOC error messages as CoAP responses are sent as error
responses, see the first paragraph in Appendix A.3.1. The text above can
rather, more generically, be:

    "The server sends to the client EDHOC message_2 in the payload of a
2.04 (Changed) response, or an EDHOC error message in the payload of a
response. If needed, the client sends to the server an EDHOC error
message in the payload of a POST request. Otherwise, the client sends to
the server EDHOC message_3 in the payload of a POST request. If needed,
the server sends to the client an EDHOC error message in the payload of
a response."

[GS] Good spotted, I modifed the original text in a different way:
NEW
EDHOC message_2 or the EDHOC error message is sent from the server to the client in the payload of the response, in the former case with response code 2.04 (Changed), in the latter with response code as specified in Section 8.3.1  EDHOC message_3 or the EDHOC error message is sent from the client to the server's resource in the payload of a POST request. If EDHOC message_4 is used, or in case of an error message, it is sent from the server to the client in the payload of the response, with response codes analogously to message_2.


[Appendix A.3.1]

* The last sentence can be extended, to mention what additionally is
defined in draft-ietf-core-oscore-edhoc, besides the EDHOC+OSCORE
request. This includes especially: a deterministic and efficient
conversion from OSCORE Sender/Recipient IDs to EDHOC connection
identifiers; web-linking and target attributes for discovering of EDHOC
resources.

[GS] Done


[Nits]

* Three instances of "key material" should be "keying material" for
consistency.
[GS] Done


* Section 3.3.1, s/and sends in message_2/and sends it in message_2
[GS] Done


* Section 3.3.2, s/and a EDHOC connection/and an EDHOC connection
[GS] Done


* Section 3.6
--- s/pre-defined int label/pre-defined integer label
--- s/Implementation can either use/Implementations can either use
[GS] Done


* Section 3.7, s/requires an 'y' parameter/requires a 'y' parameter
[GS] Done


* Section 3.8, s/protected out of scope of EDHOC/whose protection is out
of the scope of EDHOC
[GS] Done

* Section 3.9
--- s/verifying cipher suite/verifying a cipher suite
--- s/know identity of Responder/know identity of the Responder
--- s/know identity of Initiator/know identity of the Initiator
[GS] Done

* Section 4.3, s/same key kan be/same key can be
[GS] Done


* Section 5.1, s/then process according to Section 6, else process
as/then process it according to Section 6, else process it as
[GS] Done


* Section 5.3.2, s/facilitate retrieval of/facilitate the retrieval of
[GS] Done


* Section 5.4.2
--- s/facilitate retrieval of/facilitate the retrieval of
--- s/intialization vector/initialization vector
--- s/or derived application keys/or derive application keys
[GS] Done


* Section 5.4.2, s/intialization vector/initialization vector
[GS] Done


* Section 6.3.2, s/then Responder MUST discontinue/then the Responder
MUST discontinue
[GS] Done


* Section 8, s/protection is provided/protection are provided
[GS] Done

* Section 8.2
--- s/and instead rely/and instead relies
--- s/the Responders identity/the Responder's identity
--- s/Requirement for how/Requirements for how
[GS] Done


* Section 8.6
--- s/forces the initiator/forces the Initiator
[GS] Done


* Section 9.6, s/an CWT Claims Set/a CWT Claims Set
[GS] Done


* Section 9.14, s/is defined as/are defined as
[GS] Done


* Appendix A.3, s/using resource directory/using a resource directory
[GS] Done

* Appendix B, s/compatibily/compatibility
[GS] Done


* Appendix C.1, s/dignostic/diagnostic
[GS] Done

* Appendix E
--- s/which does not handle/which do not handle
--- s/with respect the current/with respect to the current
[GS] Done

* Appendix F, s/for message 1/for message_1
[GS] Done

Thanks!
Göran