Re: [PATCH net-next v3 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key

[Vlad Zolotarov <vladz@cloudius-systems.com>]

On 01/06/15 18:59, Greg Rose wrote:
> On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
> <vladz@cloudius-systems.com> wrote:
>> On 01/06/15 08:55, Gleb Natapov wrote:
>>> On Mon, Jan 05, 2015 at 03:54:52PM -0800, Greg Rose wrote:
>>>> On Mon, Jan 5, 2015 at 6:15 AM, Vlad Zolotarov
>>>> <vladz@cloudius-systems.com> wrote:
>>>>> Add the ethtool ops to VF driver to allow querying the RSS indirection
>>>>> table
>>>>> and RSS Random Key.
>>>>>
>>>>>    - PF driver: Add new VF-PF channel commands.
>>>>>    - VF driver: Utilize these new commands and add the corresponding
>>>>>                 ethtool callbacks.
>>>>>
>>>>> New in v3:
>>>>>      - Added a missing support for x550 devices.
>>>>>      - Mask the indirection table values according to PSRTYPE[n].RQPL.
>>>>>      - Minimized the number of added VF-PF commands.
>>>>>
>>>>> New in v2:
>>>>>      - Added a detailed description to patches 4 and 5.
>>>>>
>>>>> New in v1 (compared to RFC):
>>>>>      - Use "if-else" statement instead of a "switch-case" for a single
>>>>> option case.
>>>>>        More specifically: in cases where the newly added API version is
>>>>> the only one
>>>>>        allowed. We may consider using a "switch-case" back again when the
>>>>> list of
>>>>>        allowed API versions in these specific places grows up.
>>>>>
>>>>> Vlad Zolotarov (5):
>>>>>     ixgbe: Add a RETA query command to VF-PF channel API
>>>>>     ixgbevf: Add a RETA query code
>>>>>     ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>>>>>     ixgbevf: Add RSS Key query code
>>>>>     ixgbevf: Add the appropriate ethtool ops to query RSS indirection
>>>>>       table and key
>>>>>
>>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |  10 ++
>>>>>    drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  91
>>>>> +++++++++++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/ethtool.c      |  43 +++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   4 +-
>>>>>    drivers/net/ethernet/intel/ixgbevf/mbx.h          |  10 ++
>>>>>    drivers/net/ethernet/intel/ixgbevf/vf.c           | 132
>>>>> ++++++++++++++++++++++
>>>>>    drivers/net/ethernet/intel/ixgbevf/vf.h           |   2 +
>>>>>    7 files changed, 291 insertions(+), 1 deletion(-)
>>>> I've given this code a review and I don't see a way to
>>>> set a policy in the PF driver as to whether this request should be
>>>> allowed or not.  We cannot enable this query by default - it is a
>>>> security risk. To make this acceptable you need to do a
>>>> couple of things.
>>>>
>>> Can you please elaborate on the security risk this information poses?
>>> Is toeplitz hash function cryptographically strong enough so that VF
>>> cannot reconstruct the hash key from hash result provided in packet
>>> descriptor? The abstract of this paper [1] claims it is not, but I do
>>> not have access to the full article unfortunately hence the question.
>>>
>>> [1]
>>> http://ieeexplore.ieee.org/xpl/login.jsp?tp=&arnumber=5503170&url=http%3A%2F%2Fieeexplore.ieee.org%2Fxpls%2Fabs_all.jsp%3Farnumber%3D5503170
>>
>> I agree with Gleb here: when we started with just thinking about the idea of
>> this patch the possible security issue was the first thing that came into
>> our minds.
>> But eventually we couldn't come up with any security risk or attack example
>> that is exclusively caused by the fact that VF knows the indirection table
>> and/or RSS hash key of the PF.
>>
>> So, Greg, if we have missed anything and your have such an example could you
>> share it here, please?
> I don't have any examples and that is not my area of expertise.  But
> just because we can't think of a security risk or attack example
> doesn't mean there isn't one.
>
> Just add a policy hook so that the system admin can decide whether
> this information should be shared with the VFs and then we're covered
> for cases of both known and unknown exploits, risks, etc.

I absolutely disagree with u in regard of defining an RSS redirection 
table and RSS hash key as a security sensitive data. I don't know how u 
got to this conclusion.
However I don't want to argue about any longer. Let's move on.

Let's clarify one thing about this "hook". Do u agree that it should 
cover only the cases when VF shares the mentioned above data with PF - 
namely for all devices but x550?

thanks,
vlad

>
> Thanks,
>
> - Greg
>
>> Thanks,
>> vlad
>>
>>> --
>>>                          Gleb.
>>