Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 15 Mar 2017 13:36:35 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r315305 - in head: . sbin/ipfw
Message-ID:  <201703151336.v2FDaZS7057372@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ae
Date: Wed Mar 15 13:36:35 2017
New Revision: 315305
URL: https://svnweb.freebsd.org/changeset/base/315305

Log:
  Change the syntax of ipfw's named states.
  
  Since the state name is an optional argument, it often can conflict
  with other options. To avoid ambiguity now the state name must be
  prefixed with a colon.
  
  Obtained from:	Yandex LLC
  MFC after:	2 week
  Sponsored by:	Yandex LLC

Modified:
  head/UPDATING
  head/sbin/ipfw/ipfw.8
  head/sbin/ipfw/ipfw2.c

Modified: head/UPDATING
==============================================================================
--- head/UPDATING	Wed Mar 15 13:34:51 2017	(r315304)
+++ head/UPDATING	Wed Mar 15 13:36:35 2017	(r315305)
@@ -51,6 +51,12 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 12
 
 ****************************** SPECIAL WARNING: ******************************
 
+20170315:
+	The syntax of ipfw(8) named states was changed to avoid ambiguity.
+	If you have used named states in the firewall rules, you need to modify
+	them after installworld and before rebooting. Now named states must
+	be prefixed with colon.
+
 20170311:
 	The old drm (sys/dev/drm/) drivers for i915 and radeon have been
 	removed as the userland we provide cannot use them. The KMS version

Modified: head/sbin/ipfw/ipfw.8
==============================================================================
--- head/sbin/ipfw/ipfw.8	Wed Mar 15 13:34:51 2017	(r315304)
+++ head/sbin/ipfw/ipfw.8	Wed Mar 15 13:36:35 2017	(r315305)
@@ -1,7 +1,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd October 18, 2016
+.Dd March 15, 2017
 .Dt IPFW 8
 .Os
 .Sh NAME
@@ -781,7 +781,7 @@ will be executed when the packet matches
 .It Cm allow | accept | pass | permit
 Allow packets that match rule.
 The search terminates.
-.It Cm check-state Op Ar flowname | Cm any
+.It Cm check-state Op Ar :flowname | Cm :any
 Checks the packet against the dynamic ruleset.
 If a match is found, execute the action associated with
 the rule which generated this dynamic rule, otherwise
@@ -797,15 +797,15 @@ or
 .Cm limit
 rule.
 The
-.Ar flowname
+.Ar :flowname
 is symbolic name assigned to dynamic rule by
 .Cm keep-state
 opcode.
 The special flowname
-.Cm any
+.Cm :any
 can be used to ignore states flowname when matching.
 The
-.Cm default
+.Cm :default
 keyword is special name used for compatibility with old rulesets.
 .It Cm count
 Update counters for all packets that match rule.
@@ -1662,7 +1662,7 @@ specified in the same way as
 .It Cm ipversion Ar ver
 Matches IP packets whose IP version field is
 .Ar ver .
-.It Cm keep-state Op Ar flowname
+.It Cm keep-state Op Ar :flowname
 Upon a match, the firewall will create a dynamic rule, whose
 default behaviour is to match bidirectional traffic between
 source and destination IP/port using the same protocol.
@@ -1671,19 +1671,19 @@ The rule has a limited lifetime (control
 variables), and the lifetime is refreshed every time a matching
 packet is found.
 The
-.Ar flowname
+.Ar :flowname
 is used to assign additional to addresses, ports and protocol parameter
 to dynamic rule. It can be used for more accurate matching by
 .Cm check-state
 rule.
 The
-.Cm default
+.Cm :default
 keyword is special name used for compatibility with old rulesets.
 .It Cm layer2
 Matches only layer2 packets, i.e., those passed to
 .Nm
 from ether_demux() and ether_output_frame().
-.It Cm limit Bro Cm src-addr | src-port | dst-addr | dst-port Brc Ar N Op Ar flowname
+.It Cm limit Bro Cm src-addr | src-port | dst-addr | dst-port Brc Ar N Op Ar :flowname
 The firewall will only allow
 .Ar N
 connections with the same
