Add support of 'tablearg' feature for:

- 'tag' & 'untag' action parameters.
- 'tagged' & 'limit' rule options.
Rule examples:
	pipe 1 tag tablearg ip from table(1) to any
	allow ip from any to table(2) tagged tablearg
	allow tcp from table(3) to any 25 setup limit src-addr tablearg

sbin/ipfw/ipfw2.c:
1) new macros
   GET_UINT_ARG - support of 'tablearg' keyword, argument range checking.
   PRINT_UINT_ARG - support of 'tablearg' keyword.
2) strtoport(): do not silently truncate/accept invalid port list expressions
   like: '1,2-abc' or '1,2-3-4' or '1,2-3x4'. style(9) cleanup.

Approved by:	glebius (mentor)
MFC after:	1 month

sbin/ipfw/ipfw.8

@@ -1,7 +1,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd May 24, 2006
+.Dd June 15, 2006
 .Dt IPFW 8
 .Os
 .Sh NAME
@@ -1587,12 +1587,16 @@ Lookup tables currently support IPv4 addresses only.
 The
 .Cm tablearg
 feature provides the ability to use a value, looked up in the table, as
-the argument for a rule action.
+the argument for a rule action, action parameter or rule option.
 This can significantly reduce number of rules in some configurations.
 The
 .Cm tablearg
 argument can be used with the following actions:
-.Cm pipe , queue, divert, tee, netgraph, ngtee .
+.Cm pipe , queue, divert, tee, netgraph, ngtee,
+action parameters:
+.Cm tag, untag,
+rule options:
+.Cm limit, tagged.
 See the
 .Sx EXAMPLES
 Section for example usage of tables and the tablearg keyword.

sbin/ipfw/ipfw2.c

@@ -82,13 +82,42 @@ int
  */
 #define NEED1(msg)      {if (!ac) errx(EX_USAGE, msg);}
 
-#define NOT_NUMBER(str, msg) do {		\
+#define GET_UINT_ARG(arg, min, max, tok, s_x) do {			\
-	char *c;				\
+	if (!ac)							\
-	for (c = str; *c != '\0'; c++) {	\
+		errx(EX_USAGE, "%s: missing argument", match_value(s_x, tok)); \
-		if (isdigit(*c))		\
+	if (_substrcmp(*av, "tablearg") == 0) {				\
-			continue;		\
+		arg = IP_FW_TABLEARG;					\
-		errx(EX_DATAERR, msg);		\
+		break;							\
-	}					\
+	}								\
+									\
+	{								\
+	long val;							\
+	char *end;							\
+									\
+	val = strtol(*av, &end, 10);					\
+									\
+	if (!isdigit(**av) || *end != '\0' || (val == 0 && errno == EINVAL)) \
+		errx(EX_DATAERR, "%s: invalid argument: %s",		\
+		    match_value(s_x, tok), *av);			\
+									\
+	if (errno == ERANGE || val < min || val > max)			\
+		errx(EX_DATAERR, "%s: argument is out of range (%u..%u): %s", \
+		    match_value(s_x, tok), min, max, *av);		\
+									\
+	if (val == IP_FW_TABLEARG)					\
+		errx(EX_DATAERR, "%s: illegal argument value: %s",	\
+		    match_value(s_x, tok), *av);			\
+	arg = val;							\
+	}								\
+} while (0)
+
+#define PRINT_UINT_ARG(str, arg) do {					\
+	if (str != NULL)						\
+		printf("%s",str);					\
+	if (arg == IP_FW_TABLEARG)					\
+		printf("tablearg");					\
+	else								\
+		printf("%u", (uint32_t)arg);				\
 } while (0)
 
 /*
@@ -801,30 +830,39 @@ fill_newports(ipfw_insn_u16 *cmd, char *av, int proto)
 
 	while (*s) {
 		a = strtoport(av, &s, 0, proto);
-		if (s == av) /* no parameter */
+		if (s == av) 			/* empty or invalid argument */
-			break;
+			return (0);
-		if (*s == '-') { /* a range */
+
-			av = s+1;
+		switch (*s) {
+		case '-':			/* a range */
+			av = s + 1;
 			b = strtoport(av, &s, 0, proto);
-			if (s == av) /* no parameter */
+			/* Reject expressions like '1-abc' or '1-2-3'. */
-				break;
+			if (s == av || (*s != ',' && *s != '\0'))
+				return (0);
 			p[0] = a;
 			p[1] = b;
-		} else if (*s == ',' || *s == '\0' )
+			break;
+		case ',':			/* comma separated list */
+		case '\0':
 			p[0] = p[1] = a;
-		else 	/* invalid separator */
+			break;
-			errx(EX_DATAERR, "invalid separator <%c> in <%s>\n",
+		default:
+			warnx("port list: invalid separator <%c> in <%s>",
 				*s, av);
+			return (0);
+		}
+
 		i++;
 		p += 2;
-		av = s+1;
+		av = s + 1;
 	}
 	if (i > 0) {
-		if (i+1 > F_LEN_MASK)
+		if (i + 1 > F_LEN_MASK)
 			errx(EX_DATAERR, "too many ports/ranges\n");
-		cmd->o.len |= i+1; /* leave F_NOT and F_OR untouched */
+		cmd->o.len |= i + 1;	/* leave F_NOT and F_OR untouched */
 	}
