H.245v10+ support in nf_conntrack_h323?

H.245v10+ support in nf_conntrack_h323?

[Andreas Jaggi]

Hi

In a scenario where videoconferencing traffic is going through a Linux gateway/firewall, we are having some troubles with the nf_conntrack_h323 moduledropping packets.
The videoconferencing devices use version 10 of the H.245 protocol, but nf_conntrack_h323 supports version 7 (according to the comments in include/linux/nf_conntrack_h323.h).

Are there any plans to include support for version 10 (or higher) of H.245 in nf_conntrack_h323?

Thanks,
Andreas

Re: H.245v10+ support in nf_conntrack_h323?

[Mark Brown]

On Tue, Sep 01, 2009 at 11:29:10AM +0200, Andreas Jaggi wrote:

> The videoconferencing devices use version 10 of the H.245 protocol,
> but nf_conntrack_h323 supports version 7 (according to the comments in
> include/linux/nf_conntrack_h323.h).

> Are there any plans to include support for version 10 (or higher) of
> H.245 in nf_conntrack_h323?

I'd be surprised if the H.245 version were the source of your problems
here - the new protocol versions are backwards compatible and I don't
remember any changes in any of the stuff that's relevant for firewall
transit.

Re: H.245v10+ support in nf_conntrack_h323?

[Patrick McHardy]

Mark Brown wrote:
> On Tue, Sep 01, 2009 at 11:29:10AM +0200, Andreas Jaggi wrote:
> 
>> The videoconferencing devices use version 10 of the H.245 protocol,
>> but nf_conntrack_h323 supports version 7 (according to the comments in
>> include/linux/nf_conntrack_h323.h).
> 
>> Are there any plans to include support for version 10 (or higher) of
>> H.245 in nf_conntrack_h323?

AFAIK Jing Min isn't able to do further development of the H.323 helper.

> I'd be surprised if the H.245 version were the source of your problems
> here - the new protocol versions are backwards compatible and I don't
> remember any changes in any of the stuff that's relevant for firewall
> transit.

Good point. The helper should also log packets dropped due to parsing
errors. If you don't get any messages, I'd suggest to use the iptables
TRACE target to figure out where the packets are dropped exactly.

Re: H.245v10+ support in nf_conntrack_h323?

[Andreas Jaggi]

On Tue, Sep 01, 2009 at 01:25:22PM +0200, Patrick McHardy wrote:
> Mark Brown wrote:
> > I'd be surprised if the H.245 version were the source of your problems
> > here - the new protocol versions are backwards compatible and I don't
> > remember any changes in any of the stuff that's relevant for firewall
> > transit.
> 
> Good point. The helper should also log packets dropped due to parsing
> errors. If you don't get any messages, I'd suggest to use the iptables
> TRACE target to figure out where the packets are dropped exactly.

I'm quite confident that the H.245 parsing/handling is involved in the
packet dropping:
1. there are plenty of 'nf_ct_h245: packet dropped' messages
   (but no nf_ct_q931 or nf_ct_ras messages)
2. without nf_conntrack_h323 the videoconferencing works seamlessly
3. with nf_conntrack_h323 and a ...-j NOTRACK rule the videoconferencing
   works too

In our setup there is a LOG rule preceding any DROP or ACCEPT rule, and
there were no logentries for DROP rules showing up (only the nf_ct_h245:
packet dropped messages).
Would a TRACE rule provide more insight than this? (eg. by showing in
which part of the nf_conntrack_h323 code a packet is dropped?)

For me it looks like nf_conntrack_h323 is not only doing connection
tracking, but also doing protocol enforcement (by dropping packets which
do not correspond exactly to H.245v7).
Perhaps there should be a config option to disable the protocol
enforcement?

Andreas

Re: H.245v10+ support in nf_conntrack_h323?

[Patrick McHardy]

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

Andreas Jaggi wrote:
> On Tue, Sep 01, 2009 at 01:25:22PM +0200, Patrick McHardy wrote:
>> Mark Brown wrote:
>>> I'd be surprised if the H.245 version were the source of your problems
>>> here - the new protocol versions are backwards compatible and I don't
>>> remember any changes in any of the stuff that's relevant for firewall
>>> transit.
>> Good point. The helper should also log packets dropped due to parsing
>> errors. If you don't get any messages, I'd suggest to use the iptables
>> TRACE target to figure out where the packets are dropped exactly.
> 
> I'm quite confident that the H.245 parsing/handling is involved in the
> packet dropping:
> 1. there are plenty of 'nf_ct_h245: packet dropped' messages
>    (but no nf_ct_q931 or nf_ct_ras messages)

Yes, that's the H.323 helper.

> 2. without nf_conntrack_h323 the videoconferencing works seamlessly
> 3. with nf_conntrack_h323 and a ...-j NOTRACK rule the videoconferencing
>    works too
> 
> In our setup there is a LOG rule preceding any DROP or ACCEPT rule, and
> there were no logentries for DROP rules showing up (only the nf_ct_h245:
> packet dropped messages).
> Would a TRACE rule provide more insight than this? (eg. by showing in
> which part of the nf_conntrack_h323 code a packet is dropped?)

