Skip to contentSkip to conversation

proto_to_name duplication

PATCH In review Phil Oester 3 participants 6 messages Jul 21, 2006 – Jul 25, 2006 netfilter

lore
Share & tools
lore

Phil Oester

author
#0 lore raw

PATCH 1/1 proto_to_name duplication

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

extensions/libipt_multiport.c +2 -19
diff -ru ipt-orig/extensions/libipt_multiport.c ipt-new/extensions/libipt_multiport.c
@@ -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;
include/iptables.h +1 -0
diff -ru ipt-orig/include/iptables.h ipt-new/include/iptables.h
@@ -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);
iptables.c +2 -1
diff -ru ipt-orig/iptables.c ipt-new/iptables.c
@@ -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;
#1 lore raw
Quoted 2 lines · Phil Oester

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

Also applied, thanks Phil.

Quoted 5 lines

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.

author
#2 lore raw
Quoted 7 lines · Patrick McHardy
Quoted 2 lines

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

#3 lore raw
In reply to Patrick McHardy

Hi,

From: Patrick McHardy <[email protected]>
Date: Sat, 22 Jul 2006 15:47:40 +0200

Quoted 5 lines
Quoted 2 lines · Phil Oester

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

author
#4 lore raw
Quoted 10 lines · Yasuyuki KOZAKAI

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

#5 lore raw
Quoted 7 lines · Phil Oester
Quoted 5 lines · Yasuyuki KOZAKAI

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.

Quoted 8 lines
Quoted 5 lines

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.