@@ -2286,7 +2286,7 @@ are completely equivalent afterwards).
 Rules created by
 .Cm keep-state
 option also have a
-.Ar flowname
+.Ar :flowname
 taken from it.
 This name is used in matching together with addresses, ports and protocol.
 Dynamic rules will be checked at the first
@@ -2297,23 +2297,23 @@ occurrence, and the action performed upo
 as in the parent rule.
 .Pp
 Note that no additional attributes other than protocol and IP addresses
-and ports and flowname are checked on dynamic rules.
+and ports and :flowname are checked on dynamic rules.
 .Pp
 The typical use of dynamic rules is to keep a closed firewall configuration,
 but let the first TCP SYN packet from the inside network install a
 dynamic rule for the flow so that packets belonging to that session
 will be allowed through the firewall:
 .Pp
-.Dl "ipfw add check-state OUTBOUND"
-.Dl "ipfw add allow tcp from my-subnet to any setup keep-state OUTBOUND"
+.Dl "ipfw add check-state :OUTBOUND"
+.Dl "ipfw add allow tcp from my-subnet to any setup keep-state :OUTBOUND"
 .Dl "ipfw add deny tcp from any to any"
 .Pp
 A similar approach can be used for UDP, where an UDP packet coming
 from the inside will install a dynamic rule to let the response through
 the firewall:
 .Pp
-.Dl "ipfw add check-state OUTBOUND"
-.Dl "ipfw add allow udp from my-subnet to any keep-state OUTBOUND"
+.Dl "ipfw add check-state :OUTBOUND"
+.Dl "ipfw add allow udp from my-subnet to any keep-state :OUTBOUND"
 .Dl "ipfw add deny udp from any to any"
 .Pp
 Dynamic rules expire after some time, which depends on the status

Modified: head/sbin/ipfw/ipfw2.c
==============================================================================
--- head/sbin/ipfw/ipfw2.c	Wed Mar 15 13:34:51 2017	(r315304)
+++ head/sbin/ipfw/ipfw2.c	Wed Mar 15 13:36:35 2017	(r315305)
@@ -1483,7 +1483,7 @@ show_static_rule(struct cmdline_opts *co
 				    cmd->arg1, IPFW_TLV_STATE_NAME);
 			else
 				ename = NULL;
-			bprintf(bp, " %s", ename ? ename: "any");
+			bprintf(bp, " :%s", ename ? ename: "any");
 			/* avoid printing anything else */
 			flags = HAVE_PROTO | HAVE_SRCIP |
 				HAVE_DSTIP | HAVE_IP;
