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] [day] [month] [year] [list]

Date:	Mon, 14 Apr 2014 15:59:27 -0700
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	Julian Anastasov <ja@....bg>
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

On Sun, Apr 13, 2014 at 7:57 AM, Julian Anastasov <ja@....bg> wrote:
>
>         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.

Thanks for the details! This makes a lot sense to me. :)

I just cooked 3 patches based on your suggestion, I will give them
some quick tests before sending them out.

Will send them soon with you Cc'ed.

Regards.
--
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