Last Call Review of draft-ietf-core-oscore-edhoc-09
review-ietf-core-oscore-edhoc-09-secdir-lc-hardaker-2023-11-16-00
review-ietf-core-oscore-edhoc-09-secdir-lc-hardaker-2023-11-16-00
I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments. The summary of the review is: almost ready Note: this is an early review, per request In general, the document reads quiet well but I have a number of comments and suggestions below that may help if you choose to use them. Please note that I'm not an expert in CoAP and OSCORE and EDHOC, which makes anything I say questionable from the start :-) Questions/comments: - Section 2, P4: you might mention that the reverse is only usable in the classic case. I know you're describing the classic/generic case here, but it would be helpful to pre-seed the reader that this mode won't work with this draft. - Section 2, P7: you state that the server replies with 2.04 (Changed) -- is this *always* the case? Aren't there cases where an error or other response might occur? (reminder: I'm not a CoAP expert) - Section 3, "Since EDHOC message_3 may be too large..." ... "EDHOC message_3 has instead to be transported in tho CoAP payload... ". It's unclear to me reading if this is *always* the case, or only if it's too large do you do that. [note for implementation simplicity: doing it the same way either way requires less decisions to be made] - I admit confusion after reading the whole thing about where C_R, kid and the responder identifier comes from and when they're equal, etc ("is both the server's Receiptient ID (IE.e the client's sender ID) and the EDHOC Connection Identifier C_R of the server", but later "The Responder MUST choose a C_R" seem in conflict). I thought I understood it at one point (and with another read I'm sure I'd get more clarity), but it caused my secdir focused brain to think "well, if the client specifies the value and the server has to use it to distinguish between different clients, then what happens if 2 clients end up deriving/using the same value?" I think clarity about this, maybe with a solid example, would help. I was very happy to read later in the security considerations that "Even if a client did not fulfill this requirement, it would not have any impact in terms of security.", but in the end without a better understanding I remain skeptical (note: mostly that's my job as a reviewer here). - Section 3.2.1, last P: can you add a recommendation about what to do when the performance advantage might be sub-optimal, or how to detect algorithmically when this is the case? - section 3.3 P4 (not bulleted): "the server has to make sure" -- why isn't this a MUST? - section 7 bullets "This is the sum of the 64-bit MACs": IMHO, this wording doesn't match the real world math operations with the probability of attacking two MACs. I recognize that draft-ietf-lake-edhoc also says this, but I argue it's not correctly worded either. When trying to break something with two independent MACs, they're not "added" (summed) -- they're actually multiplied. - section 7: additionally, the 128-bit minimum is only assured with the current defined algorithms. I doubt future algorithms will be standardized for use that will be weaker, but the wording could be improved by saying "given the currently defined algorithms" or something to explain this "feature" is actually dependent on the IANA registry contents, eg. - section 8.3: the registry is defined with ranges from -65536 to 65535, which I note two issues with: A) no where does the section actually state this is the full range (even though the text assignment ranges implies it), and B) this space is actually 2^17 in size which seems odd. If you're going to use negative values and want space that fits into 16 bits, then the space should be -32768 to 32767. Nits/suggestions: - Abstract: it would be helpful to start the abstract with the purpose before the rest; simply move the third sentence to the first (with a bit of rewording) and it will add clarity (IMHO). - introduction, paragraph 2 (P2), sentence 1 (S1): add "with CoAP" after "the EDHOC protocol" - For figure 1 and 2, the messages contain some things that are actualy headers ("Content-Format:") and others that are labels ("Header:", "Payload:"), which is kind of confusing to read. A naive reader might actually expect "Payload:" to be in the sent message. - Figure 1, EDHOC Response: C_I is missing here. In general, please review all the messages of both Figure 1 and 2 for whether and where C_I and C_R should be included. It's a bit confusing because it might be contained within a sub-element but is listed anyway for clarity, etc, but IMHO it should be consistently put in or omitted in each message based on some formula (included or explicit). EG, C_R is technically inside EDHOC message_3 (if I read the draft right), but yet is explicitly listed as if it's separate in the payload. This may be intentional but if so then I think C_I should also be listed in the message 2 flow (EDHOC response). Same with figure 2's EDHOC response and EDHOC + OSCORE request. - Figure 2, EDHOC + OSCORE request: mention the option(s) in the payload? It might be helpful to indicate to the reader that this is where the various options go since you're talking about them. - Section 3.2 bullet 3: I note that COMB_PAYLOAD is not yet mentioned and thus could use a brief definition or maybe just a reference to the normative specification. - section 3.2: I'd be tempted to put the entire beginning of 3.2 into a subsection 3.2.1, which would make referencing it in 3.2.1 (which would become 3.2.2) less confusing and circular. You'd likely need a short intro before both sections though, but that should be easy enough. Same comment for Section 3.3 and 3.3.1. - section 3.2, bullet 5: it seems odd that this is near the last step instead of the first (IE, you're adding the signal long after doing everything to support it). Similarly, the last bullet in section 6 seems like it should really be the first bullet. - section 3.4 note to the editor: I don't understand why this note is here or why you'd want to delete the last bullet. - section 3.4, description for figure 4: it would be helpful to specify where a few of the other values come from (CBOR encoding), such as the 93 and ff values. TL;DR: some elements of your example get encoded in ways that don't match the description (eg, EDHOC option value) and could use an explanation. - The separation between 3.1 and 4.1 confused me a bit, I would have put them together but this stylistic. - 4.1.3: It seems weird that this is worded as a MUST with a bullet as opposed to a MUST NOT. I'd change the bullet into a MUST NOT and put it as the first sentence maybe. -- Wes Hardaker USC/ISI