Two bugfixes:

  + the header file contains two different opcodes (O_IPOPTS and O_IPOPT)
    for what is the same thing, and sure enough i used one in the kernel
    and the other one in userland. Be consistent!

  + "keep-state" and "limit" must be the last match pattern in a rule,
    so no matter how you enter them move them to the end of the rule.

1 files changed, 26 insertions, 15 deletions

diff --git a/sbin/ipfw/ipfw2.c b/sbin/ipfw/ipfw2.c
index 71e3358..e626353 100644
--- a/sbin/ipfw/ipfw2.c
+++ b/sbin/ipfw/ipfw2.c
@@ -996,7 +996,7 @@ show_ipfw(struct ip_fw *rule)
 				printf(" iplen %u", cmd->arg1 );
 				break;
 
-			case O_IPOPTS:
+			case O_IPOPT:
 				print_flags("ipoptions", cmd, f_ipopts);
 				break;
 
@@ -2170,7 +2170,7 @@ add(int ac, char *av[])
 	 * various flags used to record that we entered some fields.
 	 */
 	int have_mac = 0;	/* set if we have a MAC address */
-	int have_state = 0;	/* check-state or keep-state */
+	ipfw_insn *have_state = NULL;	/* check-state or keep-state */
 
 	int i;
 
@@ -2219,7 +2219,7 @@ add(int ac, char *av[])
 	action->len = 1;	/* default */
 	switch(i) {
 	case TOK_CHECKSTATE:
-		have_state = 1;
+		have_state = action;
 		action->opcode = O_CHECK_STATE;
 		break;
 
@@ -2344,10 +2344,8 @@ add(int ac, char *av[])
 		cmd = next_cmd(cmd);
 	}
 
-	if (have_state) {
-		have_state = 0;
+	if (have_state)	/* must be a check-state, we are done */
 		goto done;
-	}
 
 #define OR_START(target)					\
 	if (ac && (*av[0] == '(' || *av[0] == '{')) {		\
@@ -2610,13 +2608,13 @@ read_options:
 
 		case TOK_IPOPTS:
 			NEED1("missing argument for ipoptions");
-			fill_flags(cmd, O_IPOPTS, f_ipopts, *av);
+			fill_flags(cmd, O_IPOPT, f_ipopts, *av);
 			ac--; av++;
 			break;
 
 		case TOK_IPTOS:
 			NEED1("missing argument for iptos");
-			fill_flags(cmd, O_IPOPTS, f_iptos, *av);
+			fill_flags(cmd, O_IPTOS, f_iptos, *av);
 			ac--; av++;
 			break;
 
@@ -2697,17 +2695,18 @@ read_options:
 
 		case TOK_KEEPSTATE:
 			if (have_state)
-				errx(EX_USAGE, "only one of check-state "
+				errx(EX_USAGE, "only one of keep-state "
 					"and limit is allowed");
-			have_state = 1;
+			have_state = cmd;
 			fill_cmd(cmd, O_KEEP_STATE, 0, 0);
 			break;
 
 		case TOK_LIMIT:
 			NEED1("limit needs mask and # of connections");
 			if (have_state)
-				errx(EX_USAGE, "only one of check-state "
+				errx(EX_USAGE, "only one of keep-state "
 					"and limit is allowed");
+			have_state = cmd;
 		    {
 			ipfw_insn_limit *c = (ipfw_insn_limit *)cmd;
 
@@ -2730,7 +2729,6 @@ read_options:
 			if (c->limit_mask == 0)
 				errx(EX_USAGE, "missing limit mask");
 			ac--; av++;
-			have_state = 1;
 		    }
 			break;
 
@@ -2756,23 +2754,36 @@ done:
 	/*
 	 * generate O_PROBE_STATE if necessary
 	 */
-	if (have_state) {
+	if (have_state && have_state->opcode != O_CHECK_STATE) {
 		fill_cmd(dst, O_PROBE_STATE, 0, 0);
 		dst = next_cmd(dst);
 	}
 	/*
-	 * copy all commands but O_LOG
+	 * copy all commands but O_LOG, O_KEEP_STATE, O_LIMIT
 	 */
 	for (src = (ipfw_insn *)cmdbuf; src != cmd; src += i) {
 		i = F_LEN(src);
 
-		if (src->opcode != O_LOG) {
+		switch (src->opcode) {
+		case O_LOG:
+		case O_KEEP_STATE:
+		case O_LIMIT:
+			break;
+		default:
 			bcopy(src, dst, i * sizeof(u_int32_t));
 			dst += i;
 		}
 	}
 
 	/*
+	 * put back the have_state command as last opcode
+	 */
+	if (have_state) {
+		i = F_LEN(have_state);
+		bcopy(have_state, dst, i * sizeof(u_int32_t));
+		dst += i;
+	}
+	/*
 	 * start action section
 	 */
 	rule->act_ofs = dst - rule->cmd;