Re: [Lake] AD review draft-ietf-lake-edhoc-19

[Stephen Farrell <[e-mail]>]

Thanks Paul and Göran - please continue the discussion to
a resolution, and I'd encourage co-authors and other WG
participants to keep an eye on the mails and PRs, engaging
as needed, so we end up with improving the draft as it goes
forward.

If some chair-help is needed as we go, just ping one of us,
but my assumption is that our AD and authors should be able
to handle the discussion fine and will bring any potential
changes that need broader WG consideration to our notice
if that turns out to be needed.

Cheers,
S.

On 09/06/2023 21:23, Göran Selander wrote:
> Hi Paul,
> 
> Thanks for the review! Please see inline below responses and requests for clarifications.
> 
> I wanted to reply quickly while the context is fresh, so my co-authors have not yet looked at the responses and may have additional comments.
> 
> The updates are in https://github.com/lake-wg/edhoc/pull/407
> 
> Göran
> 
> From: Paul Wouters <[e-mail]>
> Date: Monday, 5 June 2023 at 19:49
> To: [e-mail] <[e-mail]>
> Cc: John Mattsson <[e-mail]>, Francesca Palombini <[e-mail]>, Göran Selander <[e-mail]>
> Subject: AD review draft-ietf-lake-edhoc-19
> 
> AD review draft-ietf-lake-edhoc-19
> 
> I do want to apologize for the extreme amount of time this took. I am very sorry that the WG had to wait so long for this document to proceed.
> 
> I have a few questions that I would like to see answers too, which does not necessarily mean these questions require draft changes.
> 
> Paul
> 
> 
> 
> Question 1:
> 
>          METHOD, SUITES_I = [5, 6], G_X, C_I, EAD_1
> 
> In this case, is G_X suitable for only ciphersuite 5? So even if the
> responder would want to agree with 6, it might be that the initiator
> G_X is unsuitable ? In IKEv2 this is addressed with the INVALID_KE payload,
> that tells the initiator to retry and also which DH group (KE payload) to use.
> Wouldn't EDHOC have the same (unsolved) issue ?
> 
> GS: I don’t think so. The ephemeral keys should be generated for the EDHOC key exchange algorithm (ECDH curve) of the selected cipher suite. The selected cipher suite is unique and indicated with the last integer (or only) in SUITES_I. So in this case G_X should be suitable for 6 or else message_1 is malformed. Whether G_X is suitable for 5 is not relevant because if the Responder supports cipher suite 5, which in your example is more preferred by the Initiator, then the Responder must reject this message_1 and indicate support for cipher suite 5 in the error. The Initiator should then restart EDHOC with selected cipher suite 5 (SUITES_I = 5)  and G_X suitable for 5. I added clarifying text in Note 2 of the cipher suite negotiation example, see PR #407.
> 
> 
> 
> Question 2:
> 
> Unlike IKE, here the transport has to ensure deduplication of the messages. That
> makes it more vulnerable to a denial of service attack by an attaker that more or
> less blindly can send messages. In IKE, the initial message, if totally unsuitable,
> is ignored so that if it comes from an attacker, the real initial message can still
> be received and processed. Did I miss similar language in the draft ?
> 
> GS: CoAP message deduplication as described in section 4.5 of RFC 7252 is fulfilling the assumption how to handle message duplication on transport layer, in case EDHOC is transported with CoAP. The draft also defines “EDHOC message deduplication” (Appendix G) to support transport layers which do not handle message duplication. If transport layer supports message deduplication (and EDHOC message deduplication is not used), then the implementation will not react on messages which are not duplicate on transport layer, even if application layer messages are identical. In this case, the security protocol processing applies.
> The responder will try to parse message_1. Referring to your question, it is true that “the real initial message can still be received and processed”. If parsing fails the responder may send error message or may ignore the error (see Section 8.7). That seems to be matching your comment about “if totally unsuitable, is ignored”. Perhaps I misunderstand your comment, what language is missing in the draft?
> 
> 
> Question 3:
> 
>          message_2 = (
>          G_Y_CIPHERTEXT_2 : bstr,
>          C_R : bstr / -24..23,
>          )
> 
> Why is G_Y_CIPHERTEXT_2 not split in two, the G_Y bstr and CIPHERTEXT_2 bstr ?
> Especially since in 5.3.3 it says it needs to "Decrypt CIPHERTEXT_2", so it
> needs to seperate it from G_Y_CIPHERTEXT_2. Is this an optimization for sending
> one bstr instead of two? If so, why not concatenate C_R as well ?  (I'm not a CBOR
> expert, so forgive me if this is obvious to others)
> 
> GS: G_Y_CIPHERTEXT_2 is not split, to handle the size requirements of messages, where one of the benchmarks requires the protocol to support message sizes less than 45 bytes, see Section 2.11.2.1 of draft-ietf-lake-reqs-04.html. Since the length of G_Y is known from the cipher suite this optimization is straightforward. However, the length of C_R is not a priori known, so including the raw C_R byte string in the same CBOR construction would result in two fields of two unknown lengths which may prevent unique parsing.
> 
> 
> Question 4:
> 
> Why is there an ERR_CODE of 0 meaning "success". Since "Error messages
> in EDHOC are always fatal."
> 
> It is explained in the section on how this can be used (without a need
> for interoperability).  Why not just leave ERR_Code 0 "reserved" and
> say it can be used internally ?
> 
> 
> GS: An error code for “success” was requested in the following IOTOPS thread:
> https://mailarchive.ietf.org/arch/msg/iotops/t9LTpMiZ__kFGZpceLCBaCUHN44/
> The table is updated in PR #407 to indicate that ERR_CODE = 0 is reserved and the draft does not anymore define ERR_INFO for that ERR_CODE. Does this address the comment?
> 
> 
> Question 5:
> 
>          The Initiator and Responder MAY agree out-of-band on a
>          longer oscore_key_length than the default and on a different
>          oscore_salt_length.
> 
> This implies salt can be shorter than default? Unlike key that can only be longer?
> Is that correct? If so, is there a minimum salt length ? If not, would that not
> be dangerous? (eg 0 length salt means unsalted)
> 
> GS: The default OSCORE Master Salt is null, so this is not an issue.
> 
> 
> Comments:
> 
>          Note 2. The content of the fields of message_1 may be different
>          when sent the second time, in particular the ephemeral key MUST
>          be different.
> 
> That lowercase may should be a MUST.
> 
> GS: Note 2 now says:
> “For each message_1 the Initiator MUST generate a new ephemeral ECDH key pair matching the selected cipher suite.”
> 
> 
>          the Responder is authenticated with 128-bit security against
>          online attacks (the sum of the 64-bit MACs in message_2 and
>          message_4
> 
> Since message_4 is optional, an attacker able to filter could drop this
> message altogether, in which case the "128-bit security" is really 64 bit,
> making this claim a bit too optimistic?
> 
> GS: The context for this statement is the two examples:
> 
> “As an example, if EDHOC is used with method 3, cipher suite 2, and message_4, the Responder is authenticated with 128-bit security against online attacks (the sum of the 64-bit MACs in message_2 and message_4). The same principle applies for MACs in an application protocol keyed by EDHOC as long as EDHOC is rerun when verification of the first MACs in the application protocol fails. As an example, if EDHOC with method 3 and cipher suite 2 is used as in Figure 2 of [I-D.ietf-core-oscore-edhoc<https://www.ietf.org/archive/id/draft-ietf-core-oscore-edhoc-06.txt>], 128-bit mutual authentication against online attacks can be achieved after completion of the first OSCORE request and response.”
> 
> What maybe is not clear from the previous text is that two examples are complementing each other, see Section 5.5:
> 
> ”In deployments where no protected application message is sent from the Responder to the Initiator, message_4 MUST be supported and MUST be used.”
> 
> So, indepently of whether message_4 is used or not, there is a fourth message going from Responder to Initiator, and the size of the MAC on that message adds to the security against online attacks.
> 
> Clarifications are added, see PR #407.
> 
> 
> 
> The reference to [I-D.ietf-cose-x509] should be updated to RFC 9360
> 
> GS: Reference fixed in PR #407
> 
> I feel appendici are "nice to have" or "additional background"
> materials which can be skipped by implementers. That does not seem to
> be the definition used here. I'm not sure why Appendix G (EDHOC Message
> deduplication) is an "appendix" and not a regular section of its own? Same
> for Appendix D, E and I.  Since there are 2119 keyword in there, it does
> not feel like appendix material.  Also a definition of EDHO_KeyUpdate
> seems worthy of a real section instead of an appendix.
> 
> GS: I think appendix G can make sense to move to the body (done in PR #407) since it defines application layer functionality, although examples are given with CoAP, which is introduced in Appendix A.
> 
> The authentication discussion is intentionally divided between section 3.5 and Appendix D. The introductory text of 3.5 (“3.5.0”) describe the authentication related verification in scope of the protocol and leaves other parts to the appendix. This split addresses a previous comment to make the authentication related text in the body more concise.
> 
> A similar split for similar reasons is made between section 3.8 and appendix E, where section 3.8 contains the specification of EAD and appendix E contains further details and examples.
> 
> EDHOC_KeyUpdate was introduced as a component to enable forward secrecy with the help of static context in the endpoint. Other functionality enabling this property for OSCORE has in parallell been defined in CORE, https://datatracker.ietf.org/doc/draft-ietf-core-oscore-key-update/ which applies also to the case also when EDHOC is not used to key OSCORE.  EDHOC_KeyUpdate needs to be complemented with a protocol for how to change keys in a synchronized way between Initiator and Responder. Considering there is no current user of the key update this was not considered to be worth the effort to specify that in this draft. So the current Appendix I is a stub. With this in mind the options I see is to either keep the appendix as is or remove the text from the draft.
> 
> 
>          IANA has added the media types "application/edhoc+cbor-seq" and
> 
> This is the only line that is not "requesting IANA", but has already been changed
> to IANA has added" but their IDs as TBD, so this can't have already happened. Change
> to "IANA is requested" to not confused IANA.
> 
> GS: Done
> 
> 
> 
> NITS:
> 
> Section 5.1.2 KMAC is undefined, point to RFC 8702 or NIST SP800-185 directly?
> 
> GS: Reference included
> 
> It is unclear what h'' is, until you search and find the Appendix A.1. Can this
> be explained on its first use ? (or if it is super obvious to CBOR people, I guess
> ignore my nit)
> 
> GS: Explanation included
> 
> 
> The terms Master Secret and Master Salt are unfortunate. We are trying to not
> use the term "master" anymore. I guess because of the relationship to OSCORE
> using the term, it is too late now to change this.
> 
> GS: Yes, this is something to change when OSCORE is revised.
> 
> 
>          ID_CRED_I and ID_CRED_R are used to identify and optionally
>          transport the credentials of the Initiator and the Responder,
>          respectively.
> 
> Isn't it transporting the "identity and/or credential" (not necc a credential) ?
> 
> GS: The raison d’être of ID_CRED_x is to help the receiver to retrieve CRED_x. The term “identity” is used for several purposes in the draft, but not to describe the ID_CRED fields, which contain COSE header maps. ID_CRED_x may e.g. contain CRED_x by means of the COSE header x5chain, i.e. ID_CRED_x transports an X.509 certificate (chain). COSE also support e.g. the use of URIs (x5u) pointing to CRED_x. The latter example does not really match the intuition of “identity”. So the ID_CRED field is not necessarily the identity but can be used by the receiver to identify and optionally transport the credentials. Can we explain this better?
> 
> 
>          the transport is responsible, where necessary, to handle:
>          [ ...]
>          * message duplication
> 
>        I think you mean "message duplication detection" ?
> 
> GS: Detection is perhaps a bit restrictive, CoAP not also only detects but handles message duplication (by means of Message Deduplication, see Section 4.5 of RFC 7252).
> 
> Did I get the comment right that you want to describe the solution (e.g. message deduplication) rather than the problem (e.g. message duplication)? Note that the list is somewhat inconsequential and includes both problems and solutions, so more changes are needed if we insist on making this homogenous.
> 
> 
> CRED_x is not defined. I assume it means "CRED_I or CRED_R", but I
> think it should be spelled out exlicitly. Simiarly, ID_CRED_x.
> 
> See also this:
> 
>          Any cryptographic algorithm used in the COSE header parameters
>          in ID_CRED
> 
> Here ID_CRED and not ID_CRED_x is used?
> 
> GS: Right, there is some room for clarification here. The PR now it introduces CRED_x, ID_CRED_x, and “ID_CRED field”, see 827f730.
> 
> Section 3.6 contains two definitions of ciphersuites that are sort of
> different?
> 
>          An EDHOC cipher suite consists of an ordered set of algorithms
>          from the "COSE Algorithms" and "COSE Elliptic Curves" registries
>          as well as the EDHOC MAC length.
> 
> versus:
> 
>          An EDHOC cipher suite consists of the following parameters:
>          * EDHOC AEAD algorithm
>          * EDHOC hash algorithm
>          * EDHOC MAC length in bytes (Static DH)
>          * EDHOC key exchange algorithm (ECDH curve)
>          * EDHOC signature algorithm
>          * Application AEAD algorithm
>          * Application hash algorithm
> 
> These two do not exactly match.
> 
> 
> GS: It wasn’t clear to me what the issue is. The first text is an overview, the second is a listing of the exact content. The overview matches the listing:
> 
>    *   Each cipher suite is represented by a integer label from the IANA "COSE Algorithms" registry: EDHOC AEAD & hash algorithms, application AEAD & hash algorithm, and EDHOC signature algorithm.
>    *   The EDHOC key exchange algorithm is represented by a label from the IANA "COSE Elliptic Curves" registry.
>    *   Finally, the EDHOC MAC length is measuring the in bytes.
> Is it better if we remove the overview?
> 
> 
>          Padding is intended to be discarded by the receiving application.
> 
> I find "is intended" a bit weird, as if there are other possible usages.
> Maybe: "Padding MUST be ignored and discarded by the receiving application.
> 
> GS: Done
> 
> 
> Links to other docs (drafts?) sometimes link directly to the .txt format
> instead of the "main" page of the document, eg in:
> 
>          or in X.509 certificates identified by a hash value using 'x5t'
>          [I-D.ietf-cose-x509].
> 
> and in:
> 
>          see Section 2 of [I-D.ietf-lwig-security-protocol-comparison].
> 
> It would be nicer to link to the main page of the RFC on rfc-editor.org<http://rfc-editor.org>
> (or datatracker)
> 
> GS: Both these refs are updated. (Don’t know why .txt was presented.)
> 
>          The connection identifiers do not have any cryptographic purpose
>          in EDHOC except facilitating the retrieval of security data
>          associated with the protocol state.
> 
> I think the use of "except" here is wrong, maybe:
> 
>          The connection identifiers do not have any cryptographic purpose
>          in EDHOC and only facilitate the retrieval of security data
>          associated with the protocol state.
> 
> GS: Done
> 
> 
>          demultiplex EDHOC messages from other types of messages
> 
> s/demultiplex/demultiplexing
> 
> GS: Done
> 
> 
>          and the protocol MUST be discontinued.
> 
> It's the protocol negotiation that MUST be discontinued :)
> 
> GS: I’m not sure I understand the problem (refers to this and related comments below).  I think “protocol negotiation” may be slightly confusing, since it gives the impression that a protocol is being negotiated, rather than a protocol being excuted, which I guess is what you are referring to. How about “the EDHOC protocol MUST be discontinued”, does that make more sense? I changed in the 5 occurrences of your quoted text. (An alternative formulation is “the EDHOC session MUST be discontinued” here and below.)
> 
> 
> 
>          Section 6.3.1 of [RFC9053]
> 
> Could provide a reference directly into section 6.3.1 of 9053
> 
> GS: Done
> 
> 
>          5.1. Message Processing Outline
>          This section outlines the message processing of EDHOC.
> 
> If you change the section title to "EDHOC Message Processing Outline", the
> first sentence can be deleted.
> 
> GS: Done
> 
> 
>          an error message is sent, the protocol is discontinued
> 
> The "protocol negotiation" is discontinued.
> 
> GS: Changed to: the “EDHOC protocol” is discontinued.
> 
> 
> 
>          the protocol is completed
> 
> Maybe here too, the "protocol negotiation" is completed. Or otherwise perhaps
> "protocol has established", or "EDHOC is established" ? Or "protocol exchange" is completed?
> 
> GS: Here I think “the EDHOC protocol is completed” makes sense.
> 
> [there are more instances of "protocol discontinued" etc]
> 
> GS: Changed to “EDHOC protocol discontinued” etc
> 
> 
> 
>          the attacker must have continuous interactions with the
>          collaborator, which is a significant complication.
> 
> Maybe "significant sustained attack" instead of "significant complication" ?
> 
> GS: Done
> 
> 
>          IND-CCA confidentiality
> 
> What is IND-CCA ?
> 
> GS:. Indistinguishability under chosen ciphertext attack. Explanation added.
> 
> OLD
> HKDF-Expand is not often used as a stream cipher as it is slow on long messages, and most applications require both IND-CCA confidentiality as well as integrity protection. For the encryption of message_2, any speed difference is negligible, IND-CCA does not increase security, and integrity is provided by the inner MAC (and signature depending on method).
> 
> NEW
> HKDF-Expand is not often used as a stream cipher as it is slow on long messages, and most applications require both confidentiality with indistinguishability under chosen ciphertext attack (IND-CCA) as well as integrity protection. For the encryption of message_2, any speed difference is negligible, IND-CCA does not increase security, and integrity is provided by the inner MAC (and signature depending on method).
> 
> 
> Göran
> 
>