[PATCH] RPC match and conntrack modules v2.1

[PATCH] RPC match and conntrack modules v2.1

[Ian Latter]

Hello all,

  I too had too much Xmas idle time and for some reason I couldn't get
enough coding done ... too long between programming sessions, me 
thinks.

  I have uploaded, to the ftp.netfilter.org site, a copy of the new patch-
o-matic ready code.  Let me know what you think of it ... I don't have a
lot of need for RPC traffic filtering, but when I realised that the Linux
kernel filtering for RPCs was limited to the old "record-rpc" modules, I
had to upgrade them ... I wanted some of that CheckPoint functionality
in my favourite filtering API  :oP

  The new modules will allow filtering on RPC procedures (by name or
number).  The intended usage of these modules would be with a ruleset 
like;

    # New session from client to server (rpc portmapper get)
    -A PREROUTING -t nat -i eth0 -p udp -m rpc --rpcs 100002
           -s ${client} --sport 0:1023 -d ${server} --dport 111
           -j ACCEPT

    # Continued session from client to client (port mapper answer)
    -A PREROUTING -t nat -i eth1 -m state -p udp
           -s ${server} --sport 111 -d ${client} --dport 0:1023
           --state ESTABLISHED -j ACCEPT

    # New session from client to server (procedure call)
    -A PREROUTING -t nat -i eth0 -m state -p udp
          -s ${client} --sport 0:65535
          -d ${server} --dport 32000:34000
          --state ESTABLISHED,RELATED -j ACCEPT

    # Continued session from client to server (procedure response)
    -A PREROUTING -t nat -i eth1 -m state -p udp
           -s ${server} --sport 32000:34000
           -d ${client} --dport 0:65535
           --state ESTABLISHED -j ACCEPT


  Which would allow rusers to execute from client to server;

     user@client# rusers $server


Note that if that first rule was without the --rpcs option;
    -A PREROUTING -t nat -i eth0 -p udp -m rpc
           -s ${client} --sport 0:1023 -d ${server} --dport 111
           -j ACCEPT

... then it would behave like the existing oldnat module; simply
"recording" RPC get commands via the portmapper, and letting
them through from the same host when they're a complete
procedure call.


Also note that I've updated the module for newnat, and it compiles
cleanly against 2.4.20.  I was half tempted to author the NAT module
to go with the conntrack and match, but this code has taken too 
much time already.


0f759135c2b2aae5bc39b3ed0f633749  pom-rpc-030110-01.tgz
11371 bytes size

  Comments welcome and appreciated.


Regards,


--
Ian Latter
Internet and Networking Security Officer
Macquarie University

Re: [PATCH] RPC match and conntrack modules v2.1

[Harald Welte]

[-- Attachment #1.1: Type: text/plain, Size: 2001 bytes --]

On Fri, Jan 10, 2003 at 07:26:57PM +1100, Ian Latter wrote:
> Hello all,
>   I have uploaded, to the ftp.netfilter.org site, a copy of the new patch-
> o-matic ready code.  Let me know what you think of it ... I don't have a
> lot of need for RPC traffic filtering, but when I realised that the Linux
> kernel filtering for RPCs was limited to the old "record-rpc" modules, I
> had to upgrade them ... I wanted some of that CheckPoint functionality
> in my favourite filtering API  :oP

This is cool stuff.  Thanks a lot.  The kind of contribution I really
like :)

> Also note that I've updated the module for newnat, and it compiles
> cleanly against 2.4.20.  I was half tempted to author the NAT module
> to go with the conntrack and match, but this code has taken too 
> much time already.

Sorry, I didn't understand the part about the NAT module.  Are you
saying that NAT support is not functional right now?

>   Comments welcome and appreciated.

See attached patch.  Mostly CodingStyle updates (you seem to like 8 spaces
instead of tab), but also a minor fix:

- when unloading, only unregister up to ports_n_c ports

btw: I'd really love to see the large chunk of code in the switch
statement of the match() function in ipt_rpc.c be split up in seperate
functions (thus reducing indentation and improving readability).

