[PATCH] Update SNMP basic for full IP address NAT

[PATCH] Update SNMP basic for full IP address NAT

[zze-Ganesh KERDONCUFF G ext RD-MAPS-REN]

All,

This patch applies to the netfilter file   nat_ip_snmp_basic.c

The algorithm now applies NAT to the complete IP address (and not only
the first byte)
It also recomputes the UDP checksum accordingly.

Please apply this patch to the 2.6.18.1 official release.

Many thanks to Remy Harel for his early contributions on this code.

Gilles Kerdoncuff

---

--- linux-2.6.18.1/net/ipv4/netfilter/ip_nat_snmp_basic.c
2006-10-14 05:34:03.000000000 +0200
+++ linux/net/ipv4/netfilter/ip_nat_snmp_basic.c	2006-11-02
17:02:24.000000000 +0100
@@ -70,14 +70,12 @@
 static DEFINE_SPINLOCK(snmp_lock);
 
 /* 
- * Application layer address mapping mimics the NAT mapping, but 
- * only for the first octet in this case (a more flexible system
- * can be implemented if needed).
+ * Application layer address mapping (complete IP)
  */
-struct oct1_map
+struct ip_map
 {
-	u_int8_t from;
-	u_int8_t to;
+	u_int32_t from;
+	u_int32_t to;
 };
 
                                   
@@ -612,7 +610,7 @@
 
 static inline void mangle_address(unsigned char *begin,
                                   unsigned char *addr,
-                                  const struct oct1_map *map,
+                                  const struct ip_map *map,
                                   u_int16_t *check);
 struct snmp_cnv
 {
@@ -907,6 +905,23 @@
 	csum[1] = x & 0xFF;
 }
 
