netdev - Re: [PATCH] ipv4: Elide fib_validate_source() completely when possible.

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]

Date:	Sat, 30 Jun 2012 13:45:52 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	David Miller <davem@...emloft.net>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] ipv4: Elide fib_validate_source() completely when
 possible.


	Hello,

On Fri, 29 Jun 2012, David Miller wrote:

> If rpfilter is off (or the SKB has an IPSEC path) and there are not
> tclassid users, we don't have to do anything at all when
> fib_validate_source() is invoked besides setting the itag to zero.
> 
> We monitor tclassid uses with a counter (modified only under RTNL and
> marked __read_mostly) and we protect the fib_validate_source() real
> work with a test against this counter and whether rpfilter is to be
> done.
> 
> Having a way to know whether we need no tclassid processing or not
> also opens the door for future optimized rpfilter algorithms that do
> not perform full FIB lookups.
> 
> Signed-off-by: David S. Miller <davem@...emloft.net>

> +/* Ignore rp_filter for packets protected by IPsec. */
> +int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
> +			u8 tos, int oif, struct net_device *dev,
> +			struct in_device *idev, u32 *itag)
> +{
> +	int r = secpath_exists(skb) ? 0 : IN_DEV_RPFILTER(idev);
> +

	It seems now we change the IN_DEV_ACCEPT_LOCAL policy
to depend on IN_DEV_RPFILTER. Isn't that dangerous for
home routers that disable rp_filter when using 2 or more
uplinks? I know that now rp_filter can be enabled for such
setups but sometimes users just update the kernel and can
miss such change.

	If we need the old behavior we should also check
IN_DEV_ACCEPT_LOCAL here. Then servers protected by firewall
can avoid this check by enabling accept_local. Established
stream sockets are anyways optimized by the new demux code.

	Not sure how fatal is the case of forwarding with
saddr=local_address. May be the risks for loops are same.

	If we really want a change in behavior we should
at least update the accept_local info in
Documentation/networking/ip-sysctl.txt ?

> +	if (!r && !fib_num_tclassid_users) {
> +		*itag = 0;
> +		return 0;
> +	}
> +	return __fib_validate_source(skb, src, dst, tos, oif, dev, r, idev, itag);
> +}
> +

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