Hi Joe, On Sat, Nov 17, 2018 at 05:39:22PM -0800, Joseph Touch wrote: > Reviewer: Joseph Touch > Review result: Ready with Issues [reformatted for readability] > Transport issues: > - The GEN requirements refer to signal and data channels; it should be more > clear that these would be transport associations or connections, not > necessarily separate port assignments > > - GEN-03 recommends transports that avoid > HOL blocking, but that blocking can occur at any protocol layer (e.g., when > using TCP as a tunneling layer or at the application layer) Is the concern about "conveyed over a transport not susceptible"? I don't think that one needs to read "transport" as "lowest level transport protocol" here and it would equally apply to any tunneling layer. > - SIG-001 – PLPMTUD > should be used instead of PMTUD; PMTUD (relying on ICMP, which is often blocked > and remains insecure) should be avoided. The PMTU of 1280 is relevant only for > IPv6. The use of 576 should be more clearly indicated as applying only to IPv4. > (note there is emerging PLPMTUD for UDP). The IPv4 text is phrased as it currently is since there may be tunneling scenarios that go over IPv4 at some intermediate link even if the endpoints are speaking IPv6. With respect to PMTU vs. PLPMTU, can you give any guidance on whether the literl 1280 and 576 values are equally valid for assumed PLPMTU as for "true" PMTU? > - SIG-004 should address the use of > TCP keepalives for TCP connections as a way to achieve heartbeats. I don't believe that there is an IETF consensus position about what layer is best for performing heartbeats; see the "statement regarding keepalives" thread. In particular, since this signal channel is going to support non-TCP transports and prefers UDP over TCP, it will need its own heartbeat mechanism anyway, at which point the TCP-layer functionality is going to be fairly redundant. > - SIG-004 is > self-contradictory, at first claiming that agents SHOULD avoid termination due > to heartbeat loss then later saying they MAY use heartbeat absence as > indication of defunct connections. Even though SHOULD and MAY can be used > together this way, the advise is confusing. This is a case (see below) where > the reasoning behind exceptions to SHOULDs are needed -- but the MAY clause is > a far too trivial (and, based on our experience with BGP, incorrect) condition Can you double-check this? My read is that there are two factors involved and we are only giving guidance on two quadrants of the truth table, with the two factors being "failing to receive heartbeats" and "attack traffic is present". (The attack traffic is assumed to be swamping other traffic, but when there is no attack traffic the heartbeats should be more reliable.) > - DATA-002 should suggest protocols for security, at least indicating whether > these need to be at a particular protocol layer (IP, transport, application), > etc. I think it's fine for a requirements document to limit itself to the actual cryptographic requirements. FWIW, the current data-channel spec is using TLS. -Ben > Other issues: > - the document uses SHOULD without qualifying the conditions for exception > > Nits > - the abstract is too brief > - Several requirements suggest that use of TCP avoids the need for separate > congestion control; the same should be mentioned of SCTP and DCCP. > >