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

On Tue, Jan 6, 2015 at 2:58 AM, Vlad Zolotarov
[off-list ref] wrote:

On 01/06/15 08:55, Gleb Natapov wrote:

quoted

On Mon, Jan 05, 2015 at 03:54:52PM -0800, Greg Rose wrote:

quoted

On Mon, Jan 5, 2015 at 6:15 AM, Vlad Zolotarov
[off-list ref] wrote:

quoted

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.

Thanks,

- Greg

Thanks,
vlad

quoted

--
                        Gleb.