From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net-next v1 1/5] net: allow binding socket in a VRF when there's an unbound socket Date: Tue, 25 Sep 2018 11:16:59 -0600 Message-ID: References: <20180924161326.17167-1-mmanning@vyatta.att-mail.com> <20180924161326.17167-2-mmanning@vyatta.att-mail.com> <97367b8a-dd62-0b42-3f07-30e4b897546a@vyatta.att-mail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Robert Shearman To: mmanning@vyatta.att-mail.com, netdev@vger.kernel.org Return-path: Received: from mail-pf1-f194.google.com ([209.85.210.194]:43767 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726328AbeIYXZb (ORCPT ); Tue, 25 Sep 2018 19:25:31 -0400 Received: by mail-pf1-f194.google.com with SMTP id j26-v6so11673402pfi.10 for ; Tue, 25 Sep 2018 10:17:03 -0700 (PDT) In-Reply-To: <97367b8a-dd62-0b42-3f07-30e4b897546a@vyatta.att-mail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 9/25/18 9:26 AM, Mike Manning wrote: > On 24/09/2018 23:44, David Ahern wrote: >> On 9/24/18 10:13 AM, Mike Manning wrote: >>> From: Robert Shearman >>> >>> 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 I need an explicit example here. You are saying a packet arriving on an interface not enslaved to a VRF might match a socket bound to a VRF? > 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. Why not add an l3mdev sysctl for raw sockets then? Yes, please send smaller patches. A diff stat of: 15 files changed, 109 insertions(+), 62 deletions(-) is a bit harsh. > >> >> 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. Why can't compute_score be adjusted to account for that case? > >> >> -- >> >> 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? I am not distributing my vrf tests right now. Before sending the response I quickly verified one case is easy for you to see: set the udp sysctl to 0, start a global server, send a packet to it via an interface enslaved to a VRF. It should fail ECONNREFUSED (no socket match) but instead packet reaches the server. > > 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 ping > -I Shifting packets destined to a LLA from the real device to the vrf device is a change in behavior. It is not clear to me at the moment that it will not cause a problem. > >> 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. >> > A couple of the patches are fine as is - or need a small change. It might be easier for you to send those outside of the socket lookup set.