netdev - Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support

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: <52B9278D.4020905@windriver.com>
Date:	Tue, 24 Dec 2013 14:19:57 +0800
From:	Fan Du <fan.du@...driver.com>
To:	Pablo Neira Ayuso <pablo@...filter.org>
CC:	<davem@...emloft.net>, <steffen.klassert@...unet.com>,
	<netfilter-devel@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 0/2] netfilter: IPv4/v6 IPcomp match support



On 2013年12月23日 20:13, Pablo Neira Ayuso wrote:
> Hi,
>
> On Fri, Dec 20, 2013 at 05:21:05PM +0800, Fan Du wrote:
> [...]
>>> AH is not the last header, so we still have to use ipv6_find_hdr() to
>>> find the good header instead of par->thoff. Note that the ip6_tables
>>> sets par->thoff to the last IPv6 extension header.
>>
>> I'm quite new to the internal of netfiler, especially about this part.
>> I will take a look at the code later.
>>
>>> This rises some concerns regarding your ipcomp, I think that if you
>>> use this with ah and esp, the ordering of the headers is
>>> ah+ipcomp+esp, right?
>>
>> This depends on the user land configuration of encapsulation order.
>> It can be one of the three types only(ah, esp, ipcomp), the most commonly
>> used is ah(outer)+esp(inner).
>>
>> I barely see ipcomp used in production, but I remember RFC says ipcomp
>> should be done first before esp, because after encryption in esp, the data
>> is polluted, i.e., not suitable for compressed anymore(I'm not sure the
>> details theory behind this statement.)
>
> In that case we have to use ipv6_find_hdr(..., IPPROTO_IPCOMP, ...),
> since par->thoff will point to the last header which is esp. After
> this change, the ipcomp ipv6 match will look very similar to what you
> have in ah_mt6(...) in net/ipv6/netfilter/ip6t_ah.c. Please, rework
> that in your ipcomp match patch and resend. Thanks.
>

Hi Pablo

I think we don't need to rework this patch set back to v1 by using ipv6_find_hdr
for IPv6 part, because IPcomp shared the same characteristic as esp, that's hiding
upper layer protocol.

For a packet encapsulated in order of ah->esp->ah->original packet, as you said
par->thoff is set at esp, that's why netfilter esp has a unified implementation
in net/netfilter/xt_esp.c, because it's always the last parse header netfilter
can reach.

The same rule apply with IPcomp, for example,
(1) ah->ipcomp->original packet
          ^par->thoff
(2) ipcomp->ah->original packet
       ^par->thoff

Both cases (1) and (2) par->thoff can only point into IPcomp header, so in such
circumstance, a unified implementation for both IPv4/6 is feasible, and I have
tested (2) in such implementation, it works anyway.

IMO, a unified implementation suggested by you previous is ok for this round review.


-- 
浮沉随浪只记今朝笑

--fan
--
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