tsvwg / ietf-multipath-dccp

https://datatracker.ietf.org/doc/draft-ietf-tsvwg-multipath-dccp/
8 stars 13 forks source link

ART-ART review #316

Open markusa opened 1 month ago

markusa commented 1 month ago

This is a copy from: https://datatracker.ietf.org/doc/review-ietf-tsvwg-multipath-dccp-17-artart-lc-housley-2024-10-04/

Reviewer: Russ Housley Review result: Almost Ready

I am the assigned ART-ART reviewer for this draft. Please treat these comments just like any other last call comments.

Document: draft-ietf-tsvwg-multipath-dccp-17 Reviewer: Russ Housley Review Date: 2024-10-04 IETF LC End Date: 2024-10-17 IESG Telechat date: Unknown

Summary: Almost Ready

The document is very well written. Thanks for the significant effort.

Major Concerns:

Section 3.2.4: I find the notation confusing: hostA d-key(A)=(key-a+key-b). Further, the use of "-" as part of the variable name is easy to confuse with a math operator. I suggest that the paragraph be reworded to show how to compute key_d_a and key_d_b, which also avoids the key names looking like functions. Maybe:

Key Material is exchanged in plain text between hosts, and the key parts (key_a, key_b) are used to generate the derived key (key_d) by concatenating the two parts with the local key in front. That is, key_d_a=key_a+key_b, and key_d_b=key_b+key_a.

If you accept this comment, then you might define the following:

Section 3.2.4: Is one of the Key Type mandatory-to-implement? I realize that does not make that Key Type mandatory-to-use.

Section 3.2.6: The "Key" used for the HMAC computation is the derived key (d-key) described in Section 3.2.4. This statement is ambiguous. In the proposed rewording of Section 3.2.4, the two keys values are key_d_a and key_d_b. Which one is used here? The answer appears in the bullets that follow, but it would be more clear to say it one time before the bullets. Further, both Hast A and Host B need to "perform" the HMAC-SHA256 calculation. One host is doing it to compute the value to include in the datagram, and the other is doing it to check the value that was received.

Section 3.2.6: The text says:

... In the event that an MP_HMAC cannot be associated with a suboption, unless it is an MP_HMAC sent in DCCP-Ack in response to a DCCP-Response packet containing an MP_JOIN option, this MP_HMAC MUST be ignored.

This text begs for an explanation of MP_HMAC sent in DCCP-Ack in response to a DCCP-Response packet containing an MP_JOIN option. If a sentence will not cover it, then please add a pointer to the part of the document that discusses this situation.

Section 3.6: The text says: " (if this is not possible it MUST be closed)." Please reword. Please do not burry the MUST statement in a parenthetical.

Section 4: Please add a paragraph that describes the consequences for choosing the Plain Text Key Type over ECDHE-C25519-SHA256 or ECDHE-C25519-SHA512.

Minor Concerns:

The IANA registry indicates that Feature Number 10 was given a temporary assignment. I expected to see a similar temporary assignment for Option Type 46. Maybe this document will be published as an RFC before that can happen, but it seems prudent to make the assignment.

Section 3.1: The text says:

Unlike the example in Figure 4, this document only allows the negotiation of MP-DCCP version 0, which means that client and server must support it.

I think it would be more clear to say:

Unlike the example in Figure 4, this document only allows the negotiation of MP-DCCP version 0. Therefore, successful negotiation of MP-DCCP as defined in this document, the client and the server MUST both support MP-DCCP version 0.

Section 3.2.4: s/certificate authority/certification authority (CA)/ Also, you may want to reference RFC 5280 for a definition of a CA.

Section 3.2.8: The text says:

... Note that an implementation MAY discard incoming address advertisements - for example, to avoid the required mapping state, or because advertised addresses are of no use to it (for example, IPv6 addresses when it has IPv4 only). Therefore, a host MUST treat address advertisements as soft state, and the sender MAY choose to refresh advertisements periodically.

The nesting of examples makes this paragraph very difficult. First, please separate the MUST statement and the MAY statement. Then, offer examples to explain the reasons an implementation might choose to discard incoming address advertisements or refresh advertisements periodically.

Section 3.2.9: Different names are used for the inputs to compute the HMAC key in this section. Please be consistent.

Nits:

General: Some sections use "Host A" and others use "host A". Please pick one capitalization convention and use it throughout the document.

General: Please use "bytes" (not "Bytes") when discussing the length of a field or message.

General: Please use "bytes" or "octets" when discussing the length of a field or message. I have a mild preference for octets, but consistency is desired.

Section 1.2: Please consider adding a definition of 4-tuple.

Section 2: There is something odd with the line breaks in the two paragraphs.

Section 2.1: s/Hosts A and B, respectively./Hosts A and B./

Section 2.1: s/The two subflows continues/The two subflows continue/

Section 3.2.1: s/MP_SEQ Section 3.2.5/MP_SEQ; see Section 3.2.5/

Section 3.2.2: s/packet (See Section 3.2.6 for details)/ /packet; see Section 3.2.6 for details/

Section 3.2.2: s/needing to know what the source address at the receiver is/ /the need to know the source address at the receiver/

Section 3.2.7: s/down- and uplink/down-link and up-link/

Section 3.2.9: s/MP_SEQ Section 3.2.5/MP_SEQ (Section 3.2.5)/

Section 3.2.9: s/server, respectively on the affected subflow(s) (if possible)/ /server on the affected subflow(s), if possible)/

Section 3.2.10: I expected a paragraph for each example since "Example use cases include:" is in a paragraph of its own.

Section 3.2.10: s/path values 3-15 depends/path values 3-15 depend/

Section 3.11.2: Please add white space between the adjacent paragraphs.

Section 5: s/Section 4/Section 4./