+/* Fast checksum update on 16 bits */
+static void update_csum(u_int16_t *csum,
+                        u_int16_t *old_addr,
+                        u_int16_t *new_addr,
+                        int       odd)
+{
+	/* The idea is to call 4 times the fast csum function which
recalculate the new
+	 * csum according to the new ip address in the payload
+	 */
+	int i;
+	for (i=0;i<4;i++) {
+		fast_csum((unsigned char*)csum, ((const unsigned
char*)(old_addr))+i, ((const unsigned char*)(new_addr))+i,(odd+i)%2 );
+	}
+}
+
+
+
 /* 
  * Mangle IP address.
  * 	- begin points to the start of the snmp messgae
@@ -914,24 +929,22 @@
  */
 static inline void mangle_address(unsigned char *begin,
                                   unsigned char *addr,
-                                  const struct oct1_map *map,
+                                  const struct ip_map *map,
                                   u_int16_t *check)
 {
-	if (map->from == NOCT1(*addr)) {
-		u_int32_t old;
-		
-		if (debug)
-			memcpy(&old, (unsigned char *)addr,
sizeof(old));
-			
-		*addr = map->to;
-		
+	u_int32_t old;
+
+	memcpy(&old, (unsigned char *)addr, sizeof(old));
+
+	if (map->from == old) {
+		*((u_int32_t*) addr) = map->to;
+
 		/* Update UDP checksum if being used */
 		if (*check) {
 			unsigned char odd = !((addr - begin) % 2);
 			
-			fast_csum((unsigned char *)check,
-			          &map->from, &map->to, odd);
-			          
+			update_csum(check, (u_int16_t*)&old,
(u_int16_t*)addr, odd);
+
 		}
 		
 		if (debug)
@@ -942,7 +955,7 @@
 
 static unsigned char snmp_trap_decode(struct asn1_ctx *ctx,
                                       struct snmp_v1_trap *trap,
-                                      const struct oct1_map *map,
+                                      const struct ip_map *map,
                                       u_int16_t *check)
 {
 	unsigned int cls, con, tag, len;
@@ -1036,7 +1049,7 @@
  */
 static int snmp_parse_mangle(unsigned char *msg,
                              u_int16_t len,
-                             const struct oct1_map *map,
+                             const struct ip_map *map,
                              u_int16_t *check)
 {
 	unsigned char *eoc, *end;
@@ -1215,7 +1228,7 @@
 	u_int16_t udplen = ntohs(udph->len);
 	u_int16_t paylen = udplen - sizeof(struct udphdr);
 	int dir = CTINFO2DIR(ctinfo);
-	struct oct1_map map;
+	struct ip_map map;
 
 	/*
 	 * Determine mappping for application layer addresses based
@@ -1223,12 +1236,12 @@
 	 */
 	if (dir == IP_CT_DIR_ORIGINAL) {
 		/* SNAT traps */
-		map.from =
NOCT1(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip);
-		map.to =
NOCT1(ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip);
+		map.from =
ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+		map.to = ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
 	} else {
 		/* DNAT replies */
-		map.from =
NOCT1(ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip);
-		map.to =
NOCT1(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip);
+		map.from = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
+		map.to = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
 	}
 	
 	if (map.from == map.to)

Re: [PATCH] Update SNMP basic for full IP address NAT

[Patrick McHardy]

zze-Ganesh KERDONCUFF G ext RD-MAPS-REN wrote:
> The algorithm now applies NAT to the complete IP address (and not only
> the first byte)
> It also recomputes the UDP checksum accordingly.

I'm not too familiar with SNMP NAT, please send this to
netfilter-devel@lists.netfilter.org so other netfilter
developers get a chance to look at it.

[PATCH] Update SNMP basic for full IP address NAT

[zze-Ganesh KERDONCUFF G ext RD-MAPS-REN]

 
All,

This patch applies to the netfilter file   nat_ip_snmp_basic.c

The algorithm now applies NAT to the complete IP address (and not only
the first byte) It also recomputes the UDP checksum accordingly.

Please apply this patch to the 2.6.18.1 official release.

Many thanks to Remy Harel for his early contributions on this code.

Gilles Kerdoncuff

---

--- linux-2.6.18.1/net/ipv4/netfilter/ip_nat_snmp_basic.c
2006-10-14 05:34:03.000000000 +0200
+++ linux/net/ipv4/netfilter/ip_nat_snmp_basic.c	2006-11-02
17:02:24.000000000 +0100
@@ -70,14 +70,12 @@
 static DEFINE_SPINLOCK(snmp_lock);
 
 /*
- * Application layer address mapping mimics the NAT mapping, but
- * only for the first octet in this case (a more flexible system
- * can be implemented if needed).
+ * Application layer address mapping (complete IP)
  */
-struct oct1_map
+struct ip_map
 {
-	u_int8_t from;
-	u_int8_t to;
+	u_int32_t from;
+	u_int32_t to;
 };
 
                                   
@@ -612,7 +610,7 @@
 
 static inline void mangle_address(unsigned char *begin,
                                   unsigned char *addr,
-                                  const struct oct1_map *map,
+                                  const struct ip_map *map,
                                   u_int16_t *check);  struct snmp_cnv
{ @@ -907,6 +905,23 @@
 	csum[1] = x & 0xFF;
 }
 
+/* Fast checksum update on 16 bits */
+static void update_csum(u_int16_t *csum,
+                        u_int16_t *old_addr,
+                        u_int16_t *new_addr,
+                        int       odd)
+{
+	/* The idea is to call 4 times the fast csum function which
recalculate the new
+	 * csum according to the new ip address in the payload
+	 */
+	int i;
+	for (i=0;i<4;i++) {
+		fast_csum((unsigned char*)csum, ((const unsigned
char*)(old_addr))+i, ((const unsigned char*)(new_addr))+i,(odd+i)%2 );
+	}
+}
+
+
+
 /*
  * Mangle IP address.
  * 	- begin points to the start of the snmp messgae
@@ -914,24 +929,22 @@
  */
 static inline void mangle_address(unsigned char *begin,
                                   unsigned char *addr,
-                                  const struct oct1_map *map,
+                                  const struct ip_map *map,
                                   u_int16_t *check)  {
-	if (map->from == NOCT1(*addr)) {
-		u_int32_t old;
-		
-		if (debug)
-			memcpy(&old, (unsigned char *)addr,
sizeof(old));
-			
-		*addr = map->to;
-		
+	u_int32_t old;
+
+	memcpy(&old, (unsigned char *)addr, sizeof(old));
+
+	if (map->from == old) {
+		*((u_int32_t*) addr) = map->to;
+
 		/* Update UDP checksum if being used */
 		if (*check) {
 			unsigned char odd = !((addr - begin) % 2);
 			
-			fast_csum((unsigned char *)check,
-			          &map->from, &map->to, odd);
-			          
+			update_csum(check, (u_int16_t*)&old,
(u_int16_t*)addr, odd);
+
 		}
 		
 		if (debug)
@@ -942,7 +955,7 @@
 
 static unsigned char snmp_trap_decode(struct asn1_ctx *ctx,
                                       struct snmp_v1_trap *trap,
-                                      const struct oct1_map *map,
+                                      const struct ip_map *map,
                                       u_int16_t *check)  {
 	unsigned int cls, con, tag, len;
@@ -1036,7 +1049,7 @@
  */
 static int snmp_parse_mangle(unsigned char *msg,
                              u_int16_t len,
-                             const struct oct1_map *map,
+                             const struct ip_map *map,
                              u_int16_t *check)  {
 	unsigned char *eoc, *end;
@@ -1215,7 +1228,7 @@
 	u_int16_t udplen = ntohs(udph->len);
 	u_int16_t paylen = udplen - sizeof(struct udphdr);
 	int dir = CTINFO2DIR(ctinfo);
-	struct oct1_map map;
+	struct ip_map map;
 
 	/*
 	 * Determine mappping for application layer addresses based @@
-1223,12 +1236,12 @@
 	 */
 	if (dir == IP_CT_DIR_ORIGINAL) {
 		/* SNAT traps */
-		map.from =
NOCT1(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip);
-		map.to =
NOCT1(ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip);
+		map.from =
ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+		map.to = ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
 	} else {
 		/* DNAT replies */
-		map.from =
NOCT1(ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip);
-		map.to =
NOCT1(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip);
+		map.from = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
+		map.to = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
 	}
 	
 	if (map.from == map.to)

Re: [PATCH] Update SNMP basic for full IP address NAT

[Patrick McHardy]

zze-Ganesh KERDONCUFF G ext RD-MAPS-REN wrote:
> This patch applies to the netfilter file   nat_ip_snmp_basic.c
> 
> The algorithm now applies NAT to the complete IP address (and not only
> the first byte) It also recomputes the UDP checksum accordingly.
> 
> Please apply this patch to the 2.6.18.1 official release.

What exactly is the purpose of this? My understanding of this module
(which might be wrong) is that it maps an entire /8 network, including
addresses not participating in the current connection. Your patch
changes this to only touch addresses participating in the connection
and leave the others unmapped. Is that really what you want?
In either case we can not break the old behaviour, so I suggest you
add an module option to control whether the entire network or just
a single address will be translated (ideally by just letting the user
specify a prefix length) and default to the old behaviour.

Re: [PATCH] Update SNMP basic for full IP address NAT

[Gilles Kerdoncuff]

Hi Patrick,

** The purpose of my patch, for SNMP, is to fully benefit of the NAT 
feature on a residential gateway (RFC2962 - Basic method)

SNMP traps from any device in the local network can be sent to a Network 
Management Station (NMS) in the ouside world (using the MASQUERADE or 
SNAT rule found on any gateway). The local IP of the agent in the 
payload is natted to the public address.

In addition, SNMP requests from outside world NMS can reach a device in 
the local network with a specific DNAT rule. Some requests ask for the 
IP address table. In that case, the local IP in the response, its natted 
to the public IP of the gateway.

So when we have to NAT from a local network to an internet public 
address, the full IP has to be natted, not only the first byte.


** I understand that you'd like to keep the /8 behavior, yet, something 
still troubles me with the current ip_nat_snmp_basic.c file :

The from/to fields of the oct1_map structure are filled-in by calling 
the NOCT1 (0xFF mask) macro on the tuple IP.
Which means that on a 192.168.1.x address, it takes the x part.

On the second hand, the mangle_address routine is called with 
"ctx->pointer - 4", which points to the beginning of the IP. So, the 
routine will compare the x part to the '192' part !!

Am I misinterpretting the code or is it really a bug ?
My understanding is that the '192' part of the address should be subject 
to the NAT translation, correct ?

** Anyway, if any use case for the /8 behavior exists, I don't mind 
adding a parameter to test only the first 8,16,24 or 32 bits of the 
address, keeping /8 as a default.


BR
Gilles



Patrick McHardy wrote:
> Gilles KERDONCUFF wrote:
>   
>> This patch applies to the netfilter file   nat_ip_snmp_basic.c
>>
>> The algorithm now applies NAT to the complete IP address (and not only
>> the first byte) It also recomputes the UDP checksum accordingly.
>>
>> Please apply this patch to the 2.6.18.1 official release.
>>     
>
> What exactly is the purpose of this? My understanding of this module
> (which might be wrong) is that it maps an entire /8 network, including
> addresses not participating in the current connection. Your patch
> changes this to only touch addresses participating in the connection
> and leave the others unmapped. Is that really what you want?
> In either case we can not break the old behaviour, so I suggest you
> add an module option to control whether the entire network or just
> a single address will be translated (ideally by just letting the user
> specify a prefix length) and default to the old behaviour.
>
>   

 

Re: [PATCH] Update SNMP basic for full IP address NAT

[Patrick McHardy]

Gilles Kerdoncuff wrote:
> ** I understand that you'd like to keep the /8 behavior, yet, something
> still troubles me with the current ip_nat_snmp_basic.c file :
> 
> The from/to fields of the oct1_map structure are filled-in by calling
> the NOCT1 (0xFF mask) macro on the tuple IP.
> Which means that on a 192.168.1.x address, it takes the x part.

I'm confused - you seem to be right, but quick testing shows it behaves
correctly and takes the 192 part.

> On the second hand, the mangle_address routine is called with
> "ctx->pointer - 4", which points to the beginning of the IP. So, the
> routine will compare the x part to the '192' part !!
> 
> Am I misinterpretting the code or is it really a bug ?
> My understanding is that the '192' part of the address should be subject
> to the NAT translation, correct ?

Yes.

> ** Anyway, if any use case for the /8 behavior exists, I don't mind
> adding a parameter to test only the first 8,16,24 or 32 bits of the
> address, keeping /8 as a default.

Please do so. But why only 8,16,24,32? I don't believe allowing
any prefix len will be harder to do.

Re: [PATCH] Update SNMP basic for full IP address NAT

[Gilles Kerdoncuff]

Patrick McHardy wrote:
> Gilles Kerdoncuff wrote:
>   
>> ** I understand that you'd like to keep the /8 behavior, yet, something
>> still troubles me with the current ip_nat_snmp_basic.c file :
>>
>> The from/to fields of the oct1_map structure are filled-in by calling
>> the NOCT1 (0xFF mask) macro on the tuple IP.
>> Which means that on a 192.168.1.x address, it takes the x part.
>>     
>
> I'm confused - you seem to be right, but quick testing shows it behaves
> correctly and takes the 192 part.
>
>   
This makes things clear. This is one of those little/big endian problems :
I'm using this module on a MIPS platform, you probably tested it on an 
Intel architecture

If you look at ip_conntrack_ftp.c, it uses ntohl routine when extracting 
the tuple IP. This is the way it should be done.

In the SNMP module, add ntohl after the
    map.to = NOCT1(
and
    map.from = NOCT1(
code lines
Run your quick test again, you will see that the bug is there !

I will fix that in my updated patch.


>> ** Anyway, if any use case for the /8 behavior exists, I don't mind
>> adding a parameter to test only the first 8,16,24 or 32 bits of the
>> address, keeping /8 as a default.
>>     
>
> Please do so. But why only 8,16,24,32? I don't believe allowing
> any prefix len will be harder to do.
>   
Ok, I'll do so as soon as I can find a few hours to code and test it.
Thanks a lot.

Gilles.

 

Re: [PATCH] Update SNMP basic for full IP address NAT

[Patrick McHardy]

Gilles Kerdoncuff wrote:
> This makes things clear. This is one of those little/big endian problems :
> I'm using this module on a MIPS platform, you probably tested it on an
> Intel architecture
> 
> If you look at ip_conntrack_ftp.c, it uses ntohl routine when extracting
> the tuple IP. This is the way it should be done.
> 
> In the SNMP module, add ntohl after the
>    map.to = NOCT1(
> and
>    map.from = NOCT1(
> code lines
> Run your quick test again, you will see that the bug is there !
> 
> I will fix that in my updated patch.

Please send a seperate patch for this since its unrelated to
your changes.

> Ok, I'll do so as soon as I can find a few hours to code and test it.
> Thanks a lot.

Please make sure you do your patch on top of the nf_nat tree
(http://people.netfilter.org/~kaber/nf-2.6.20-nat.git/) and
update both the old and the new helper. The mail I just sent
to netfilter-devel explains how to clone that tree.