Hi Tero, I have published a new version of the draft which I believe addresses the issues you raise in your WGLC readiness review below. As such, I'd like to request the document enter WGLC.
Changes indicated inline below. > On Jan 17, 2021, at 4:27 PM, Tero Kivinen <kivi...@iki.fi> wrote: > > First, thanks for the Joe for very good TSV review for this draft. > > I am now going through this draft to see whether it is ready for WGLC, > and while reviewing it I have some comments on it. On the other hand I > think it is starting to get ready for WGLC... > > In section 2.2.3 fragmentation etc, the draft talks about replay > protection, but in the ESP replay attack protection is optional, and > does not need to be done by the receiver, altough the sequence number > generation MUST be done by sender. Modified the text to include "optional" and make more clear we are only referring to the window method that is commonly used for this. > Also normally in ESP seqeuence numbers are not required to be back to > back, there could be cases where they would be incrementing by larger > value, and cases where the sequence numbers are not sent out in order. > This can happen for example if multiple encryption boards are used, > and each of them have separate coordinated sequence number generation. > This can also cause reordering in some cases. The draft now says that > "sender MUST Send the inner-packet fragments back-to-back", which > adds a new requirement for ESP. It might be good idea to add note, > that says this changes requirement from RFC4303, which only requires > that sequence numbers are incremented, not by how much (for example > draft-ietf-lwig-minimal-esp proposes that in some enviroments it might > be better to use time for sequence number generation so they do not > need to store sequence number while sleeping). Added paragraph on new "incrementing sequence numbers by one" requirement. > Section 2.2.3 does not change the replay protection parts, just talks > about that similar method than what is used in the replay protection > can be used for reassembly. This may cause issues if the ESP level > does not do replay protection, but the reassembly code assumes it is > done. The text also seems to indicate that the window sizes for the > reassembly and replay protection could be different. The lack of the assumption is also more clear with the additional text about the window sizes when both IP-TFS and replay protection are in use. > If reassembly and replay windows are not same, I think the smaller one > of them is used, as if replay window is smaller, then too old packets > are dropped before they would reach the larger reassembly window, and > same happens for other way around. Added text to point this out. > I think we could just say that when using this detection of replays is > mandatory and that replay window and the reassembly window are same. Continued to leave the option to use replay protection or not to the user. This change isn't required so I'd rather leave it as optional as the base ESP spec does. > Also note, that increasing replay window size does not have that much > effect on the memory consumption of the SA, but increasing reassembly > window size does have. I.e., if the quite often used 1024 packet > replay window size is matched with same sized reassembly window of > 1024 * 1024 byte packets, that will mean each SA could consume up to 1 > MB for reassembly window. > > Also even when the 2.2.3 says in that there is no need for timers, I > disagree with that, as during rekey or other operations it might be > possible that the traffic from the SA is moved to the newly created > SA, and then few minutes later the old SA is deleted. Keeping the > reassembly buffers for that long just because there is no incoming > packets is just wasting data. Added a more text saying timers are OK and could save some memory. > In section 2.2.2 and 2.2.3.1 the draft tries to fill the packets to > full, even when there would only be one byte fitting on the frame, and > I think this is bad idea. I would think it would be better to say that > if less than 5% (or some other fixed configurable amount) of the > packet is available and the packet to be added would be fragmented, > then packet is padded instead of fragmenting, and the packet to be > added is moved to next outer packet. > > I think there is too much emphasis on not wasting the bandwidth and > too much emphasis on the robustness of the protocol. Small amount of > padding that will avoid fragmentation is much more desirable than not > wasting few % of the bandwidth. Added sentence describing a method for minimum fragment size configuration as well. > In section 2.2.5 it says that ECN value is not mapped, because of > constant-send-rate, but it leaves out the other part, i.e., if some > device is sending inner data faster than the constant-send-rate then > the IPTFS should set the ECN for inner packets when adding them to the > aggregated frame, just to indicate this information to the other end. Added paragraph indicating ECN can still be set on inner fragments according to RFC3168. > In section 2.2.7 the draft says there is no effective MTU, which is > true, but the question is that should there be one? I mean even if the > original sender will automatically detect that 63kB UDP packets go > through without fragmentation that does not mean that it will be good > idea to use 63kB packets as that will cause latency for all other > packets sharing the same SA. I think it would be useful to be able to > restrict the effective MTU of the tunnel. Added sentence indicating MTU MAY still be explicitly configured on the tunnel. > In section 2.3 why does the text say: > > In other words, an SA that has > AGGFRAG_PAYLOAD enabled MUST NOT have non-AGGFRAG_PAYLOAD payloads > such as IP (IP protocol 4), TCP transport (IP protocol 6), or ESP pad > packets (protocol 59) intermixed with non-empty AGGFRAG_PAYLOAD > payloads. > > why is that restriction with only non-empty AGGFRAG_PAYLOAD payloads? > When can empty AGGFRAG_PAYLOAD used intermixed with other payloads? Added sentence explaining this and referring back to "Empty Payload" section which also explains this. > The rekeying of the SA is not specified fully. It does say that > rekeying cannot change the USE_AGGFRAG information, but it does not > say that rekeyed SA actually inherits the USE_AGGFRAG flag. It should > explictly mention that. Added sentence indicating the inheritance. > Also it does not mention whether frame is allowed to to be fragmented > across the old and new SA. It should say that as sequence numbers are > different for the SAs, the packet can never be fragmented so that part > of it is in old SA, and part of it is in the newly created SA. Added sentence saying inner packets should not be fragmented across different SAs. > In section 2.2.4 draft says: ESP payload length is equal to the > AGGFRAG_PAYLOAD header length. This would indicate that this packet is > not the same length than other frames. I think it would be better to > say that this has just the AGGFRAG_PAYLOAD header, and then rest of > the frame is padding. These are only used to transmit congestion control information back to the sender on non-IP-TFS SAs, there's no reason to pad them out. Once again I believe we are ready for WGLC. Thanks, Chris. > -- > kivi...@iki.fi > > _______________________________________________ > IPsec mailing list > IPsec@ietf.org > https://www.ietf.org/mailman/listinfo/ipsec >
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
