The following reply was made to PR kern/46159; it has been noted by GNATS. From: "Aaron Gifford" <[EMAIL PROTECTED]> To: [EMAIL PROTECTED] Cc: Subject: Re: kern/46159: [ipfw] [patch] ipfw dynamic rules lifetime feature Date: Sat, 26 Aug 2006 19:24:28 -0600
------=_Part_157133_22428623.1156641868079 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline Various PRs that have dealt with this issue: kern/46159 (still open) kern/28713 (closed, but should not have been in my opinion) kern/22065 (closed in favor of kern/28713) Hi, Here is the latest up-to-date version of the ipfw dynamic rule lifetime feature patch for 6.1-STABLE (STABLE as of 16 Aug. 2006) taken from my personal web site at http://www.adg.us/computers/ipfwpatch.html (currently down/unreachable due to gvinum issues, but should be back shortly) where patches for versions are available for FreeBSD from the 4.x and 5.x eras as well: I would very much like to see this feature become part of the mainstream distribution. Even with ipfw's built-in keepalive mechanisms, per-rule lifetime controls remain a powerful tool in my opinion, and as Oleg Koreshkov indicated in his PR submission, very likely in his opinion too. It grants firewall administrators much greater control over the flow network traffic. The per-rule lifetime features have been in use on many production machines that I manage for well over six years now. There have been zero stability issues. The patches to add these features are simple and straightforward. There have been no negative problems brought up at any time in the past when I have posted about the patches to public mailing lists, and no negative problems mentioned in any of the PR reports submitted. The only arguments against inclusion were that the features were not useful. I believe that was just the personal opinion of one of the maintainers at the time (and I respect his opinion and his FreeBSD contributions, but in this particular case respectfully disagree), but it doesn't account for the usefulness that I and others have found even after ipfw added built-in keepalive abilities. On various hosts I have used the lifetime features, when ipfw keepalives were added, I rewrote my rulesets to remove the use of per-rule lifetimes in hopes that I could stop maintaining the patchsets. Unfortunately, I discovered there were places in my own rulesets where keepalives could not fully replace the per-rule lifetime features, and so I've had to keep maintaining the patches to add these features to ipfw as I have done for over six years now. So please carefully consider adding per-rule lifetimes to ipfw rules. They're useful. I think a wider audience of ipfw network admins. will find them useful. Thank you. Aaron Gifford --- /usr/src/sys/netinet/ip_fw.h.orig Sat Jul 29 02:24:12 2006 +++ /usr/src/sys/netinet/ip_fw.h Wed Aug 16 14:52:55 2006 @@ -101,7 +101,7 @@ O_VERSRCREACH, /* none */ O_PROBE_STATE, /* none */ - O_KEEP_STATE, /* none */ + O_KEEP_STATE, /* u32 = optional lifetime */ O_LIMIT, /* ipfw_insn_limit */ O_LIMIT_PARENT, /* dyn_type, not an opcode. */ @@ -432,6 +432,7 @@ u_int64_t bcnt; /* byte match counter */ struct ipfw_flow_id id; /* (masked) flow id */ u_int32_t expire; /* expire time */ + u_int32_t lifetime; /* per-rule lifetime */ u_int32_t bucket; /* which bucket in hash table */ u_int32_t state; /* state of this rule (typically a * combination of TCP flags) --- /usr/src/sys/netinet/ip_fw2.c.orig Sat Jul 29 02:24:12 2006 +++ /usr/src/sys/netinet/ip_fw2.c Wed Aug 16 14:54:37 2006 @@ -1261,7 +1261,7 @@ } } } - q->expire = time_second + dyn_ack_lifetime; + q->expire = time_second + q->lifetime; break; case BOTH_SYN | BOTH_FIN: /* both sides closed */ @@ -1284,11 +1284,16 @@ q->expire = time_second + dyn_rst_lifetime; break; } - } else if (pkt->proto == IPPROTO_UDP) { - q->expire = time_second + dyn_udp_lifetime; } else { - /* other protocols */ - q->expire = time_second + dyn_short_lifetime; + /* + * UDP and other protocols: + * NOTE: The value of q->lifetime was set at the time this + * dynamic rule was created. It was either explicitly set + * by the ruleset creator to a specific value, or was pre- + * set to either dyn_udp_lifetime for UDP, or to + * dyn_short_lifetime for non-UDP protocols. + */ + q->expire = time_second + q->lifetime; } done: if (match_direction) @@ -1350,7 +1355,8 @@ * - "parent" rules for the above (O_LIMIT_PARENT). */ static ipfw_dyn_rule * -add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule) +add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule, + u_int32_t lifetime) { ipfw_dyn_rule *r; int i; @@ -1382,7 +1388,20 @@ } r->id = *id; - r->expire = time_second + dyn_syn_lifetime; + r->lifetime = lifetime; + if (r->id.proto == IPPROTO_TCP) { + r->lifetime = r->lifetime ? r->lifetime : dyn_ack_lifetime; + r->expire = time_second + dyn_syn_lifetime; + } else { + if (r->lifetime == 0) { + if (r->id.proto == IPPROTO_UDP) { + r->lifetime = dyn_udp_lifetime; + } else { + r->lifetime = dyn_short_lifetime; + } + } + r->expire = time_second + r->lifetime; + } r->rule = rule; r->dyn_type = dyn_type; r->pcnt = r->bcnt = 0; @@ -1437,7 +1456,7 @@ return q; } } - return add_dyn_rule(pkt, O_LIMIT_PARENT, rule); + return add_dyn_rule(pkt, O_LIMIT_PARENT, rule, 0); } /** @@ -1490,7 +1509,8 @@ switch (cmd->o.opcode) { case O_KEEP_STATE: /* bidir rule */ - add_dyn_rule(&args->f_id, O_KEEP_STATE, rule); + add_dyn_rule(&args->f_id, O_KEEP_STATE, rule, + ((ipfw_insn_u32 *)cmd)->d[0]); break; case O_LIMIT: { /* limit number of sessions */ @@ -1549,7 +1569,7 @@ return (1); } } - add_dyn_rule(&args->f_id, O_LIMIT, (struct ip_fw *)parent); + add_dyn_rule(&args->f_id, O_LIMIT, (struct ip_fw *)parent, 0); break; } default: @@ -3645,7 +3665,6 @@ DEB(printf("ipfw: opcode %d\n", cmd->opcode);) switch (cmd->opcode) { case O_PROBE_STATE: - case O_KEEP_STATE: case O_PROTO: case O_IP_SRC_ME: case O_IP_DST_ME: @@ -3677,6 +3696,7 @@ goto bad_size; break; + case O_KEEP_STATE: case O_UID: case O_GID: case O_JAIL: --- /usr/src/sbin/ipfw/ipfw2.c.orig Mon Aug 7 13:32:57 2006 +++ /usr/src/sbin/ipfw/ipfw2.c Wed Aug 16 14:52:55 2006 @@ -260,6 +260,7 @@ TOK_IN, TOK_LIMIT, TOK_KEEPSTATE, + TOK_LIFETIME, TOK_LAYER2, TOK_OUT, TOK_DIVERTED, @@ -394,6 +395,7 @@ { "in", TOK_IN }, { "limit", TOK_LIMIT }, { "keep-state", TOK_KEEPSTATE }, + { "lifetime", TOK_LIFETIME }, { "bridged", TOK_LAYER2 }, { "layer2", TOK_LAYER2 }, { "out", TOK_OUT }, @@ -1939,6 +1941,8 @@ case O_KEEP_STATE: printf(" keep-state"); + if (cmd32->d[0]) + printf(" lifetime %u", cmd32->d[0]); break; case O_LIMIT: { @@ -3857,6 +3861,9 @@ struct ip_fw *rule; + /* Temporary pointer to the most recent keep-state command: */ + ipfw_insn_u32 *cmd_keepstate = (ipfw_insn_u32 *)0; + /* * various flags used to record that we entered some fields. */ @@ -4558,7 +4565,20 @@ errx(EX_USAGE, "only one of keep-state " "and limit is allowed"); have_state = cmd; - fill_cmd(cmd, O_KEEP_STATE, 0, 0); + cmd->opcode = O_KEEP_STATE; + cmd->len = F_INSN_SIZE(ipfw_insn_u32); + cmd32->d[0] = 0; + cmd_keepstate = cmd32; + break; + + case TOK_LIFETIME: + if (cmd_keepstate == (ipfw_insn_u32 *)0) + errx(EX_USAGE, "lifetime must immediately " + "follow keep-state"); + NEED1("lifetime requires # of seconds"); + cmd_keepstate->d[0] = strtoul(*av, NULL, 0); + cmd_keepstate = (ipfw_insn_u32 *)0; + ac--; av++; break; case TOK_LIMIT: { --- /usr/src/sbin/ipfw/ipfw.8.orig Sat Jul 29 02:24:12 2006 +++ /usr/src/sbin/ipfw/ipfw.8 Wed Aug 16 14:52:55 2006 @@ -1282,14 +1282,35 @@ .It Cm ipversion Ar ver Matches IP packets whose IP version field is .Ar ver . -.It Cm keep-state +.It Xo Cm keep-state +.Op Cm lifetime Ar number +.Xc 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. -The rule has a limited lifetime (controlled by a set of +The rule has a limited lifetime controlled by a set of .Xr sysctl 8 -variables), and the lifetime is refreshed every time a matching +variables, and the lifetime is refreshed every time a matching packet is found. +.Pp +The default limited rule lifetime behavior may be modified +for a specific rule by appending +.Cm lifetime Ar number +immediately after +.Cm keep-state . +Doing so will explicitly set the dynamic rule lifetime to the +specified number of seconds, overriding the default lifetime +behavior for the specified rule. +.Pp +For TCP rules, explicitly setting a rule lifetime overrides the +default setting stored in the +.Xr sysctl 8 +variable +.Em net.inet.ip.fw.dyn_ack_lifetime . +For UDP rules, it overrides +.Em net.inet.ip.fw.dyn_udp_lifetime . +For all other rules, it overrides +.Em net.inet.ip.fw.dyn_short_lifetime . .It Cm layer2 Matches only layer2 packets, i.e., those passed to .Nm ------=_Part_157133_22428623.1156641868079 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline Various PRs that have dealt with this issue:<br> kern/46159 (still open)<br> kern/28713 (closed, but should not have been in my opinion)<br> kern/22065 (closed in favor of kern/28713)<br><br><br>Hi,<br><br>Here is the latest up-to-date version of the ipfw dynamic rule lifetime feature patch for 6.1-STABLE (STABLE as of 16 Aug. 2006) taken from my personal web site at <a href="http://www.adg.us/computers/ipfwpatch.html";>http://www.adg.us/computers/ipfwpatch.html</a> (currently down/unreachable due to gvinum issues, but should be back shortly) where patches for versions are available for FreeBSD from the 4.x and 5.x eras as well:<br><br>I would very much like to see this feature become part of the mainstream distribution. Even with ipfw's built-in keepalive mechanisms, per-rule lifetime controls remain a powerful tool in my opinion, and as Oleg Koreshkov indicated in his PR submission, very likely in his opinion too. It grants firewall administrators much greater control over the flow network traffic.<br><br>The per-rule lifetime features have been in use on many production machines that I manage for well over six years now. There have been zero stability issues. The patches to add these features are simple and straightforward. <br><br>There have been no negative problems brought up at any time in the past when I have posted about the patches to public mailing lists, and no negative problems mentioned in any of the PR reports submitted. The only arguments against inclusion were that the features were not useful. I believe that was just the personal opinion of one of the maintainers at the time (and I respect his opinion and his FreeBSD contributions, but in this particular case respectfully disagree), but it doesn't a ccount for the usefulness that I and others have found even after ipfw added built-in keepalive abilities. <br><br>On various hosts I have used the lifetime features, when ipfw keepalives were added, I rewrote my rulesets to remove the use of per-rule lifetimes in hopes that I could stop maintaining the patchsets. Unfortunately, I discovered there were places in my own rulesets where keepalives could not fully replace the per-rule lifetime features, and so I've had to keep maintaining the patches to add these features to ipfw as I have done for over six years now. <br><br>So please carefully consider adding per-rule lifetimes to ipfw rules. They're useful. I think a wider audience of ipfw network admins. will find them useful.<br><br>Thank you.<br>Aaron Gifford<br><br>--- /usr/src/sys/netinet/ip_fw.h.orig Sat Jul 29 02:24:12 2006 <br>+++ /usr/src/sys/netinet/ip_fw.h Wed Aug 16 14:52:55 2006<br>@@ -101,7 +101,7 @@<br> O_VERSRCREACH, /* none */<br><br> O_PROBE_STATE, &n bsp; /* none */ <br>- O_KEEP_STATE, /* none */<br>+ O_KEEP_STATE, /* u32 = optional lifetime */<br> O_LIMIT,   ; /* ipfw_insn_limit */<br> O_LIMIT_PARENT, /* dyn_type, not an opcode. */ <br><br>@@ -432,6 +432,7 @@<br> u_int64_t bcnt; /* byte match counter */<br> struct ipfw_flow_id id; /* (masked) flow id */<br> u_int32_t expire; /* expire time */ <br>+ u_int32_t lifetime; /* per-rule lifetime */<br> u_int32_t bucket; /* which bucket in hash table */<br> u_int32_t st ate; /* state of this rule (typically a <br> * combination of TCP flags)<br>--- /usr/src/sys/netinet/ip_fw2.c.orig Sat Jul 29 02:24:12 2006<br>+++ /usr/src/sys/netinet/ip_fw2.c Wed Aug 16 14:54:37 2006<br>@@ -1261,7 +1261,7 @@ <br> }<br> }<br> }<br>-&n bsp; q->expire = time_second + dyn_ack_lifetime;<br>+ q->expire = time_second + q->lifetime; <br> break;<br><br> case BOTH_SYN | BOTH_FIN: /* both sides closed */<br>@@ -1284,11 +1284,16 @@<br> q->ex pire = time_second + dyn_rst_lifetime;<br> break; <br> }<br>- } else if (pkt->proto == IPPROTO_UDP) {<br>- q->expire = time_second + dyn_udp_lifetime;<br> } else {<br>- /* other protocols */<br>- q->expire = time_second + dyn_short_lifetime; <br>+ /*<br>+ * UDP and other protocols:<br>+ * NOTE: The value of q->lifetime was set at the time this<br>+ * dynamic rule was cr eated. It was either explicitly set <br>+ * by the ruleset creator to a specific value, or was pre-<br>+ * set to either dyn_udp_lifetime for UDP, or to<br>+ * dyn_short_lifetime for non-UDP protocols.<br>+   ; */ <br>+ q->expire = time_second + q->lifetime;<br> }<br> done:<br> if (match_direction)<br>@@ -1350,7 +1355,8 @@<br> * - "parent" rules for the above (O_LIMIT_PARENT).<br> */<br> static ipfw_dyn_rule *<br>-add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule)<br>+add_dyn_rule(struct ipfw_flow_id *id, u_int8_t dyn_type, struct ip_fw *rule,<br>+ u_int32_t lifetime) <br> {<br> ipfw_dyn_rule *r;<br> int i;<br>@@ -1382,7 +1388,20 @@<br> }<br><br> r->id = *id;<br>- r->expire = time_second + dyn_syn_lifetime;<br>+ r->lifetime = lifetime;<br> + if (r->id.proto == IPPROTO_TCP) {<br>+ r->lifetime = r->lifetime ? r->lifetime : dyn_ack_lifetime;<br>+ r->expire = time_second + dyn_syn_lifetime;<br>+ } else {<br> + if (r->lifetime == 0) {<br>+ if (r->id.proto == IPPROTO_UDP) {<br>+ r->lifetime = dyn_udp_lifetime;<br>+ } else {<br>+ r->lifetime = dyn_short_lifetime; <br>+ }<br>+ }<br>+ r->expire = time_second + r->lifetime;<br>+ }<br> r->rule = rule;<br>   ; r->dyn_type = dyn_type;<br> r->pcnt = r->bcnt = 0; <br>@@ -1437,7 +1456,7 @@<br> return q;<br> }<br> }<br>- return add_dyn_rule(pkt, O_LIMIT_PARENT, rule);<br>+ return add_dyn_rule(pkt, O_LIMIT_PARENT, rule, 0); <br> }<br><br> /**<br>@@ -1490,7 +1509,8 @@<br><br> switch (cmd->o.opcode) {<br> case O_KEEP_STATE: /* bidir rule */<br>- add_dyn_rule(&args->f_id, O_KEEP_STATE, rule);<br>+ add_dyn_rule(&args->f_id, O_ KEEP_STATE, rule, <br>+ ((ipfw_insn_u32 *)cmd)->d[0]);<br> break;<br><br> case O_LIMIT: { /* limit number of sessions */<br>@@ -1549,7 +1569,7 @@<br>   ; return (1); <br> }<br> }<br>- add_dyn_rule(&args->f_id, O_LIMIT, (struct ip_fw *)parent);<br>+ add_dyn_rule(&args-&g t;f_id, O_LIMIT, (struct ip_fw *)parent, 0); <br> break;<br> }<br> default:<br>@@ -3645,7 +3665,6 @@<br> DEB(printf("ipfw: opcode %d\n", cmd->opcode);)<br> switch (cmd->opcode) {<br>&nbs p; case O_PROBE_STATE: <br>- case O_KEEP_STATE:<br> case O_PROTO:<br> case O_IP_SRC_ME:<br> case O_IP_DST_ME:<br>@@ -3677,6 +3696,7 @@<br> goto bad_size;<br> break;<br><br>+ case O_KEEP_STATE:<br> case O_UID:<br> case O_GID:<br> &n bsp; case O_JAIL:<br>--- /usr/src/sbin/ipfw/ipfw2.c.orig Mon Aug 7 13:32:57 2006 <br>+++ /usr/src/sbin/ipfw/ipfw2.c Wed Aug 16 14:52:55 2006<br>@@ -260,6 +260,7 @@<br> TOK_IN,<br> TOK_LIMIT,<br> TOK_KEEPSTATE,<br>+ TOK_LIFETIME,<br> TOK_LAYER2,<br> TOK_OUT,<br> TOK_DIVERTED,<br>@@ -394,6 +395,7 @@<br> { "in", TOK_IN },<br> { "limit", TOK_LIMIT },<br> { "keep-state", &n bsp; TOK_KEEPSTATE },<br> + { "lifetime", TOK_LIFETIME },<br> { "bridged", TOK_LAYER2 },<br> { "layer2", TOK_LAYER2 },<br> { "out", TOK_OUT }, <br>@@ -1939,6 +1941,8 @@<br><br> case O_KEEP_STATE:<br> printf(" keep-state");<br>+ &nb sp; if (cmd32->d[0])<br>+ printf(" lifetime %u", cmd32->d[0]); <br> break;<br><br> case O_LIMIT: {<br>@@ -3857,6 +3861,9 @@<br><br> struct ip_fw *rule;<br><br>+ /* Temporar y pointer to the most recent keep-state command: */ <br>+ ipfw_insn_u32 *cmd_keepstate = (ipfw_insn_u32 *)0;<br>+<br> /*<br> * various flags used to record that we entered some fields.<br> */<br>@@ -4558,7 +4565,20 @@<br> errx(EX_USAGE, "only one of keep-state " <br> "and limit is allowed");<br> have_state = cmd;<br>- & nbsp; fill_cmd(cmd, O_KEEP_STATE, 0, 0);<br>+ cmd->opcode = O_KEEP_STATE; <br>+ cmd->len = F_INSN_SIZE(ipfw_insn_u32);<br>+ cmd32->d[0] = 0;<br>+ cmd_keepstate = cmd32;<br>+ &nbs p; break;<br>+<br>+ case TOK_LIFETIME: <br>+ if (cmd_keepstate == (ipfw_insn_u32 *)0)<br>+ errx(EX_USAGE, "lifetime must immediately "<br>+   ; "follow keep-state"); <br>+ NEED1("lifetime requires # of seconds");<br>+ cmd_keepstate->d[0] = strtoul(*av, NULL, 0);<br>+ cmd_keepstate = (ipfw_insn_u32 *)0;<br>+ ac--; av++; <br> break;<br><br> case TOK_LIMIT: {<br>--- /usr/src/sbin/ipfw/ipfw.8.orig Sat Jul 29 02:24:12 2006<br>+++ /usr/src/sbin/ipfw/ipfw.8 Wed Aug 16 14:52:55 2006<br>@@ -1282,14 +1282,35 @@ <br> .It Cm ipversion Ar ver<br> Matches IP packets whose IP version field is<br> .Ar ver .<br>-.It Cm keep-state<br>+.It Xo Cm keep-state<br>+.Op Cm lifetime Ar number<br>+.Xc<br> Upon a match, the firewall will create a dynamic rule, whose <br> default behaviour is to match bidirectional traffic between<br> source and destination IP/port using the same protocol.<br>-The rule has a limited lifetime (controlled by a set of<br>+The rule has a limited lifetime controlled by a set of <br> .Xr sysctl 8<br>-variables), and the lifetime is refreshed every time a matching<br>+variables, and the lifetime is refreshed every time a matching<br> packet is found.<br>+.Pp<br>+The default limited rule lifetime behavior may be modified <br>+for a specific rule by appending<br>+.Cm lifetime Ar number<br>+immediately after<br>+.Cm keep-state .<br>+Doing so will explicitly set the dynamic rule lifetime to the<br>+specified number of seconds, overriding the default lifetime <br>+behavior for the specified rule.<br>+.Pp<br>+For TCP rules, explicitly setting a rule lifetime overrides the<br>+default setting stored in the<br>+.Xr sysctl 8<br>+variable<br>+.Em net.inet.ip.fw.dyn_ack_lifetime .<br> +For UDP rules, it overrides<br>+.Em net.inet.ip.fw.dyn_udp_lifetime .<br>+For all other rules, it overrides<br>+.Em net.inet.ip.fw.dyn_short_lifetime .<br> .It Cm layer2<br> Matches only layer2 packets, i.e., those passed to <br> .Nm<br><br> ------=_Part_157133_22428623.1156641868079-- _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw To unsubscribe, send any mail to "[EMAIL PROTECTED]"
