iptables: allow service names in [DS]NAT targets

PATCH Superseded Phil Oester 3 participants 4 messages Jul 8, 2013 – Jul 25, 2013 netfilter

Share & tools

Phil Oester

author

2013-07-08 16:46 UTC #0 lore raw

PATCH v2 1/1 iptables: allow service names in [DS]NAT targets

As reported by Alexander Hoogerhuis, the [DS]NAT targets do not allow use of service names in the --to argument. The same problem was fixed in the REDIRECT target in commit 84d758b3 ("extensions: REDIRECT: fix --to-ports parser"). Use a similar fix here.

This closes bugzilla #514.

Phil

Signed-off-by Phil Oester <[email protected]>

---
v2: also handle IPv6 [DS]NAT targets

extensions/libip6t_DNAT.c +18 ~~-24~~

index eaa6bf1..f842e40 100644
@@ -46,7 +46,7 @@ static const struct xt_option_entry DNAT_opts[] = {
 static void
 parse_to(const char *orig_arg, int portok, struct nf_nat_range *range)
 {
-	char *arg, *start, *end = NULL, *colon = NULL, *dash, *error;
+	char *arg, *start, *end = "", *colon = NULL, *dash;
 	const struct in6_addr *ip;
 
 	arg = strdup(orig_arg);

@@ -60,8 +60,7 @@ parse_to(const char *orig_arg, int portok, struct nf_nat_range *range)
 		colon = strchr(arg, ':');
 		if (colon && strchr(colon+1, ':'))
 			colon = NULL;
-	}
-	else {
+	} else {
 		start++;
 		end = strchr(start, ']');
 		if (end == NULL)

@@ -73,7 +72,7 @@ parse_to(const char *orig_arg, int portok, struct nf_nat_range *range)
 	}
 
 	if (colon) {
-		int port;
+		unsigned int port, maxport;
 
 		if (!portok)
 			xtables_error(PARAMETER_PROBLEM,

@@ -81,34 +80,29 @@ parse_to(const char *orig_arg, int portok, struct nf_nat_range *range)
 
 		range->flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
 
-		port = atoi(colon+1);
-		if (port <= 0 || port > 65535)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Port `%s' not valid\n", colon+1);
-
-		error = strchr(colon+1, ':');
-		if (error)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Invalid port:port syntax - use dash\n");
+		if (!xtables_strtoui(colon + 1, &end, &port, 0, UINT16_MAX) &&
+		    (port = xtables_service_to_port(colon + 1, NULL)) == (unsigned)-1)
+			xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", colon);
 
-		dash = strchr(colon, '-');
-		if (!dash) {
+		switch (*end) {
+		case '\0':
 			range->min_proto.tcp.port
 				= range->max_proto.tcp.port
 				= htons(port);
-		} else {
-			int maxport;
+			break;
+		case '-':
+			if (!xtables_strtoui(end + 1, NULL, &maxport, 0, UINT16_MAX) &&
+			    (maxport = xtables_service_to_port(end + 1, NULL)) == (unsigned)-1)
+				xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", colon);
 
-			maxport = atoi(dash + 1);
-			if (maxport <= 0 || maxport > 65535)
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port `%s' not valid\n", dash+1);
 			if (maxport < port)
-				/* People are stupid. */
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port range `%s' funky\n", colon+1);
+				xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", colon);
+
 			range->min_proto.tcp.port = htons(port);
 			range->max_proto.tcp.port = htons(maxport);
+			break;
+		default:
+			xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", colon);
 		}
 		/* Starts with colon or [] colon? No IP info...*/
 		if (colon == arg || colon == arg+2) {

extensions/libip6t_SNAT.c +18 ~~-24~~

74 lines not on this page · raw message

extensions/libipt_DNAT.c +18 ~~-22~~

index ff18799..e452cfc 100644
@@ -67,7 +67,7 @@ static struct xt_entry_target *
 parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 {
 	struct nf_nat_ipv4_range range;
-	char *arg, *colon, *dash, *error;
+	char *arg, *colon, *dash;
 	const struct in_addr *ip;
 
 	arg = strdup(orig_arg);

@@ -77,7 +77,8 @@ parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 	colon = strchr(arg, ':');
 
 	if (colon) {
-		int port;
+		char *end = "";
+		unsigned int port, maxport;
 
 		if (!portok)
 			xtables_error(PARAMETER_PROBLEM,

@@ -85,34 +86,29 @@ parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 
 		range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
 
-		port = atoi(colon+1);
-		if (port <= 0 || port > 65535)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Port `%s' not valid\n", colon+1);
-
-		error = strchr(colon+1, ':');
-		if (error)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Invalid port:port syntax - use dash\n");
+		if (!xtables_strtoui(colon + 1, &end, &port, 0, UINT16_MAX) &&
+		    (port = xtables_service_to_port(colon + 1, NULL)) == (unsigned)-1)
+			xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", arg);
 
-		dash = strchr(colon, '-');
-		if (!dash) {
+		switch (*end) {
+		case '\0':
 			range.min.tcp.port
 				= range.max.tcp.port
 				= htons(port);
-		} else {
-			int maxport;
+			break;
+		case '-':
+			if (!xtables_strtoui(end + 1, NULL, &maxport, 0, UINT16_MAX) &&
+			    (maxport = xtables_service_to_port(end + 1, NULL)) == (unsigned)-1)
+				xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", arg);
 
-			maxport = atoi(dash + 1);
-			if (maxport <= 0 || maxport > 65535)
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port `%s' not valid\n", dash+1);
 			if (maxport < port)
-				/* People are stupid. */
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port range `%s' funky\n", colon+1);
+				xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", arg);
+
 			range.min.tcp.port = htons(port);
 			range.max.tcp.port = htons(maxport);
+			break;
+		default:
+			xtables_param_act(XTF_BAD_VALUE, "DNAT", "--to", arg);
 		}
 		/* Starts with a colon? No IP info...*/
 		if (colon == arg) {

extensions/libipt_SNAT.c +18 ~~-22~~

index 1a24f3d..01be677 100644
@@ -67,7 +67,7 @@ static struct xt_entry_target *
 parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 {
 	struct nf_nat_ipv4_range range;
-	char *arg, *colon, *dash, *error;
+	char *arg, *colon, *dash;
 	const struct in_addr *ip;
 
 	arg = strdup(orig_arg);

@@ -77,7 +77,8 @@ parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 	colon = strchr(arg, ':');
 
 	if (colon) {
-		int port;
+		char *end = "";
+		unsigned int port, maxport;
 
 		if (!portok)
 			xtables_error(PARAMETER_PROBLEM,

@@ -85,34 +86,29 @@ parse_to(const char *orig_arg, int portok, struct ipt_natinfo *info)
 
 		range.flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
 
-		port = atoi(colon+1);
-		if (port <= 0 || port > 65535)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Port `%s' not valid\n", colon+1);
-
-		error = strchr(colon+1, ':');
-		if (error)
-			xtables_error(PARAMETER_PROBLEM,
-				   "Invalid port:port syntax - use dash\n");
+		if (!xtables_strtoui(colon + 1, &end, &port, 0, UINT16_MAX) &&
+		    (port = xtables_service_to_port(colon + 1, NULL)) == (unsigned)-1)
+			xtables_param_act(XTF_BAD_VALUE, "SNAT", "--to", arg);
 
-		dash = strchr(colon, '-');
-		if (!dash) {
+		switch (*end) {
+		case '\0':
 			range.min.tcp.port
 				= range.max.tcp.port
 				= htons(port);
-		} else {
-			int maxport;
+			break;
+		case '-':
+			if (!xtables_strtoui(end + 1, NULL, &maxport, 0, UINT16_MAX) &&
+			    (maxport = xtables_service_to_port(end + 1, NULL)) == (unsigned)-1)
+				xtables_param_act(XTF_BAD_VALUE, "SNAT", "--to", arg);
 
-			maxport = atoi(dash + 1);
-			if (maxport <= 0 || maxport > 65535)
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port `%s' not valid\n", dash+1);
 			if (maxport < port)
-				/* People are stupid. */
-				xtables_error(PARAMETER_PROBLEM,
-					   "Port range `%s' funky\n", colon+1);
+				xtables_param_act(XTF_BAD_VALUE, "SNAT", "--to", arg);
+
 			range.min.tcp.port = htons(port);
 			range.max.tcp.port = htons(maxport);
+			break;
+		default:
+			xtables_param_act(XTF_BAD_VALUE, "SNAT", "--to", arg);
 		}
 		/* Starts with a colon? No IP info...*/
 		if (colon == arg) {

Pablo Neira Ayuso

maintainer

2013-07-24 19:00 UTC #1 lore raw

Quoted 4 lines · Phil Oester

As reported by Alexander Hoogerhuis, the [DS]NAT targets do not allow use of service names in the --to argument. The same problem was fixed in the REDIRECT target in commit 84d758b3 ("extensions: REDIRECT: fix --to-ports parser"). Use a similar fix here.

While testing this I noticed that this works:

--to-source 1.1.1.1:telnet
--to-source 1.1.1.1-1.1.1.10:1025-3000
--to-source 1.1.1.1-1.1.1.10:telnet

But this does not:

--to-source 1.1.1.1-1.1.1.10:telnet-http iptables v1.4.19.1: SNAT: Bad value for "--to" option:
"1.1.1.1-1.1.1.10:telnet-ssh"

I think it should, for consistency (even if I have to confess that it looks a bit ugly to me).

If you decide to address this and send me a new version to support this, then it would be also good to update the manpage to say that we support services starting 1.4.20.

Thanks.

Jozsef Kadlecsik

2013-07-24 21:11 UTC #2 lore raw

Quoted 18 lines · Pablo Neira Ayuso

Quoted 4 lines · Phil Oester
As reported by Alexander Hoogerhuis, the [DS]NAT targets do not allow use of service names in the --to argument. The same problem was fixed in the REDIRECT target in commit 84d758b3 ("extensions: REDIRECT: fix --to-ports parser"). Use a similar fix here.
While testing this I noticed that this works:
--to-source 1.1.1.1:telnet
--to-source 1.1.1.1-1.1.1.10:1025-3000
--to-source 1.1.1.1-1.1.1.10:telnet
But this does not:
--to-source 1.1.1.1-1.1.1.10:telnet-http iptables v1.4.19.1: SNAT: Bad value for "--to" option:
"1.1.1.1-1.1.1.10:telnet-ssh"

Quoted 6 lines

I think it should, for consistency (even if I have to confess that it looks a bit ugly to me).
If you decide to address this and send me a new version to support this, then it would be also good to update the manpage to say that we support services starting 1.4.20.

That is still ambiguous - there are service names with dash. So I suggest to support the notation '[name-with-dash]' in order to explicitly express and handle such cases.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences

          H-1525 Budapest 114, POB. 49, Hungary

Phil Oester

author

2013-07-25 00:17 UTC #3 lore raw

Quoted 10 lines · Jozsef Kadlecsik

Quoted 9 lines · Pablo Neira Ayuso
But this does not:
--to-source 1.1.1.1-1.1.1.10:telnet-http iptables v1.4.19.1: SNAT: Bad value for "--to" option:
"1.1.1.1-1.1.1.10:telnet-ssh"
I think it should, for consistency (even if I have to confess that it looks a bit ugly to me).

Quoted 7 lines

Quoted 3 lines
If you decide to address this and send me a new version to support this, then it would be also good to update the manpage to say that we support services starting 1.4.20.
That is still ambiguous - there are service names with dash. So I suggest to support the notation '[name-with-dash]' in order to explicitly express and handle such cases.

Or perhaps as an alternative, we don't allow more than one port if one wishes to use service names? It seems the port parser is going to get so complicated it will lead to bugs. Particularly since ip6tables uses [ ] for addresses to disambiguate them from the :port section. Now we'd have to be able to handle multiple [] arguments.

So these would be acceptable:

    :22-23
    :ssh
    :wap-push (port 2948)

this would not:

:ssh-telnet

Phil