Date:   Tue, 25 Sep 2018 16:26:38 +0100
From:   Mike Manning <mmanning@...tta.att-mail.com>
To:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org
Cc:     Robert Shearman <rshearma@...tta.att-mail.com>
Subject: Re: [PATCH net-next v1 1/5] net: allow binding socket in a VRF when
 there's an unbound socket

On 24/09/2018 23:44, David Ahern wrote:
> On 9/24/18 10:13 AM, Mike Manning wrote:
>> From: Robert Shearman <rshearma@...tta.att-mail.com>
>>
>> There is no easy way currently for applications that want to receive
>> packets in the default VRF to be isolated from packets arriving in
>> VRFs, which makes using VRF-unaware applications in a VRF-aware system
>> a potential security risk.
> 
> That comment is not correct.
> 
> The point of the l3mdev sysctl's is to prohibit this case. Setting
> net.ipv4.{tcp,udp}_l3mdev_accept=0 means that a packet arriving on an
> interface enslaved to a VRF can not be received by a global socket.
Hi David, thanks for reviewing this. The converse does not hold though, 
i.e. there is no guarantee that the unbound socket will be selected for 
packets when not in a VRF, if there is an unbound socket and a socket 
bound to a VRF. Also, such packets should not be handled by the socket 
in the VRF if there is no unbound socket. We also had an issue with raw 
socket lookup device bind matching. I can break this particular patch 
into smaller patches and provide more detail, would this help? I will 
also update/break up the other patches according to your comments.

> 
> Setting the l3mdev to 1 allows the default socket to work across VRFs.
> If that is not what you want for a given app or a given VRF, then one
> option is to add netfilter rules on the VRF device to prohibit it. I
> just verified this works for both tcp and udp.

Netfilter is per application and so does not scale. I have not checked 
if it is suitable for packet handling on raw sockets.

> 
> Further, overlapping binds are allowed using SO_REUSEPORT meaning I can
> have a server running in the default vrf bound to a port AND a server
> running bound to a specific vrf and the same port:
> 
> udp    UNCONN     0      0      *%red:12345                 *:*
>             users:(("vrf-test",pid=1376,fd=3))
> udp    UNCONN     0      0       *:12345                 *:*
>          users:(("vrf-test",pid=1375,fd=3))
> 
> tcp    LISTEN     0      1      *%red:12345                 *:*
>             users:(("vrf-test",pid=1356,fd=3))
> tcp    LISTEN     0      1       *:12345                 *:*
>          users:(("vrf-test",pid=1352,fd=3))
> 
> For packets arriving on an interface enslaved to a VRF the socket lookup
> will pick the VRF server over the global one.

Agreed, but the converse is not guaranteed to hold i.e. packets that are 
not in a VRF may be handled by a socket bound to a VRF.

We do use SO_REUSEPORT for our own applications so as to run instances 
in the default and other VRFs, but still require these patches (or 
similar) due to how packets are handled when there is an unbound socket 
and sockets bound to different VRFs.

> 
> --
> 
> With this patch set I am seeing a number of tests failing -- socket
> connections working when they should not or not working when they
> should. I only skimmed the results. I am guessing this patch is the
> reason, but that is just a guess.
> 
> You need to make sure all permutations of:
> 1. net.ipv4.{tcp,udp}_l3mdev_accept={0,1},
> 2. connection in the default VRF and in a VRF,
> 3. locally originated and remote traffic,
> 4. ipv4 and ipv6
>

We are using raw, datagram and stream sockets for ipv4 & ipv6, require 
connectivity for local and remote addresses where appropriate and need 
route leaking between VRFs when configured, we are unaware of any 
outstanding bugs. Is there some way that I can run/analyze the tests 
that are failing for you?

Also cf patch 2/5 note that ping to link-local addresses is handled 
consistently with that to global addresses in a VRF, so this now 
succeeds if ping is done in the VRF, i.e. 'sudo ip vrf exec <vrf> ping 
<ll> -I <intf>

> continue to work as expected meaning packets flow when they should and
> fail with the right error when they should not. I believe the UDP cases
> were the main ones failing.
> 
> Given the test failures, I did not look at the code changes in the patch.
> 

Powered by blists - more mailing lists