netdev - Re: [Patch net] ipv4: fib: check forwarding before checking send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.11.1404131746100.1423@ja.home.ssi.bg>
Date:	Sun, 13 Apr 2014 17:57:36 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Cong Wang <xiyou.wangcong@...il.com>
cc:	David Miller <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Cong Wang <cwang@...pensource.com>
Subject: Re: [Patch net] ipv4: fib: check forwarding before checking
 send_redirects


	Hello,

On Fri, 11 Apr 2014, Cong Wang wrote:

> On Wed, Apr 9, 2014 at 1:03 AM, Julian Anastasov <ja@....bg> wrote:
> >
> >         I'm still not sure that it is valid to process
> > input route on loopback device. There is also a case
> > of broadcasts/multicasts that should preserve their local output
> > route while being looped in ip_mc_output(). In this
> > case device is not loopback. I need more time to check
> > the code again...

	It looks like LOOPBACK_IFINDEX is used only for
flowi4_iif. Users can distinguish input from output route
lookups by 'ip rule ... iif lo' matching. This trick works
because the lo interface was never used for input routes.
This is documented in iproute2/doc/ip-cref.tex, look for
'iif NAME' for the ip rules.

	As result, input routes are not expected on loopback.
Not sure if you can configure your setup in different way.
We still can solve it by fixing another problem, read below.

	Considering how flowi_iif is matched in fib_rule_match(),
it is wrong to provide 0 for the flowi4_iif field in
__fib_validate_source(). By this way, users can not match
this reverse flow with 'ip rule ... iif lo'. Of course,
such rules are rarely used.

	There are other places that need to provide
LOOPBACK_IFINDEX instead of 0:

- include/net/flow.h flowi4_init_output()
- net/ipv4/ipmr.c reg_vif_xmit(): skb->skb_iif ? : LOOPBACK_IFINDEX;
- net/ipv4/netfilter/ipt_rpfilter.c rpfilter_mt()

	Simply, flowi4_iif must not contain 0, it does not
look logical to ignore all ip rules with specified iif.

	So, we can implement such changes:

- all places that provide LOOPBACK_IFINDEX to fib_validate_source()
should be changed to provide 0 for oif. By this way the
'dev->ifindex != oif' check will work for your setup.

- then __fib_validate_source() will use:

	-	fl4.flowi4_iif = oif;
	+	fl4.flowi4_iif = oif ? : LOOPBACK_IFINDEX;

	What we achieve is:

- correct 'ip rule ... iif lo' matching for rp_filter checks

- fib_validate_source() will not call __fib_validate_source()
for loopback interface (when rp_filter is 0).

	By this way we do not add extra check in fib_validate_source(),
it goes in __fib_validate_source. The special semantic for
'lo' is coded at the right place, just for flowi4_iif.

Regards

--
Julian Anastasov <ja@....bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html