Thanks for the thorough review! Comments inline..
Martin Duke via Datatracker <nore...@ietf.org> writes:
Martin Duke has entered the following ballot position for draft-ietf-ipsecme-iptfs-13: Discuss When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ for more information about how to handle DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-ipsecme-iptfs/ ---------------------------------------------------------------------- DISCUSS: ----------------------------------------------------------------------
One point which I think will be simple to address: (6) As malformed packets are sometimes an attack vector, it would be good to specify behavior in response to pathological BlockOffsets, for instance: - What if two BlockOffset fields disagree? e.g., with 500 byte outer packets, what if the sequence of block offsets is {0, 750, 100}? Does the third packet have 250 or 100 bytes of the first data block? Drop the packet, kill the SA, ignore one and accept the other, or something else?
The block offset is pointing at the start of the next packet (which may be beyond the current packets boundary). So it also represents what is left in the current inner packet being reassembled. When the offset doesn't agree with the known length of the inner being reassembled, the inner is simply dropped and you move to the start of the next packet (which is what the block offset points to). It should be noted that these values are in the cipher text (i.e. they are encrypted inside the ESP wrapper), so getting bad values here is almost for sure due to a bug/corruption on a validated sender rather than an attack. :)
- What if a pad block is in a packet with a BlockOffset greater than the packet length? Would the receiver skip over the specified bytes in the subsequent packet, even though padding is supposed to only be at the end of packets?
This situation can't occur as pad blocks are very simple and hard to mess up. :) Pad blocks start with 4 0-bits and their length is "the rest of the packet". By definition if the block offset points past the end of the outer packet, there is no pad and the payload is entirely made up of the current inner packet being reassembled.
---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- Thanks to Joe Touch for 2 TSVART reviews, and for addressing his comments. Also thanks for the very literate discussion of congestion control. (2.2.3) It would be nice to at least suggest a default number for the reordering window. In TCP, we traditionally use 3, but really any suggestion for the clueless is fine.
We could add the text "TCP traditionally uses 3" if you'd like. :)
(3) Please clarify: is TsVal the actual tranmission time, or the time the packet is queued for the next transmission opportunity?
It has to be when when queued as the value is set prior to ESP encryption.
(3) This probably just needs a bit more explanation, but reading this document, and not knowing much about ESP, I could not figure out the case where the return path does not support AGGFRAG_PAYLOAD. IIUC, IKEv2 negotiates this for the pair explicitly, so this case cannot arise. Otherwise, how is this negotiated? Why would a tunnel endpoint support just AGGFRAG without payloads but not with?
The most common case (for this admittedly uncommon scenario) would be static configuration of the SAs, where only one side is configured to use IP-TFS.
NITS (2.4.1) update the [RFC8229] reference to RFC8229bis?
We wouldn't want to block on this. The normal "updates/replaces" pointers should take care of things if/when RFC8229bis gets published, right?
(6.1) "The value 5 was chosen to not conflict with other used values." IIUC the values here are just Protocol numbers from the registry. So maybe it's better to be more explicit and say that this cannot be used with RFC1819 streams?
They are specific to ESP, but have traditionally been drawn from IP protocol numbers. This isn't a requirement though. If you feel strong we could add that explicit text, but I think it's pretty obvious this is only for ESP payloads. Thanks again for your thorough review!! Chris.
signature.asc
Description: PGP signature
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec