Re: pfsync and policy routing states patch

2010-02-01 Thread Henning Brauer
* Romey Valadez  [2010-01-15 00:53]:
> this patch apply to OpenBSD v4.6 -stable

we really don't care much for diffs to -stable.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services, http://bsws.de
Full-Service ISP - Secure Hosting, Mail and DNS Services
Dedicated Servers, Rootservers, Application Hosting



pfsync and policy routing states patch - version2

2010-01-20 Thread Romey Valadez
Hi tech@,

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_tstate;  /* active state level   */
u_int8_twscale; /* window scaling factor*/
-   u_int8_tpad[6];
+   u_int8_tnexthop_idx;/* nexthop in pool index*/
+   u_int8_tpad[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

pfsync and policy routing states patch

2010-01-18 Thread Romey Valadez
Hi All

I'm testing this patch to permit synchronization of states that use
route-to or reply-to for policy routing, this patch apply to OpenBSD
v4.6 -stable, this is the problem:

when pfsync send an state that uses route-to or reply-to option to the
other machine, it only sends only current next-hop address in rt_addr
member of pfsync_state struct, the peer gets the pfsync data and it
will try to insert the state and bind it to ruleset, when the
secondary machine gets active and a packet going to pass through pf
will try to process pf_route if the state matches with a rule with
route/reply option, but the interface information hadn't inserted into
the state so the state will has st->rt_kif = NULL then the packet will
be droped in pf_route function.

To complete the route/reply option of one state and keep compatibility
with pfsync_state structure, we can use the data space holding by pad
member into pfsync_state_peer, it seems to not be used by other
function, so we can free rename it, I rename it as ex_info (extra info),
so we can use it to put the info to complete the state. The ex_info
data is 6 bytes length so we can't put the st->rt_kif->pfik_name data
here. We can put the relative position of the rt_kif into the
route/reply pool, this manner we need only 2 bytes to send the integer
that represents the relative position into the pool, this position
will only tell us
where the interface can be found, it no necesary be the exact pool, so
when a peer
receives the pfsync data it can bind it into the pool if the both
ruleset have a checksum match.

I  made some tests and it is running fine, this patch can be ported to
the -current version of cvs, but we need to make a little changes
beacuse rpool member of pf_rule struct has a new name: route.

The modified functions was: pfsync_state_export, pfsync_state_import
and pfsync_in_upd, the pfsync_in_upd was inserted a new state with
flags equal to zero, now it insert the new state with the pkt->flags,
so if the ruleset checksum matches this state will be attached to the
ruleset in pfsync_state_import function

===
RCS file: src/sys/net/RCS/if_pfsync.c,v
retrieving revision 1.127
diff -u -r1.127 src/sys/net/if_pfsync.c
--- src/sys/net/if_pfsync.c 2010/01/13 23:06:38 1.127
+++ src/sys/net/if_pfsync.c 2010/01/14 21:45:18
@@ -398,6 +398,9 @@
 void
 pfsync_state_export(struct pfsync_state *sp, struct pf_state *st)
 {
+   struct pf_pooladdr *acur = NULL;
+   struct pf_rule *r = NULL;
+   int count = 0;
bzero(sp, sizeof(struct pfsync_state));

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

+   /* insert the rpool position reference for route-to states */
+   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) {
+   count++;
+   if(st->rt_kif == acur->kif)
+   break;
+   }
+   }
+   bcopy(&count, &sp->dst.ex_info, sizeof(count));
+   }
+
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 +482,9 @@
struct pfi_kif  *kif;
int pool_flags;
int error;
+   struct pf_pooladdr *acur;
+   int count;
+   int i;

