netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit)
@ 2011-12-02 17:37 Pete Holland
  2011-12-04 22:49 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Pete Holland @ 2011-12-02 17:37 UTC (permalink / raw)
  To: netfilter-devel

From: Peter Holland <pholland27@gmail.com>

Make the logging of dropped packets due to ct helper rejection
conditional on LOG_INVALID.
This is consistent with the other uses of nf_log_packet.
Use protocol from conntrack tuple (original direction).
Without this check, there is a possible DoS based on traffic induced
log generation.
(specifically this was noted in the wild by an attacker against the SIP helper)

Signed-off-by: Peter Holland <pholland27@gmail.com>
---
--- net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c.orig	2011-11-29
11:34:36.683717278 -0800
+++ net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c	2011-12-02
09:32:00.727064563 -0800
@@ -116,8 +116,10 @@ static unsigned int ipv4_confirm(unsigne
 	ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
 			   ct, ctinfo);
 	if (ret != NF_ACCEPT) {
-		nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
-			      "nf_ct_%s: dropping packet", helper->name);
+		if (LOG_INVALID(nf_ct_net(ct),
+				ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum))			
+			nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
+				      "nf_ct_%s: dropping packet", helper->name);
 		return ret;
 	}

--- net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c.orig	2011-11-29
11:35:00.221028814 -0800
+++ net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c	2011-12-02
09:32:45.457026870 -0800
@@ -180,8 +180,10 @@ static unsigned int ipv6_confirm(unsigne

 	ret = helper->help(skb, protoff, ct, ctinfo);
 	if (ret != NF_ACCEPT) {
-		nf_log_packet(NFPROTO_IPV6, hooknum, skb, in, out, NULL,
-			      "nf_ct_%s: dropping packet", helper->name);
+		if (LOG_INVALID(nf_ct_net(ct),
+				ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum))	
+			nf_log_packet(NFPROTO_IPV6, hooknum, skb, in, out, NULL,
+				      "nf_ct_%s: dropping packet", helper->name);
 		return ret;
 	}
 out:

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit)
  2011-12-02 17:37 [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit) Pete Holland
@ 2011-12-04 22:49 ` Pablo Neira Ayuso
  2011-12-05 18:09   ` Pete Holland
  0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-04 22:49 UTC (permalink / raw)
  To: Pete Holland; +Cc: netfilter-devel

Hi Peter,

On Fri, Dec 02, 2011 at 09:37:04AM -0800, Pete Holland wrote:
> From: Peter Holland <pholland27@gmail.com>
> 
> Make the logging of dropped packets due to ct helper rejection
> conditional on LOG_INVALID.
> This is consistent with the other uses of nf_log_packet.
> Use protocol from conntrack tuple (original direction).
> Without this check, there is a possible DoS based on traffic induced
> log generation.
> (specifically this was noted in the wild by an attacker against the SIP helper)
> 
> Signed-off-by: Peter Holland <pholland27@gmail.com>
> ---
> --- net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c.orig	2011-11-29
> 11:34:36.683717278 -0800
> +++ net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c	2011-12-02
> 09:32:00.727064563 -0800
> @@ -116,8 +116,10 @@ static unsigned int ipv4_confirm(unsigne
>  	ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
>  			   ct, ctinfo);
>  	if (ret != NF_ACCEPT) {
> -		nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
> -			      "nf_ct_%s: dropping packet", helper->name);
> +		if (LOG_INVALID(nf_ct_net(ct),
> +				ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum))

You can use nf_ct_protonum here.

> +			nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
> +				      "nf_ct_%s: dropping packet", helper->name);
>  		return ret;
>  	}

Below you can find:

/* adjust seqs for loopback traffic only in outgoing direction */
if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
    !nf_is_loopback_packet(skb)) {
        typeof(nf_nat_seq_adjust_hook) seq_adjust;

        seq_adjust = rcu_dereference(nf_nat_seq_adjust_hook);
        if (!seq_adjust || !seq_adjust(skb, ct, ctinfo)) {
                NF_CT_STAT_INC_ATOMIC(nf_ct_net(ct), drop);
                                      ^^^^^^^^^^^^

Please, declare in ipv4_confirm:

struct net *net = nf_ct_net(ct);

And use the net pointer in that function.

Same thing for the IPv6 side.

Thank you.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit)
  2011-12-04 22:49 ` Pablo Neira Ayuso
@ 2011-12-05 18:09   ` Pete Holland
  2011-12-05 22:37     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 4+ messages in thread
From: Pete Holland @ 2011-12-05 18:09 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

would you like that I check *net for NULL as is done for *ct, *help,
and *helper?

On Sun, Dec 4, 2011 at 2:49 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Peter,
>
> On Fri, Dec 02, 2011 at 09:37:04AM -0800, Pete Holland wrote:
>> From: Peter Holland <pholland27@gmail.com>
>>
>> Make the logging of dropped packets due to ct helper rejection
>> conditional on LOG_INVALID.
>> This is consistent with the other uses of nf_log_packet.
>> Use protocol from conntrack tuple (original direction).
>> Without this check, there is a possible DoS based on traffic induced
>> log generation.
>> (specifically this was noted in the wild by an attacker against the SIP helper)
>>
>> Signed-off-by: Peter Holland <pholland27@gmail.com>
>> ---
>> --- net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c.orig       2011-11-29
>> 11:34:36.683717278 -0800
>> +++ net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c    2011-12-02
>> 09:32:00.727064563 -0800
>> @@ -116,8 +116,10 @@ static unsigned int ipv4_confirm(unsigne
>>       ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
>>                          ct, ctinfo);
>>       if (ret != NF_ACCEPT) {
>> -             nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
>> -                           "nf_ct_%s: dropping packet", helper->name);
>> +             if (LOG_INVALID(nf_ct_net(ct),
>> +                             ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum))
>
> You can use nf_ct_protonum here.
>
>> +                     nf_log_packet(NFPROTO_IPV4, hooknum, skb, in, out, NULL,
>> +                                   "nf_ct_%s: dropping packet", helper->name);
>>               return ret;
>>       }
>
> Below you can find:
>
> /* adjust seqs for loopback traffic only in outgoing direction */
> if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
>    !nf_is_loopback_packet(skb)) {
>        typeof(nf_nat_seq_adjust_hook) seq_adjust;
>
>        seq_adjust = rcu_dereference(nf_nat_seq_adjust_hook);
>        if (!seq_adjust || !seq_adjust(skb, ct, ctinfo)) {
>                NF_CT_STAT_INC_ATOMIC(nf_ct_net(ct), drop);
>                                      ^^^^^^^^^^^^
>
> Please, declare in ipv4_confirm:
>
> struct net *net = nf_ct_net(ct);
>
> And use the net pointer in that function.
>
> Same thing for the IPv6 side.
>
> Thank you.
--
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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit)
  2011-12-05 18:09   ` Pete Holland
@ 2011-12-05 22:37     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2011-12-05 22:37 UTC (permalink / raw)
  To: Pete Holland; +Cc: netfilter-devel

On Mon, Dec 05, 2011 at 10:09:51AM -0800, Pete Holland wrote:
> would you like that I check *net for NULL as is done for *ct, *help,
> and *helper?

No. net is not ever NULL.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-12-05 22:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-02 17:37 [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID (resubmit) Pete Holland
2011-12-04 22:49 ` Pablo Neira Ayuso
2011-12-05 18:09   ` Pete Holland
2011-12-05 22:37     ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).