Re: [IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-10 Thread Christian Hopps


Martin Duke  writes:


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.


Can do.

Thanks,
Chris.



Martin

On Wed, Aug 10, 2022 at 12:32 PM Christian Hopps 
wrote:


Martin Duke  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  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 

Re: [IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-10 Thread Martin Duke
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  wrote:

>
> Martin Duke  writes:
>
> > Comments inline.
> >
> > On Tue, Aug 9, 2022 at 8:56 PM Christian Hopps 
> > wrote:
> >
> >
> > Thanks for the thorough review! Comments inline..
> >
> > Martin Duke via Datatracker  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

Re: [IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-10 Thread Christian Hopps


Martin Duke  writes:


Comments inline.

On Tue, Aug 9, 2022 at 8:56 PM Christian Hopps 
wrote:


Thanks for the thorough review! Comments inline..

Martin Duke via Datatracker  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 

Re: [IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-10 Thread Martin Duke
Comments inline.

On Tue, Aug 9, 2022 at 8:56 PM Christian Hopps  wrote:

>
> Thanks for the thorough review! Comments inline..
>
> Martin Duke via Datatracker  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.

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.


>
> > --
> > 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?


>
> > 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 

Re: [IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-09 Thread Christian Hopps


Thanks for the thorough review! Comments inline..

Martin Duke via Datatracker  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


[IPsec] Martin Duke's Discuss on draft-ietf-ipsecme-iptfs-13: (with DISCUSS and COMMENT)

2022-08-09 Thread Martin Duke via Datatracker
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?

- 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?


--
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.

(3) Please clarify: is TsVal the actual tranmission time, or the time the
packet is queued for the next transmission opportunity?

(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?

NITS
(2.4.1) update the [RFC8229] reference to RFC8229bis?

(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?



___
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec