Hi Joseph, First, thanks for your review. This is an initial reply to clarify a couple points [inline].
> On Dec 4, 2020, at 1:26 AM, Joseph Touch via Datatracker <nore...@ietf.org> > wrote: > > Reviewer: Joseph Touch > Review result: Not Ready > > This documents most significant transport issue is the use of path MTU > discovery. Sec 2 recommends the use of path MTU discovery [RFC1191] [RFC8201], > but this is known to be problematic black holing where ICMPs are blocked or > filtered (which should be noted everywhere PMTU is suggested, including Sec > 4.2). It would be more appropriate to incorporate PLPMTUD [RFC4821] [RFC8899], > especially in conjunction with congestion control, as both assume > bidirectional > stateful ingress/egress coordination. This is particularly important given the > sending side’s prerogative to change the size of the tunnel EMTU_S (see > below). Thanks, I will look into adding text on this. An implementation can use receipt of the time echo in the CC header as an ACK of receipt of a tunnel packet that included that time val so probing using this method could be suggested as well. For non-CC the user is expected to be in control of the path domain (otherwise it wouldn't be able to correctly set the fixed send rate) so no PMTU (or simple PMTU if they really wish to use it -- unlikely) should suffice. > The most significant concern is not at the transport layer – it is the > assumption of in-order delivery and insufficient consideration of the impact > of > loss at the network layer. Loss could orphan received fragments – for which a > timeout should be recommended. Loss or reordering could generate faulty > reassembly at the egress – which is actually very likely here, e.g., when a > short packet is followed by a sequence of packets that are exactly the tunnel > MAP (as defined in draft-ietf-intarea-tunnels). As noted below, persistent > fragmentation could make this situation worse, esp. in the presence of > frequent > reordering or loss. In-order deliver is not assumed. Correct reordering is required, and is done on ingress by IPTFS through use of the ESP sequence number. IPsec/ESP includes a sequence number and a window for anti-replay protection. This also provides for loss detection (as the sliding window moves) allowing for reaping of orphaned fragments. Perhaps we should add a bit of text highlight this though? (FWIW my test suite for our implementation of IPTFS includes a fairly comprehensive set of tests of out-of-order delivery including drops. :) > Other significant issues, largely at the network/tunnel layer: > > There is no clear utility in having the blockoffset point past the end of the > current packet. It serves – at best – as only a partially useful check on the > next packet. I.e., if the two blockoffsets disagree, presumably a packet is > lost – but if they agree, it cannot be concluded that a packet is not lost. It > is sufficient that it points to the end of the tunnel packet. No harm either though, right? Having implemented this, I can tell you that it does help detect bugs in ones code while in development. :) > Although the mechanism allows for padding, it appears too aggressive in trying > to be work-conserving. Consider a tunnel that could support 1000B payloads; if > a stream of packets comes in as 200B, 1000B, 1000B, 1000B, etc. (more 1000B > packets), EVERY 1000B would be fragmented, which means loss of a single tunnel > packet would cause two inner packets to be incomplete. The protocol should > allow padding in some cases to avoid fragmentation, e.g., every 100th packet > it > might allow insertion of padding rather splitting a packet across the stream. > The heuristic shown here is just an example. This is allowed by the protocol. There is no restriction specified on when an implementation can insert padding in the tunnel packet stream. In fact unless the tunnel is being 100% utilized the expectation is that it will be sending extra padding often -- that being one key to the traffic flow security. Your suggestion I think is that it could be beneficial to consider reducing the available bandwidth by forcing padding to align packets even if in the presence of an offered load >= 100% of the tunnel capacity . Again, this is certainly allowed, but when and if to do this seems to be a great topic for research, and would be outside the scope of this initial protocol specification I think. Do you think we should add text explicitly mentioning that one could do this if one wanted to though? > Fig 1 shows the blocksize as always occurring in the latter half of a word; is > this always the case? If so, it would be useful to indicate the left side of > that word as “ESP – con’t”. If not, other examples should be provided. The "..." is just omitting the details of other IPTFS fields which are normatively specified in section 6. The intent was to give an overview of what is being discussed in the text below, but not normatively define the header format. > At the end of Sec 2.2, the blockoffset helps recover the next inner packet, > but > the term “full” in that sentence implies the inner packet is entirely inside > that outer packet (which it may not be). will remove "full". > The option of congestion control and the claim of “unidirectional” should be > discussed more consistently (i.e., mention the need for bidirectional channels > when mentioning thec claim of unidirectionality). Will do. > Like all tunnels, this approach needs to more clearly indicate a number of > different MTU values and how they interact [draft-ietf-intarea-tunnels], both > for the underlying network and the tunnel provided: - EMTU_S - > EMTU_R - Path MTU - Link MTU > > It may also be useful to use the terms developed in that doc, e.g., tunnel > link > packet (the packet that traverses a tunnel) as well as inner vs. outer > fragmentation, tunnel maximum atomic packet, etc. > > There are a number of other tunnel considerations that should be addressed, > again as discussed in draft-ietf-intarea-tunnels. These include: - The > impact of tunnel traversal on the inner hopcount/TTL (there should be none, as > per that doc; hopcount should be adjusted by routers, not tunnels) - > Impact of errors in the tunnel on the ingress - Specification of the > EMTU_R of the tunnel itself, and how that is determined - What the > ingress should do if an incoming packet exceeds EMTU_R - It should be > noted that this is a unicast tunnel - What you expect if there are > multiple paths between the tunnel ingress and egress - Does the tunnel > itself have a flow ID? (if the outer packet is IPv6) If so, is it fixed or > intended to vary arbitrarily (and if so, how)? - If the outer packet is > IPv4, do you expect to use DF=1? If not, how are you handling ID issues in RFC > 6864? If so, it might be useful to add a reminder that the ID can be anything > (including constant, which may be useful in avoiding a covert channel). Will go back through the document keeping this in mind. Thanks again, Chris.
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec