Hi, Valery, Thanks - I will confirm when the update is posted so this can be closed out.
Joe — Dr. Joe Touch, temporal epistemologist www.strayalpha.com > On May 24, 2022, at 4:27 AM, Valery Smyslov <s...@elvis.ru> wrote: > > Hi Joe, > > please see inline. > > Regards, > Valery. > > From: to...@strayalpha.com <mailto:to...@strayalpha.com> > [mailto:to...@strayalpha.com <mailto:to...@strayalpha.com>] > Sent: Monday, May 23, 2022 6:14 PM > To: Valery Smyslov > Cc: tsv-art; draft-ietf-ipsecme-rfc8229bis....@ietf.org > <mailto:draft-ietf-ipsecme-rfc8229bis....@ietf.org>; ipsec@ietf.org > <mailto:ipsec@ietf.org>; last-c...@ietf.org <mailto:last-c...@ietf.org> > Subject: [***SPAM***] Re: [Tsv-art] Tsvart last call review of > draft-ietf-ipsecme-rfc8229bis-06 > > Hi Valery, > > Notes below. > > Joe > > > On May 23, 2022, at 4:53 AM, Valery Smyslov <s...@elvis.ru > <mailto:s...@elvis.ru>> wrote: > > Hi Joseph, > > thank for your review, much appreciated. More inline. > > > Reviewer: Joseph Touch > Review result: Ready with Issues > > This document has been reviewed as part of the transport area review team's > ongoing effort to review key IETF documents. These comments were written > primarily for the transport area directors, but are copied to the document's > authors and WG to allow them to address any issues raised and also to the IETF > discussion list for information. > > When done at the time of IETF Last Call, the authors should consider this > review as part of the last-call comments they receive. Please always CC > tsv-...@ietf.org <mailto:tsv-...@ietf.org> if you reply to or forward this > review. > > Overall, this document adds useful clarifications to the original RFC on > tunneling IPsec over TCP. There are a number of issues that should be > addressed > as it proceeds, as noted below. All can be addressed relatively directly > (i.e., > none create new open issues). > > General comments: > > The document lacks (and would benefit from) a section providing details of the > differences in this update. > > Good point. > > can add the following text at the end of 1.1: > > In particular: > o The interpretation of the Length field preceding every message is clarified > o Use of NAT_DETECTION_*_IP notifications is clarified > o Retransmission behavior is clarified > o Using cookie and puzzles is described with more detail > o Error handling is clarified > o Implications of TCP encapsulation on IPsec SA processing are expanded > o Interaction of TCP encapsulation with MOBIKE is clarified > o Section describing interactions with other IKEv2 extensions is added > o Recommendation for TLS encapsulation include using TLS 1.3 > > Is it OK? > > Sure; it might also be useful to indicate the section where each is addressed > in most detail. > > Done. > >> Figures should include captions. > > I would leave this to RFC Editor (we tried to keep RFC 8229 text when > possible, > and it doesn't have captions too). > > The RFC Editor isn’t likely to care. These can be added without changing the > intent of RFC 8229; in most cases, the caption is fairly obvious from the > text. > > I’ve added titles for the figures defining headers and prefix > formats > and removed anchors from figures in Appendix B (so they are now > “inline”). > >> Given the new document adds primarily clarifications, it would be preferable >> if >> the header numbering were not gratuitously modified vs. the original. The new >> section 2 should be demoted to 1.2 as per the original; this would go a long >> way to avoiding unnecessary confusion between the two. > > OK, makes sense. > > > Specific suggestions and concerns: > > Section 3 clarifies the meaning of the 16-bit length field as including both > the message and the message length field. This is counterintuitive and > problematic, notably because ESP messages could be up to 65535 bytes long. > This > possibility should be addressed (e.g., prohibit tunneling of messages over > 65533 bytes). > > This is a a good catch, we'll add this clarification. > > > Section 4.2 claims the length cannot be 0 or 1 bytes; again, this suggests it > might have been better to have the length field no include the length itself. > > The design decision that length field includes both the message and the > message > length was made back in 2016 when RFC 8229 was developed to align > it with 3GPP’s recommendation. We are not in a position to change this design. > > Understood. > > >> Regardless, it seems there are other lengths that are equally invalid (isn’t >> there a min ESP header size? What about the IP packet header inside)? The >> true >> min should be indicated. > > TCP encapsulation is used for IKE too and "NAT keepalive" messages > may still be sent by IKE (even they are not needed for TCP), which are 1 byte > long. > > It might be useful to mention that. > > This possibility is mentioned in Section 7.6. Added a clarification > referencing this section. > > It's a good question whether empty messages (with Length = 2) are OK. > >>> From receiver's point of view following the Postel's rule I'd simply ignore >>> them >> and don't tear down TCP... >> >> Added a clarification for receiving empty messages. >> >> >> Section 7.1 suggests closing idle TCP connections to clean up resources; this >> is inconsistent with TCP’s basic premise (don’t clean it up until those >> resources are used for a new connection). There should be a more direct >> reason >> given for this change. >> >> If a TCP connection is no more associated with any SA, then it SHOULD be >> deleted by TCP Originator. In some cases the TCP FIN/ACK messages >> will not reach the Responder (e.g. due to network problems), >> so this TCP connection will become an orphan on Responder, >> since no new traffic will ever be sent over it. >> We see no reason to waste Responder's resources in this case - this is the >> reason. >> Note, that this recommendation is MAY, you are free to ignore it. > > It might be useful to indicate that the reason is to conserve responder IPsec > resources, i.e., this is (to TCP) an application consideration, not a TCP one. > > Done. > > > Section 7.1 mentions a keep-alive; it would be useful to explain whether this > is intended to use TCP keepalives or IPsec keepalives or both. It may also be > important to indicate how these keepalives might interact. This might refer to > the more detailed discussion in Section 7.6. > > Section 7.1 talks about IKE keep-alive messages (more formally called > "liveness check" messages). > The idea is that an encrypted and authenticated message must be received, > so that the responder may learn new SPIs, so TCP keep-alives are not suited. > > Will add clarification. > > > Section 7.2 on retransmissions should explain the need for IPsec to continue > to > retransmit messages even though the transport ensures reliable delivery (e.g., > that messages could be ignored or delayed elsewhere in receiver processing). > > There is generally no need to do it (except for the situations when TCP > connection > is replaced with a new one while waiting for the response). And Section 7.2 > describes it very clear, we think. However, there is no harm if the initiator > retransmits > the request (following IKEv2 recommendations on exponential back-off between > retransmissions), > so some implementations might choose not to complicate retransmission logic > and always follow > the same pattern regardless on the transport (note, that TCP encapsulation > adds quite a lot of changes to IKE codebase, so it's a valid desire to > minimize them). > Thus "SHOULD NOT" retransmit in the first button in 7.2. > > It might be useful to indicate this rationale there. > > Done. > > > Section 7.7 discusses the relation of the IPsec DFs to the outer TCP DFs. As > with all tunnels, there need be no direct relation. The outer TCP header acts > as a link layer protocol and its frag/reassembly need have no correlation to > the inner payload. Additionally, it is nonsensical to relate the two for TCP > as > a tunnel because it does not preserve message boundaries of the carried IPsec > traffic anyway. It might be useful to mention this, rather than indicating > this > as “not possible”. (i.e., even if it were possible, it would be incorrect to > do > so). > > We believe that is exactly what Section 7.7 1-st bullet says. > Do you want some specific text to be added here? > > It might be useful to indicate that you’re doing what tunnels (not IPsec > tunnels) should be doing, not creating a new solution for this specific > approach. > > Added a clarification. > > > (Note, that we are not in a position to discuss generic considerations > of using tunnels, we just explain that what is required > by RFC 4301, is not possible with TCP. We don't want to discuss > here whether RFC 4301 requirements are wrong). > > RFC4301’s tunnels are not the tunnel you’re describing here; agreed that > those tunnel considerations are not relevant. > > >> Section 10.1 indicates problems with TCP-in-TCP. It would probably be useful >> to >> provide a citation with better treatment of this issue (e.g,, >> https://www.spiedigitallibrary.org/conference-proceedings-of-spie/6011/1/Understanding-TCP-over-TCP-- >> >> <https://www.spiedigitallibrary.org/conference-proceedings-of-spie/6011/1/Understanding-TCP-over-TCP--> >> effects-of-TCP-tunneling-on/10.1117/12.630496.short?SSO=1, >> https://link.springer.com/chapter/10.1007/978-981-13-3329-3_32 >> <https://link.springer.com/chapter/10.1007/978-981-13-3329-3_32>). This is >> more >> commonly referred to as “TCP meltdown”; bufferbloat is a different phenomenon >> with a different cause (in-network queuing that relies on tail-drop and/or >> lacks ECN), and does not appear relevant to the issues presented in this >> section. > > We'd be happy to include good references, but It seems that both articles are > behind paywalls, > or at least require some subscription to be able to read. Do you have some > free references? > Or probably you can craft a proper text to be included in this section? > > Being behind a paywall does not disqualify a document as an appropriate > citation, especially when that is the primary source. > > Here’s from/to text: > > FROM: > TCP-in-TCP can also lead to increased buffering, or bufferbloat. > This can occur when the window size of the outer TCP connection is > reduced and becomes smaller than the window sizes of the inner TCP > connections. This can lead to packets backing up in the outer TCP > connection's send buffers. In order to limit this effect, the outer > TCP connection should have limits on its send buffer size and on the > rate at which it reduces its window size. > TO: > > TCP-in-TCP can also lead to “TCP meltdown”, where stacked instances > of TCP can result in significant impacts to performance [Ho05]. > For the case in this document, such meltdown can occur when the window > > size of the outer TCP connection is smaller than the window sizes of the > inner TCP > connections. The resulting interactions can lead to packets backing up in > the outer TCP > connection's send buffers. In order to limit this effect, the outer > TCP connection should have limits on its send buffer size and on the > rate at which it reduces its window size. > > Here’s the bibtex: > @inproceedings{10.1117/12.630496, > author = {Osamu Honda and Hiroyuki Ohsaki and Makoto Imase and Mika Ishizuka > and Junichi Murayama}, > title = {{Understanding TCP over TCP: effects of TCP tunneling on end-to-end > throughput and latency}}, > volume = {6011}, > booktitle = {Performance, Quality of Service, and Control of Next-Generation > Communication and Sensor Networks III}, > editor = {Mohammed Atiquzzaman and Sergey I. Balandin}, > organization = {International Society for Optics and Photonics}, > publisher = {SPIE}, > pages = {138 -- 146}, > keywords = {TCP, TCP over TCP, TCP tunnel, Performance Evaluation, Goodput, > Round-Trip Time}, > year = {2005}, > doi = {10.1117/12.630496}, > URL = {https://doi.org/10.1117/12.630496 <https://doi.org/10.1117/12.630496>} > } > > Thank you for the text! > >>> Section 11 (security considerations) mentions the new vulnerability >>> introduced >>> by the outer TCP layer, but only DoS via SYN-flooding. This connection is >>> also >>> susceptible to RST and other spoofing attacks attacks (RFC4953), which >>> should >>> be noted as well. Data injection attacks are not possible, but all the rest >>> of >>> the TCP machinery remains vulnerable. >> >> Good point, will add clarification and cite RFC 4953. >> >> Regards, >> Valery. >> >> _______________________________________________ >> Tsv-art mailing list >> tsv-...@ietf.org <mailto:tsv-...@ietf.org> >> https://www.ietf.org/mailman/listinfo/tsv-art >> <https://www.ietf.org/mailman/listinfo/tsv-art> > > -- > last-call mailing list > last-c...@ietf.org <mailto:last-c...@ietf.org> > https://www.ietf.org/mailman/listinfo/last-call > <https://www.ietf.org/mailman/listinfo/last-call>
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
