FTP Masquarade connection tracking bugfix

Rusty Russell rusty@rustcorp.com.au
Tue, 16 Oct 2001 10:47:57 +1000


In message <20011015093414.A14110@pc0519sd.sysdata.siemens.hu> you write:
> after upgrading from kernel version 2.2.x to 2.4.10, we found a small
> parsing problem in FTP connection tracking code (ip_conntrack_ftp.c).
> It recognises the FTP commands PORT and EPRT with trailing carriage
> return '\r' only, but some ftp client sends '\n'. It should be also
> accepted, as was in the 2.2.x.

I preferred the previous version I saw, with the whole search
structure duplicated.

Actually, how does this patch work for you?

Cheers,
Rusty.
--
Premature optmztion is rt of all evl. --DK

diff -urN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.4.12-official/net/ipv4/netfilter/ip_conntrack_ftp.c working-2.4.12-ftp/net/ipv4/netfilter/ip_conntrack_ftp.c
--- linux-2.4.12-official/net/ipv4/netfilter/ip_conntrack_ftp.c	Wed Oct 10 17:37:58 2001
+++ working-2.4.12-ftp/net/ipv4/netfilter/ip_conntrack_ftp.c	Tue Oct 16 10:33:03 2001
@@ -29,47 +29,47 @@
 #define DEBUGP(format, args...)
 #endif
 
-static int try_rfc959(const char *, size_t, u_int32_t [], char);
-static int try_eprt(const char *, size_t, u_int32_t [], char);
-static int try_espv_response(const char *, size_t, u_int32_t [], char);
+static int try_rfc959(const char *, size_t, u_int32_t [], const char *);
+static int try_eprt(const char *, size_t, u_int32_t [], const char *);
+static int try_espv_response(const char *, size_t, u_int32_t [], const char *);
 
 static struct ftp_search {
 	enum ip_conntrack_dir dir;
 	const char *pattern;
 	size_t plen;
 	char skip;
-	char term;
+	const char *term;
 	enum ip_ct_ftp_type ftptype;
-	int (*getnum)(const char *, size_t, u_int32_t[], char);
+	int (*getnum)(const char *, size_t, u_int32_t[], const char *);
 } search[] = {
 	{
 		IP_CT_DIR_ORIGINAL,
-		"PORT",	sizeof("PORT") - 1, ' ', '\r',
+		"PORT",	sizeof("PORT") - 1, ' ', "\r\n",
 		IP_CT_FTP_PORT,
 		try_rfc959,
 	},
 	{
 		IP_CT_DIR_REPLY,
-		"227 ",	sizeof("227 ") - 1, '(', ')',
+		"227 ",	sizeof("227 ") - 1, '(', ")",
 		IP_CT_FTP_PASV,
 		try_rfc959,
 	},
 	{
 		IP_CT_DIR_ORIGINAL,
-		"EPRT", sizeof("EPRT") - 1, ' ', '\r',
+		"EPRT", sizeof("EPRT") - 1, ' ', "\r\n",
 		IP_CT_FTP_EPRT,
 		try_eprt,
 	},
 	{
 		IP_CT_DIR_REPLY,
-		"229 ", sizeof("229 ") - 1, '(', ')',
+		"229 ", sizeof("229 ") - 1, '(', ")",
 		IP_CT_FTP_EPSV,
 		try_espv_response,
 	},
 };
 
 static int try_number(const char *data, size_t dlen, u_int32_t array[],
-		      int array_size, char sep, char term)
+		      int array_size, char sep, const char *term)
 {
 	u_int32_t i, len;
 
@@ -83,9 +83,9 @@
 		else if (*data == sep)
 			i++;
 		else {
-			/* Unexpected character; true if it's the
+			/* Unexpected character; true if it's a
 			   terminator and we're finished. */
-			if (*data == term && i == array_size - 1)
+			if (strchr(term, *data) && i == array_size - 1)
 				return len;
 
 			DEBUGP("Char %u (got %u nums) `%u' unexpected\n",
@@ -100,7 +100,7 @@
 
 /* Returns 0, or length of numbers: 192,168,1,1,5,6 */
 static int try_rfc959(const char *data, size_t dlen, u_int32_t array[6],
-		       char term)
+		      const char *term)
 {
 	return try_number(data, dlen, array, 6, ',', term);
 }
@@ -131,17 +131,18 @@
 
 /* Returns 0, or length of numbers: |1|132.235.1.2|6275| */
 static int try_eprt(const char *data, size_t dlen, u_int32_t array[6],
-		    char term)
+		    const char *term)
 {
-	char delim;
+	char delim[2];
 	int length;
 
+	delim[1] = '\0';
 	/* First character is delimiter, then "1" for IPv4, then
            delimiter again. */
 	if (dlen <= 3) return 0;
-	delim = data[0];
-	if (isdigit(delim) || delim < 33 || delim > 126
-	    || data[1] != '1' || data[2] != delim)
+	delim[0] = data[0];
+	if (isdigit(delim[0]) || delim[0] < 33 || delim[0] > 126
+	    || data[1] != '1' || data[2] != delim[0])
 		return 0;
 
 	DEBUGP("EPRT: Got |1|!\n");
@@ -152,12 +153,12 @@
 
 	DEBUGP("EPRT: Got IP address!\n");
 	/* Start offset includes initial "|1|", and trailing delimiter */
-	return get_port(data, 3 + length + 1, dlen, delim, array+4);
+	return get_port(data, 3 + length + 1, dlen, delim[0], array+4);
 }
 
 /* Returns 0, or length of numbers: |||6446| */
 static int try_espv_response(const char *data, size_t dlen, u_int32_t array[6],
-			     char term)
+			     const char *term)
 {
 	char delim;
 
@@ -174,11 +175,12 @@
 /* Return 1 for match, 0 for accept, -1 for partial. */
 static int find_pattern(const char *data, size_t dlen,
 			const char *pattern, size_t plen,
-			char skip, char term,
+			char skip, const char *term,
 			unsigned int *numoff,
 			unsigned int *numlen,
 			u_int32_t array[6],
-			int (*getnum)(const char *, size_t, u_int32_t[], char))
+			int (*getnum)(const char *, size_t, u_int32_t[], 
+				      const char *))
 {
 	size_t i;