[PATCH] proto_to_name duplication

[PATCH] proto_to_name duplication

[Phil Oester]

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

Update multiport match to use the iptables version of proto_to_name
instead of reinventing the wheel.

I looked at a similar ipv6 cleanup, but the ipv6header header match
makes this somewhat trickier.

Incidentally - it's probably time to remove the superfluous mport
match.  Anyone still using it can probably stick with older userspace.

Phil


[-- Attachment #2: patch-proto_to_name --]
[-- Type: text/plain, Size: 2142 bytes --]

diff -ru ipt-orig/extensions/libipt_multiport.c ipt-new/extensions/libipt_multiport.c
--- ipt-orig/extensions/libipt_multiport.c	2006-07-20 10:01:54.000000000 -0700
+++ ipt-new/extensions/libipt_multiport.c	2006-07-21 14:58:53.000000000 -0700
@@ -51,23 +51,6 @@
 	{0}
 };
 
-static char *
-proto_to_name(u_int8_t proto)
-{
-	switch (proto) {
-	case IPPROTO_TCP:
-		return "tcp";
-	case IPPROTO_UDP:
-		return "udp";
-	case IPPROTO_SCTP:
-		return "sctp";
-	case IPPROTO_DCCP:
-		return "dccp";
-	default:
-		return NULL;
-	}
-}
-
 static unsigned int
 parse_multi_ports(const char *portstring, u_int16_t *ports, const char *proto)
 {
@@ -143,7 +126,7 @@
 		exit_error(PARAMETER_PROBLEM,
 			   "multiport only works with TCP or UDP");
 
-	if ((proto = proto_to_name(entry->ip.proto)) != NULL)
+	if ((proto = proto_to_name(entry->ip.proto, 1)) != NULL)
 		return proto;
 	else if (!entry->ip.proto)
 		exit_error(PARAMETER_PROBLEM,
@@ -264,7 +247,7 @@
 {
 	struct servent *service;
 
-	if ((service = getservbyport(htons(port), proto_to_name(proto))))
+	if ((service = getservbyport(htons(port), proto_to_name(proto, 1))))
 		return service->s_name;
 
 	return NULL;
diff -ru ipt-orig/include/iptables.h ipt-new/include/iptables.h
--- ipt-orig/include/iptables.h	2006-07-20 10:01:54.000000000 -0700
+++ ipt-new/include/iptables.h	2006-07-21 15:29:18.000000000 -0700
@@ -151,6 +151,7 @@
 extern void register_match(struct iptables_match *me);
 extern void register_target(struct iptables_target *me);
 
+extern char *proto_to_name(u_int8_t proto, int nolookup);
 extern int service_to_port(const char *name, const char *proto);
 extern u_int16_t parse_port(const char *port, const char *proto);
 extern struct in_addr *dotted_to_addr(const char *dotted);
diff -ru ipt-orig/iptables.c ipt-new/iptables.c
--- ipt-orig/iptables.c	2006-07-20 10:01:54.000000000 -0700
+++ ipt-new/iptables.c	2006-07-21 15:29:28.000000000 -0700
@@ -229,9 +229,10 @@
 	{ "esp", IPPROTO_ESP },
 	{ "ah", IPPROTO_AH },
 	{ "sctp", IPPROTO_SCTP },
+	{ "dccp", IPPROTO_DCCP },
 };
 
-static char *
+char *
 proto_to_name(u_int8_t proto, int nolookup)
 {
 	unsigned int i;

Re: [PATCH] proto_to_name duplication

[Patrick McHardy]

Phil Oester wrote:
> Update multiport match to use the iptables version of proto_to_name
> instead of reinventing the wheel.

Also applied, thanks Phil.

> I looked at a similar ipv6 cleanup, but the ipv6header header match
> makes this somewhat trickier.
> 
> Incidentally - it's probably time to remove the superfluous mport
> match.  Anyone still using it can probably stick with older userspace.

You're probably right, but as long as it doesn't cause any work I
also have no problem with keeping it. Do you know how long ago the
mport patch was removed from pom? Couldn't find anything in the
SVN history.

Re: [PATCH] proto_to_name duplication

[Phil Oester]

On Sat, Jul 22, 2006 at 03:47:40PM +0200, Patrick McHardy wrote:
> > Incidentally - it's probably time to remove the superfluous mport
> > match.  Anyone still using it can probably stick with older userspace.
> 
> You're probably right, but as long as it doesn't cause any work I
> also have no problem with keeping it. Do you know how long ago the
> mport patch was removed from pom? Couldn't find anything in the
> SVN history.

It was removed in mid-May.  But you're right - might as well just leave it
alone unless it becomes a maintenance headache.

Phil

Re: [PATCH] proto_to_name duplication

[Yasuyuki KOZAKAI]

Hi,

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 22 Jul 2006 15:47:40 +0200

> Phil Oester wrote:
> > Update multiport match to use the iptables version of proto_to_name
> > instead of reinventing the wheel.
> 
> Also applied, thanks Phil.

This allows iptables to send the invalid entry to kernel when
people do 'iptables -p icmp -m multiport --sports 10000 ...',
for example. Of cause kernel can reject this, but iptables cannot
output useful error message after that.

check_proto() should not allow protocols other than tcp, udp, sctp and
dccp. That's why libip{,6}t_multiport have the other version of
proto_to_name().

Please revert this or add check for protocol number.

Regards,

-- Yasuyuki Kozakai

Re: [PATCH] proto_to_name duplication

[Phil Oester]

On Mon, Jul 24, 2006 at 02:38:32PM +0900, Yasuyuki KOZAKAI wrote:
> This allows iptables to send the invalid entry to kernel when
> people do 'iptables -p icmp -m multiport --sports 10000 ...',
> for example. Of cause kernel can reject this, but iptables cannot
> output useful error message after that.
> 
> check_proto() should not allow protocols other than tcp, udp, sctp and
> dccp. That's why libip{,6}t_multiport have the other version of
> proto_to_name().
> 
> Please revert this or add check for protocol number.

Good catch.  Probably should just revert this one for simplicity.

Phil

Re: [PATCH] proto_to_name duplication

[Patrick McHardy]

Phil Oester wrote:
> On Mon, Jul 24, 2006 at 02:38:32PM +0900, Yasuyuki KOZAKAI wrote:
> 
>>This allows iptables to send the invalid entry to kernel when
>>people do 'iptables -p icmp -m multiport --sports 10000 ...',
>>for example. Of cause kernel can reject this, but iptables cannot
>>output useful error message after that.
>>
>>check_proto() should not allow protocols other than tcp, udp, sctp and
>>dccp. That's why libip{,6}t_multiport have the other version of
>>proto_to_name().
>>
>>Please revert this or add check for protocol number.
> 
> 
> Good catch.  Probably should just revert this one for simplicity.


Agreed, I've reverted it. Thanks.