Subject: Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

From: Jakub Kicinski <[email-masked]>
Date: Wed, 18 Oct 2023 16:50:20 -0700

On Wed, 18 Oct 2023 11:12:13 -0700 Alexander Duyck wrote:
> > > Based on earlier comments it doesn't change the inputs, it just
> > > changes how I have to handle the data and the key. It starts reducing
> > > things down to something like the Intel implementation of Flow
> > > Director in terms of how the key gets generated and hashed.  
> >
> > About Flow Director I know only that it is bad :)  
> 
> Yeah, and that is my concern w/ the symmetric XOR is that it isn't
> good. It opens up the toeplitz hash to exploitation. You can target
> the same bucket by just making sure that source IP and port XOR with
> destination IP and port to the same value. That can be done by adding
> the same amount to each side. So there are 2^144 easily predictable
> possible combinations that will end up in the same hash bucket. Seems
> like it might be something that could be exploitable. That is why I
> want it marked out as a separate algo since it is essentially
> destroying entropy before we even get to the Toeplitz portion of the
> hash. As such it isn't a hash I would want to use for anything that is
> meant to spread workload since it is so easily exploitable.

I see your point.

Which is not to say that I know what to do about it. crc or any
future secure algo will get destroyed all the same. It's the input
entropy that gets destroyed, independently of the algo.

We already support xor, and it doesn't come with a warning saying
it's insecure so we kind of assume user knows what they are doing.

I think the API we pick for configuring sym-xor should be the same as
sym-sort. And the "makes algo insecure" argument won't apply to sort.

IMO fat warning in the documentation and ethtool man saying that this
makes the algo (any / all) vulnerable to attack would be enough.
Willem?