On Tue, 17 Sep 2013, Ivan Popovski wrote:

>Hi
>
>I've been asked, by net admin, to implement pf.conf simplification for
>divert-to rule. Reason is that divert-to is written to support only one
>port per line and because of that there are situations where admins
>must write lot of lines only because different ports. After looking at
>pfctl/parse.y I've found that patch (for 5.3) would be trivial and
>wouldn't break anything, ie. works for one port and port range at the
>same time.
>
>Please let me know if there is interest for this and ofc if something
>needs to be fixed.
>
>Here is an example.
>
>Now:
>
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42240 modulate state probability 20%
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42241 modulate state probability 20%
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42242 modulate state probability 20%
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42243 modulate state probability 20%
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42244 modulate state

This appears to be intended to divide connections equally among five
ports, but (given that the probability applies only to the packets which
actually reach the rule) doesn't it actually divide them as 20%, 16%,
12.8%, 10.24%, 40.96%?  To get an (approximately) equal distribution I
think you'd need to use probabilities 20%, 25%, 33%, 50%.

If using a port range were to implicitly divide connections equally
among those ports, this problem would go away.  But that's not what your
patch does.

        Dave

>After patching:
>
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42240:42243 modulate state probability 20%
>pass in quick inet proto tcp from 192.168.1.0/24 to any port 21 divert-to 
>127.0.0.1 port 42244 modulate state
>
>Patch:
>
>Index: parse.y
>===================================================================
>RCS file: /cvs/src/sbin/pfctl/parse.y,v
>retrieving revision 1.621
>diff -u -r1.621 parse.y
>--- parse.y    16 Jan 2013 01:49:20 -0000      1.621
>+++ parse.y    17 Sep 2013 15:45:20 -0000
>@@ -261,7 +261,7 @@
>       u_int8_t                 set_prio[2];
>       struct {
>               struct node_host        *addr;
>-              u_int16_t               port;
>+              u_int16_t               port, port_top;
>       }                        divert, divert_packet;
>       struct redirspec         nat;
>       struct redirspec         rdr;
>@@ -475,7 +475,7 @@
> %type <v.i>                   sourcetrack flush unaryop statelock
> %type <v.b>                   action
> %type <v.b>                   flags flag blockspec prio
>-%type <v.range>               portplain portstar portrange
>+%type <v.range>               portstar portrange
> %type <v.hashkey>             hashkey
> %type <v.proto>               proto proto_list proto_item
> %type <v.number>              protoval
>@@ -2078,6 +2078,28 @@
>                                       r.divert.addr =
>                                           $8.divert.addr->addr.v.a.addr;
>                               }
>+                              if ($8.divert.port_top &&
>+                                  $8.divert.port_top < r.divert.port) {
>+                                      yyerror("invalid divert port range: "
>+                                          "%u:%u", ntohs(r.divert.port),
>+                                          ntohs($8.divert.port_top));
>+                                      YYERROR;
>+                              }
>+
>+#define NHS_LT(x, y) (ntohs(x) < ntohs(y))
>+#define NHS_INC(x) x = htons(ntohs(x) + 1)
>+                              while(NHS_LT(r.divert.port,
>+                                  $8.divert.port_top)) {
>+                                      expand_rule(&r, 1, $4, &$8.nat, &$8.rdr,
>+                                          &$8.rroute, $6, $7.src_os,
>+                                          $7.src.host, $7.src.port,
>+                                          $7.dst.host, $7.dst.port,
>+                                          $8.uid, $8.gid, $8.rcv,
>+                                          $8.icmpspec, "");
>+                                      NHS_INC(r.divert.port);
>+                              }
>+#undef NHS_INC
>+#undef NHS_LT
>                       }
>                       r.divert_packet.port = $8.divert_packet.port;
>
>@@ -2197,7 +2219,7 @@
>                       }
>                       filter_opts.rtableid = $2;
>               }
>-              | DIVERTTO STRING PORT portplain {
>+              | DIVERTTO STRING PORT portrange {
>                       if ((filter_opts.divert.addr = host($2)) == NULL) {
>                               yyerror("could not parse divert address: %s",
>                                   $2);
>@@ -2210,6 +2232,7 @@
>                               yyerror("invalid divert port: %u", ntohs($4.a));
>                               YYERROR;
>                       }
>+                      filter_opts.divert.port_top = $4.b;
>               }
>               | DIVERTREPLY {
>                       filter_opts.divert.port = 1;    /* some random value */
>@@ -3073,15 +3096,6 @@
>                       $$->op = $2;
>                       $$->next = NULL;
>                       $$->tail = $$;
>-              }
>-              ;
>-
>-portplain     : numberstring                  {
>-                      if (parseport($1, &$$, 0) == -1) {
>-                              free($1);
>-                              YYERROR;
>-                      }
>-                      free($1);
>               }
>               ;
>
>
>

-- 
Dave Anderson
<[email protected]>


Reply via email to