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

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.

--
Gary Jennejohn