netdev - Re: [PATCH/RFC 01/10] Implement local diversion of IPv4 skbs

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: <45ACD0E4.2070908@trash.net>
Date:	Tue, 16 Jan 2007 14:19:32 +0100
From:	Patrick McHardy <kaber@...sh.net>
To:	KOVACS Krisztian <hidden@...abit.hu>
CC:	netfilter-devel@...ts.netfilter.org, netdev@...r.kernel.org,
	Balazs Scheidler <bazsi@...abit.hu>
Subject: Re: [PATCH/RFC 01/10] Implement local diversion of IPv4 skbs

KOVACS Krisztian wrote:
> On Wednesday 10 January 2007 13:19, Patrick McHardy wrote:
> 
>>>  Of course it's true that doing early lookups and storing that
>>>reference in the skb widens the window considerably, but I think this
>>>race is already handled. Or is there anything I don't see?
>>
>>You're right, it seems to be handled properly (except I think there is
>>a race between sk_common_release calling xfrm_sk_free_policy and f.e.
>>udp calling __xfrm_policy_check, will look into that).
>>
>>It probably shouldn't be cached anyway, with nf_queue for example
>>the window could be _really_ large.
> 
> 
>   Patrick, I seem to be out of ideas how this could be done 
> without "caching" the socket lookup. The problem is that it's not only 
> caching in some cases. For example we can do something like this:
> 
>   iptables -t tproxy -A PREROUTING -s X -d Y -p tcp --dport 80 \
>            -j TPROXY --to proxy_ip:proxy_port
> 
>   In this case the TPROXY target does a socket lookup for 
> proxy_ip:proxy_port and stores that socket reference in skb->sk. 
> Obviously if you don't do this then TCP will do a lookup on the packet's 
> original destination address/port and it won't work.
> 
>   Unfortunately I don't see any way how this could be solved without 
> storing the result of the lookup... So while I agree that having that 
> socket reference in the skb is risky, as previously skb->sk was unused on 
> the input path, I simply don't have any other idea. (Unless your load 
> iptable_tproxy skb->sk==NULL on input is still true with these patches, 
> so I think there should be absolutely no problems with tproxy unused.)

One (not very pretty) possibility would be to store the address/port
somewhere in the skb and use it for the socket lookup. I think thats
also what the 2.2 code did. Other than that I don't have any ideas
either, but I'm not too familiar with that code, maybe someone else
could explain whether caching the sockets would really be a problem
and why.

>   Other possible problems which came to my mind:
> 
> - The previous version was missing IPv4 fragment reassembly: we obviously 
> need this to be able to do socket lookups, so now I've added this to 
> iptable_tproxy.

Makes sense.

> - IP_FREEBIND does not require NET_ADMIN capability, combined with the 
> relaxed source address on ip_output() this means that we provide a way to 
> do IPv4 address forging for unprivileged users. As we must not break 
> anything it looks like we need a separate socket option for disabling 
> output source address checks (this would obviously require NET_ADMIN).

Also sounds reasonable.
-
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