Hi pf@

I sent this patch to t...@openbsd.org mail list, I sending to you to
get your suggestions, please send me any suggestions.



here is a version 2 of the patch to correct the synchronization of
states that use route-to policy, thanks to Bret Lambert for your
patience to teach me and your suggestions.

This patch apply to OpenBSD v4.6 -stable, nexthop_idx will give us the
reference into the route pool that have a same interface that the
current state is using to reach the nexthop, this reference not need
to point to exact current pool we need only a reference to the correct
interface, so in the standby peer we only look at that reference into
the pool to take out the interface reference, so we can complete the
rt_kif info in the state.

this patch is applied under sys/net, it modifies pfsync_state_import,
pfsync_state_export, pfsync_in_upd functions and pfsync_state_peer
struct.

===================================================================
RCS file: RCS/pfvar.h,v
retrieving revision 1.290
diff -u -r1.290 pfvar.h
--- pfvar.h     2010/01/13 23:33:23     1.290
+++ pfvar.h     2010/01/20 22:26:23
@@ -826,7 +826,8 @@
       u_int16_t       mss;            /* Maximum segment size option  */
       u_int8_t        state;          /* active state level           */
       u_int8_t        wscale;         /* window scaling factor        */
-       u_int8_t        pad[6];
+       u_int8_t        nexthop_idx;    /* nexthop in pool index        */
+       u_int8_t        pad[5];
 } __packed;

 struct pfsync_state_key {

===================================================================
RCS file: RCS/if_pfsync.c,v
retrieving revision 1.127
diff -u -r1.127 if_pfsync.c
--- if_pfsync.c 2010/01/13 23:06:38     1.127
+++ if_pfsync.c 2010/01/20 22:28:39
@@ -398,6 +398,8 @@
 void
 pfsync_state_export(struct pfsync_state *sp, struct pf_state *st)
 {
+       struct pf_pooladdr *acur = NULL;
+       struct pf_rule *r = NULL;
       bzero(sp, sizeof(struct pfsync_state));

       /* copy from state key */
@@ -449,6 +451,20 @@
       else
               sp->nat_rule = htonl(st->nat_rule.ptr->nr);

+       /* insert the rpool position reference for route-to states */
+       sp->dst.nexthop_idx = 0;
+       if(st->rt_kif) {
+               if(sp->rule != htonl(-1) && ntohl(sp->rule) <
pf_main_ruleset.rules[PF_RULESET_FILTER].active.rcount)
+                       r =
pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr_array[ntohl(sp->rule)];
+               if(r != NULL) {
+                       TAILQ_FOREACH(acur, &r->rpool.list, entries) {
+                               sp->dst.nexthop_idx++;
+                               if(st->rt_kif == acur->kif)
+                                       break;
+                       }
+               }
+       }
+
       pf_state_counter_hton(st->packets[0], sp->packets[0]);
       pf_state_counter_hton(st->packets[1], sp->packets[1]);
       pf_state_counter_hton(st->bytes[0], sp->bytes[0]);
@@ -465,6 +481,8 @@
       struct pfi_kif  *kif;
       int pool_flags;
       int error;
+       struct pf_pooladdr *acur;
+       u_int8_t count;

       if (sp->creatorid == 0 && pf_status.debug >= PF_DEBUG_MISC) {
               printf("pfsync_state_import: invalid creator id:"
@@ -563,6 +581,18 @@
       st->nat_rule.ptr = NULL;
       st->anchor.ptr = NULL;
       st->rt_kif = NULL;
+       /* try to insert the route-to kif for this state */
+       if(r != &pf_default_rule && r->rpool.cur) {
+               count = 0;
+               TAILQ_FOREACH(acur, &r->rpool.list, entries) {
+                       count++;
+                       if(count == sp->dst.nexthop_idx) {
+                               st->rt_kif = acur->kif;
+                               break;
+                       }
+               }
+       }
+

       st->pfsync_time = time_uptime;
       st->sync_state = PFSYNC_S_NONE;
@@ -916,7 +946,7 @@
               st = pf_find_state_byid(&id_key);
               if (st == NULL) {
                       /* insert the update */
-                       if (pfsync_state_import(sp, 0))
+                       if (pfsync_state_import(sp, pkt->flags))
                               pfsyncstats.pfsyncs_badstate++;
                       continue;
               }


I tested and it is working fine, any suggestions are welcomed.


Here is the original problem that I trying to solve with this patch:

if one rule with policy routing using route-to or reply-to creates a
state, this will inserted on the peer with pfsycn protocol with
rt_addr to the nexthop ip address but without the nexthop interface
output reference, in pfsync_state_import the rt_kif is inserted as:

st->rt_kif = NULL;

pfsync will bind the state to the ruleset if the checksum of ruleset
on both firewall is the same.

so when a failover happens:
pf_test probes the next packet coming in on an interface and it
belongs to a state and the action is pass it, if the rule to this
state has a route-to/reply-to routing then pf_route is called.

The pf_route function needs the interface to do the routing of the
packet, here is pf_route code fragment in pf.c :


      } else {
              if (TAILQ_EMPTY(&r->rpool.list)) {
                      DPFPRINTF(PF_DEBUG_URGENT,
                          ("pf_route: TAILQ_EMPTY(&r->rpool.list)\n"));
                      goto bad;
              }
              if (s == NULL) {
                      pf_map_addr(AF_INET, r, (struct pf_addr *)&ip->ip_src,
                          &naddr, NULL, &sn);
                      if (!PF_AZERO(&naddr, AF_INET))
                              dst->sin_addr.s_addr = naddr.v4.s_addr;
                      ifp = r->rpool.cur->kif ?
                          r->rpool.cur->kif->pfik_ifp : NULL;
              } else {
                      if (!PF_AZERO(&s->rt_addr, AF_INET))
                              dst->sin_addr.s_addr =
                                  s->rt_addr.v4.s_addr;
                      ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
              }
      }
      if (ifp == NULL)
              goto bad;

if the state exists and s->rt_kif isn't null then ifp will take the
interface info from its rt_kif->pfik_ifp

ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;

if the ifp==NULL then the packet will be droped and the tcp session
will be lossed, so we can not reach a high availability firewall for
pf routed rules.

- Romey

Reply via email to