[PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[Pete Holland]

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 the IPPROTO_RAW filter since it is unclear based on the caller
what protocol it actually is.
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-11-29
11:41:55.031121908 -0800
@@ -116,8 +116,9 @@ 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, IPPROTO_RAW))			
+			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-11-29
11:38:49.541033773 -0800
@@ -180,8 +180,9 @@ 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), IPPROTO_RAW))	
+			nf_log_packet(NFPROTO_IPV6, hooknum, skb, in, out, NULL,
+				      "nf_ct_%s: dropping packet", helper->name);
 		return ret;
 	}
 out:

Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[Pete Holland]

it occurred to me that I should be able to extract the protocol number
from the the tuplehash in struct nf_conn.  the original
direction tuple should always be there, and I could get it from there.

so instead of using IPPROTO_RAW, I could use
ct->tuplehash[IP_CT_ORIGINAL].src.l3num

i'm still pretty new in the netfilter code, so any thoughts are
greatly appreciated

On Tue, Nov 29, 2011 at 12:08 PM, Pete Holland <pholland27@gmail.com> 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 the IPPROTO_RAW filter since it is unclear based on the caller
> what protocol it actually is.
> 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-11-29
> 11:41:55.031121908 -0800
> @@ -116,8 +116,9 @@ 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, IPPROTO_RAW))
> +                       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-11-29
> 11:38:49.541033773 -0800
> @@ -180,8 +180,9 @@ 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), IPPROTO_RAW))
> +                       nf_log_packet(NFPROTO_IPV6, hooknum, skb, in, out, NULL,
> +                                     "nf_ct_%s: dropping packet", helper->name);
>                return ret;
>        }
>  out:
>
--
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

Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[Pete Holland]

sorry that should be
ct->tuplehash[IP_CT_ORIGINAL].tuple.src.l3num

On Wed, Nov 30, 2011 at 10:33 AM, Pete Holland <pholland27@gmail.com> wrote:
> it occurred to me that I should be able to extract the protocol number
> from the the tuplehash in struct nf_conn.  the original
> direction tuple should always be there, and I could get it from there.
>
> so instead of using IPPROTO_RAW, I could use
> ct->tuplehash[IP_CT_ORIGINAL].src.l3num
>
> i'm still pretty new in the netfilter code, so any thoughts are
> greatly appreciated
>
> On Tue, Nov 29, 2011 at 12:08 PM, Pete Holland <pholland27@gmail.com> 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 the IPPROTO_RAW filter since it is unclear based on the caller
>> what protocol it actually is.
>> 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-11-29
>> 11:41:55.031121908 -0800
>> @@ -116,8 +116,9 @@ 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, IPPROTO_RAW))
>> +                       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-11-29
>> 11:38:49.541033773 -0800
>> @@ -180,8 +180,9 @@ 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), IPPROTO_RAW))
>> +                       nf_log_packet(NFPROTO_IPV6, hooknum, skb, in, out, NULL,
>> +                                     "nf_ct_%s: dropping packet", helper->name);
>>                return ret;
>>        }
>>  out:
>>
>
--
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

Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[Patrick McHardy]

On 11/30/2011 07:35 PM, Pete Holland wrote:
> sorry that should be
> ct->tuplehash[IP_CT_ORIGINAL].tuple.src.l3num

I'd prefer that to the IPPROTO_RAW usage.

> On Wed, Nov 30, 2011 at 10:33 AM, Pete Holland<pholland27@gmail.com>  wrote:
>> it occurred to me that I should be able to extract the protocol number
>> from the the tuplehash in struct nf_conn.  the original
>> direction tuple should always be there, and I could get it from there.
>>
>> so instead of using IPPROTO_RAW, I could use
>> ct->tuplehash[IP_CT_ORIGINAL].src.l3num
>>
>> i'm still pretty new in the netfilter code, so any thoughts are
>> greatly appreciated
>>
>> On Tue, Nov 29, 2011 at 12:08 PM, Pete Holland<pholland27@gmail.com>  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 the IPPROTO_RAW filter since it is unclear based on the caller
>>> what protocol it actually is.
>>> 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>

Re: [PATCH 1/1] netfilter: conntrack: make call to nf_log_packet due to helper rejection conditional on LOG_INVALID

[Pete Holland]

src.l3num is the protocol number in the ethernet header (ipv4 or ipv6)
so what I actually want is:
ct->tuplehash[IP_CT_ORIGINAL].tuple.dst.protonum
which is the the L4 protocol number

i've built and tested this (by forcing a ret of -NF_ACCEPT) and it
logs (or not) appropriately with the sysctl settings

i'll resubmit the patch with this change

cheers,
Pete

On Wed, Nov 30, 2011 at 10:55 AM, Patrick McHardy <kaber@trash.net> wrote:
> On 11/30/2011 07:35 PM, Pete Holland wrote:
>>
>> sorry that should be
>> ct->tuplehash[IP_CT_ORIGINAL].tuple.src.l3num
>
> I'd prefer that to the IPPROTO_RAW usage.
>
>> On Wed, Nov 30, 2011 at 10:33 AM, Pete Holland<pholland27@gmail.com>
>>  wrote:
>>>
>>> it occurred to me that I should be able to extract the protocol number
>>> from the the tuplehash in struct nf_conn.  the original
>>> direction tuple should always be there, and I could get it from there.
>>>
>>> so instead of using IPPROTO_RAW, I could use
>>> ct->tuplehash[IP_CT_ORIGINAL].src.l3num
>>>
>>> i'm still pretty new in the netfilter code, so any thoughts are
>>> greatly appreciated
>>>
>>> On Tue, Nov 29, 2011 at 12:08 PM, Pete Holland<pholland27@gmail.com>
>>>  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 the IPPROTO_RAW filter since it is unclear based on the caller
>>>> what protocol it actually is.
>>>> 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>
>
>
--
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