netfilter: nf_conntrack: log packets dropped by helpers

netfilter: nf_conntrack: log packets dropped by helpers

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 114 bytes --]

I'm about to commit a couple of patches from some private trees
to my nf-next-2.6.git tree. Any comments welcome.

[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 2377 bytes --]

commit ec4988f91a1750b181cfc87461c3df48b0ab0fc7
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Aug 25 14:21:24 2009 +0200

    netfilter: nf_conntrack: log packets dropped by helpers
    
    Log packets dropped by helpers using the netfilter logging API. This
    is useful in combination with nfnetlink_log to analyze those packets
    in userspace for debugging.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 7d2ead7..44fa394 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -26,6 +26,7 @@
 #include <net/netfilter/ipv4/nf_conntrack_ipv4.h>
 #include <net/netfilter/nf_nat_helper.h>
 #include <net/netfilter/ipv4/nf_defrag_ipv4.h>
+#include <net/netfilter/nf_log.h>
 
 int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
 			      struct nf_conn *ct,
@@ -113,8 +114,11 @@ static unsigned int ipv4_confirm(unsigned int hooknum,
 
 	ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
 			   ct, ctinfo);
-	if (ret != NF_ACCEPT)
+	if (ret != NF_ACCEPT) {
+		nf_log_packet(AF_INET, hooknum, skb, in, out, NULL,
+			      "nf_ct_%s: dropping packet", helper->name);
 		return ret;
+	}
 
 	if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status)) {
 		typeof(nf_nat_seq_adjust_hook) seq_adjust;
diff --git a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
index 2a15c2d..6b76778 100644
--- a/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c
@@ -27,6 +27,7 @@
 #include <net/netfilter/nf_conntrack_l3proto.h>
 #include <net/netfilter/nf_conntrack_core.h>
 #include <net/netfilter/ipv6/nf_conntrack_ipv6.h>
+#include <net/netfilter/nf_log.h>
 
 static bool ipv6_pkt_to_tuple(const struct sk_buff *skb, unsigned int nhoff,
 			      struct nf_conntrack_tuple *tuple)
@@ -176,8 +177,11 @@ static unsigned int ipv6_confirm(unsigned int hooknum,
 	}
 
 	ret = helper->help(skb, protoff, ct, ctinfo);
-	if (ret != NF_ACCEPT)
+	if (ret != NF_ACCEPT) {
+		nf_log_packet(AF_INET6, hooknum, skb, in, out, NULL,
+			      "nf_ct_%s: dropping packet", helper->name);
 		return ret;
+	}
 out:
 	/* We've seen it coming out the other side: confirm it */
 	return nf_conntrack_confirm(skb);

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Jan Engelhardt]

On Tuesday 2009-08-25 14:24, Patrick McHardy wrote:

>I'm about to commit a couple of patches from some private trees
>to my nf-next-2.6.git tree. Any comments welcome.
>
> 
> int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
> 			      struct nf_conn *ct,
>@@ -113,8 +114,11 @@ static unsigned int ipv4_confirm(unsigned int hooknum,
> 
> 	ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
> 			   ct, ctinfo);
>-	if (ret != NF_ACCEPT)
>+	if (ret != NF_ACCEPT) {
>+		nf_log_packet(AF_INET, hooknum, skb, in, out, NULL,
>+			      "nf_ct_%s: dropping packet", helper->name);
> 		return ret;
>+	}

These calls should probably be using NFPROTO_*.

Also, the actual helpers should be audited and any "we dropped things"
messages be removed since now that is done by your patch.

>+	if (ret != NF_ACCEPT) {
>+		nf_log_packet(AF_INET6, hooknum, skb, in, out, NULL,
>+			      "nf_ct_%s: dropping packet", helper->name);
> 		return ret;
>+	}

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Patrick McHardy]

Jan Engelhardt wrote:
> On Tuesday 2009-08-25 14:24, Patrick McHardy wrote:
> 
>> I'm about to commit a couple of patches from some private trees
>> to my nf-next-2.6.git tree. Any comments welcome.
>>
>>
>> int (*nf_nat_seq_adjust_hook)(struct sk_buff *skb,
>> 			      struct nf_conn *ct,
>> @@ -113,8 +114,11 @@ static unsigned int ipv4_confirm(unsigned int hooknum,
>>
>> 	ret = helper->help(skb, skb_network_offset(skb) + ip_hdrlen(skb),
>> 			   ct, ctinfo);
>> -	if (ret != NF_ACCEPT)
>> +	if (ret != NF_ACCEPT) {
>> +		nf_log_packet(AF_INET, hooknum, skb, in, out, NULL,
>> +			      "nf_ct_%s: dropping packet", helper->name);
>> 		return ret;
>> +	}
> 
> These calls should probably be using NFPROTO_*.

Good point, I'll fix that up.

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Pascal Hambourg]

Hello,

May I ask what are the reasons for a helper to drop packets ?

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Patrick McHardy]