No, it only shows the path of the packet through the ruleset.

> For me it looks like nf_conntrack_h323 is not only doing connection
> tracking, but also doing protocol enforcement (by dropping packets which
> do not correspond exactly to H.245v7).
> Perhaps there should be a config option to disable the protocol
> enforcement?

Its unfortunately necessary to drop packets in some cases after parsing
errors when the helper might have already (partially) mangled the
packet.

You could try this patch in combination with ulogd and the pcap output
plugin to capture the packets which are dropped by the helper for
analysis.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2386 bytes --]

commit 74f7a6552c8d76ffc5e11eb8d9d6c07238b9ae77
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Aug 25 15:33:08 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 9ac2fdc..aa95bb8 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(NFPROTO_IPV4, 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 a7f4cd6..5f2ec20 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(NFPROTO_IPV6, 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: H.245v10+ support in nf_conntrack_h323?

[Andreas Jaggi]

On Tue, Sep 01, 2009 at 02:20:51PM +0200, Patrick McHardy wrote:
> Its unfortunately necessary to drop packets in some cases after parsing
> errors when the helper might have already (partially) mangled the
> packet.

Do the helpers also mangle packets when there is no NAT involved?
(in out setup we don't use NAT, btw)
If they don't, would it be reasonable to return NF_ACCEPT when there is
no NAT. For example like in the patch below?

> You could try this patch in combination with ulogd and the pcap output
> plugin to capture the packets which are dropped by the helper for
> analysis.

Unfortunately the videoconferencing system is currently not available
for testing (and might be so for while). But I can have some tcpdumps from
both sides of the Linux Gateway from which I can extract the packets that
were dropped by the H.323 helper.

Andreas


--- a/net/netfilter/nf_conntrack_h323_main.c	2009-05-19 01:52:34.000000000 +0200
+++ b/net/netfilter/nf_conntrack_h323_main.c	2009-09-01 17:36:08.000000000 +0200
@@ -596,8 +596,12 @@
 		}
 
 		/* Process H.245 signal */
-		if (process_h245(skb, ct, ctinfo, &data, dataoff, &mscm) < 0)
-			goto drop;
+		if (process_h245(skb, ct, ctinfo, &data, dataoff, &mscm) < 0) {
+			if (ct->status & IPS_NAT_MASK)
+				goto drop;
+			else
+				break;
+		}
 	}
 
 	spin_unlock_bh(&nf_h323_lock);
@@ -1141,8 +1145,12 @@
 		}
 
 		/* Process Q.931 signal */
-		if (process_q931(skb, ct, ctinfo, &data, dataoff, &q931) < 0)
-			goto drop;
+		if (process_q931(skb, ct, ctinfo, &data, dataoff, &q931) < 0) {
+			if (ct->status & IPS_NAT_MASK)
+				goto drop;
+			else
+				break;
+		}
 	}
 
 	spin_unlock_bh(&nf_h323_lock);
@@ -1716,7 +1724,8 @@
 	}
 
 	/* Process RAS message */
-	if (process_ras(skb, ct, ctinfo, &data, &ras) < 0)
+	if ((process_ras(skb, ct, ctinfo, &data, &ras) < 0) &&
+	    (ct->status & IPS_NAT_MASK))
 		goto drop;
 
       accept:

Re: H.245v10+ support in nf_conntrack_h323?

[Andreas Jaggi]

By using the tcpdumps made during the video conferencing test, I was
able to reproduce the problem with tcpreplay.

Further investigation (eg. sprinkling a couple printks over
nf_conntrack_h323_main.c) showed that the H.245 packet is dropped
because the __nf_ct_expect_check() fails with -EMFILE.
This because max_expected of the H.245 expect policy is reached.

max_expected is set to 'H323_RTP_CHANNEL_MAX * 4 + 2' in
nf_conntrack_h323_main.c and H323_RTP_CHANNEL_MAX is defined as '4'.

Increasing H323_RTP_CHANNEL_MAX solves the problem that H.245 packets
are dropped!

Now I would like to propose a patch to fix this permanently, but I don't
know what a reasonable value for H323_RTP_CHANNEL_MAX would be (only
that the current value is too low for our video conferencing setup).

How was the current value of H323_RTP_CHANNEL_MAX determined? And what
would be the implications of increasing this value?

Andreas

Re: H.245v10+ support in nf_conntrack_h323?

[Mark Brown]

On Tue, Sep 01, 2009 at 02:10:33PM +0200, Andreas Jaggi wrote:

> For me it looks like nf_conntrack_h323 is not only doing connection
> tracking, but also doing protocol enforcement (by dropping packets which
> do not correspond exactly to H.245v7).
> Perhaps there should be a config option to disable the protocol
> enforcement?

It's not going to be a deliberate attempt at enforcement that's the
issue here - doing any substantial enforcement would require a full
decode of the PER messages, involving having a full copy of the (rather
large) ASN.1 in the kernel.  It's much more likely that a shortcut taken
in decoding the message is getting confused by something that the
endpoint is choosing to send, a brief glance at the ASN.1 code certainly
suggests that to me.  Chances are it's not even a new v10 feature that's
causing the issue.