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

Alexander:


In-line

On 5/6/24 3:27 AM, Alexander Leidinger 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?
>

We used to use an RBtree but this had CPU consequences that @gallatin 
and I worked on. Using this customized hash-list which is tuned for TCP 
sequence numbers saved us about 10% CPU IIRR over the RB tree.


With these new fixes I think we are ok and I really don't want to 
introduce more overhead cpu wise :)


R

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