I don't know, but I think esp. in cases where you have lots of rpc
activity (NFS?, I'm not an RPC expert) it might be wise to use a slab
cache for the 'struct request_p' list items.

Please integrate my patch [and maybe consider my suggestions, that's of
course up to you], test it and resubmit it.

Thanks!

> Regards,
> Ian Latter

-- 
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
"If this were a dictatorship, it'd be a heck of a lot easier, just so long
 as I'm the dictator."  --  George W. Bush Dec 18, 2000

[-- Attachment #1.2: rpc.patch.update --]
[-- Type: text/plain, Size: 23771 bytes --]

--- rpc.patch	Fri Jan 10 14:24:58 2003
+++ rpc.patch.new	Fri Jan 10 14:40:40 2003
@@ -1,6 +1,6 @@
 --- linux/net/ipv4/netfilter/ip_conntrack_rpc_tcp.c.old	Thu Jan  1 10:00:00 1970
 +++ linux/net/ipv4/netfilter/ip_conntrack_rpc_tcp.c	Fri Jan 10 18:53:15 2003
-@@ -0,0 +1,477 @@
+@@ -0,0 +1,482 @@
 +/* RPC extension for IP (TCP) connection tracking, Version 2.1
 + * (C) 2000 by Marcelo Barbosa Lima <marcelo.lima@dcc.unicamp.br>
 + *       - original rpc tracking module
@@ -28,20 +28,20 @@
 + **
 + *      Note to all:
 + *
-+ *        RPCs should not be exposed to the internet - ask the Pentagon;
++ *	RPCs should not be exposed to the internet - ask the Pentagon;
 + *
-+ *          "The unidentified crackers pleaded guilty in July to charges
-+ *           of juvenile delinquency stemming from a string of Pentagon
-+ *           network intrusions in February.
-+ *
-+ *           The youths, going by the names TooShort and Makaveli, used
-+ *           a common server security hole to break in, according to
-+ *           Dane Jasper, owner of the California Internet service
-+ *           provider, Sonic. They used the hole, known as the 'statd'
-+ *           exploit, to attempt more than 800 break-ins, Jasper said."
++ *	  "The unidentified crackers pleaded guilty in July to charges
++ *	   of juvenile delinquency stemming from a string of Pentagon
++ *	   network intrusions in February.
++ *
++ *	   The youths, going by the names TooShort and Makaveli, used
++ *	   a common server security hole to break in, according to
++ *	   Dane Jasper, owner of the California Internet service
++ *	   provider, Sonic. They used the hole, known as the 'statd'
++ *	   exploit, to attempt more than 800 break-ins, Jasper said."
 + *
-+ *        From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
-+ *        URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
++ *	From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
++ *	URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
 + **
 + */
 +
@@ -88,12 +88,13 @@
 +#include <linux/netfilter_ipv4/listhelp.h>
 +
 +/* For future conections RPC, using client's cache bindings
-+ * I'll use ip_conntrack_lock to lock these lists           */
++ * I'll use ip_conntrack_lock to lock these lists	   */
 +
 +LIST_HEAD(request_p_list_tcp);
 +
 +
-+static void delete_request_p(unsigned long request_p_ul) {
++static void delete_request_p(unsigned long request_p_ul) 
++{
 +	struct request_p *p = (void *)request_p_ul;
 +	
 +	WRITE_LOCK(&ipct_rpc_tcp_lock);
@@ -104,7 +105,8 @@
 +}
 +
 +
-+static void req_cl(struct request_p * r){
++static void req_cl(struct request_p * r)
++{
 +	WRITE_LOCK(&ipct_rpc_tcp_lock);
 +	del_timer(&r->timeout);
 +	LIST_DELETE(&request_p_list_tcp, r);
@@ -114,38 +116,41 @@
 +}
 +
 +
-+static void clean_request(struct list_head *list){
++static void clean_request(struct list_head *list)
++{
 +	struct list_head *first = list->prev;
 +	struct list_head *temp = list->next;
 +	struct list_head *aux;
 +
-+	if(list_empty(list)) return;
++	if (list_empty(list))
++		return;
 +
-+	    while(first != temp) {
++	while (first != temp) {
 +		aux = temp->next;
 +		req_cl((struct request_p *)temp);
 +		temp = aux;	
-+	    }
++	}
 +	req_cl((struct request_p *)temp);
 +	return;
 +}
 +
 +
 +static void alloc_request_p(u_int32_t xid, u_int16_t proto, u_int32_t ip,
-+		     u_int16_t port){
++		     u_int16_t port)
++{
 +	struct request_p *req_p;
-+        
-+        /* Verifies if entry already exists */
-+        WRITE_LOCK(&ipct_rpc_tcp_lock);
-+        req_p = LIST_FIND(&request_p_list_tcp, request_p_cmp,
++	
++	/* Verifies if entry already exists */
++	WRITE_LOCK(&ipct_rpc_tcp_lock);
++	req_p = LIST_FIND(&request_p_list_tcp, request_p_cmp,
 +		struct request_p *, xid, ip, port);
 +
-+        if(req_p){
++	if (req_p) {
 +		/* Refresh timeout */
 +		if (del_timer(&req_p->timeout)) {
-+		   req_p->timeout.expires = jiffies + EXP;
-+		   add_timer(&req_p->timeout);	
-+	        } 
++			req_p->timeout.expires = jiffies + EXP;
++			add_timer(&req_p->timeout);	
++		} 
 +		WRITE_UNLOCK(&ipct_rpc_tcp_lock);
 +		return;	
 +
@@ -154,7 +159,7 @@
 +	
 +	/* Allocate new request_p */
 +	req_p = (struct request_p *) kmalloc(sizeof(struct request_p), GFP_ATOMIC);
-+        if(!req_p) {
++	if (!req_p) {
 + 		DEBUGP("can't allocate request_p\n");
 +		return;			
 +	}
@@ -162,10 +167,10 @@
 +		{ { NULL, NULL }, jiffies + EXP, (unsigned long)req_p,
 +			NULL }}); 
 +      
-+        /*Initialize timer */
++	/* Initialize timer */
 +	init_timer(&req_p->timeout);
 +	req_p->timeout.function = delete_request_p;
-+        add_timer(&req_p->timeout); 
++	add_timer(&req_p->timeout); 
 +
 +	/* Put in list */
 +	WRITE_LOCK(&ipct_rpc_tcp_lock);
@@ -194,8 +199,8 @@
 +
 +	DEBUGP("new packet to evaluate ..\n");
 +
-+        /* This works for packets like handshake packets, ignore */
-+        if (len == ((tcph->doff + iph->ihl) * 4)) {
++	/* This works for packets like handshake packets, ignore */
++	if (len == ((tcph->doff + iph->ihl) * 4)) {
 +		DEBUGP("packet has no data (may still be handshaking). [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -228,13 +233,13 @@
 +	}
 +
 +
-+        /* If packet is client packet */
++	/* If packet is client packet */
 +	if (dir == IP_CT_DIR_ORIGINAL) {
 +
 +		DEBUGP("packet is from the initiator. [cont]\n");
 +
 +		/* Tests if packet len is ok */
-+		if((tcplen - (tcph->doff * 4)) != 60) {
++		if ((tcplen - (tcph->doff * 4)) != 60) {
 +			DEBUGP("packet length is not correct. [skip]\n");
 +			return NF_ACCEPT;
 +		}
@@ -249,7 +254,7 @@
 +		data = data + 5;
 +
 +		/* Is one get port procedure? */
-+		if(IXDR_GET_INT32(data) == 3) { 
++		if (IXDR_GET_INT32(data) == 3) { 
 +
 +			data++;
 +		
@@ -285,7 +290,7 @@
 +	DEBUGP("packet is from the receiver. [cont]\n");
 +
 +	/* Tests if packet len is ok */
-+	if((tcplen - (tcph->doff * 4)) != 32) {
++	if ((tcplen - (tcph->doff * 4)) != 32) {
 +		DEBUGP("packet length is not correct. [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -299,21 +304,21 @@
 +		ct->tuplehash[!dir].tuple.src.u.all);
 +
 +	/* Drop unexpected packets */
-+	if(!req_p) {
++	if (!req_p) {
 +		DEBUGP("packet is not expected. [skip]\n");
 +		return NF_ACCEPT;
 +	}
 +	data = data++;
 +
 +	/* Verifies if packet is really a reply packet */
-+	if(IXDR_GET_INT32(data) != 1) {
++	if (IXDR_GET_INT32(data) != 1) {
 +		DEBUGP("packet is not a valid RPC reply. [skip]\n");
 +		return NF_ACCEPT;
 +	}
 +	data++;
 +
 +	/* Is status accept? */
-+	if(IXDR_GET_INT32(data)) {
++	if (IXDR_GET_INT32(data)) {
 +		DEBUGP("packet is not an RPC accept. [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -323,18 +328,18 @@
 +	data = data + IXDR_GET_INT32(data) + 2;
 +
 +	/* Is accpet status "success"? */
-+	if(IXDR_GET_INT32(data)) {
++	if (IXDR_GET_INT32(data)) {
 +		DEBUGP("packet is not an RPC accept status of success. [skip]\n");
 +		return NF_ACCEPT;
 +	}
 +	data++;
 +
-+	/* Get server port number */          
++	/* Get server port number */	  
 +	port_buf = (u_int16_t) IXDR_GET_INT32(data);
 +
 +	/* Alright. This binding must be in internal expect list;
 +	 * If port == 0, then this service is not registred.   */
-+	if(port_buf) {
++	if (port_buf) {
 +		DEBUGP("port found: %u\n", port_buf);
 +
 +		memset(&expect, 0, sizeof(expect));
@@ -345,7 +350,7 @@
 +		exp->mask.src.ip = 0xffffffff;
 +		exp->mask.dst.ip = 0xffffffff;
 +
-+		switch(req_p->proto) {
++		switch (req_p->proto) {
 +			case IPPROTO_UDP:
 +				exp->tuple.src.u.udp.port = 0;
 +				exp->tuple.dst.u.udp.port = htons(port_buf);
@@ -463,7 +468,7 @@
 +	DEBUGP("cleaning request list\n");
 +	clean_request(&request_p_list_tcp);
 +
-+	for (port = 0; (port < MAX_PORTS) && ports[port]; port++) {
++	for (port = 0; (port < ports_n_c) && ports[port]; port++) {
 +		DEBUGP("unregistering port %d\n", ports[port]);
 +		ip_conntrack_helper_unregister(&rpc_helpers[port]);
 +	}
@@ -480,7 +485,7 @@
 +
 --- linux/net/ipv4/netfilter/ip_conntrack_rpc_udp.c.old	Thu Jan  1 10:00:00 1970
 +++ linux/net/ipv4/netfilter/ip_conntrack_rpc_udp.c	Fri Jan 10 18:53:09 2003
-@@ -0,0 +1,466 @@
+@@ -0,0 +1,471 @@
 +/* RPC extension for IP (UDP) connection tracking, Version 2.1
 + * (C) 2000 by Marcelo Barbosa Lima <marcelo.lima@dcc.unicamp.br>
 + *       - original rpc tracking module
@@ -508,20 +513,20 @@
 + **
 + *      Note to all:
 + *
-+ *        RPCs should not be exposed to the internet - ask the Pentagon;
++ *	RPCs should not be exposed to the internet - ask the Pentagon;
 + *
-+ *          "The unidentified crackers pleaded guilty in July to charges
-+ *           of juvenile delinquency stemming from a string of Pentagon
-+ *           network intrusions in February.
-+ *
-+ *           The youths, going by the names TooShort and Makaveli, used
-+ *           a common server security hole to break in, according to
-+ *           Dane Jasper, owner of the California Internet service
-+ *           provider, Sonic. They used the hole, known as the 'statd'
-+ *           exploit, to attempt more than 800 break-ins, Jasper said."
++ *	  "The unidentified crackers pleaded guilty in July to charges
++ *	   of juvenile delinquency stemming from a string of Pentagon
++ *	   network intrusions in February.
++ *
++ *	   The youths, going by the names TooShort and Makaveli, used
++ *	   a common server security hole to break in, according to
++ *	   Dane Jasper, owner of the California Internet service
++ *	   provider, Sonic. They used the hole, known as the 'statd'
++ *	   exploit, to attempt more than 800 break-ins, Jasper said."
 + *
-+ *        From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
-+ *        URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
++ *	From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
++ *	URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
 + **
 + */
 +
@@ -573,7 +578,8 @@
 +LIST_HEAD(request_p_list_udp);
 +
 +
-+static void delete_request_p(unsigned long request_p_ul) {
++static void delete_request_p(unsigned long request_p_ul)
++{
 +	struct request_p *p = (void *)request_p_ul;
 +	
 +	WRITE_LOCK(&ipct_rpc_udp_lock);
@@ -584,7 +590,8 @@
 +}
 +
 +
-+static void req_cl(struct request_p * r){
++static void req_cl(struct request_p * r)
++{
 +	WRITE_LOCK(&ipct_rpc_udp_lock);
 +	del_timer(&r->timeout);
 +	LIST_DELETE(&request_p_list_udp, r);
@@ -594,25 +601,28 @@
 +}
 +
 +
-+static void clean_request(struct list_head *list){
++static void clean_request(struct list_head *list)
++{
 +	struct list_head *first = list->prev;
 +	struct list_head *temp = list->next;
 +	struct list_head *aux;
 +
-+	if(list_empty(list)) return;
++	if (list_empty(list))
++		return;
 +
-+	    while(first != temp) {
++	while (first != temp) {
 +		aux = temp->next;
 +		req_cl((struct request_p *)temp);
 +		temp = aux;	
-+	    }
++	}
 +	req_cl((struct request_p *)temp);
 +	return;
 +}
 +
 +
 +static void alloc_request_p(u_int32_t xid, u_int16_t proto, u_int32_t ip,
-+		     u_int16_t port){
++		     u_int16_t port)
++{
 +	struct request_p *req_p;
 +
 +	/* Verifies if entry already exists */
@@ -620,7 +630,7 @@
 +	req_p = LIST_FIND(&request_p_list_udp, request_p_cmp,
 +			struct request_p *, xid, ip, port);
 +
-+	if(req_p) {
++	if (req_p) {
 +		/* Refresh timeout */
 +		if (del_timer(&req_p->timeout)) {
 +			req_p->timeout.expires = jiffies + EXP;
@@ -634,7 +644,7 @@
 +	
 +	/* Allocate new request_p */
 +	req_p = (struct request_p *) kmalloc(sizeof(struct request_p), GFP_ATOMIC);
-+	if(!req_p) {
++	if (!req_p) {
 + 		DEBUGP("can't allocate request_p\n");
 +		return;			
 +	}
@@ -686,7 +696,7 @@
 +
 +
 +	/* FIXME: Source route IP option packets --RR */
-+	if(*chsm){
++	if (*chsm) {
 +		if (csum_tcpudp_magic(iph->saddr, iph->daddr, udplen, IPPROTO_UDP,
 +		    csum_partial((char *)udph, udplen, 0))) {
 +			DEBUGP("[note: failure to get past this error may indicate source routing]\n");
@@ -757,7 +767,7 @@
 +	}
 +
 +	/* Tests if packet len is ok */
-+	if((udplen - sizeof(struct udphdr)) != 28) {
++	if ((udplen - sizeof(struct udphdr)) != 28) {
 +		DEBUGP("packet length is not correct. [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -768,21 +778,21 @@
 +		ct->tuplehash[!dir].tuple.src.u.all);
 +
 +	/* Drop unexpected packets */
-+	if(!req_p) {
++	if (!req_p) {
 +		DEBUGP("packet is not expected. [skip]\n");
 +		return NF_ACCEPT;
 +	}
 +	data = data++;
 +
 +	/* Verifies if packet is really a reply packet */
-+	if(IXDR_GET_INT32(data) != 1) {
++	if (IXDR_GET_INT32(data) != 1) {
 +		DEBUGP("packet is not a valid RPC reply. [skip]\n");
 +		return NF_ACCEPT;
 +	}
 +	data++;
 +
 +	/* Is status accept? */
-+	if(IXDR_GET_INT32(data)) {
++	if (IXDR_GET_INT32(data)) {
 +		DEBUGP("packet is not an RPC accept. [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -792,7 +802,7 @@
 +	data = data + IXDR_GET_INT32(data) + 2;
 +
 +	/* Is accpet status "success"? */
-+	if(IXDR_GET_INT32(data)) {
++	if (IXDR_GET_INT32(data)) {
 +		DEBUGP("packet is not an RPC accept status of success. [skip]\n");
 +		return NF_ACCEPT;
 +	}
@@ -803,7 +813,7 @@
 +
 +	/* Alright. This binding must be in internal expect list;
 +	 * If port == 0, then this service is not registred.   */
-+	if(port_buf) {
++	if (port_buf) {
 +		DEBUGP("port found: %u\n", port_buf);
 +
 +		memset(&expect, 0, sizeof(expect));
@@ -932,7 +942,7 @@
 +	DEBUGP("cleaning request list\n");
 +	clean_request(&request_p_list_udp);
 +
-+	for (port = 0; (port < MAX_PORTS) && ports[port]; port++) {
++	for (port = 0; (port < ports_n_c) && ports[port]; port++) {
 +		DEBUGP("unregistering port %d\n", ports[port]);
 +		ip_conntrack_helper_unregister(&rpc_helpers[port]);
 +	}
@@ -977,20 +987,20 @@
 + **
 + *      Note to all:
 + *
-+ *        RPCs should not be exposed to the internet - ask the Pentagon;
++ *	RPCs should not be exposed to the internet - ask the Pentagon;
 + *
-+ *          "The unidentified crackers pleaded guilty in July to charges
-+ *           of juvenile delinquency stemming from a string of Pentagon
-+ *           network intrusions in February.
-+ *
-+ *           The youths, going by the names TooShort and Makaveli, used
-+ *           a common server security hole to break in, according to
-+ *           Dane Jasper, owner of the California Internet service
-+ *           provider, Sonic. They used the hole, known as the 'statd'
-+ *           exploit, to attempt more than 800 break-ins, Jasper said."
++ *	  "The unidentified crackers pleaded guilty in July to charges
++ *	   of juvenile delinquency stemming from a string of Pentagon
++ *	   network intrusions in February.
++ *
++ *	   The youths, going by the names TooShort and Makaveli, used
++ *	   a common server security hole to break in, according to
++ *	   Dane Jasper, owner of the California Internet service
++ *	   provider, Sonic. They used the hole, known as the 'statd'
++ *	   exploit, to attempt more than 800 break-ins, Jasper said."
 + *
-+ *        From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
-+ *        URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
++ *	From: Wired News; "Pentagon Kids Kicked Off Grid" - Nov 6, 1998
++ *	URL:  http://www.wired.com/news/politics/0,1283,16098,00.html
 + **
 + */
 +
@@ -1082,22 +1092,22 @@
 +{
 +	int   proc_ctr;
 +	char *proc_ptr;
-+        unsigned int proc_num;
++	unsigned int proc_num;
 +
 +	DEBUGP("entered match_rpcs [%i] [%i] ..\n", i_procs, proc);
 +
-+	if(i_procs == -1)
++	if (i_procs == -1)
 +		return 1;
 +
-+        for (proc_ctr=0; proc_ctr <= i_procs; proc_ctr++) {
++	for (proc_ctr=0; proc_ctr <= i_procs; proc_ctr++) {
 +
-+                proc_ptr = c_procs;
-+                proc_ptr += proc_ctr * IPT_RPC_CHAR_LEN;
-+                proc_num = k_atoi(proc_ptr);
++		proc_ptr = c_procs;
++		proc_ptr += proc_ctr * IPT_RPC_CHAR_LEN;
++		proc_num = k_atoi(proc_ptr);
 +
-+		if(proc_num == proc)
++		if (proc_num == proc)
 +			return 1;
-+        }
++	}
 +	return 0;
 +}
 +
@@ -1111,7 +1121,7 @@
 +	const u_int32_t *data;
 +	enum ip_conntrack_dir dir;
 +	const struct tcphdr *tcp;
-+        const struct ipt_rpc_info *rpcinfo = matchinfo;
++	const struct ipt_rpc_info *rpcinfo = matchinfo;
 +	struct request_p *req_p;
 +	u_int32_t xid;
 +	int port, portsok;
@@ -1125,7 +1135,7 @@
 +		dir = CTINFO2DIR(ctinfo);
 +
 +		/* we only want the client to server packets for matching */
-+		if(dir != IP_CT_DIR_ORIGINAL)
++		if (dir != IP_CT_DIR_ORIGINAL)
 +			return 0;
 +
 +		/* This does sanity checking on UDP or TCP packets,
@@ -1134,24 +1144,24 @@
 +		 * communications with the portmapper.
 +		 */
 +
-+		switch(ct->tuplehash[0].tuple.dst.protonum) {
++		switch (ct->tuplehash[0].tuple.dst.protonum) {
 +
 +		case IPPROTO_UDP:
 +			DEBUGP("PROTO_UDP [cont]\n");
-+			if(offset == 0 && datalen < sizeof(struct udphdr)) {
++			if (offset == 0 && datalen < sizeof(struct udphdr)) {
 +				DEBUGP("UDP packet does not contain a complete UDP header. [drop]\n");
 +				return 0;
 +			}
-+			for(port=0,portsok=0; port <= ports_n_c; port++) {
-+				if(ntohs(ct->tuplehash[dir].tuple.dst.u.all) == ports[port]) {
++			for (port=0,portsok=0; port <= ports_n_c; port++) {
++				if (ntohs(ct->tuplehash[dir].tuple.dst.u.all) == ports[port]) {
 +					portsok++;
 +					break;
 +				}
 +			}
-+			if(portsok == 1) {
-+				if((datalen - sizeof(struct udphdr)) != 56) {
++			if (portsok == 1) {
++				if ((datalen - sizeof(struct udphdr)) != 56) {
 +					DEBUGP("UDP packet length is not correct for RPC content. [skip]\n");
-+					if(rpcinfo->strict == 1)
++					if (rpcinfo->strict == 1)
 +						*hotdrop = 1;
 +					return 0;
 +				}
@@ -1165,7 +1175,7 @@
 +
 +				/* Get RPC requestor */
 +				data = (const u_int32_t *)hdr + 7;
-+				if(IXDR_GET_INT32(data) != 3) {
++				if (IXDR_GET_INT32(data) != 3) {
 +					DEBUGP("UDP packet contains an invalid (non \"get\") requestor. [skip]\n");
 +					if(rpcinfo->strict == 1)
 +						*hotdrop = 1;
@@ -1175,8 +1185,8 @@
 +
 +				/* Get RPC procedure */
 +				data = (const u_int32_t *)hdr + 12;
-+                		if(match_rpcs((char *)&rpcinfo->c_procs,
-+				   rpcinfo->i_procs, IXDR_GET_INT32(data)) == 0) {
++				if (match_rpcs((char *)&rpcinfo->c_procs,
++				    rpcinfo->i_procs, IXDR_GET_INT32(data)) == 0) {
 +					DEBUGP("UDP packet contains illegal procedure request [%u]. [drop]\n",
 +						(unsigned int)IXDR_GET_INT32(data));
 +
@@ -1187,7 +1197,7 @@
 +							  ct->tuplehash[dir].tuple.src.ip,
 +							  ct->tuplehash[dir].tuple.src.u.all);
 +
-+					if(req_p){
++					if (req_p) {
 +						DEBUGP("found req_p for xid=%u proto=17 %u.%u.%u.%u:%u\n", xid, 
 +						NIPQUAD(ct->tuplehash[dir].tuple.src.ip),
 +						ntohs(ct->tuplehash[dir].tuple.src.u.all));
@@ -1217,20 +1227,20 @@
 +		
 +		case IPPROTO_TCP:
 +			DEBUGP("PROTO_TCP [cont]\n");
-+			if(offset == 0 && datalen < sizeof(struct tcphdr)) {
++			if (offset == 0 && datalen < sizeof(struct tcphdr)) {
 +				DEBUGP("TCP packet does not contain a complete TCP header. [drop]\n");
 +				return 0;
 +			}	
-+			if(ntohs(ct->tuplehash[dir].tuple.dst.u.all) == 111) { 
++			if (ntohs(ct->tuplehash[dir].tuple.dst.u.all) == 111) { 
 +
 +				tcp = hdr;
-+				if(datalen == (tcp->doff * 4)) {
++				if (datalen == (tcp->doff * 4)) {
 +					DEBUGP("TCP packet does not contain any data. [match]\n");
 +					return (1 && (!offset));
 +				}
 +
 +				/* Tests if packet len is ok */
-+				if((datalen - (tcp->doff * 4)) != 60) {
++				if ((datalen - (tcp->doff * 4)) != 60) {
 +					DEBUGP("TCP packet length is not correct for RPC content. [skip]\n");
 +					if(rpcinfo->strict == 1)
 +						*hotdrop = 1;
@@ -1246,9 +1256,9 @@
 +
 +				/* Get RPC requestor */
 +				data = (const u_int32_t *)tcp + tcp->doff + 6;	
-+				if(IXDR_GET_INT32(data) != 3) {
++				if (IXDR_GET_INT32(data) != 3) {
 +					DEBUGP("TCP packet contains an invalid (non \"get\") requestor. [skip]\n");
-+					if(rpcinfo->strict == 1)
++					if (rpcinfo->strict == 1)
 +						*hotdrop = 1;
 +					return 0;
 +				}
@@ -1256,8 +1266,8 @@
 +
 +				/* Get RPC procedure */
 +				data = (const u_int32_t *)tcp + tcp->doff + 11;
-+                		if(match_rpcs((char *)&rpcinfo->c_procs,
-+				   rpcinfo->i_procs, IXDR_GET_INT32(data)) == 0) {
++				if (match_rpcs((char *)&rpcinfo->c_procs,
++				    rpcinfo->i_procs, IXDR_GET_INT32(data)) == 0) {
 +					DEBUGP("TCP packet contains illegal procedure request [%u]. [drop]\n",
 +						(unsigned int)IXDR_GET_INT32(data));
 +
@@ -1267,7 +1277,7 @@
 +							  ct->tuplehash[dir].tuple.src.ip,
 +							  ct->tuplehash[dir].tuple.src.u.all);
 +
-+					if(req_p){
++					if (req_p) {
 +						DEBUGP("found req_p for xid=%u proto=6 %u.%u.%u.%u:%u\n", xid, 
 +							NIPQUAD(ct->tuplehash[dir].tuple.src.ip),
 +							ntohs(ct->tuplehash[dir].tuple.src.u.all));
@@ -1306,19 +1316,19 @@
 +
 +
 +static int checkentry(const char *tablename, const struct ipt_ip *ip, void *matchinfo,
-+                   unsigned int matchsize, unsigned int hook_mask)
++		   unsigned int matchsize, unsigned int hook_mask)
 +{
-+        if (hook_mask
-+            & ~((1 << NF_IP_PRE_ROUTING) | (1 << NF_IP_FORWARD) | (1 << NF_IP_POST_ROUTING)
++	if (hook_mask
++	    & ~((1 << NF_IP_PRE_ROUTING) | (1 << NF_IP_FORWARD) | (1 << NF_IP_POST_ROUTING)
 +		| (1 << NF_IP_LOCAL_IN) | (1 << NF_IP_LOCAL_OUT))) {
 +		printk("ipt_rpc: only valid for PRE_ROUTING, FORWARD, POST_ROUTING, LOCAL_IN and/or LOCAL_OUT targets.\n");
-+                return 0;
-+        }
++		return 0;
++	}
 +
-+        if (matchsize != IPT_ALIGN(sizeof(struct ipt_rpc_info)))
-+                return 0;
++	if (matchsize != IPT_ALIGN(sizeof(struct ipt_rpc_info)))
++		return 0;
 +
-+        return 1;
++	return 1;
 +}
 +
 +
@@ -1331,7 +1341,7 @@
 +{
 +	int port;
 +
-+        DEBUGP("incrementing usage counts\n");
++	DEBUGP("incrementing usage counts\n");
 +	__MOD_INC_USE_COUNT(ip_conntrack_rpc_udp);
 +	__MOD_INC_USE_COUNT(ip_conntrack_rpc_tcp);
 +
@@ -1354,7 +1364,7 @@
 +	DEBUGP("unregistering match\n");
 +	ipt_unregister_match(&rpc_match);
 +
-+        DEBUGP("decrementing usage counts\n");
++	DEBUGP("decrementing usage counts\n");
 +	__MOD_DEC_USE_COUNT(ip_conntrack_rpc_tcp);
 +	__MOD_DEC_USE_COUNT(ip_conntrack_rpc_udp);
 +}
@@ -1411,25 +1421,25 @@
 +#define EXPIRES (180 * HZ)
 +
 +/* For future conections RPC, using client's cache bindings
-+ * I'll use ip_conntrack_lock to lock these lists           */
++ * I'll use ip_conntrack_lock to lock these lists	   */
 +
 +/* This identifies each request and stores protocol */
 +struct request_p {
-+   struct list_head list;
++	struct list_head list;
 +
-+   u_int32_t xid;   
-+   u_int32_t ip;
-+   u_int16_t port;
++	u_int32_t xid;   
++	u_int32_t ip;
++	u_int16_t port;
 +	
-+   /* Protocol */
-+   u_int16_t proto;
++	/* Protocol */
++	u_int16_t proto;
 +
-+   struct timer_list timeout;
++	struct timer_list timeout;
 +};
 +
 +static inline int request_p_cmp(const struct request_p *p, u_int32_t xid, 
 +				u_int32_t ip, u_int32_t port) {
-+	return(p->xid == xid && p->ip == ip && p->port);
++	return (p->xid == xid && p->ip == ip && p->port);
 +
 +}
 +

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] RPC match and conntrack modules v2.1

[Ian Latter]

Hi Harald,

> This is cool stuff.  Thanks a lot.  The kind of contribution I really
> like :)

Cool, thanks .. :-)  I think there will be a lot of embedded firewall
manufacturers that will enjoy this one ...


> Sorry, I didn't understand the part about the NAT module.  Are you
> saying that NAT support is not functional right now?
That's correct - there was no NAT built into record-rpc, not even in an
old API.  I wanted to make the module set complete by writing the 
corresponding NAT module(s) but I didn't get time.  I might be re-inspired
later in January if no one else picks it up -- the code should be almost
identical to the RSH work module that I wrote, because the connection
handling is quite similar ..... actually ... maybe it doesn't need a nat helper;
the connections are all initiated from the client to the server, and the
internal protocols aren't carrying payloads that need to be rewritten ... so
as long as the nat stuff handles "related" streams sanely, then its
probably ok ...
 
> See attached patch.  Mostly CodingStyle updates (you seem to like 8 spaces
> instead of tab), but also a minor fix:
Cool ... could you resent it as a "zip" attachment ... my mail software
decodes text attachments which will make all the tabs in your patch
spaces again ...
 
> - when unloading, only unregister up to ports_n_c ports
ok

> btw: I'd really love to see the large chunk of code in the switch
> statement of the match() function in ipt_rpc.c be split up in seperate
> functions (thus reducing indentation and improving readability).
possibly could be ... the gumby way might be to do udp and tcp
functions ... but a better way might be to pull out the rpc payload
stuff .... I can take a look at that.
 
> I don't know, but I think esp. in cases where you have lots of rpc
> activity (NFS?, I'm not an RPC expert) it might be wise to use a slab
> cache for the 'struct request_p' list items.
I have not done that before, is there  an example some place in the
existing modules?

> Please integrate my patch [and maybe consider my suggestions, that's of
> course up to you], test it and resubmit it.
hopefully be done by the end of the weekend/early next week.
 
> Thanks!
Thanks for taking a look at it, and providing feed back ...

 
> > Regards,
> > Ian Latter
> 
> -- 
> - Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
> 
===========================================================================
=
> "If this were a dictatorship, it'd be a heck of a lot easier, just so long
>  as I'm the dictator."  --  George W. Bush Dec 18, 2000


--
Ian Latter
Internet and Networking Security Officer
Macquarie University

Re: [PATCH] RPC match and conntrack modules v2.1

[Harald Welte]

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

On Sat, Jan 11, 2003 at 09:47:01AM +1100, Ian Latter wrote:

> Cool, thanks .. :-)  I think there will be a lot of embedded firewall
> manufacturers that will enjoy this one ...

agreed :)

> That's correct - there was no NAT built into record-rpc, not even in an
> old API.  I wanted to make the module set complete by writing the 
> corresponding NAT module(s) but I didn't get time.  I might be re-inspired
> later in January if no one else picks it up -- the code should be almost
> identical to the RSH work module that I wrote, because the connection
> handling is quite similar ..... actually ... maybe it doesn't need a
> nat helper; the connections are all initiated from the client to the
> server, and the internal protocols aren't carrying payloads that need
> to be rewritten ... so as long as the nat stuff handles "related"
> streams sanely, then its probably ok ...

no problem, let's just ignore NAT for now.

> > See attached patch.  Mostly CodingStyle updates (you seem to like 8 spaces
> > instead of tab), but also a minor fix:
> Cool ... could you resent it as a "zip" attachment ... my mail software
> decodes text attachments which will make all the tabs in your patch
> spaces again ...

well, here it is as .gz again.  Strange mailclient you must be using...

> > btw: I'd really love to see the large chunk of code in the switch
> > statement of the match() function in ipt_rpc.c be split up in seperate
> > functions (thus reducing indentation and improving readability).
> possibly could be ... the gumby way might be to do udp and tcp
> functions ... but a better way might be to pull out the rpc payload
> stuff .... I can take a look at that.

thanks.

> > I don't know, but I think esp. in cases where you have lots of rpc
> > activity (NFS?, I'm not an RPC expert) it might be wise to use a slab
> > cache for the 'struct request_p' list items.
> I have not done that before, is there  an example some place in the
> existing modules?

yes, it's done in ip_conntrack_core. see kmem_cache_create /
kmem_cache_alloc / and related functions.   The idea is for the memory
management to keep a pool of memory chunks of the requested size around,
in case they are heavily allocated / deallocated.  We do this with
struct sk_buff, struct ip_conntrack.

I'm not sure whether it's worth the effort in the rpc case... But if I'm
not mistaken in NFSv2, every single file in an 'ls' is a seperate RPC
request/reply, isn't it?

> > Please integrate my patch [and maybe consider my suggestions, that's of
> > course up to you], test it and resubmit it.
> hopefully be done by the end of the weekend/early next week.

no problem, I'll be off over the wekend anyway.

> > Thanks!
> Thanks for taking a look at it, and providing feed back ...

You're welcome.
> --
> Ian Latter

-- 
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
"If this were a dictatorship, it'd be a heck of a lot easier, just so long
 as I'm the dictator."  --  George W. Bush Dec 18, 2000

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] RPC match and conntrack modules v2.1

[Ian Latter]

Hi Harald,

  I didn't get your gz attachment, but I reassembled your patch from
the mailling list archive.

  I have done all bar the cache implementation.  You may be  right
about its usage (and hence its performance) but I'm not sure -- if
NFS only does one RPC lookup per procedure then the ls data will
be a packet per file in the expectation, not the decision making 
process, which will put it out of my hands ... I don't have the gear
to test that particular scenario on hand.
  I would like to apply the cache stuff, but it looked a little challenging
for the time I had available.  I am also interested in combining the 
tcp and udp modules into a single module.
  I have done a fair bit of work on its cleanliness since you last
saw it.

  The file is on the ftp server -- it applies cleanly to 2.4.20.  I think
you'll be much happier with that implementation (v2.2).

  11572 Jan 12 21:08 pom-rpc-030112-01.tgz
  afd50da3f913ba860b69c74030b0a056  pom-rpc-030112-01.tgz


Let me know how you find it ...



----- Original Message -----
>From: "Harald Welte" <laforge@gnumonks.org>
>To: "Ian Latter" <Ian.Latter@mq.edu.au>
>Subject:  Re: [PATCH] RPC match and conntrack modules v2.1
>Date: Sat, 11 Jan 2003 01:05:55 +0100
>
> On Sat, Jan 11, 2003 at 09:47:01AM +1100, Ian Latter wrote:
> 
> > Cool, thanks .. :-)  I think there will be a lot of embedded firewall
> > manufacturers that will enjoy this one ...
> 
> agreed :)
> 
> > That's correct - there was no NAT built into record-rpc, not even in an
> > old API.  I wanted to make the module set complete by writing the 
> > corresponding NAT module(s) but I didn't get time.  I might be re-inspired
> > later in January if no one else picks it up -- the code should be almost
> > identical to the RSH work module that I wrote, because the connection
> > handling is quite similar ..... actually ... maybe it doesn't need a
> > nat helper; the connections are all initiated from the client to the
> > server, and the internal protocols aren't carrying payloads that need
> > to be rewritten ... so as long as the nat stuff handles "related"
> > streams sanely, then its probably ok ...
> 
> no problem, let's just ignore NAT for now.
> 
> > > See attached patch.  Mostly CodingStyle updates (you seem to like 8 spaces
> > > instead of tab), but also a minor fix:
> > Cool ... could you resent it as a "zip" attachment ... my mail software
> > decodes text attachments which will make all the tabs in your patch
> > spaces again ...
> 
> well, here it is as .gz again.  Strange mailclient you must be using...
> 
> > > btw: I'd really love to see the large chunk of code in the switch
> > > statement of the match() function in ipt_rpc.c be split up in seperate
> > > functions (thus reducing indentation and improving readability).
> > possibly could be ... the gumby way might be to do udp and tcp
> > functions ... but a better way might be to pull out the rpc payload
> > stuff .... I can take a look at that.
> 
> thanks.
> 
> > > I don't know, but I think esp. in cases where you have lots of rpc
> > > activity (NFS?, I'm not an RPC expert) it might be wise to use a slab
> > > cache for the 'struct request_p' list items.
> > I have not done that before, is there  an example some place in the
> > existing modules?
> 
> yes, it's done in ip_conntrack_core. see kmem_cache_create /
> kmem_cache_alloc / and related functions.   The idea is for the memory
> management to keep a pool of memory chunks of the requested size around,
> in case they are heavily allocated / deallocated.  We do this with
> struct sk_buff, struct ip_conntrack.
> 
> I'm not sure whether it's worth the effort in the rpc case... But if I'm
> not mistaken in NFSv2, every single file in an 'ls' is a seperate RPC
> request/reply, isn't it?
> 
> > > Please integrate my patch [and maybe consider my suggestions, that's of
> > > course up to you], test it and resubmit it.
> > hopefully be done by the end of the weekend/early next week.
> 
> no problem, I'll be off over the wekend anyway.
> 
> > > Thanks!
> > Thanks for taking a look at it, and providing feed back ...
> 
> You're welcome.
> > --
> > Ian Latter
> 
> -- 
> - Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
> 
===========================================================================
=
> "If this were a dictatorship, it'd be a heck of a lot easier, just so long
>  as I'm the dictator."  --  George W. Bush Dec 18, 2000
> 

--
Ian Latter
Internet and Networking Security Officer
Macquarie University

Re: [PATCH] RPC match and conntrack modules v2.1

[Harald Welte]

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

On Sun, Jan 12, 2003 at 09:33:07PM +1100, Ian Latter wrote:
> Hi Harald,
> 
>   I didn't get your gz attachment, but I reassembled your patch from
> the mailling list archive.

sorry, I missed that attachment :(

>   I have done all bar the cache implementation.  

Thanks. let's consider the slab cache as optional optimization, if
needed this can be changed later.

>   I have done a fair bit of work on its cleanliness since you last
> saw it.

great.  I'll apply your patch to CVS now.

-- 
- Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
============================================================================
"If this were a dictatorship, it'd be a heck of a lot easier, just so long
 as I'm the dictator."  --  George W. Bush Dec 18, 2000

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] RPC match and conntrack modules v2.1

[Ian Latter]

Fantastic ... thanks.



----- Original Message -----
>From: "Harald Welte" <laforge@gnumonks.org>
>To: "Ian Latter" <Ian.Latter@mq.edu.au>
>Subject:  Re: [PATCH] RPC match and conntrack modules v2.1
>Date: Mon, 13 Jan 2003 02:32:26 +0100
>
> On Sun, Jan 12, 2003 at 09:33:07PM +1100, Ian Latter wrote:
> > Hi Harald,
> > 
> >   I didn't get your gz attachment, but I reassembled your patch from
> > the mailling list archive.
> 
> sorry, I missed that attachment :(
> 
> >   I have done all bar the cache implementation.  
> 
> Thanks. let's consider the slab cache as optional optimization, if
> needed this can be changed later.
> 
> >   I have done a fair bit of work on its cleanliness since you last
> > saw it.
> 
> great.  I'll apply your patch to CVS now.
> 
> -- 
> - Harald Welte / laforge@gnumonks.org               http://www.gnumonks.org/
> 
==========================================================================
==
> "If this were a dictatorship, it'd be a heck of a lot easier, just so long
>  as I'm the dictator."  --  George W. Bush Dec 18, 2000
> 

--
Ian Latter
Internet and Networking Security Officer
Macquarie University