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

From: Alexander Leidinger <Alexander_at_Leidinger.net>
Date: Mon, 06 May 2024 07:27:31 UTC
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?

Bye,
Alexander.

-- 
http://www.Leidinger.net Alexander@Leidinger.net: PGP 0x8F31830F9F2772BF
http://www.FreeBSD.org    netchild@FreeBSD.org  : PGP 0x8F31830F9F2772BF