* Alexander Bluhm <alexander.bl...@gmx.net> [2011-08-30 20:59]:
> When pf_test_rule() is called for fragments that have not been
> reassembled, the address copy is not done anymore.

good catch, new diff below.

> I think pf_setup_pdesc() should not call pf_test_rule() at all and
> just fill the pd struct.

indeed, the test_rule call in the fragment case is a bit weird.

> But that is more work so I would suggest
> to copy the PF_ACPY() to the handle fragments that aren't reassembled
> by normalization.

yup :)

Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.771
diff -u -p -r1.771 pf.c
--- pf.c        30 Aug 2011 00:40:47 -0000      1.771
+++ pf.c        31 Aug 2011 15:01:06 -0000
@@ -2762,9 +2762,6 @@ pf_test_rule(struct pf_rule **rm, struct
        u_int16_t                virtual_type, virtual_id;
        u_int8_t                 icmptype = 0, icmpcode = 0;
 
-       PF_ACPY(&pd->nsaddr, pd->src, pd->af);
-       PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
-
        bzero(&act, sizeof(act));
        act.prio[0] = act.prio[1] = PF_PRIO_NOTSET;
        bzero(sns, sizeof(sns));
@@ -2782,14 +2779,6 @@ pf_test_rule(struct pf_rule **rm, struct
        }
 
        switch (pd->virtual_proto) {
-       case IPPROTO_TCP:
-               pd->nsport = th->th_sport;
-               pd->ndport = th->th_dport;
-               break;
-       case IPPROTO_UDP:
-               pd->nsport = pd->hdr.udp->uh_sport;
-               pd->ndport = pd->hdr.udp->uh_dport;
-               break;
 #ifdef INET
        case IPPROTO_ICMP:
                icmptype = pd->hdr.icmp->icmp_type;
@@ -2820,9 +2809,6 @@ pf_test_rule(struct pf_rule **rm, struct
                }
                break;
 #endif /* INET6 */
-       default:
-               pd->nsport = pd->ndport = 0;
-               break;
        }
 
        pd->osport = pd->nsport;
@@ -5679,6 +5665,13 @@ pf_setup_pdesc(sa_family_t af, int dir, 
                                    m, *off, pd, a, ruleset, *hdrlen);
                        if (*action != PF_PASS)
                                REASON_SET(reason, PFRES_FRAG);
+
+                       PF_ACPY(&pd->nsaddr, pd->src, pd->af);
+                       PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
+                       if (pd->sport)
+                               pd->nsport = *pd->sport;
+                       if (pd->dport)
+                               pd->ndport = *pd->dport;
                        return (-1);
                }
                break;
@@ -5849,6 +5842,14 @@ pf_setup_pdesc(sa_family_t af, int dir, 
        }
 #endif /* INET6 */
        }
+
+       PF_ACPY(&pd->nsaddr, pd->src, pd->af);
+       PF_ACPY(&pd->ndaddr, pd->dst, pd->af);
+       if (pd->sport)
+               pd->nsport = *pd->sport;
+       if (pd->dport)
+               pd->ndport = *pd->dport;
+
        return (0);
 }

Reply via email to