Thanks for the explanation of the half-duplex mode.
Would it be too much to include the following requirements? You seem to
think they are redundant but they are not obvious to me from reading the
text.
Senders MUST encode a BlockLength consistent with the immediately preceding
packet. Specifically, if the immediately preceding packet had a Pad Data
Block, the
BlockLength MUST be zero, as Pad Data Blocks cannot be fragmented. The
BlockLength MUST be consistent with the remaining size implied by the
native length
encoding of the fragmented inner packet.
To account for misbehaving senders, a receiver SHOULD gracefully handle the
case where the BlockLengths of consecutive packets, and/or the inner packet
they
share, do not agree. It MAY drop the inner packet, or one or both of the
outer packets.
Martin
On Wed, Aug 10, 2022 at 12:32 PM Christian Hopps <cho...@chopps.org> wrote:
>
> Martin Duke <martin.h.d...@gmail.com> writes:
>
> > Comments inline.
> >
> > On Tue, Aug 9, 2022 at 8:56 PM Christian Hopps <cho...@chopps.org>
> > wrote:
> >
> >
> > Thanks for the thorough review! Comments inline..
> >
> > Martin Duke via Datatracker <nore...@ietf.org> writes:
> >
> > > (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. :)
> >
> >
> > Do I understand correctly that the inner packet's native length field
> > is the ground truth, rather than the block offset? I actually don't
> > care how these conflicts are resolved, just that the text resolves
> > them.
>
> That's correct, it's the only place the actual length is, no duplication.
> The block offset always points at the start of the next packet.
>
> From 2.2.1:
>
> Likewise, the
> length of the data block is extracted from the encapsulated IPv4's
> Total Length or IPv6's Payload Length fields.
>
> From 2.2:
> [.. diagram showing "DataBlocks" and "BlockOffset" ..]
>
> If the BlockOffset value is zero it means that the DataBlocks data
> begins with a new data block.
>
> Conversely, if the BlockOffset value is non-zero it points to the
> start of the new data block, and the initial DataBlocks data belongs
> to the data block that is still being re-assembled.
>
>
> > I am not an expert on these attacks, nor do I have a well-thought-out
> > threat model, but IIUC these sorts of problems usually manifest as
> > buffer overflows and the like, not as injected packets. In any case,
> > it's better to have well-defined protocol behavior on unexpected
> > input.
> >
> >
> >
> > > - 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.
> >
> >
> > OK. The document seems to define a pad block as a kind of data block,
> > and the BlockOffset field applies to data blocks. So it would be
> > legal to have an all-padding packet with a BlockOffset > outer packet
> > size, IIUC.
>
> No, pad blocks are always from their start to the end of the outer packet.
> You would never be fragmenting (thus "continuing" in the next packet) a pad
> block.
>
> Again from 2.2:
>
> Conversely, if the BlockOffset value is non-zero it points to the
> start of the new data block, and the initial DataBlocks data belongs
> to the data block that is still being re-assembled.
>
> Pad blocks are never fragmented or reassembled.
>
> From 6.1.3.3: Pad Data Block
> 1 2 3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> | 0x0 | Padding ...
> +-+-+-+-+-+-+-+-+-+-+-
> Figure 9: Pad Data Block format
> Type:
> A 4-bit value of 0x0 indicating a padding data block.
> Padding:
> Extends to end of the encapsulating packet.
>
> > >
> >
> ----------------------------------------------------------------------
> > > 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.
> > :)
> >
> >
> > Sure.
> >
> >
> >
> > > (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.
> >
> >
> > OK, please clarify in the text.
> >
> >
> >
> > > (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.
> >
> >
> > I guess my confusion is that this case is not about interacting with
> > legacy devices; they still have to be updated to support AGGFRAG
> > without payloads. is there
> > really that big of a win to implement just the headers without
> > supporting payloads?
>
> I believe the case that people have in mind is when they only want IPTFS
> in one direction, not b/c the other direction doesn't support IP-TFS (it
> has to to handle the no payload case), but b/c they don't want/need to
> secure that second SA with TFS.
>
> Thanks,
> Chris.
>
>
> >
> >
> > > 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?
> >
> >
> > The general practice is to prefer the more up-to-date reference, and
> > as 8229bis is going through it shouldn't really block. But I'm not
> > going to insist.
> >
> >
> >
> > > (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.
> >
> >
> > I don't feel strongly.
> >
> >
> >
> > Thanks again for your thorough review!!
> > Chris.
>
>
_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec
