Re: [PACTH nf-next] netfilter: nf_reject_ipv4: don't send tcp RST if the packet is non-TCP

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

Hi Liping,

On Mon, Jun 20, 2016 at 09:26:28PM +0800, Liping Zhang wrote:
> From: Liping Zhang <liping.zhang@spreadtrum.com>
> 
> In iptables, if the user add a rule to send tcp RST and specify the
> non-TCP protocol, such as UDP, kernel will reject this request. But
> in nftables, this validity check only occurs in nft tool, i.e. only
> in userspace.
> 
> This means that user can add such a rule like follows via nfnetlink:
>   "nft add rule filter forward ip protocol udp reject with tcp reset"
> 
> This will generate some confusing tcp RST packets. So we should send
> tcp RST only when it is TCP packet.
> 
> Signed-off-by: Liping Zhang <liping.zhang@spreadtrum.com>
> ---
>  net/ipv4/netfilter/nf_reject_ipv4.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
> index b6ea57e..fd82202 100644
> --- a/net/ipv4/netfilter/nf_reject_ipv4.c
> +++ b/net/ipv4/netfilter/nf_reject_ipv4.c
> @@ -24,6 +24,9 @@ const struct tcphdr *nf_reject_ip_tcphdr_get(struct sk_buff *oldskb,
>  	if (ip_hdr(oldskb)->frag_off & htons(IP_OFFSET))
>  		return NULL;
>  
> +	if (ip_hdr(oldskb)->protocol != IPPROTO_TCP)
> +		return NULL;
> +
>  	oth = skb_header_pointer(oldskb, ip_hdrlen(oldskb),
>  				 sizeof(struct tcphdr), _oth);
>  	if (oth == NULL)

A different check/log is made for ip6:
nf_reject_ip6_tcphdr_get():
        /* IP header checks: fragment, too short. */
        if (proto != IPPROTO_TCP || *otcplen < sizeof(struct tcphdr)) {
                pr_debug("proto(%d) != IPPROTO_TCP or too short (len = %d)\n",
                         proto, *otcplen);
                return NULL;
        }

Would be nice to have some consistency on this log message as it
increases debug-ability.

  Marcelo