Re: git: fce03f85c5bf - main - TCP can be subject to Sack Attacks lets fix this issue.

On Mon, 6 May 2024 11:11:36 +0000
Gary Jennejohn <garyj@gmx.de> wrote:

> On Mon, 06 May 2024 09:27:31 +0200
> Alexander Leidinger <Alexander@Leidinger.net> wrote:
>
> > Am 2024-05-05 15:10, schrieb Randall Stewart:
> > > The branch main has been updated by rrs:
> > >
> > > URL:
> > > https://cgit.FreeBSD.org/src/commit/?id=fce03f85c5bfc0d73fb5c43ac1affad73efab11a
> > >
> > > commit fce03f85c5bfc0d73fb5c43ac1affad73efab11a
> > > Author:     Randall Stewart <rrs@FreeBSD.org>
> > > AuthorDate: 2024-05-05 13:08:47 +0000
> > > Commit:     Randall Stewart <rrs@FreeBSD.org>
> > > CommitDate: 2024-05-05 13:08:47 +0000
> > >
> > >     TCP can be subject to Sack Attacks lets fix this issue.
> > >
> > >     There is a type of attack that a TCP peer can launch on a
> > > connection. This is for sure in Rack or BBR and probably even the
> > > default stack if it uses lists in sack processing. The idea of the
> > > attack is that the attacker is driving you to look at 100's of sack
> > > blocks that only update 1 byte. So for example if you have 1 - 10,000
> > > bytes outstanding the attacker sends in something like:
> > >
> > >     ACK 0 SACK(1-512) SACK(1024 - 1536), SACK(2048-2536), SACK(4096 -
> > > 4608), SACK(8192-8704)
> > >     This first sack looks fine but then the attacker sends
> > >
> > >     ACK 0 SACK(1-512) SACK(1025 - 1537), SACK(2049-2537), SACK(4097 -
> > > 4609), SACK(8193-8705)
> > >     ACK 0 SACK(1-512) SACK(1027 - 1539), SACK(2051-2539), SACK(4099 -
> > > 4611), SACK(8195-8707)
> > >     ...
> > >     These blocks are making you hunt across your linked list and split
> > > things up so that you have an entry for every other byte. Has your list
> > > grows you spend more and more CPU running through the lists. The idea
> > > here is the attacker chooses entries as far apart as possible that make
> > > you run through the list. This example is small but in theory if the
> > > window is open to say 1Meg you could end up with 100's of thousands
> > > link list entries.
> >
> > Would it make sense to use a tree list (generic example:
> > https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/list/TreeList.html)
> > instead of a linked list additional/independently to what you committed?
> >
> > > diff --git a/sys/netinet/tcp_stacks/sack_filter.c
> > > b/sys/netinet/tcp_stacks/sack_filter.c
> > > index e82fcee2ffac..fc9ee8454a1e 100644
> > > --- a/sys/netinet/tcp_stacks/sack_filter.c
> > > +++ b/sys/netinet/tcp_stacks/sack_filter.c
> >
> > >  #ifndef _KERNEL
> > > +
> > > +static u_int tcp_fixed_maxseg(const struct tcpcb *tp)
> > > +{
> > > +	/* Lets pretend their are timestamps on for user space */
> > > +	return (tp->t_maxseg - 12);
> > > +}
> >
> > Typo in the comment?
> >
>
> Yes.  Should be Let's as a contraction of Let us.
>

And their should be there, which I just noticed.

--
Gary Jennejohn