if (sp->creatorid == 0 && pf_status.debug >= PF_DEBUG_MISC) {
printf("pfsync_state_import: invalid creator id:"
@@ -563,6 +583,19 @@
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) {
+   bcopy(&sp->dst.ex_info, &count, sizeof(count));
+   i = 0;
+   TAILQ_FOREACH(acur, &r->rpool.list, entries) {
+   i++;
+   if(i == count) {
+   st->rt_kif = acur->kif;
+   break;
+   }
+   }
+   }
+

st->pfsync_time = time_uptime;
st->sync_state = PFSYNC_S_NONE;
@@ -916,7 +949,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;
}



pfsync and policy routing states patch

2010-01-14 Thread Romey Valadez
Hi All

I'm testing this patch to permit synchronization of states that use
route-to or reply-to for policy routing, this patch apply to OpenBSD
v4.6 -stable, this is the problem:

when pfsync send an state that uses route-to or reply-to option to the
other machine, it only sends only current next-hop address in rt_addr
member of pfsync_state struct, the peer gets the pfsync data and it
will try to insert the state and bind it to ruleset, when the
secondary machine gets active and a packet going to pass through pf
will try to process pf_route if the state matches with a rule with
route/reply option, but the interface information hadn't inserted into
the state so the state will has st->rt_kif = NULL then the packet will
be droped in pf_route function.

To complete the route/reply option of one state and keep compatibility
with pfsync_state structure, we can use the data space holding by pad
member into pfsync_state_peer, it seems to not be used by other
function, so we can free rename, I rename it as ex_info (extra info),
so we can use it to put the info to complete the state. The ex_info
data is 6 bytes length so we can't put the st->rt_kif->pfik_name data
here. We can put the relative position of the rt_kif into the
route/reply pool, this manner we need only 2 bytes to send the integer
that represents the relative position into the pool, so when a peer
receives the pfsync data it can bind it into the pool if the both
ruleset have a checksum match.

I  made some tests and it is running fine, this patch can be ported to
the -current version of cvs, but we need to make a little changes
beacuse rpool member of pf_rule struct has a new name: route.

The modified functions was: pfsync_state_export, pfsync_state_import
and pfsync_in_upd, the pfsync_in_upd was inserted a new state with
flags equal to zero, now it insert the new state with the pkt->flags,
so if the ruleset checksum matches this state will be attached to the
ruleset in pfsync_state_import function

===
RCS file: src/sys/net/RCS/if_pfsync.c,v
retrieving revision 1.127
diff -u -r1.127 src/sys/net/if_pfsync.c
--- src/sys/net/if_pfsync.c 2010/01/13 23:06:38 1.127
+++ src/sys/net/if_pfsync.c 2010/01/14 21:45:18
@@ -398,6 +398,9 @@
 void
 pfsync_state_export(struct pfsync_state *sp, struct pf_state *st)
 {
+   struct pf_pooladdr *acur = NULL;
+   struct pf_rule *r = NULL;
+   int count = 0;
bzero(sp, sizeof(struct pfsync_state));

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

+   /* insert the rpool position reference for route-to states */
+   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) {
+   count++;
+   if(st->rt_kif == acur->kif)
+   break;
+   }
+   }
+   bcopy(&count, &sp->dst.ex_info, sizeof(count));
+   }
+
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 +482,9 @@
struct pfi_kif  *kif;
int pool_flags;
int error;
+   struct pf_pooladdr *acur;
+   int count;
+   int i;

if (sp->creatorid == 0 && pf_status.debug >= PF_DEBUG_MISC) {
printf("pfsync_state_import: invalid creator id:"
@@ -563,6 +583,19 @@
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) {
+   bcopy(&sp->dst.ex_info, &count, sizeof(count));
+   i = 0;
+   TAILQ_FOREACH(acur, &r->rpool.list, entries) {
+   i++;
+   if(i == count) {
+   st->rt_kif = acur->kif;
+   break;
+   }
+   }
+   }
+

st->pfsync_time = time_uptime;
st->sync_state = PFSYNC_S_NONE;
@@ -916,7 +949,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;
}


===
RCS file: src/sys/net/RCS/pfvar.h,v
retriev