Next step on my quest to unify pf_test and pf_test6. Move the fragment handling and some other protocol specific tests into pf_setup_pdesc(). IPv6 already does this mostly but only IPv4 did not. So this diff brings that more into sync. It also includes some additional cleanups.
Works for me but needs some more testing. -- :wq Claudio Index: net/if_pflog.c =================================================================== RCS file: /cvs/src/sys/net/if_pflog.c,v retrieving revision 1.34 diff -u -p -r1.34 if_pflog.c --- net/if_pflog.c 22 May 2011 13:21:24 -0000 1.34 +++ net/if_pflog.c 24 May 2011 15:56:53 -0000 @@ -334,7 +334,7 @@ pflog_bpfcopy(const void *src_arg, void /* rewrite addresses if needed */ memset(&pd, 0, sizeof(pd)); pd.hdr.any = &pf_hdrs; - if (pf_setup_pdesc(pfloghdr->af, pfloghdr->dir, &pd, mfake, &action, + if (pf_setup_pdesc(pfloghdr->af, pfloghdr->dir, &pd, &mfake, &action, &reason, NULL, NULL, NULL, NULL, &off, &hdrlen) == -1) return; Index: net/pf.c =================================================================== RCS file: /cvs/src/sys/net/pf.c,v retrieving revision 1.747 diff -u -p -r1.747 pf.c --- net/pf.c 2 Jun 2011 22:08:40 -0000 1.747 +++ net/pf.c 8 Jun 2011 06:08:24 -0000 @@ -5466,10 +5466,12 @@ pf_get_divert(struct mbuf *m) } int -pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf *m, +pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0, u_short *action, u_short *reason, struct pfi_kif *kif, struct pf_rule **a, struct pf_rule **r, struct pf_ruleset **ruleset, int *off, int *hdrlen) { + struct mbuf *m = *m0; + if (pd->hdr.any == NULL) panic("pf_setup_pdesc: no storage for headers provided"); @@ -5479,13 +5481,23 @@ pf_setup_pdesc(sa_family_t af, int dir, case AF_INET: { struct ip *h; + /* Check for illegal packets */ + if (m->m_pkthdr.len < (int)sizeof(struct ip)) { + *action = PF_DROP; + REASON_SET(reason, PFRES_SHORT); + return (-1); + } + h = mtod(m, struct ip *); *off = h->ip_hl << 2; - if (*off < (int)sizeof(*h)) { + + if (*off < (int)sizeof(struct ip) || + *off > ntohs(h->ip_len)) { *action = PF_DROP; REASON_SET(reason, PFRES_SHORT); return (-1); } + pd->src = (struct pf_addr *)&h->ip_src; pd->dst = (struct pf_addr *)&h->ip_dst; pd->sport = pd->dport = NULL; @@ -5500,9 +5512,33 @@ pf_setup_pdesc(sa_family_t af, int dir, pd->tot_len = ntohs(h->ip_len); pd->rdomain = rtable_l2(m->m_pkthdr.rdomain); - /* fragments not reassembled handled later */ - if (h->ip_off & htons(IP_MF | IP_OFFMASK)) - return (0); + if (h->ip_off & htons(IP_MF | IP_OFFMASK)) { + if (!pf_status.reass) { + /* + * handle fragments that aren't reassembled by + * normalization + */ + if (kif == NULL || r == NULL) /* pflog */ + *action = PF_DROP; + else + *action = pf_test_fragment(r, dir, kif, + m, pd, a, ruleset); + if (*action == PF_DROP) + REASON_SET(reason, PFRES_FRAG); + return (-1); + } + /* packet reassembly */ + if (pf_normalize_ip(m0, dir, reason, pd) != PF_PASS) { + *action = PF_DROP; + return (-1); + } + m = *m0; + if (m == NULL) { + /* packet has been reassembled, no error */ + *action = PF_PASS; + return (-1); + } + } switch (h->ip_p) { case IPPROTO_TCP: { @@ -5551,6 +5587,39 @@ pf_setup_pdesc(sa_family_t af, int dir, struct ip6_hdr *h; int terminal = 0; + /* Check for illegal packets */ + if (m->m_pkthdr.len < (int)sizeof(struct ip6_hdr)) { + *action = PF_DROP; + REASON_SET(reason, PFRES_SHORT); + return (-1); + } + + /* packet reassembly */ + if (pf_status.reass && + pf_normalize_ip6(m0, dir, reason, pd) != PF_PASS) { + *action = PF_DROP; + return (-1); + } + m = *m0; + if (m == NULL) { + /* packet has been reassembled, no error */ + *action = PF_PASS; + return (-1); + } + + h = mtod(m, struct ip6_hdr *); +#if 1 + /* + * we do not support jumbogram yet. if we keep going, zero + * ip6_plen will do something bad, so drop the packet for now. + */ + if (htons(h->ip6_plen) == 0) { + *action = PF_DROP; + REASON_SET(reason, PFRES_NORM); + return (-1); + } +#endif + h = mtod(m, struct ip6_hdr *); pd->src = (struct pf_addr *)&h->ip6_src; pd->dst = (struct pf_addr *)&h->ip6_dst; @@ -5756,7 +5825,6 @@ pf_test(int dir, struct ifnet *ifp, stru struct pfi_kif *kif; u_short action, reason = 0; struct mbuf *m = *m0; - struct ip *h; struct pf_rule *a = NULL, *r = &pf_default_rule; struct pf_state *s = NULL; struct pf_ruleset *ruleset = NULL; @@ -5788,13 +5856,6 @@ pf_test(int dir, struct ifnet *ifp, stru panic("non-M_PKTHDR is passed to pf_test"); #endif /* DIAGNOSTIC */ - if (m->m_pkthdr.len < (int)sizeof(*h)) { - action = PF_DROP; - REASON_SET(&reason, PFRES_SHORT); - pd.pflog |= PF_LOG_FORCE; - goto done; - } - if (m->m_pkthdr.pf.flags & PF_TAG_GENERATED) return (PF_PASS); @@ -5806,40 +5867,21 @@ pf_test(int dir, struct ifnet *ifp, stru return (PF_PASS); } - /* packet reassembly here if 1) enabled 2) we deal with a fragment */ - h = mtod(m, struct ip *); - if (pf_status.reass && - (h->ip_off & htons(IP_MF | IP_OFFMASK)) && - pf_normalize_ip(m0, dir, kif, &reason, &pd) != PF_PASS) { - action = PF_DROP; - goto done; - } - m = *m0; /* pf_normalize messes with m0 */ - if (m == NULL) - return (PF_PASS); - h = mtod(m, struct ip *); - - if (pf_setup_pdesc(AF_INET, dir, &pd, m, &action, &reason, kif, &a, &r, + if (pf_setup_pdesc(AF_INET, dir, &pd, m0, &action, &reason, kif, &a, &r, &ruleset, &off, &hdrlen) == -1) { if (action != PF_PASS) pd.pflog |= PF_LOG_FORCE; goto done; } pd.eh = eh; - - /* handle fragments that didn't get reassembled by normalization */ - if (h->ip_off & htons(IP_MF | IP_OFFMASK)) { - action = pf_test_fragment(&r, dir, kif, m, - &pd, &a, &ruleset); - goto done; - } + m = *m0; /* pf_setup_pdesc -> pf_normalize messes with m0 */ switch (pd.proto) { case IPPROTO_TCP: { if ((pd.hdr.tcp->th_flags & TH_ACK) && pd.p_len == 0) pqid = 1; - action = pf_normalize_tcp(dir, kif, m, 0, off, h, &pd); + action = pf_normalize_tcp(dir, m, off, &pd); if (action == PF_DROP) goto done; action = pf_test_state_tcp(&s, dir, kif, m, off, &pd, @@ -5921,6 +5963,7 @@ done: if (action != PF_DROP) { if (s) { /* The non-state case is handled in pf_test_rule() */ + struct ip *h = mtod(m, struct ip *); if (action == PF_PASS && h->ip_hl > 5 && !(s->state_flags & PFSTATE_ALLOWOPTS)) { action = PF_DROP; @@ -5953,7 +5996,7 @@ done: #ifdef ALTQ if (action == PF_PASS && qid) { m->m_pkthdr.pf.qid = qid; - m->m_pkthdr.pf.hdr = h; /* hints for ecn */ + m->m_pkthdr.pf.hdr = mtod(m, caddr_t); /* hints for ecn */ } #endif /* ALTQ */ @@ -6037,7 +6080,6 @@ pf_test6(int fwdir, struct ifnet *ifp, s u_short action, reason = 0; struct mbuf *m = *m0; struct m_tag *mtag; - struct ip6_hdr *h; struct pf_rule *a = NULL, *r = &pf_default_rule; struct pf_state *s = NULL; struct pf_ruleset *ruleset = NULL; @@ -6070,13 +6112,6 @@ pf_test6(int fwdir, struct ifnet *ifp, s panic("non-M_PKTHDR is passed to pf_test6"); #endif /* DIAGNOSTIC */ - if (m->m_pkthdr.len < (int)sizeof(*h)) { - action = PF_DROP; - REASON_SET(&reason, PFRES_SHORT); - pd.pflog |= PF_LOG_FORCE; - goto done; - } - if (m->m_pkthdr.pf.flags & PF_TAG_GENERATED) return (PF_PASS); @@ -6088,44 +6123,21 @@ pf_test6(int fwdir, struct ifnet *ifp, s return (PF_PASS); } - /* packet reassembly */ - if (pf_status.reass && - pf_normalize_ip6(m0, fwdir, kif, &reason, &pd) != PF_PASS) { - action = PF_DROP; - goto done; - } - m = *m0; /* pf_normalize messes with m0 */ - if (m == NULL) - return (PF_PASS); - h = mtod(m, struct ip6_hdr *); - -#if 1 - /* - * we do not support jumbogram yet. if we keep going, zero ip6_plen - * will do something bad, so drop the packet for now. - */ - if (htons(h->ip6_plen) == 0) { - action = PF_DROP; - REASON_SET(&reason, PFRES_NORM); - pd.pflog |= PF_LOG_FORCE; - goto done; - } -#endif - - if (pf_setup_pdesc(AF_INET6, dir, &pd, m, &action, &reason, kif, &a, &r, - &ruleset, &off, &hdrlen) == -1) { + if (pf_setup_pdesc(AF_INET6, dir, &pd, m0, &action, &reason, kif, &a, + &r, &ruleset, &off, &hdrlen) == -1) { if (action != PF_PASS) pd.pflog |= PF_LOG_FORCE; goto done; } pd.eh = eh; + m = *m0; /* pf_setup_pdesc -> pf_normalize messes with m0 */ switch (pd.proto) { case IPPROTO_TCP: { if ((pd.hdr.tcp->th_flags & TH_ACK) && pd.p_len == 0) pqid = 1; - action = pf_normalize_tcp(dir, kif, m, 0, off, h, &pd); + action = pf_normalize_tcp(dir, m, off, &pd); if (action == PF_DROP) goto done; action = pf_test_state_tcp(&s, dir, kif, m, off, &pd, @@ -6239,7 +6251,7 @@ done: #ifdef ALTQ if (action == PF_PASS && qid) { m->m_pkthdr.pf.qid = qid; - m->m_pkthdr.pf.hdr = h; /* add hints for ecn */ + m->m_pkthdr.pf.hdr = mtod(m, caddr_t); /* add hints for ecn */ } #endif /* ALTQ */ Index: net/pf_norm.c =================================================================== RCS file: /cvs/src/sys/net/pf_norm.c,v retrieving revision 1.133 diff -u -p -r1.133 pf_norm.c --- net/pf_norm.c 24 May 2011 14:01:52 -0000 1.133 +++ net/pf_norm.c 24 May 2011 16:57:05 -0000 @@ -739,30 +739,22 @@ pf_refragment6(struct mbuf **m0, struct #endif /* INET6 */ int -pf_normalize_ip(struct mbuf **m0, int dir, struct pfi_kif *kif, - u_short *reason, struct pf_pdesc *pd) +pf_normalize_ip(struct mbuf **m0, int dir, u_short *reason, + struct pf_pdesc *pd) { struct mbuf *m = *m0; struct ip *h = mtod(m, struct ip *); - int hlen = h->ip_hl << 2; u_int16_t fragoff = (ntohs(h->ip_off) & IP_OFFMASK) << 3; u_int16_t mff = (ntohs(h->ip_off) & IP_MF); - /* Check for illegal packets */ - if (hlen < (int)sizeof(struct ip)) - goto drop; - - if (hlen > ntohs(h->ip_len)) - goto drop; + /* We will need other tests here */ + if (!fragoff && !mff) + goto no_fragment; /* Clear IP_DF if we're in no-df mode */ if (pf_status.reass & PF_REASS_NODF && h->ip_off & htons(IP_DF)) h->ip_off &= htons(~IP_DF); - /* We will need other tests here */ - if (!fragoff && !mff) - goto no_fragment; - /* We're dealing with a fragment now. Don't allow fragments * with IP_DF to enter the cache. If the flag was cleared by * no-df above, fine. Otherwise drop it. @@ -789,16 +781,12 @@ pf_normalize_ip(struct mbuf **m0, int di pd->flags |= PFDESC_IP_REAS; return (PF_PASS); - - drop: - REASON_SET(reason, PFRES_NORM); - return (PF_DROP); } #ifdef INET6 int -pf_normalize_ip6(struct mbuf **m0, int dir, struct pfi_kif *kif, - u_short *reason, struct pf_pdesc *pd) +pf_normalize_ip6(struct mbuf **m0, int dir, u_short *reason, + struct pf_pdesc *pd) { struct mbuf *m = *m0; struct ip6_hdr *h = mtod(m, struct ip6_hdr *); @@ -826,7 +814,6 @@ pf_normalize_ip6(struct mbuf **m0, int d switch (proto) { case IPPROTO_FRAGMENT: goto fragment; - break; case IPPROTO_AH: case IPPROTO_ROUTING: case IPPROTO_DSTOPTS: @@ -938,8 +925,7 @@ pf_normalize_ip6(struct mbuf **m0, int d #endif /* INET6 */ int -pf_normalize_tcp(int dir, struct pfi_kif *kif, struct mbuf *m, int ipoff, - int off, void *h, struct pf_pdesc *pd) +pf_normalize_tcp(int dir, struct mbuf *m, int off, struct pf_pdesc *pd) { struct tcphdr *th = pd->hdr.tcp; u_short reason; Index: net/pfvar.h =================================================================== RCS file: /cvs/src/sys/net/pfvar.h,v retrieving revision 1.332 diff -u -p -r1.332 pfvar.h --- net/pfvar.h 24 May 2011 14:01:52 -0000 1.332 +++ net/pfvar.h 24 May 2011 16:57:22 -0000 @@ -1743,7 +1743,7 @@ void pf_rm_rule(struct pf_rulequeue struct pf_rule *); struct pf_divert *pf_find_divert(struct mbuf *); int pf_setup_pdesc(sa_family_t, int, - struct pf_pdesc *, struct mbuf *, + struct pf_pdesc *, struct mbuf **, u_short *, u_short *, struct pfi_kif *, struct pf_rule **, struct pf_rule **, struct pf_ruleset **, int *, int *); @@ -1777,12 +1777,9 @@ int pf_match_gid(u_int8_t, gid_t, gid_t, int pf_refragment6(struct mbuf **, struct m_tag *mtag, int); void pf_normalize_init(void); -int pf_normalize_ip(struct mbuf **, int, struct pfi_kif *, u_short *, - struct pf_pdesc *); -int pf_normalize_ip6(struct mbuf **, int, struct pfi_kif *, u_short *, - struct pf_pdesc *); -int pf_normalize_tcp(int, struct pfi_kif *, struct mbuf *, int, int, void *, - struct pf_pdesc *); +int pf_normalize_ip(struct mbuf **, int, u_short *, struct pf_pdesc *); +int pf_normalize_ip6(struct mbuf **, int, u_short *, struct pf_pdesc *); +int pf_normalize_tcp(int, struct mbuf *, int, struct pf_pdesc *); void pf_normalize_tcp_cleanup(struct pf_state *); int pf_normalize_tcp_init(struct mbuf *, int, struct pf_pdesc *, struct tcphdr *, struct pf_state_peer *, struct pf_state_peer *);