Re: [nft PATCH 4/4 v3] nft: complete reject support

["Álvaro Neira Ayuso" <alvaroneay@gmail.com>]

El 22/09/14 10:20, Patrick McHardy escribió:
> On Sun, Sep 21, 2014 at 09:32:37PM +0200, Alvaro Neira Ayuso wrote:
>> This patch allows to use the reject action in rules. Example:
>>
>>    nft add rule filter input udp dport 22 reject
>>
>> In this rule, we assume that the reason is network unreachable. Also
>> we can specify the reason with the option "with" and the reason. Example:
>>
>>    nft add rule filter input tcp dport 22 reject with host-unreach
>>
>> In the bridge tables and inet tables, we can use this action too. Example:
>>
>>    nft add rule inet filter input reject with icmp-host-unreach
>>
>> In this rule above, this generates a meta nfproto dependency to match
>> ipv4 traffic because we use a icmpv4 reason to reject.
>>
>> If the reason is not specified, we infer it from the context.
>
> There are a couple of things I don't like very much about this concept.
>
> First of all, the types have redundant information (icmp-*, icmpv6-*).
> This might not look to bad if you only consider the reject statement,
> but the redundancy becomes very obvious if you consider a set declaration
> which explicitly specifies the data type and still has to use the icmp
> prefix for every value.
>
> Its also not consistent with the remaining keywords, we don't use
> tcp-syn, tcp-fin etc.
>
> I also don't like having a reject specific family, without taking extra
> care this prevents protocol conflict detection and determination of
> the protocol for following expressions.
>
> I'd rather have this very explicit:
>
> reject with icmp host-unreachable
> reject with icmpv6 host-unreachable
> reject with icmp
>
> So the protocol would be an explizit parameter without strcmp hacks and
> the type keywords would not encode the protocol. I'd also not be
> against just using the exact same syntax as for matches:
>
> reject with icmp type host-unreach (note the extra "type").
>
>> Insufficient context for using reject
>>    * nft add rule inet filter input reject
>>    * nft add rule bridge filter input reject
>
> Well, we do have the inet reject expression and I consider it important
> that we actually support it, we want to help make easier rulesets.
>
> It does actually work today without a specific type as in your example,
> so why wouldn't this work anymore?
>
> I actually think we should add full support for this by adding an
> inet-specific ICMP type table which is the intersection of the ICMP
> and ICMPv6 types for inet and map those to the corresponding real
> types:
>
> nft inet filter input reject with host-unreachable
>

I have seen the ICMP and ICMPv6 types and I have done this map:

CODE		 |	ICMPv6		|	ICMPv4
admin-prohibited | admin-prohibited	|	admin-prohibited
port-unreach	 | port-unreach		|	port-unreach
no-route	 | no-route		|	net-unreach
host-unreach	 | addr-unreach		|	host-unreach

What do you think?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html