-	return i;
+	return (i);
 }
 
 static struct _s_x icmpcodes[] = {
@@ -1475,28 +1513,33 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 				print_unreach6_code(cmd->arg1);
 			break;
 
-#define	PRINT_WITH_ARG(o)						\
+		case O_SKIPTO:
-			if (cmd->arg1 == IP_FW_TABLEARG)		\
+			PRINT_UINT_ARG("skipto ", cmd->arg1);
-				printf("%s tablearg", (o));		\
-			else						\
-				printf("%s %u", (o), cmd->arg1);	\
 			break;
 
-		case O_SKIPTO:
-			PRINT_WITH_ARG("skipto");
 		case O_PIPE:
-			PRINT_WITH_ARG("pipe");
+			PRINT_UINT_ARG("pipe ", cmd->arg1);
+			break;
+
 		case O_QUEUE:
-			PRINT_WITH_ARG("queue");
+			PRINT_UINT_ARG("queue ", cmd->arg1);
+			break;
+
 		case O_DIVERT:
-			PRINT_WITH_ARG("divert");
+			PRINT_UINT_ARG("divert ", cmd->arg1);
+			break;
+
 		case O_TEE:
-			PRINT_WITH_ARG("tee");
+			PRINT_UINT_ARG("tee ", cmd->arg1);
+			break;
+
 		case O_NETGRAPH:
-			PRINT_WITH_ARG("netgraph");
+			PRINT_UINT_ARG("netgraph ", cmd->arg1);
+			break;
+
 		case O_NGTEE:
-			PRINT_WITH_ARG("ngtee");
+			PRINT_UINT_ARG("ngtee ", cmd->arg1);
-#undef PRINT_WITH_ARG
+			break;
 
 		case O_FORWARD_IP:
 		    {
@@ -1542,9 +1585,9 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 	}
 	if (tagptr) {
 		if (tagptr->len & F_NOT)
-			printf(" untag %hu", tagptr->arg1);
+			PRINT_UINT_ARG(" untag ", tagptr->arg1);
 		else
-			printf(" tag %hu", tagptr->arg1);
+			PRINT_UINT_ARG(" tag ", tagptr->arg1);
 	}
 
 	/*
@@ -1898,8 +1941,7 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 				printf(" keep-state");
 				break;
 
-			case O_LIMIT:
+			case O_LIMIT: {
-			    {
 				struct _s_x *p = limit_masks;
 				ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
 				uint8_t x = c->limit_mask;
@@ -1912,9 +1954,9 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 						printf("%s%s", comma, p->s);
 						comma = ",";
 					}
-				printf(" %d", c->conn_limit);
+				PRINT_UINT_ARG(" ", c->conn_limit);
-			    }
 				break;
+			}
 
 			case O_IP6:
 				printf(" ip6");
@@ -1934,10 +1976,10 @@ show_ipfw(struct ip_fw *rule, int pcwidth, int bcwidth)
 
 			case O_TAGGED:
 				if (F_LEN(cmd) == 1)
-				    printf(" tagged %hu", cmd->arg1 );
+					PRINT_UINT_ARG(" tagged ", cmd->arg1);
 				else
-				    print_newports((ipfw_insn_u16 *)cmd, 0,
+					print_newports((ipfw_insn_u16 *)cmd, 0,
-					O_TAGGED);
+					    O_TAGGED);
 				break;
 
 			default:
@@ -4061,18 +4103,18 @@ add(int ac, char *av[])
 			break;
 
 		case TOK_TAG:
-		case TOK_UNTAG:
+		case TOK_UNTAG: {
-			{
+			uint16_t tag;
+
 			if (have_tag)
-				errx(EX_USAGE, "tag and untag cannot be specified more than once");
+				errx(EX_USAGE, "tag and untag cannot be "
-			NEED1("missing tag number");
+				    "specified more than once");
-			NOT_NUMBER(*av, "invalid tag number");
+			GET_UINT_ARG(tag, 1, 65534, i, rule_action_params);
 			have_tag = cmd;
-			fill_cmd(cmd, O_TAG, (i == TOK_TAG) ? 0: F_NOT,
+			fill_cmd(cmd, O_TAG, (i == TOK_TAG) ? 0: F_NOT, tag);
-			    strtoul(*av, NULL, 0));
 			ac--; av++;
-			}
 			break;
+		}
 
 		default:
 			abort();
@@ -4517,39 +4559,38 @@ add(int ac, char *av[])
 			fill_cmd(cmd, O_KEEP_STATE, 0, 0);
 			break;
 
-		case TOK_LIMIT:
+		case TOK_LIMIT: {
-			if (open_par)
-				errx(EX_USAGE, "limit cannot be part "
-				    "of an or block");
-			if (have_state)
-				errx(EX_USAGE, "only one of keep-state "
-					"and limit is allowed");
-			NEED1("limit needs mask and # of connections");
-			have_state = cmd;
-		    {
 			ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
+			int val;
+
+			if (open_par)
+				errx(EX_USAGE,
+				    "limit cannot be part of an or block");
+			if (have_state)
+				errx(EX_USAGE, "only one of keep-state and"
+				    "limit is allowed");
+			have_state = cmd;
 
 			cmd->len = F_INSN_SIZE(ipfw_insn_limit);
 			cmd->opcode = O_LIMIT;
-			c->limit_mask = 0;
+			c->limit_mask = c->conn_limit = 0;
-			c->conn_limit = 0;
-			for (; ac >1 ;) {
-				int val;
 
-				val = match_token(limit_masks, *av);
+			while (ac > 0) {
-				if (val <= 0)
+				if ((val = match_token(limit_masks, *av)) <= 0)
 					break;
 				c->limit_mask |= val;
 				ac--; av++;
 			}
-			c->conn_limit = atoi(*av);
+
-			if (c->conn_limit == 0)
-				errx(EX_USAGE, "limit: limit must be >0");
 			if (c->limit_mask == 0)
-				errx(EX_USAGE, "missing limit mask");
+				errx(EX_USAGE, "limit: missing limit mask");
+
+			GET_UINT_ARG(c->conn_limit, 1, 65534, TOK_LIMIT,
+			    rule_options);
+
 			ac--; av++;
-		    }
 			break;
+		}
 
 		case TOK_PROTO:
 			NEED1("missing protocol");
@@ -4664,14 +4705,17 @@ add(int ac, char *av[])
 			break;
 
 		case TOK_TAGGED:
-			NEED1("missing tag number");
+			if (ac > 0 && strpbrk(*av, "-,")) {
-			if (strpbrk(*av, "-,")) {
 				if (!add_ports(cmd, *av, 0, O_TAGGED))
-					errx(EX_DATAERR, "invalid tag %s", *av);
+					errx(EX_DATAERR, "tagged: invalid tag"
-			} else {
+					    " list: %s", *av);
-				NOT_NUMBER(*av, "invalid tag number");
+			}
-				fill_cmd(cmd, O_TAGGED, 0,
+			else {
-				    strtoul(*av, NULL, 0));
+				uint16_t tag;
+
+				GET_UINT_ARG(tag, 1, 65534, TOK_TAGGED,
+				    rule_options);
+				fill_cmd(cmd, O_TAGGED, 0, tag);
 			}
 			ac--; av++;
 			break;

sys/netinet/ip_fw2.c

@@ -1417,7 +1417,7 @@ lookup_dyn_parent(struct ipfw_flow_id *pkt, struct ip_fw *rule)
  */
 static int
 install_state(struct ip_fw *rule, ipfw_insn_limit *cmd,
-    struct ip_fw_args *args)
+    struct ip_fw_args *args, uint32_t tablearg)
 {
 	static int last_log;
 
@@ -1465,11 +1465,19 @@ install_state(struct ip_fw *rule, ipfw_insn_limit *cmd,
 	case O_LIMIT: {		/* limit number of sessions */
 		struct ipfw_flow_id id;
 		ipfw_dyn_rule *parent;
+		uint32_t conn_limit;
 		uint16_t limit_mask = cmd->limit_mask;
 
+		conn_limit = (cmd->conn_limit == IP_FW_TABLEARG) ?
+		    tablearg : cmd->conn_limit;
+		  
 		DEB(
-		printf("ipfw: %s: O_LIMIT rule, conn_limit: %u\n",
+		if (cmd->conn_limit == IP_FW_TABLEARG)
-		    __func__, cmd->conn_limit);
+			printf("ipfw: %s: O_LIMIT rule, conn_limit: %u "
+			    "(tablearg)\n", __func__, conn_limit);
+		else
+			printf("ipfw: %s: O_LIMIT rule, conn_limit: %u\n",
+			    __func__, conn_limit);
 		)
 
 		id.dst_ip = id.src_ip = id.dst_port = id.src_port = 0;
@@ -1497,10 +1505,10 @@ install_state(struct ip_fw *rule, ipfw_insn_limit *cmd,
 			return (1);
 		}
 
-		if (parent->count >= cmd->conn_limit) {
+		if (parent->count >= conn_limit) {
 			/* See if we can remove some expired rule. */
 			remove_dyn_rule(rule, parent);
-			if (parent->count >= cmd->conn_limit) {
+			if (parent->count >= conn_limit) {
 				if (fw_verbose && last_log != time_uptime) {
 					last_log = time_uptime;
 					log(LOG_SECURITY | LOG_DEBUG,
@@ -2895,10 +2903,13 @@ do {									\
 				match = is_ipv4;
 				break;
 
-			case O_TAG:
+			case O_TAG: {
+				uint32_t tag = (cmd->arg1 == IP_FW_TABLEARG) ?
+				    tablearg : cmd->arg1;
+
 				/* Packet is already tagged with this tag? */
-				mtag = m_tag_locate(m, MTAG_IPFW,
+				mtag = m_tag_locate(m, MTAG_IPFW, tag, NULL);
-						((ipfw_insn *) cmd)->arg1, NULL);
+
 				/* We have `untag' action when F_NOT flag is
 				 * present. And we must remove this mtag from
 				 * mbuf and reset `match' to zero (`match' will
@@ -2910,18 +2921,21 @@ do {									\
 					if (mtag != NULL)
 						m_tag_delete(m, mtag);
 				} else if (mtag == NULL) {
-					mtag = m_tag_alloc(MTAG_IPFW,
+					if ((mtag = m_tag_alloc(MTAG_IPFW,
-						((ipfw_insn *) cmd)->arg1, 0, M_NOWAIT);
+					    tag, 0, M_NOWAIT)) != NULL)
-					if (mtag != NULL)
 						m_tag_prepend(m, mtag);
 				}
 				match = (cmd->len & F_NOT) ? 0: 1;
 				break;
+			}
+
+			case O_TAGGED: {
+				uint32_t tag = (cmd->arg1 == IP_FW_TABLEARG) ?
+				    tablearg : cmd->arg1;
 
-			case O_TAGGED: 
 				if (cmdlen == 1) {
-					match = (m_tag_locate(m, MTAG_IPFW, 
+					match = m_tag_locate(m, MTAG_IPFW,
-						((ipfw_insn *) cmd)->arg1, NULL) != NULL);
+					    tag, NULL) != NULL;
 					break;
 				}
 
@@ -2943,6 +2957,7 @@ do {									\
 						    mtag->m_tag_id <= p[1];
 				}
 				break;
+			}
 				
 			/*
 			 * The second set of opcodes represents 'actions',
@@ -2988,7 +3003,7 @@ do {									\
 			case O_LIMIT:
 			case O_KEEP_STATE:
 				if (install_state(f,
-				    (ipfw_insn_limit *)cmd, args)) {
+				    (ipfw_insn_limit *)cmd, args, tablearg)) {
 					retval = IP_FW_DENY;
 					goto done; /* error/limit violation */
 				}