[PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Yasuyuki KOZAKAI]

[-- Attachment #1: Type: Text/Plain, Size: 1331 bytes --]


This patch follows the recent changes to ip_conntrack.

Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

===========

    [NETFILTER]: Fix multiple problems with TCP window tracking

    The first attached patch addresses several problems in the current TCP
    connection tracking in the 2.6 tree. Some of the problems was reported,
    others was discovered by nfsim tests:

    - tcp_sack function was not safe against nonlinear skbs
    - practically arbitrary RST segments (addresses, ports assumed to be
      known) could cause connection teardown in conntrack (thanks to Tim
      Burress for the bugreport and patch)
    - article on which the code was based falsely assumed that packets
      must fit completely into the window: packets must at least overlap
      (thanks to Phil Oester for the bugreport and patch)
    - state table slightly changed to handle ACK packets sent by server to
      late resent SYNs
    - tracking reopening connections reworked
    - cosmetic change: when window tracking is ignored by setting
      ip_conntrack_tcp_be_liberal to nonzero, it's ignored completely from
      now on

    Signed-off-by: Patrick McHardy <kaber@trash.net>

-----------------------------------------------------------------
Yasuyuki Kozakai @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

[-- Attachment #2: 01-tcp-window.patch --]
[-- Type: Text/Plain, Size: 10962 bytes --]

diff -Nur linux-2.6.12-rc4-nfct/net/netfilter/nf_conntrack_proto_tcp.c linux-2.6.12-rc4-nfct-1-tcp-window/net/netfilter/nf_conntrack_proto_tcp.c
--- linux-2.6.12-rc4-nfct/net/netfilter/nf_conntrack_proto_tcp.c	2005-05-11 15:21:47.000000000 +0900
+++ linux-2.6.12-rc4-nfct-1-tcp-window/net/netfilter/nf_conntrack_proto_tcp.c	2005-05-19 13:52:56.000000000 +0900
@@ -261,7 +261,7 @@
  *	sSS -> sSR	Standard open.
  *	sSR -> sSR	Retransmitted SYN/ACK.
  *	sES -> sIG	Late retransmitted SYN/ACK?
- *	sFW -> sIG
+ *	sFW -> sIG	Might be SYN/ACK answering ignored SYN
  *	sCW -> sIG
  *	sLA -> sIG
  *	sTW -> sIG
@@ -280,10 +280,10 @@
  *	sCL -> sCL
  */
 /* 	     sNO, sSS, sSR, sES, sFW, sCW, sLA, sTW, sCL, sLI	*/
-/*ack*/	   { sIV, sIG, sIV, sES, sCW, sCW, sTW, sTW, sCL, sIV },
+/*ack*/	   { sIV, sIV, sSR, sES, sCW, sCW, sTW, sTW, sCL, sIV },
 /*
- *	sSS -> sIG	Might be a half-open connection.
- *	sSR -> sIV	Simultaneous open.
+ *	sSS -> sIV	Might be a half-open connection.
+ *	sSR -> sSR	Might answer late resent SYN.
  *	sES -> sES	:-)
  *	sFW -> sCW	Normal close request answered by ACK.
  *	sCW -> sCW
@@ -359,14 +359,19 @@
    http://www.nluug.nl/events/sane2000/papers.html
    http://www.iae.nl/users/guido/papers/tcp_filtering.ps.gz
    
-   The boundaries and the conditions are slightly changed:
-   
+   The boundaries and the conditions are changed according to RFC793:
+   the packet must intersect the window (i.e. segments may be
+   after the right or before the left edge) and thus receivers may ACK
+   segments after the right edge of the window.
+
    	td_maxend = max(sack + max(win,1)) seen in reply packets
 	td_maxwin = max(max(win, 1)) + (sack - ack) seen in sent packets
+	td_maxwin += seq + len - sender.td_maxend
+			if seq + len > sender.td_maxend
 	td_end    = max(seq + len) seen in sent packets
    
-   I. 	Upper bound for valid data:	seq + len <= sender.td_maxend
-   II. 	Lower bound for valid data:	seq >= sender.td_end - receiver.td_maxwin
+   I.   Upper bound for valid data:	seq <= sender.td_maxend
+   II.  Lower bound for valid data:	seq + len >= sender.td_end - receiver.td_maxwin
    III.	Upper bound for valid ack:      sack <= receiver.td_end
    IV.	Lower bound for valid ack:	ack >= receiver.td_end - MAXACKWINDOW
 
@@ -455,10 +460,13 @@
 static void tcp_sack(const struct sk_buff *skb, unsigned int dataoff,
 		     struct tcphdr *tcph, __u32 *sack)
 {
-	__u32 tmp;
-	unsigned char *ptr;
         unsigned char buff[(15 * 4) - sizeof(struct tcphdr)];
+	unsigned char *ptr;
 	int length = (tcph->doff*4) - sizeof(struct tcphdr);
+	__u32 tmp;
+
+	if (!length)
+		return;
 
 	ptr = skb_header_pointer(skb, dataoff + sizeof(struct tcphdr),
 				 length, buff);
@@ -515,7 +523,7 @@
 
 static int tcp_in_window(struct ip_ct_tcp *state, 
                          enum ip_conntrack_dir dir,
-                         unsigned int *index,
+                         unsigned int index,
                          const struct sk_buff *skb,
 			 unsigned int dataoff,
                          struct tcphdr *tcph,
@@ -614,20 +622,23 @@
 		ack = sack = receiver->td_end;
 	}
 
-	if (seq == end)
+	if (seq == end
+	    && (!tcph->rst
+		|| (seq == 0 && state->state == TCP_CONNTRACK_SYN_SENT)))
 		/*
 		 * Packets contains no data: we assume it is valid
 		 * and check the ack value only.
+		 * However RST segments are always validated by their
+		 * SEQ number, except when seq == 0 (reset sent answering
+		 * SYN.
 		 */
 		seq = end = sender->td_end;
 
 	DEBUGP("tcp_in_window: src=%u.%u.%u.%u:%hu dst=%u.%u.%u.%u:%hu "
-	       "seq=%u ack=%u sack =%u win=%u end=%u trim=%u\n",
+	       "seq=%u ack=%u sack =%u win=%u end=%u\n",
 		NIPQUAD(iph->saddr), ntohs(tcph->source),
 		NIPQUAD(iph->daddr), ntohs(tcph->dest),
-		seq, ack, sack, win, end, 
-		after(end, sender->td_maxend) && before(seq, sender->td_maxend)
-		? sender->td_maxend : end);
+		seq, ack, sack, win, end);
 	DEBUGP("tcp_in_window: sender end=%u maxend=%u maxwin=%u scale=%i "
 	       "receiver end=%u maxend=%u maxwin=%u scale=%i\n",
 		sender->td_end, sender->td_maxend, sender->td_maxwin,
@@ -635,24 +646,15 @@
 		receiver->td_end, receiver->td_maxend, receiver->td_maxwin,
 		receiver->td_scale);
 
-	/* Ignore data over the right edge of the receiver's window. */
-	if (after(end, sender->td_maxend) &&
-	    before(seq, sender->td_maxend)) {
-		end = sender->td_maxend;
-		if (*index == TCP_FIN_SET)
-			*index = TCP_ACK_SET;
-	}
 	DEBUGP("tcp_in_window: I=%i II=%i III=%i IV=%i\n",
-		before(end, sender->td_maxend + 1) 
-		    || before(seq, sender->td_maxend + 1),
-	    	after(seq, sender->td_end - receiver->td_maxwin - 1) 
-	    	    || after(end, sender->td_end - receiver->td_maxwin - 1),
+		before(seq, sender->td_maxend + 1),
+		after(end, sender->td_end - receiver->td_maxwin - 1),
 	    	before(sack, receiver->td_end + 1),
 	    	after(ack, receiver->td_end - MAXACKWINDOW(sender)));
 
 	if (sender->loose || receiver->loose ||
-	    (before(end, sender->td_maxend + 1) &&
-	     after(seq, sender->td_end - receiver->td_maxwin - 1) &&
+	    (before(seq, sender->td_maxend + 1) &&
+	     after(end, sender->td_end - receiver->td_maxwin - 1) &&
 	     before(sack, receiver->td_end + 1) &&
 	     after(ack, receiver->td_end - MAXACKWINDOW(sender)))) {
 	    	/*
@@ -669,6 +671,11 @@
 			sender->td_maxwin = swin;
 		if (after(end, sender->td_end))
 			sender->td_end = end;
+		/*
+		 * Update receiver data.
+		 */
+		if (after(end, sender->td_maxend))
+			receiver->td_maxwin += end - sender->td_maxend;
 		if (after(sack + win, receiver->td_maxend - 1)) {
 			receiver->td_maxend = sack + win;
 			if (win == 0)
@@ -678,7 +685,7 @@
 		/* 
 		 * Check retransmissions.
 		 */
-		if (*index == TCP_ACK_SET) {
+		if (index == TCP_ACK_SET) {
 			if (state->last_dir == dir
 			    && state->last_seq == seq
 			    && state->last_ack == ack
@@ -703,16 +710,16 @@
 		if (LOG_INVALID(IPPROTO_TCP))
 			nf_log_packet(pf, 0, skb, NULL, NULL,
 			"nf_ct_tcp: %s ",
-			before(end, sender->td_maxend + 1) ?
-			after(seq, sender->td_end - receiver->td_maxwin - 1) ?
+			before(seq, sender->td_maxend + 1) ?
+			after(end, sender->td_end - receiver->td_maxwin - 1) ?
 			before(sack, receiver->td_end + 1) ?
 			after(ack, receiver->td_end - MAXACKWINDOW(sender)) ? "BUG"
-			: "ACK is under the lower bound (possibly overly delayed ACK)"
-			: "ACK is over the upper bound (ACKed data has never seen yet)"
-			: "SEQ is under the lower bound (retransmitted already ACKed data)"
+			: "ACK is under the lower bound (possible overly delayed ACK)"
+			: "ACK is over the upper bound (ACKed data not seen yet)"
+			: "SEQ is under the lower bound (already ACKed data retransmitted)"
 			: "SEQ is over the upper bound (over the window of the receiver)");
 
-		res = nf_ct_tcp_be_liberal && !tcph->rst;
+		res = nf_ct_tcp_be_liberal;
   	}
   
 	DEBUGP("tcp_in_window: res=%i sender end=%u maxend=%u maxwin=%u "
@@ -903,13 +910,12 @@
 	switch (new_state) {
 	case TCP_CONNTRACK_IGNORE:
 		/* Either SYN in ORIGINAL
-		 * or SYN/ACK in REPLY
-		 * or ACK in REPLY direction (half-open connection). */
+		 * or SYN/ACK in REPLY. */
 		if (index == TCP_SYNACK_SET
 		    && conntrack->proto.tcp.last_index == TCP_SYN_SET
 		    && conntrack->proto.tcp.last_dir != dir
-		    && after(ntohl(th->ack_seq),
-		    	     conntrack->proto.tcp.last_seq)) {
+		    && ntohl(th->ack_seq) ==
+		    	     conntrack->proto.tcp.last_end) {
 			/* This SYN/ACK acknowledges a SYN that we earlier 
 			 * ignored as invalid. This means that the client and
 			 * the server are both in sync, while the firewall is
@@ -929,6 +935,8 @@
 		conntrack->proto.tcp.last_index = index;
 		conntrack->proto.tcp.last_dir = dir;
 		conntrack->proto.tcp.last_seq = ntohl(th->seq);
+		conntrack->proto.tcp.last_end =
+		    segment_seq_plus_len(ntohl(th->seq), skb->len, dataoff, th);
 
 		WRITE_UNLOCK(&tcp_lock);
 		if (LOG_INVALID(IPPROTO_TCP))
@@ -946,7 +954,12 @@
 				  "nf_ct_tcp: invalid state ");
 		return -NF_ACCEPT;
 	case TCP_CONNTRACK_SYN_SENT:
-		if (old_state >= TCP_CONNTRACK_TIME_WAIT) {
+		if (old_state < TCP_CONNTRACK_TIME_WAIT)
+			break;
+		if ((conntrack->proto.tcp.seen[dir].flags &
+			IP_CT_TCP_FLAG_CLOSE_INIT)
+		    || after(ntohl(th->seq),
+			     conntrack->proto.tcp.seen[dir].td_end)) {
 		    	/* Attempt to reopen a closed connection.
 		    	* Delete this connection and look up again. */
 		    	WRITE_UNLOCK(&tcp_lock);
@@ -955,22 +968,16 @@
 		    					    conntrack);
 		    	return -NF_REPEAT;
 		}
-		break;
 	case TCP_CONNTRACK_CLOSE:
 		if (index == TCP_RST_SET
-		    && ((test_bit(IPS_SEEN_REPLY_BIT, &conntrack->status)
-		         && conntrack->proto.tcp.last_index <= TCP_SYNACK_SET)
-			|| (!test_bit(IPS_ASSURED_BIT, &conntrack->status)
-			 && conntrack->proto.tcp.last_index == TCP_ACK_SET))
-		    && after(ntohl(th->ack_seq),
-		    	     conntrack->proto.tcp.last_seq)) {
-			/* Ignore RST closing down invalid SYN or ACK
-			   we had let trough. */ 
-		    	WRITE_UNLOCK(&tcp_lock);
-			if (LOG_INVALID(IPPROTO_TCP))
-				nf_log_packet(pf, 0, skb, NULL, NULL, 
-					  "nf_ct_tcp: invalid RST (ignored) ");
-			return NF_ACCEPT;
+		    && test_bit(IPS_SEEN_REPLY_BIT, &conntrack->status)
+		    && conntrack->proto.tcp.last_index == TCP_SYN_SET
+		    && ntohl(th->ack_seq) == conntrack->proto.tcp.last_end) {
+			/* RST sent to invalid SYN we had let trough
+			 * SYN was in window then, tear down connection.
+			 * We skip window checking, because packet might ACK
+			 * segments we ignored in the SYN. */
+			goto in_window;
 		}
 		/* Just fall trough */
 	default:
@@ -978,16 +985,14 @@
 		break;
 	}
 
-	if (!tcp_in_window(&conntrack->proto.tcp, dir, &index, 
+	if (!tcp_in_window(&conntrack->proto.tcp, dir, index,
 			   skb, dataoff, th, pf)) {
 		WRITE_UNLOCK(&tcp_lock);
 		return -NF_ACCEPT;
 	}
+     in_window:
 	/* From now on we have got in-window packets */
-
-	/* If FIN was trimmed off, we don't change state. */
 	conntrack->proto.tcp.last_index = index;
-	new_state = tcp_conntracks[dir][index][old_state];
 
 	DEBUGP("tcp_conntracks: src=%u.%u.%u.%u:%hu dst=%u.%u.%u.%u:%hu "
 	       "syn=%i ack=%i fin=%i rst=%i old=%i new=%i\n",
@@ -998,6 +1003,10 @@
 		old_state, new_state);
 
 	conntrack->proto.tcp.state = new_state;
+	if (old_state != new_state
+	    && (new_state == TCP_CONNTRACK_FIN_WAIT
+		|| new_state == TCP_CONNTRACK_CLOSE))
+		conntrack->proto.tcp.seen[dir].flags |= IP_CT_TCP_FLAG_CLOSE_INIT;
 	timeout = conntrack->proto.tcp.retrans >= nf_ct_tcp_max_retrans
 		  && *tcp_timeouts[new_state] > nf_ct_tcp_timeout_max_retrans
 		  ? nf_ct_tcp_timeout_max_retrans : *tcp_timeouts[new_state];
@@ -1028,7 +1037,7 @@
 	return NF_ACCEPT;
 }
  
-  /* Called when a new connection for this protocol found. */
+/* Called when a new connection for this protocol found. */
 static int tcp_new(struct nf_conn *conntrack,
 		   const struct sk_buff *skb,
 		   unsigned int dataoff)

Re: [PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Michal Rokos]

Hello,

>>>>> "YK" == Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> writes:
YK>
YK> This patch follows the recent changes to ip_conntrack.
YK>

could I ask you against what is this patch?

I'm using 
http://svn.netfilter.org/netfilter/trunk/patch-o-matic-ng/nf_conntrack and 
I'm getting rejects when applying this patch...

Namely:
***************
*** 515,521 ****

  static int tcp_in_window(struct ip_ct_tcp *state,
                           enum ip_conntrack_dir dir,
-                          unsigned int *index,
                           const struct sk_buff *skb,
                         unsigned int dataoff,
                           struct tcphdr *tcph,
--- 523,529 ----

  static int tcp_in_window(struct ip_ct_tcp *state,
                           enum ip_conntrack_dir dir,
+                          unsigned int index,
                           const struct sk_buff *skb,
                         unsigned int dataoff,
                           struct tcphdr *tcph,
but I have there: static int tcp_in_window(struct nf_ct_tcp *state,

When I tried to apply '[PATCH NF_CONNTRACK 1/9]: unify common symbols of 
{ip,nf}_conntrack' you posted 2005-03-22, I'm getting rejects too...

So it seems I follow odd repository.

Do you have any hint what repository should I track?

Thank you.

 Michal

nf_conntrack tree

[Patrick McHardy]

I'm working on getting nf_conntrack ready for submission. To make it
easier for people to participate, I've exported my tree through
bitkeeper and as plain patch. It currently contains nf_conntrack as
found in pomng plus a couple of ported ip_conntrack fixes:

bk://numenor.coreworks.de/nf-2.6-nfct
http://numenor.coreworks.de/~kaber

The tree and the diff will be kept up-to-date when changes are made.

In my opinion the most important issue at this time is making
ip_conntrack matches and targets work properly with nf_conntrack and
adding their IPv6 equivalents. Before doing the IPv6 part, I would like
to get sorted out if we can share the IPv4 matches/targets between
ip_conntrack and nf_conntrack.

Regards
Patrick

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

Hi, Patrick,

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 16 Mar 2005 01:35:32 +0100

> I'm working on getting nf_conntrack ready for submission. To make it
> easier for people to participate, I've exported my tree through
> bitkeeper and as plain patch. It currently contains nf_conntrack as
> found in pomng plus a couple of ported ip_conntrack fixes:
> 
> bk://numenor.coreworks.de/nf-2.6-nfct
> http://numenor.coreworks.de/~kaber
> 
> The tree and the diff will be kept up-to-date when changes are made.

Thanks. I'll look that.

> In my opinion the most important issue at this time is making
> ip_conntrack matches and targets work properly with nf_conntrack and
> adding their IPv6 equivalents. Before doing the IPv6 part, I would like
> to get sorted out if we can share the IPv4 matches/targets between
> ip_conntrack and nf_conntrack.

There are issues to do this.
	1. conflicting symbols when including ip_conntrack.h and nf_conntrack.
	   -> easy work. We can change symbols in nf_conntrack_*.h.

	2. I would not like to change UI. If ip_conntrack isn't loaded when
	   these matches/targets are loaded, I want kernel to load ip_conntrack
	   automatically. I suggests changes in init() as following.

		int ret = -1;	   

		if (get_symbols(need_ip_conntrack) != NULL) {
			ret = ip_register_match(match_func_with_ip_conntrack);

		} else if (get_symbols(need_nf_conntrack) != NULL) {
			ret = ip_register_match(match_func_with_nf_conntrack);
		} else
			return -1;

		return ret;

	  I don't know this usage of get_symbols() is right or not.
	  If right, ip_conntrack will be loaded automatically in the case that
	  ip_conntrack and nf_conntrack are not loaded.
	  If either of them is loaded, it will be used.

	3. The symbol "ip_conntrack_untracked" which depends on ip_conntrack.
	   state, conntrack, NOTRACK use this symbol. How about defining
	   "void *nf_ct_untracked" in net/core/netfilter.c and
	   set &ip_conntrack_untracked.general/&nf_conntrack_untracked.general
	   to it when initializing ip_conntrack/nf_conntrack ?

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: nf_conntrack tree

[Pablo Neira]

Yasuyuki KOZAKAI wrote:
> 	3. The symbol "ip_conntrack_untracked" which depends on ip_conntrack.
> 	   state, conntrack, NOTRACK use this symbol. How about defining
> 	   "void *nf_ct_untracked" in net/core/netfilter.c and
> 	   set &ip_conntrack_untracked.general/&nf_conntrack_untracked.general
> 	   to it when initializing ip_conntrack/nf_conntrack ?

Something I have in mind. I think that we could use one of the bits of 
nfcache explicitely mark invalid connections. If we get an invalid 
packet, conntrack can set the NFC_INVALID bit. That way we interpret a 
NULL pointer in skb->nfct as a untracked connection and kill that fake 
conntrack.

--
Pablo

Re: nf_conntrack tree

[Jozsef Kadlecsik]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1145 bytes --]

Hi,

On Wed, 16 Mar 2005, Pablo Neira wrote:

> Yasuyuki KOZAKAI wrote:
> > 	3. The symbol "ip_conntrack_untracked" which depends on ip_conntrack.
> > 	   state, conntrack, NOTRACK use this symbol. How about defining
> > 	   "void *nf_ct_untracked" in net/core/netfilter.c and
> > 	   set &ip_conntrack_untracked.general/&nf_conntrack_untracked.general
> > 	   to it when initializing ip_conntrack/nf_conntrack ?
>
> Something I have in mind. I think that we could use one of the bits of
> nfcache explicitely mark invalid connections. If we get an invalid
> packet, conntrack can set the NFC_INVALID bit. That way we interpret a
> NULL pointer in skb->nfct as a untracked connection and kill that fake
> conntrack.

When working on a conntrack-friendly TARPIT target, I added "general" glue
function support in order to avoid unnecessary cross-dependency. Have a
look at the attached patch.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlec@sunserv.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

[-- Attachment #2: TARPIT.patch --]
[-- Type: TEXT/PLAIN, Size: 18166 bytes --]

diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/include/linux/netfilter.h linux-2.6.9-TARPIT/include/linux/netfilter.h
--- linux-2.6.9-orig/include/linux/netfilter.h	2004-10-18 23:54:08.000000000 +0200
+++ linux-2.6.9-TARPIT/include/linux/netfilter.h	2005-02-08 12:33:13.000000000 +0100
@@ -178,12 +178,18 @@
 ip6t_find_target_lock(const char *name, int *error, struct semaphore *mutex);
 extern inline struct arpt_target *
 arpt_find_target_lock(const char *name, int *error, struct semaphore *mutex);
-extern void (*ip_ct_attach)(struct sk_buff *, struct sk_buff *);
 
 #ifdef CONFIG_NETFILTER_DEBUG
 extern void nf_dump_skb(int pf, struct sk_buff *skb);
 #endif
 
+/* Glue functions to avoid unnecessary dependencies */
+struct nf_ip_glue_functions {
+       void (*ip_ct_attach)(struct sk_buff *, struct sk_buff *);
+       void (*ip_ct_notrack)(struct sk_buff *);
+};
+extern struct nf_ip_glue_functions nf_ip_glue;
+
 /* FIXME: Before cache is ever used, this must be implemented for real. */
 extern void nf_invalidate_cache(int pf);
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/core/netfilter.c linux-2.6.9-TARPIT/net/core/netfilter.c
--- linux-2.6.9-orig/net/core/netfilter.c	2004-10-18 23:54:07.000000000 +0200
+++ linux-2.6.9-TARPIT/net/core/netfilter.c	2005-02-09 16:27:51.000000000 +0100
@@ -48,6 +48,7 @@
 struct list_head nf_hooks[NPROTO][NF_MAX_HOOKS];
 static LIST_HEAD(nf_sockopts);
 static spinlock_t nf_hook_lock = SPIN_LOCK_UNLOCKED;
+struct nf_ip_glue_functions nf_ip_glue = {};
 
 /* 
  * A queue handler may be registered for each protocol.  Each is protected by
@@ -802,12 +803,6 @@
 EXPORT_SYMBOL(nf_log_unregister);
 EXPORT_SYMBOL(nf_log_packet);
 
-/* This does not belong here, but ipt_REJECT needs it if connection
-   tracking in use: without this, connection may not be in hash table,
-   and hence manufactured ICMP or RST packets will not be associated
-   with it. */
-void (*ip_ct_attach)(struct sk_buff *, struct sk_buff *);
-
 void __init netfilter_init(void)
 {
 	int i, h;
@@ -818,7 +813,7 @@
 	}
 }
 
-EXPORT_SYMBOL(ip_ct_attach);
+EXPORT_SYMBOL(nf_ip_glue);
 EXPORT_SYMBOL(ip_route_me_harder);
 EXPORT_SYMBOL(nf_getsockopt);
 EXPORT_SYMBOL(nf_hook_slow);
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/Kconfig linux-2.6.9-TARPIT/net/ipv4/netfilter/Kconfig
--- linux-2.6.9-orig/net/ipv4/netfilter/Kconfig	2004-10-18 23:54:55.000000000 +0200
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/Kconfig	2005-02-08 12:36:33.000000000 +0100
@@ -412,6 +412,25 @@
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_TARGET_TARPIT
+	tristate 'TARPIT target support'
+	depends on IP_NF_FILTER
+	help
+	  Adds a TARPIT target to iptables, which captures and holds
+	  incoming TCP connections using no local per-connection resources.
+	  Connections are accepted, but immediately switched to the persist
+	  state (0 byte window), in which the remote side stops sending data
+	  and asks to continue every 60-240 seconds.  Attempts to close the
+	  connection are ignored, forcing the remote side to time out the
+	  connection in 12-24 minutes.
+	
+	  This offers similar functionality to LaBrea
+	  <http://www.hackbusters.net/LaBrea/> but doesn't require dedicated
+	  hardware or IPs.  Any TCP port that you would normally DROP or 
+	  REJECT can instead become a TARPIT.
+	  
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 # NAT + specific targets
 config IP_NF_NAT
 	tristate "Full NAT"
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/Makefile linux-2.6.9-TARPIT/net/ipv4/netfilter/Makefile
--- linux-2.6.9-orig/net/ipv4/netfilter/Makefile	2004-10-18 23:53:43.000000000 +0200
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/Makefile	2005-02-08 12:36:56.000000000 +0100
@@ -70,6 +70,7 @@
 
 # targets
 obj-$(CONFIG_IP_NF_TARGET_REJECT) += ipt_REJECT.o
+obj-$(CONFIG_IP_NF_TARGET_TARPIT) += ipt_TARPIT.o
 obj-$(CONFIG_IP_NF_TARGET_TOS) += ipt_TOS.o
 obj-$(CONFIG_IP_NF_TARGET_ECN) += ipt_ECN.o
 obj-$(CONFIG_IP_NF_TARGET_DSCP) += ipt_DSCP.o
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/ip_conntrack_core.c linux-2.6.9-TARPIT/net/ipv4/netfilter/ip_conntrack_core.c
--- linux-2.6.9-orig/net/ipv4/netfilter/ip_conntrack_core.c	2004-10-18 23:53:05.000000000 +0200
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/ip_conntrack_core.c	2005-02-09 16:44:33.000000000 +0100
@@ -1176,6 +1176,18 @@
 	nf_conntrack_get(nskb->nfct);
 }
 
+/* Used by the NOTRACK and TARPIT targets. */
+static void ip_conntrack_notrack(struct sk_buff *skb)
+{
+	/* Attach fake conntrack entry. 
+	   If there is a real ct entry correspondig to this packet, 
+	   it'll hang aroun till timing out. We don't deal with it
+	   for performance reasons. JK */
+	skb->nfct = &ip_conntrack_untracked.ct_general;
+	skb->nfctinfo = IP_CT_NEW;
+	nf_conntrack_get(skb->nfct);
+}
+
 static inline int
 do_kill(const struct ip_conntrack_tuple_hash *i,
 	int (*kill)(const struct ip_conntrack *i, void *data),
@@ -1290,7 +1302,8 @@
    supposed to kill the mall. */
 void ip_conntrack_cleanup(void)
 {
-	ip_ct_attach = NULL;
+	nf_ip_glue.ip_ct_attach = NULL;
+	nf_ip_glue.ip_ct_notrack = NULL;
 	/* This makes sure all current packets have passed through
            netfilter framework.  Roll on, two-stage module
            delete... */
@@ -1379,8 +1392,9 @@
 	for (i = 0; i < ip_conntrack_htable_size; i++)
 		INIT_LIST_HEAD(&ip_conntrack_hash[i]);
 
-	/* For use by ipt_REJECT */
-	ip_ct_attach = ip_conntrack_attach;
+	/* Set glue functions */
+	nf_ip_glue.ip_ct_attach = ip_conntrack_attach;
+	nf_ip_glue.ip_ct_notrack = ip_conntrack_notrack;
 
 	/* Set up fake conntrack:
 	    - to never be deleted, not in any hashes */
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/ipt_NOTRACK.c linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_NOTRACK.c
--- linux-2.6.9-orig/net/ipv4/netfilter/ipt_NOTRACK.c	2004-10-18 23:53:11.000000000 +0200
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_NOTRACK.c	2004-12-18 12:27:50.000000000 +0100
@@ -15,18 +15,17 @@
        const void *targinfo,
        void *userinfo)
 {
+	void (*notrack)(struct sk_buff *);
+	
 	/* Previously seen (loopback)? Ignore. */
 	if ((*pskb)->nfct != NULL)
 		return IPT_CONTINUE;
 
-	/* Attach fake conntrack entry. 
-	   If there is a real ct entry correspondig to this packet, 
-	   it'll hang aroun till timing out. We don't deal with it
-	   for performance reasons. JK */
-	(*pskb)->nfct = &ip_conntrack_untracked.ct_general;
-	(*pskb)->nfctinfo = IP_CT_NEW;
-	nf_conntrack_get((*pskb)->nfct);
-
+	/* Avoid module unload race with nf_ip_glue being NULLed out */
+	if ((notrack = nf_ip_glue.ip_ct_notrack) != NULL) {
+		mb(); /* Just to be sure: must be read before executing this */
+		notrack(*pskb);
+	}
 	return IPT_CONTINUE;
 }
 
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/ipt_REJECT.c linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_REJECT.c
--- linux-2.6.9-orig/net/ipv4/netfilter/ipt_REJECT.c	2004-10-18 23:54:55.000000000 +0200
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_REJECT.c	2004-12-16 11:15:06.000000000 +0100
@@ -45,8 +45,8 @@
 {
 	void (*attach)(struct sk_buff *, struct sk_buff *);
 
-	/* Avoid module unload race with ip_ct_attach being NULLed out */
-	if (skb->nfct && (attach = ip_ct_attach) != NULL) {
+	/* Avoid module unload race with nf_ip_glue being NULLed out */
+	if (skb->nfct && (attach = nf_ip_glue.ip_ct_attach) != NULL) {
 		mb(); /* Just to be sure: must be read before executing this */
 		attach(new_skb, skb);
 	}
diff -urN --exclude-from=/usr/src/diff.exclude linux-2.6.9-orig/net/ipv4/netfilter/ipt_TARPIT.c linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_TARPIT.c
--- linux-2.6.9-orig/net/ipv4/netfilter/ipt_TARPIT.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.9-TARPIT/net/ipv4/netfilter/ipt_TARPIT.c	2005-02-09 15:00:03.000000000 +0100
@@ -0,0 +1,338 @@
+/* 
+ * Kernel module to capture and hold incoming TCP connections using 
+ * no local per-connection resources.
+ * 
+ * Based on ipt_REJECT.c and offering functionality similar to 
+ * LaBrea <http://www.hackbusters.net/LaBrea/>.
+ * 
+ * Copyright (c) 2002 Aaron Hopkins <tools@die.net>
+ * 
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * 
+ * Goal:
+ * - Allow incoming TCP connections to be established.
+ * - Passing data should result in the connection being switched to the 
+ *   persist state (0 byte window), in which the remote side stops sending 
+ *   data and asks to continue every 60 seconds.
+ * - Attempts to shut down the connection should be ignored completely, so 
+ *   the remote side ends up having to time it out.
+ *
+ * This means:
+ * - Reply to TCP SYN,!ACK,!RST,!FIN with SYN-ACK, window 5 bytes
+ * - Reply to TCP SYN,ACK,!RST,!FIN with RST to prevent spoofing
+ * - Reply to TCP !SYN,!RST,!FIN with ACK, window 0 bytes, rate-limited
+ */
+
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <net/ip.h>
+#include <net/tcp.h>
+#include <net/icmp.h>
+#include <net/route.h>
+#include <linux/random.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+
+#if 0
+#define DEBUGP printk
+#else
+#define DEBUGP(format, args...)
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Aaron Hopkins <tools@die.net>");
+MODULE_DESCRIPTION("iptables TARPIT target module");
+
+/* From ipt_REJECT.c */
+static inline struct rtable *route_reverse(struct sk_buff *skb, int hook)
+{
+	struct iphdr *iph = skb->nh.iph;
+	struct dst_entry *odst;
+	struct flowi fl = {};
+	struct rtable *rt;
+
+	/* We don't require ip forwarding to be enabled to be able to
+	 * send a RST reply for bridged traffic. */
+	if (hook == NF_IP_LOCAL_IN
+#ifdef CONFIG_BRIDGE_NETFILTER
+	    || (skb->nf_bridge && skb->nf_bridge->mask & BRNF_BRIDGED)
+#endif
+	   ) {
+		fl.nl_u.ip4_u.daddr = iph->saddr;
+		if (hook == NF_IP_LOCAL_IN)
+			fl.nl_u.ip4_u.saddr = iph->daddr;
+		fl.nl_u.ip4_u.tos = RT_TOS(iph->tos);
+
+		if (ip_route_output_key(&rt, &fl) != 0)
+			return NULL;
+	} else {
+		/* non-local src, find valid iif to satisfy
+		 * rp-filter when calling ip_route_input. */
+		fl.nl_u.ip4_u.daddr = iph->daddr;
+		if (ip_route_output_key(&rt, &fl) != 0)
+			return NULL;
+
+		odst = skb->dst;
+		if (ip_route_input(skb, iph->saddr, iph->daddr,
+		                   RT_TOS(iph->tos), rt->u.dst.dev) != 0) {
+			dst_release(&rt->u.dst);
+			return NULL;
+		}
+		dst_release(&rt->u.dst);
+		rt = (struct rtable *)skb->dst;
+		skb->dst = odst;
+	}
+
+	if (rt->u.dst.error) {
+		dst_release(&rt->u.dst);
+		rt = NULL;
+	}
+
+	return rt;
+}
+
+/* Send reply */
+static void tarpit_tcp(struct sk_buff *oskb, int hook)
+{
+	struct sk_buff *nskb;
+	struct rtable *ort = (struct rtable*)oskb->dst, *nrt;
+	struct tcphdr _otcph, *otcph, *ntcph;
+	u_int32_t tmp_addr;
+	u_int16_t tmp_port;
+	int hh_len;
+	void (*notrack)(struct sk_buff *);
+
+	otcph = skb_header_pointer(oskb, oskb->nh.iph->ihl * 4,
+				   sizeof(_otcph), &_otcph);
+	if (otcph == NULL)
+		return;
+
+	/* No replies for RST or FIN */
+	if (otcph->rst || otcph->fin)
+		return;
+
+	/* No reply to !SYN,!ACK.  Rate-limit replies to !SYN,ACKs */
+	if (!otcph->syn && (!otcph->ack || !xrlim_allow(&ort->u.dst, 1*HZ)))
+		return;
+
+	/* FIXME: check checksum */
+
+	/* Reverse routing */
+	if ((nrt = route_reverse(oskb, hook)) == NULL)
+		return;
+
+	hh_len = LL_RESERVED_SPACE(nrt->u.dst.dev);
+
+	/* We need a linear, writeable skb.  We also need to expand
+	   headroom in case hh_len of incoming interface < hh_len of
+	   outgoing interface */
+	nskb = skb_copy_expand(oskb, hh_len, skb_tailroom(oskb),
+			       GFP_ATOMIC);
+	if (!nskb) {
+		dst_release(&nrt->u.dst);
+		return;
+	}
+
+	dst_release(nskb->dst);
+	nskb->dst = &nrt->u.dst;
+
+	/* This packet will not be the same as the other: clear nf fields */
+	nf_reset(nskb);
+	nskb->nfcache = 0;
+	nskb->nfmark = 0;
+#ifdef CONFIG_BRIDGE_NETFILTER
+	nf_bridge_put(nskb->nf_bridge);
+	nskb->nf_bridge = NULL;
+#endif
+
+	ntcph = (struct tcphdr *)((u_int32_t*)nskb->nh.iph + nskb->nh.iph->ihl);
+
+	/* Truncate to length (no data) */
+	ntcph->doff = sizeof(struct tcphdr)/4;
+	skb_trim(nskb, nskb->nh.iph->ihl*4 + sizeof(struct tcphdr));
+	nskb->nh.iph->tot_len = htons(nskb->len);
+
+	/* Swap source and dest */
+	tmp_addr = nskb->nh.iph->saddr;
+	nskb->nh.iph->saddr = nskb->nh.iph->daddr;
+	nskb->nh.iph->daddr = tmp_addr;
+	tmp_port = ntcph->source;
+	ntcph->source = ntcph->dest;
+	ntcph->dest = tmp_port;
+
+	/* Use supplied sequence number or make a new one */
+	ntcph->seq = otcph->ack ? otcph->ack_seq 
+                     : htonl(secure_tcp_sequence_number(nskb->nh.iph->saddr, 
+						        nskb->nh.iph->daddr, 
+						        ntcph->source, 
+			     			        ntcph->dest));
+
+	/* Our SYN-ACKs must have a >0 window */
+	ntcph->window = (otcph->syn && !otcph->ack) ? htons(5) : 0;
+
+	ntcph->urg_ptr = 0;
+
+	/* Reset flags */
+	((u_int8_t *)ntcph)[13] = 0;
+
+	if (otcph->syn && otcph->ack) {
+		ntcph->rst = 1;
+		ntcph->ack_seq = 0;
+	} else {
+		ntcph->syn = otcph->syn;
+		ntcph->ack = 1;
+		ntcph->ack_seq = htonl(ntohl(otcph->seq) + otcph->syn);
+	}
+
+	/* Adjust TCP checksum */
+	ntcph->check = 0;
+	ntcph->check = tcp_v4_check(ntcph, sizeof(struct tcphdr),
+				   nskb->nh.iph->saddr,
+				   nskb->nh.iph->daddr,
+				   csum_partial((char *)ntcph,
+						sizeof(struct tcphdr), 0));
+
+	/* Adjust IP TTL */
+	nskb->nh.iph->ttl = sysctl_ip_default_ttl;
+
+	/* Set DF, id = 0 */
+	nskb->nh.iph->frag_off = htons(IP_DF);
+	nskb->nh.iph->id = 0;
+
+	/* Adjust IP checksum */
+	nskb->nh.iph->check = 0;
+	nskb->nh.iph->check = ip_fast_csum((unsigned char *)nskb->nh.iph, 
+					   nskb->nh.iph->ihl);
+
+	/* "Never happens" */
+	if (nskb->len > dst_pmtu(nskb->dst))
+		goto free_nskb;
+
+	/* NOTRACK reply packet, if possible */
+	/* Avoid module unload race with nf_ip_glue being NULLed out */
+	if ((oskb->nfct == NULL)
+	    && (notrack = nf_ip_glue.ip_ct_notrack) != NULL) {
+		mb(); /* Just to be sure: must be read before executing this */
+		notrack(nskb);
+	}
+
+	NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, nskb, NULL, nskb->dst->dev,
+		ip_finish_output);
+	return;
+
+ free_nskb:
+	kfree_skb(nskb);
+}
+
+
+static unsigned int tarpit(struct sk_buff **pskb,
+			   const struct net_device *in,
+			   const struct net_device *out,
+			   unsigned int hooknum,
+			   const void *targinfo,
+			   void *userinfo)
+{
+	struct sk_buff *skb;
+	struct rtable *rt;
+	struct iphdr *iph = (*pskb)->nh.iph;
+
+	/* Raw table: we must call routing first */
+	if ((*pskb)->dst == NULL && hooknum == NF_IP_PRE_ROUTING)
+		ip_route_input(*pskb, iph->daddr, iph->saddr, iph->tos,
+			       (struct net_device *)in);
+	skb = *pskb;
+	rt = (struct rtable*)skb->dst; 
+	
+	/* Do we have an input route cache entry? */
+	if (!rt)	
+		return NF_DROP;
+
+        /* No replies to physical multicast/broadcast */
+        if (skb->pkt_type != PACKET_HOST && skb->pkt_type != PACKET_OTHERHOST)
+     		return NF_DROP;
+
+        /* Now check at the protocol level */
+	if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
+                return NF_DROP;
+
+	/* Our naive response construction doesn't deal with IP
+           options, and probably shouldn't try. */
+	if (skb->nh.iph->ihl*4 != sizeof(struct iphdr))
+		return NF_DROP;
+
+        /* We aren't interested in fragments */
+	if (skb->nh.iph->frag_off & htons(IP_OFFSET))
+                return NF_DROP;
+
+	/* A truncated TCP header isn't going to be useful */
+	if (skb->len < (skb->nh.iph->ihl*4) + sizeof(struct tcphdr))
+		return NF_DROP;
+
+	tarpit_tcp(skb, hooknum);
+
+	return NF_DROP;
+}
+
+
+static int check(const char *tablename,
+		 const struct ipt_entry *e,
+		 void *targinfo,
+		 unsigned int targinfosize,
+		 unsigned int hook_mask)
+{
+	/* Only allow these for input/forward packet filtering. */
+	if (!(strcmp(tablename, "filter") == 0
+	      || strcmp(tablename, "raw") == 0)) {
+		DEBUGP("TARPIT: bad table %s'.\n", tablename);
+		return 0;
+	}
+	if ((strcmp(tablename, "filter") == 0
+	     && (hook_mask & ~((1 << NF_IP_LOCAL_IN) 
+                               | (1 << NF_IP_FORWARD))) != 0)
+	    || (strcmp(tablename, "raw") == 0
+	        && (hook_mask & ~(1 << NF_IP_PRE_ROUTING)) != 0)) {
+		DEBUGP("TARPIT: bad hook mask %X\n", hook_mask);
+		return 0;
+	}
+
+	/* Must specify that it's a TCP packet */
+	if (e->ip.proto != IPPROTO_TCP || (e->ip.invflags & IPT_INV_PROTO)) {
+		DEBUGP("TARPIT: not valid for non-tcp\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static struct ipt_target ipt_tarpit_reg = { 
+	.name = "TARPIT",
+	.target = tarpit,
+	.checkentry = check,
+	.me = THIS_MODULE
+};
+
+static int __init init(void)
+{
+	return ipt_register_target(&ipt_tarpit_reg);
+}
+
+static void __exit fini(void)
+{
+	ipt_unregister_target(&ipt_tarpit_reg);
+}
+
+module_init(init);
+module_exit(fini);

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

Hi, Pablo and Jozsef,

> On Wed, 16 Mar 2005, Pablo Neira wrote:
> 
> > Yasuyuki KOZAKAI wrote:
> > > 	3. The symbol "ip_conntrack_untracked" which depends on ip_conntrack.
> > > 	   state, conntrack, NOTRACK use this symbol. How about defining
> > > 	   "void *nf_ct_untracked" in net/core/netfilter.c and
> > > 	   set &ip_conntrack_untracked.general/&nf_conntrack_untracked.general
> > > 	   to it when initializing ip_conntrack/nf_conntrack ?
> >
> > Something I have in mind. I think that we could use one of the bits of
> > nfcache explicitely mark invalid connections. If we get an invalid
> > packet, conntrack can set the NFC_INVALID bit. That way we interpret a
> > NULL pointer in skb->nfct as a untracked connection and kill that fake
> > conntrack.

If we kill the fake conntrack, ip_conntrack cannot decide the skb should be
tracked or not.

But using nfcache sounds good. Instead of NFC_INVALID, how about NFC_NOTRACK ?
If NFC_NOTRACK is set, ip_conntrack can figure out that's untracked skb.
And matches can distinguish between invalid skb and untracked one by checking
nfcache.

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Date: Wed, 16 Mar 2005 11:13:57 +0100 (CET)

> When working on a conntrack-friendly TARPIT target, I added "general" glue
> function support in order to avoid unnecessary cross-dependency. Have a
> look at the attached patch.

That's useful for only NOTRACK target.
The symbol ip_conntrack_untracked remains in state and conntrack matches.

So I think that using nfcache flag is better in the current.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: nf_conntrack tree

[Patrick McHardy]

Jozsef Kadlecsik wrote:
> When working on a conntrack-friendly TARPIT target, I added "general" glue
> function support in order to avoid unnecessary cross-dependency. Have a
> look at the attached patch.

I thought about this. I thought it would require too many glue functions
that are mostly only have a single user, but actually it doesn't look so
bad. ipt_conntrack (worst one AFAICT) should be ok with a struct
nf_conntrack_common + two glue functions:

tuple_proto_cmp
tuple_addr_cmp(direction, src/dst)

Seems to be the best idea so far. We could also get rid of some of the
uglies in net/core/netfilter.c.

Regards
Patrick

Re: nf_conntrack tree

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
>>In my opinion the most important issue at this time is making
>>ip_conntrack matches and targets work properly with nf_conntrack and
>>adding their IPv6 equivalents. Before doing the IPv6 part, I would like
>>to get sorted out if we can share the IPv4 matches/targets between
>>ip_conntrack and nf_conntrack.
> 
> 
> There are issues to do this.
> 	1. conflicting symbols when including ip_conntrack.h and nf_conntrack.
> 	   -> easy work. We can change symbols in nf_conntrack_*.h.

Yes. We could also move users of the ip_conntrack.h symbols over to the
nf_conntrack.h symbols. This would allow to kill a lot of them.

> 	2. I would not like to change UI. If ip_conntrack isn't loaded when
> 	   these matches/targets are loaded, I want kernel to load ip_conntrack
> 	   automatically. I suggests changes in init() as following.
> 
> 		int ret = -1;	   
> 
> 		if (get_symbols(need_ip_conntrack) != NULL) {
> 			ret = ip_register_match(match_func_with_ip_conntrack);
> 
> 		} else if (get_symbols(need_nf_conntrack) != NULL) {
> 			ret = ip_register_match(match_func_with_nf_conntrack);
> 		} else
> 			return -1;
> 
> 		return ret;
> 
> 	  I don't know this usage of get_symbols() is right or not.
> 	  If right, ip_conntrack will be loaded automatically in the case that
> 	  ip_conntrack and nf_conntrack are not loaded.
> 	  If either of them is loaded, it will be used.

Not sure if it works, but that would be a possibility.

> 	3. The symbol "ip_conntrack_untracked" which depends on ip_conntrack.
> 	   state, conntrack, NOTRACK use this symbol. How about defining
> 	   "void *nf_ct_untracked" in net/core/netfilter.c and
> 	   set &ip_conntrack_untracked.general/&nf_conntrack_untracked.general
> 	   to it when initializing ip_conntrack/nf_conntrack ?

I was thinking about creating a struct nf_conntrack_common that is
used in both struct ip_conntrack and nf_conntrack for things like
mark, flags, ... We could then simply use a status flag for untracked.
It would also help connmark/CONNMARK and a couple of others, the
main problem are the tuples (needed by ipt_conntrack), the ip_conntrack
tuples don't match the nf_conntrack tuples.

Regards
Patrick

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

Hi,

From: Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp>
Date: Wed, 16 Mar 2005 15:48:33 +0900 (JST)

> 	2. I would not like to change UI. If ip_conntrack isn't loaded when
> 	   these matches/targets are loaded, I want kernel to load ip_conntrack
> 	   automatically. I suggests changes in init() as following.
> 
> 		int ret = -1;	   
> 
> 		if (get_symbols(need_ip_conntrack) != NULL) {
> 			ret = ip_register_match(match_func_with_ip_conntrack);
> 
> 		} else if (get_symbols(need_nf_conntrack) != NULL) {
> 			ret = ip_register_match(match_func_with_nf_conntrack);
> 		} else
> 			return -1;
> 
> 		return ret;
> 
> 	  I don't know this usage of get_symbols() is right or not.
> 	  If right, ip_conntrack will be loaded automatically in the case that
> 	  ip_conntrack and nf_conntrack are not loaded.
> 	  If either of them is loaded, it will be used.

Oh, this is bad code. What I really want to do is following.
Anyway, this issue would be resolved.

 static struct ipt_match helper_match = {
 	.name		= "helper",
-	.match		= &match,
 	.checkentry	= &check,
 	.me		= THIS_MODULE,
 };
 
 static int __init init(void)
 {
-	need_ip_conntrack();
+	if (!request_module("ip_conntrack"))
+		helper_match.match = &ip_match;
+	} else if (!request_module("nf_conntrack"))
+		helper_match.match = &nf_match;
+	} else
+		return -1;
+
 	return ipt_register_match(&helper_match);
 }
 
-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: nf_conntrack tree

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:

> Oh, this is bad code. What I really want to do is following.
> Anyway, this issue would be resolved.
> 
>  static struct ipt_match helper_match = {
>  	.name		= "helper",
> -	.match		= &match,
>  	.checkentry	= &check,
>  	.me		= THIS_MODULE,
>  };
>  
>  static int __init init(void)
>  {
> -	need_ip_conntrack();
> +	if (!request_module("ip_conntrack"))
> +		helper_match.match = &ip_match;
> +	} else if (!request_module("nf_conntrack"))
> +		helper_match.match = &nf_match;
> +	} else
> +		return -1;
> +
>  	return ipt_register_match(&helper_match);
>  }

That won't work when nf_conntrack is already loaded. Do you think
it is necessary to allow people to build both as modules? It would
be easier to just force them to choose, and would probably result
in more people actually using it since ip_conntrack wouldn't be used
by default anymore.

Regards
Patrick

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 16 Mar 2005 20:32:54 +0100

> Yasuyuki KOZAKAI wrote:
> 
> > Oh, this is bad code. What I really want to do is following.
> > Anyway, this issue would be resolved.
> > 
> >  static struct ipt_match helper_match = {
> >  	.name		= "helper",
> > -	.match		= &match,
> >  	.checkentry	= &check,
> >  	.me		= THIS_MODULE,
> >  };
> >  
> >  static int __init init(void)
> >  {
> > -	need_ip_conntrack();
> > +	if (!request_module("ip_conntrack"))
> > +		helper_match.match = &ip_match;
> > +	} else if (!request_module("nf_conntrack"))
> > +		helper_match.match = &nf_match;
> > +	} else
> > +		return -1;
> > +
> >  	return ipt_register_match(&helper_match);
> >  }
> 
> That won't work when nf_conntrack is already loaded. Do you think
> it is necessary to allow people to build both as modules?

You're right. #ifdef or other staff is needed.

>                                                           It would
> be easier to just force them to choose, and would probably result
> in more people actually using it since ip_conntrack wouldn't be used
> by default anymore.

But I want this flexibility if possible so that we can test matches/targets
with ip_conntrack/nf_conntrack without recompiling.

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: nf_conntrack tree

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
>>That won't work when nf_conntrack is already loaded. Do you think
>>it is necessary to allow people to build both as modules?
> 
> 
> You're right. #ifdef or other staff is needed.

If we go the way Joszef propsed, namely having something like
a global struct nf_ct_ops *, we can simply add a flag to it
which specifies which conntrack is usec.

>>                                                          It would
>>be easier to just force them to choose, and would probably result
>>in more people actually using it since ip_conntrack wouldn't be used
>>by default anymore.
> 
> But I want this flexibility if possible so that we can test matches/targets
> with ip_conntrack/nf_conntrack without recompiling.

I see that it would be useful for testing, but I fear that it will
result in additional complexity and bugs with little gain for users.
Let's see how it works out, if it really does cause a lot of extra
complexity it would prefer to only allow one at a time.

Regards
Patrick

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

Hi, Patrick,

OK, I agree introducing nf_ct_ops.

But in the current, it's not acceptable to introduce nf_conntrack_common
as you said in other mail. I think it needs many changes to ip_conntrack and
ip_nat, it may be difficult to change nf_conn structure
(e.g. optimization), and it will confuse us - What we unify is ?

My suggestion is pulling out common definitions of enum and flags to some
common header files, and making {ip,nf}_conntrack*.h include them.
No change to ip_conntrack_*.c is necessary.

Regards,
-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>


From: Patrick McHardy <kaber@trash.net>
Date: Wed, 16 Mar 2005 21:04:06 +0100

> Yasuyuki KOZAKAI wrote:
> >>That won't work when nf_conntrack is already loaded. Do you think
> >>it is necessary to allow people to build both as modules?
> > 
> > 
> > You're right. #ifdef or other staff is needed.
> 
> If we go the way Joszef propsed, namely having something like
> a global struct nf_ct_ops *, we can simply add a flag to it
> which specifies which conntrack is usec.
> 
> >>                                                          It would
> >>be easier to just force them to choose, and would probably result
> >>in more people actually using it since ip_conntrack wouldn't be used
> >>by default anymore.
> > 
> > But I want this flexibility if possible so that we can test matches/targets
> > with ip_conntrack/nf_conntrack without recompiling.
> 
> I see that it would be useful for testing, but I fear that it will
> result in additional complexity and bugs with little gain for users.
> Let's see how it works out, if it really does cause a lot of extra
> complexity it would prefer to only allow one at a time.
> 
> Regards
> Patrick
> 

Re: nf_conntrack tree

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> Hi, Patrick,
> 
> OK, I agree introducing nf_ct_ops.
> 
> But in the current, it's not acceptable to introduce nf_conntrack_common
> as you said in other mail. I think it needs many changes to ip_conntrack and
> ip_nat, it may be difficult to change nf_conn structure
> (e.g. optimization), and it will confuse us - What we unify is ?

We're unifying things common to ip_conntrack and nf_conntrack, like
mark, status, ... in order to keep the number of accessor functions
in nf_ct_ops to a minimum.

> My suggestion is pulling out common definitions of enum and flags to some
> common header files, and making {ip,nf}_conntrack*.h include them.
> No change to ip_conntrack_*.c is necessary.

Yes, that is one part. But some matches/targets need to get some
information from the conntrack. I've made a list of what is needed, so
we can talk in more concrete terms:

ipt_state:
- recognize untracked

ipt_helper:
- ct->master->helper->name

ipt_conntrack:
- recognize untracked
- tuples
- status
- expires

ipt_connmark/IPT_CONNMARK:
- mark

Creating a struct nf_conntrack_common should be ok for untracked,
status, expires, mark and master->helper->name. The tuples can't be
handled this way, for them we need nf_ct_ops.

Regards
Patrick

Re: nf_conntrack tree

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Sun, 20 Mar 2005 17:42:57 +0100

> > But in the current, it's not acceptable to introduce nf_conntrack_common
> > as you said in other mail. I think it needs many changes to ip_conntrack and
> > ip_nat, it may be difficult to change nf_conn structure
> > (e.g. optimization), and it will confuse us - What we unify is ?
>
> We're unifying things common to ip_conntrack and nf_conntrack, like
> mark, status, ... in order to keep the number of accessor functions
> in nf_ct_ops to a minimum.

It's very poor joke. Please don't mind. :)

At first, let me clarify. I imagine that your suggestion is changing definition
of "struct ip_connntrack" as follow.

struct ip_conntrack {
	...
	struct nf_conntrack_common common;
	...
}

This would change many parts of ip_conntrack and we'd have to rework on this
issue when we want to change some member of "struct nf_conn". I think
duplicating matches/targets is better than this way.

Or what you suggest is implementing glue function which gathers the common
parameters from ip_conntrack/nf_conn and put them in nf_conntrack_common
structure ? This introduces a few overheads, but acceptable for me.

> Yes, that is one part. But some matches/targets need to get some
> information from the conntrack. I've made a list of what is needed, so
> we can talk in more concrete terms:

OK,

> ipt_state:
> - recognize untracked
> 
> ipt_helper:
> - ct->master->helper->name
> 
> ipt_conntrack:
> - recognize untracked
> - tuples
> - status
> - expires
> 
> ipt_connmark/IPT_CONNMARK:
> - mark

We have to be careful about "mark". Users may enable NF_IP_CONNTRACK_MARK and
disable NF_CONNTRACK_MARK.

> Creating a struct nf_conntrack_common should be ok for untracked,
> status, expires, mark and master->helper->name. The tuples can't be
> handled this way, for them we need nf_ct_ops.

Regards,

-----------------------------------------------------------------
Yasuyuki KOZAKAI @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: [PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Yasuyuki KOZAKAI]

Hi, Michal,

From: Michal Rokos <michal@rokos.info>
Date: Mon, 23 May 2005 15:46:58 +0200

> Hello,
> 
> >>>>> "YK" == Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> writes:
> YK>
> YK> This patch follows the recent changes to ip_conntrack.
> YK>
> 
> could I ask you against what is this patch?
> 
> I'm using 
> http://svn.netfilter.org/netfilter/trunk/patch-o-matic-ng/nf_conntrack and 
> I'm getting rejects when applying this patch...

Patrick prepared bk tree for submission as follows and I have been
sending patches to him since then to avoid duplicated work and
inconsistency between svn and his tree.

From: Patrick McHardy <kaber@trash.net>
Subject: nf_conntrack tree
Date: Wed, 16 Mar 2005 01:35:32 +0100

> I'm working on getting nf_conntrack ready for submission. To make it
> easier for people to participate, I've exported my tree through
> bitkeeper and as plain patch. It currently contains nf_conntrack as
> found in pomng plus a couple of ported ip_conntrack fixes:
> 
> bk://numenor.coreworks.de/nf-2.6-nfct
> http://numenor.coreworks.de/~kaber
> 
> The tree and the diff will be kept up-to-date when changes are made.

In this time, I applied
	http://numenor.coreworks.de/~kaber/nf_conntrack-03052005_01.diff

to cloned linus tree at 2005/05/22 00:37(JST: +9:00) before making their
patches.

Which tree should I try to keep to update ?

Regards,

-----------------------------------------------------------------
Yasuyuki Kozakai @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: [PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Patrick McHardy]

Yasuyuki KOZAKAI wrote:
> In this time, I applied
> 	http://numenor.coreworks.de/~kaber/nf_conntrack-03052005_01.diff
> 
> to cloned linus tree at 2005/05/22 00:37(JST: +9:00) before making their
> patches.
> 
> Which tree should I try to keep to update ?

I'm only updateing this tree myself and I think it contains some extra
fixes not found in pom, so probably best to work of this tree for now.
If it makes things easier for you I can extract the additional patches,
apply them to pom and continue to work with that ..

Regards
Patrick

Re: [PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Michal Rokos]

Hello,

>>>>> "YK" == Yasuyuki KOZAKAI <yasuyuki.kozakai@toshiba.co.jp> writes:
YK> Patrick prepared bk tree for submission as follows and I have been
YK> sending patches to him since then to avoid duplicated work and
YK> inconsistency between svn and his tree.

Thank you very much for this. It helped me a lot!

Patrick, do you have something else than BitKeeper repository? Most of the 
people now are using either git, subversion, or CVS...

 Michal

Re: [PATCH NF_CONNTRACK 1/9]: Fix multiple problems with TCP window tracking

[Patrick McHardy]

Michal Rokos wrote:
> YK> Patrick prepared bk tree for submission as follows and I have been
> YK> sending patches to him since then to avoid duplicated work and
> YK> inconsistency between svn and his tree.
> 
> Thank you very much for this. It helped me a lot!
> 
> Patrick, do you have something else than BitKeeper repository? Most of the 
> people now are using either git, subversion, or CVS...

I wanted to convert it to git for a long time, I'll probably
do it tomorrow.

Regards
Patrick