On Tue, Jun 21, 2016 at 05:12:39PM +0200, Mike Belopuhov wrote:
> On Tue, Jun 21, 2016 at 16:08 +0200, Alexander Bluhm wrote:
> > On Tue, Jun 21, 2016 at 02:45:42PM +0200, Mike Belopuhov wrote:
> > > Unless I'm wrong, I have to retract my OK and ask you to fix
> > > the sport bit instead.
> >
> > Yes, fixing it in pf_get_sport() is more correct. I will try
> > to make a diff.
> >
>
> Cool.
I have found an issue with pf_get_sport(), it only works for out
rules. In my use case I have an in rule that does nat and diverts
to a socket. Collisions with existing states were not found.
This diff fixes it, now I can trigger such a log message:
Jun 21 11:18:14 q70 /bsd: pf: pf: NAT proxy port allocation (10000-10001) failed
Unfortunately it does not solve my orignal state key reuse problem.
But I suggest fixing things step by step.
ok?
bluhm
Index: net/pf_lb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.53
diff -u -p -r1.53 pf_lb.c
--- net/pf_lb.c 15 Jun 2016 11:36:06 -0000 1.53
+++ net/pf_lb.c 22 Jun 2016 17:08:06 -0000
@@ -155,6 +155,7 @@ pf_get_sport(struct pf_pdesc *pd, struct
struct pf_state_key_cmp key;
struct pf_addr init_addr;
u_int16_t cut;
+ int dir = (pd->dir == PF_IN) ? PF_OUT : PF_IN;
bzero(&init_addr, sizeof(init_addr));
if (pf_map_addr(pd->naf, r, &pd->nsaddr, naddr, &init_addr, sn, &r->nat,
@@ -182,9 +183,9 @@ pf_get_sport(struct pf_pdesc *pd, struct
key.af = pd->naf;
key.proto = pd->proto;
key.rdomain = pd->rdomain;
- PF_ACPY(&key.addr[0], &pd->ndaddr, key.af);
- PF_ACPY(&key.addr[1], naddr, key.af);
- key.port[0] = pd->ndport;
+ PF_ACPY(&key.addr[pd->didx], &pd->ndaddr, key.af);
+ PF_ACPY(&key.addr[pd->sidx], naddr, key.af);
+ key.port[pd->didx] = pd->ndport;
/*
* port search; start random, step;
@@ -194,20 +195,20 @@ pf_get_sport(struct pf_pdesc *pd, struct
pd->proto == IPPROTO_ICMP || pd->proto == IPPROTO_ICMPV6)) {
/* XXX bug: icmp states dont use the id on both
* XXX sides (traceroute -I through nat) */
- key.port[1] = pd->nsport;
- if (pf_find_state_all(&key, PF_IN, NULL) == NULL) {
+ key.port[pd->sidx] = pd->nsport;
+ if (pf_find_state_all(&key, dir, NULL) == NULL) {
*nport = pd->nsport;
return (0);
}
} else if (low == 0 && high == 0) {
- key.port[1] = pd->nsport;
- if (pf_find_state_all(&key, PF_IN, NULL) == NULL) {
+ key.port[pd->sidx] = pd->nsport;
+ if (pf_find_state_all(&key, dir, NULL) == NULL) {
*nport = pd->nsport;
return (0);
}
} else if (low == high) {
- key.port[1] = htons(low);
- if (pf_find_state_all(&key, PF_IN, NULL) == NULL) {
+ key.port[pd->sidx] = htons(low);
+ if (pf_find_state_all(&key, dir, NULL) == NULL) {
*nport = htons(low);
return (0);
}
@@ -223,16 +224,16 @@ pf_get_sport(struct pf_pdesc *pd, struct
cut = arc4random_uniform(1 + high - low) + low;
/* low <= cut <= high */
for (tmp = cut; tmp <= high; ++(tmp)) {
- key.port[1] = htons(tmp);
- if (pf_find_state_all(&key, PF_IN, NULL) ==
+ key.port[pd->sidx] = htons(tmp);
+ if (pf_find_state_all(&key, dir, NULL) ==
NULL && !in_baddynamic(tmp, pd->proto)) {
*nport = htons(tmp);
return (0);
}
}
for (tmp = cut - 1; tmp >= low; --(tmp)) {
- key.port[1] = htons(tmp);
- if (pf_find_state_all(&key, PF_IN, NULL) ==
+ key.port[pd->sidx] = htons(tmp);
+ if (pf_find_state_all(&key, dir, NULL) ==
NULL && !in_baddynamic(tmp, pd->proto)) {
*nport = htons(tmp);
return (0);
