Re: [IPsec] Artart last call review of draft-ietf-ipsecme-rfc8229bis-06
Hi Mark, thank you for your review! Regards, Valery. > -Original Message- > From: Mark Nottingham via Datatracker [mailto:nore...@ietf.org] > Sent: Sunday, May 22, 2022 7:12 AM > To: a...@ietf.org > Cc: draft-ietf-ipsecme-rfc8229bis@ietf.org; ipsec@ietf.org; > last-c...@ietf.org > Subject: Artart last call review of draft-ietf-ipsecme-rfc8229bis-06 > > Reviewer: Mark Nottingham > Review result: Ready > > I have reviewed this document and found it well-written and reasonably easy to > understand (although I lack expertise in the subject matter). > > I did not identify any intersection with ART area concerns or standards. ___ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
Re: [IPsec] Tsvart last call review of draft-ietf-ipsecme-rfc8229bis-06
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 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? > 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). > 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. > 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'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... > 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. > 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
Re: [IPsec] ESP Signally to higher layers
Thanks Chris. Helps a bit. On 5/20/22 20:27, Christian Hopps wrote: Robert Moskowitz writes: This is an item that goes back to the beginning of ESP work: Minimally, how does the higher level 'learn' that it is secure: E2E or TE2TE? Encrypted/Authenticated/CrCed... ? And as ESP has a seq#, how might it be convied to the higher layer? Case in point: MAVlink has a 1-byte seq# in its payload. How might this be provided by ESP? https://mavlink.io/en/guide/message_signing.html So I have been thinking about this vis-a-vis diet-esp. What is the mechanism/trigger that can best work across a number of higher layers to inform of operating environment and values available (seq#)? Is this done anywhere now? If you're asking for a generic API mechanism in unix, for datagrams it would be recvmsg. Recvmsg uses a msghdr which can include control data (cmsghdr). That is the way that lower layer information associated with packets is passed up to the application. man recvmsg man cmsg I don't know if any ESP data is currently passed with this method though. Thanks, Chris. Bob ___ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec ___ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec ___ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
Re: [IPsec] ESP Signally to higher layers
On 5/21/22 07:13, Michael Richardson wrote: Robert Moskowitz wrote: > This is an item that goes back to the beginning of ESP work: > Minimally, how does the higher level 'learn' that it is secure: Are you asking how *TCP* learns of this, or how an application with an open socket(2) learns of this? App TCP or UDP. > Encrypted/Authenticated/CrCed... ? > And as ESP has a seq#, how might it be convied to the higher layer? Do you mean replay counter here, or did you mean SPI? SPI SHOULD have no value to the higher layer. It is the actual Seq # that may be of value. Preferably, never, because it will get rekeyed, so really, whatever you want to do really needs to be communicated abstracted to the key daemon, who will do the right thing, and keep track of updates to the SPI# > Case in point: MAVlink has a 1-byte seq# in its payload. How might > this be provided by ESP? Now I think maybe you really do mean sequence/replay counter. Yes. > https://mavlink.io/en/guide/message_signing.html > So I have been thinking about this vis-a-vis diet-esp. What is the > mechanism/trigger that can best work across a number of higher layers > to inform of operating environment and values available (seq#)? > Is this done anywhere now? Doubtful. MAVlink does its own seq# processing, so if I squeeze it out in transporting the MAVlink packet, I need to rebuild it when passing it up to MAVlink. Now a formal SCHC layer SHOULD be able to do this. One would think. There are other layer 5 protocols with Seq #. Like RTP. ___ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
Re: [IPsec] [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 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 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. >> 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. >> 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. > 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... > >> 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. > >> Section 7.1 me