@@ -2076,7 +2076,7 @@ show_static_rule(struct cmdline_opts *co
 
 			case O_KEEP_STATE:
 				bprintf(bp, " keep-state");
-				bprintf(bp, " %s",
+				bprintf(bp, " :%s",
 				    object_search_ctlv(fo->tstate, cmd->arg1,
 				    IPFW_TLV_STATE_NAME));
 				break;
@@ -2095,7 +2095,7 @@ show_static_rule(struct cmdline_opts *co
 						comma = ",";
 					}
 				bprint_uint_arg(bp, " ", c->conn_limit);
-				bprintf(bp, " %s",
+				bprintf(bp, " :%s",
 				    object_search_ctlv(fo->tstate, cmd->arg1,
 				    IPFW_TLV_STATE_NAME));
 				break;
@@ -2198,7 +2198,7 @@ show_dyn_state(struct cmdline_opts *co, 
 	} else
 		bprintf(bp, " UNKNOWN <-> UNKNOWN");
 	if (d->kidx != 0)
-		bprintf(bp, " %s", object_search_ctlv(fo->tstate,
+		bprintf(bp, " :%s", object_search_ctlv(fo->tstate,
 		    d->kidx, IPFW_TLV_STATE_NAME));
 }
 
@@ -3714,27 +3714,25 @@ compile_rule(char *av[], uint32_t *rbuf,
 	case TOK_CHECKSTATE:
 		have_state = action;
 		action->opcode = O_CHECK_STATE;
-		if (*av == NULL) {
+		if (*av == NULL ||
+		    match_token(rule_options, *av) == TOK_COMMENT) {
 			action->arg1 = pack_object(tstate,
 			    default_state_name, IPFW_TLV_STATE_NAME);
 			break;
 		}
-		if (strcmp(*av, "any") == 0)
-			action->arg1 = 0;
-		else if ((i = match_token(rule_options, *av)) != -1) {
-			action->arg1 = pack_object(tstate,
-			    default_state_name, IPFW_TLV_STATE_NAME);
-			if (i != TOK_COMMENT)
-				warn("Ambiguous state name '%s', '%s'"
-				    " used instead.\n", *av,
-				    default_state_name);
-			break;
-		} else if (state_check_name(*av) == 0)
-			action->arg1 = pack_object(tstate, *av,
-			    IPFW_TLV_STATE_NAME);
-		else
-			errx(EX_DATAERR, "Invalid state name %s", *av);
-		av++;
+		if (*av[0] == ':') {
+			if (strcmp(*av + 1, "any") == 0)
+				action->arg1 = 0;
+			else if (state_check_name(*av + 1) == 0)
+				action->arg1 = pack_object(tstate, *av + 1,
+				    IPFW_TLV_STATE_NAME);
+			else
+				errx(EX_DATAERR, "Invalid state name %s",
+				    *av);
+			av++;
+			break;
+		}
+		errx(EX_DATAERR, "Invalid state name %s", *av);
 		break;
 
 	case TOK_ACCEPT:
@@ -4577,22 +4575,16 @@ read_options:
 			if (have_state)
 				errx(EX_USAGE, "only one of keep-state "
 					"and limit is allowed");
-			if (*av == NULL ||
-			    (i = match_token(rule_options, *av)) != -1) {
-				if (*av != NULL && i != TOK_COMMENT)
-					warn("Ambiguous state name '%s',"
-					    " '%s' used instead.\n", *av,
-					    default_state_name);
-				uidx = pack_object(tstate, default_state_name,
-				    IPFW_TLV_STATE_NAME);
-			} else {
-				if (state_check_name(*av) != 0)
+			if (*av != NULL && *av[0] == ':') {
+				if (state_check_name(*av + 1) != 0)
 					errx(EX_DATAERR,
 					    "Invalid state name %s", *av);
-				uidx = pack_object(tstate, *av,
+				uidx = pack_object(tstate, *av + 1,
 				    IPFW_TLV_STATE_NAME);
 				av++;
-			}
+			} else
+				uidx = pack_object(tstate, default_state_name,
+				    IPFW_TLV_STATE_NAME);
 			have_state = cmd;
 			fill_cmd(cmd, O_KEEP_STATE, 0, uidx);
 			break;
@@ -4629,22 +4621,16 @@ read_options:
 			    TOK_LIMIT, rule_options);
 			av++;
 
-			if (*av == NULL ||
-			    (i = match_token(rule_options, *av)) != -1) {
-				if (*av != NULL && i != TOK_COMMENT)
-					warn("Ambiguous state name '%s',"
-					    " '%s' used instead.\n", *av,
-					    default_state_name);
-				cmd->arg1 = pack_object(tstate,
-				    default_state_name, IPFW_TLV_STATE_NAME);
-			} else {
-				if (state_check_name(*av) != 0)
+			if (*av != NULL && *av[0] == ':') {
+				if (state_check_name(*av + 1) != 0)
 					errx(EX_DATAERR,
 					    "Invalid state name %s", *av);
-				cmd->arg1 = pack_object(tstate, *av,
+				cmd->arg1 = pack_object(tstate, *av + 1,
 				    IPFW_TLV_STATE_NAME);
 				av++;
-			}
+			} else
+				cmd->arg1 = pack_object(tstate,
+				    default_state_name, IPFW_TLV_STATE_NAME);
 			break;
 		}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703151336.v2FDaZS7057372>