Pascal Hambourg wrote:
> Hello,
> 
> May I ask what are the reasons for a helper to drop packets ?

Mainly parsing errors, memory allocation errors and bugs.

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Jan Engelhardt]

On Wednesday 2009-08-26 14:55, Patrick McHardy wrote:

>Pascal Hambourg wrote:
>> Hello,
>> 
>> May I ask what are the reasons for a helper to drop packets ?
>
>Mainly parsing errors, memory allocation errors and bugs.

I am not so quite sure whether parsing errors (like, a port > 65535 for 
FTP) should cause them to be dropped; after all, it might be, for 
example, a vendor-specific form of a protocol that just does not fit the 
nf_conntrack_ftp parsing exactly. That is to say, packets should be let 
through, though without setting up expectations.

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Jozsef Kadlecsik]

On Wed, 26 Aug 2009, Jan Engelhardt wrote:

> On Wednesday 2009-08-26 14:55, Patrick McHardy wrote:
> 
> >Pascal Hambourg wrote:
> >> Hello,
> >> 
> >> May I ask what are the reasons for a helper to drop packets ?
> >
> >Mainly parsing errors, memory allocation errors and bugs.
> 
> I am not so quite sure whether parsing errors (like, a port > 65535 for 
> FTP) should cause them to be dropped; after all, it might be, for 
> example, a vendor-specific form of a protocol that just does not fit the 
> nf_conntrack_ftp parsing exactly. That is to say, packets should be let 
> through, though without setting up expectations.

I usually say that conntrack is not a policy-decision machine and 
therefore should not drop packets.

However, for the protocol helpers I strongly believe if the helper can't 
figure out the protocol elements due to any kind of parser error, then 
the packet should be dropped. Otherwise we may open up possible DoS attack 
vectors to sloppy server/client implementations.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Pascal Hambourg]

Jozsef Kadlecsik a écrit :
> 
> I usually say that conntrack is not a policy-decision machine and 
> therefore should not drop packets.

I strongly agree.

> However, for the protocol helpers I strongly believe if the helper can't 
> figure out the protocol elements due to any kind of parser error, then 
> the packet should be dropped. Otherwise we may open up possible DoS attack 
> vectors to sloppy server/client implementations.

As a user, I do not expect packets to be dropped when I just enable
conntrack without loading any filtering rule. Wouldn't it be better (if
possible) to flag these packets so they can be dropped - or not - by
iptables ?
--
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: netfilter: nf_conntrack: log packets dropped by helpers

[Jozsef Kadlecsik]

On Wed, 26 Aug 2009, Pascal Hambourg wrote:

> Jozsef Kadlecsik a ?crit :
> > 
> > I usually say that conntrack is not a policy-decision machine and 
> > therefore should not drop packets.
> 
> I strongly agree.
> 
> > However, for the protocol helpers I strongly believe if the helper can't 
> > figure out the protocol elements due to any kind of parser error, then 
> > the packet should be dropped. Otherwise we may open up possible DoS attack 
> > vectors to sloppy server/client implementations.
> 
> As a user, I do not expect packets to be dropped when I just enable
> conntrack without loading any filtering rule. Wouldn't it be better (if
> possible) to flag these packets so they can be dropped - or not - by
> iptables ?

There are a few cases when the conntrack core cannot but drop packets. But 
those are very exceptional indeed.

But here we are discussing the protocol helpers: in order to reliably and 
safely handle the related connection - and prevent to be fooled by 
deliberately broken, mangled, etc patterns - the helpers too may have no 
choice but to drop the unparseable packet. And it's not up to the user to 
decide whether the packet should still be let through, because it could 
defeat the access control rules. For a good example just have a look at 
the article 'Breaking through a Firewall using a forged FTP command' in 
Phrack #63.

Best regards, 
Jozsef 
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
          H-1525 Budapest 114, POB. 49, Hungary

Re: netfilter: nf_conntrack: log packets dropped by helpers

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2009-08-26 14:55, Patrick McHardy wrote:
> 
>> Pascal Hambourg wrote:
>>> Hello,
>>>
>>> May I ask what are the reasons for a helper to drop packets ?
>> Mainly parsing errors, memory allocation errors and bugs.
> 
> I am not so quite sure whether parsing errors (like, a port > 65535 for 
> FTP) should cause them to be dropped; after all, it might be, for 
> example, a vendor-specific form of a protocol that just does not fit the 
> nf_conntrack_ftp parsing exactly. That is to say, packets should be let 
> through, though without setting up expectations.

Thats what usually happens. There are a few exceptions though, most
notably the SIP helper, which drops packets since they might already
have been mangled.