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

From: Ahmed Zaki <ahmed.zaki@intel.com>
To: Jakub Kicinski <kuba@kernel.org>, <willemdebruijn.kernel@gmail.com>
Cc: mkubecek@suse.cz, andrew@lunn.ch,
	Wojciech Drewek <wojciech.drewek@intel.com>,
	corbet@lwn.net, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, jesse.brandeburg@intel.com,
	edumazet@google.com, anthony.l.nguyen@intel.com,
	horms@kernel.org, vladimir.oltean@nxp.com,
	intel-wired-lan@lists.osuosl.org, pabeni@redhat.com,
	davem@davemloft.net
Subject: Re: [Intel-wired-lan] [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type
Date: Fri, 20 Oct 2023 15:24:41 -0600	[thread overview]
Message-ID: <45c6ab9f-50f6-4e9e-a035-060a4491bded@intel.com> (raw)
In-Reply-To: <20231018165020.55cc4a79@kernel.org>

On 2023-10-18 17:50, Jakub Kicinski wrote:
> 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?

Please advise on the next step. Should I send a new version with the Doc 
warning, or will you use v5?

Thanks.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan