On Wed, Jun 08, 2011 at 09:53:24AM +0200, Claudio Jeker wrote:
> 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.
New version with fixes found by bluhm@ (mostly make sure that the right
header and mbufs are used in all cases).
--
: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 10 Jun 2011 08:22:36 -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,41 @@ 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);
+ }
+ /* refetch header, recalc offset and update pd */
+ h = mtod(m, struct ip *);
+ *off = h->ip_hl << 2;
+ pd->src = (struct pf_addr *)&h->ip_src;
+ pd->dst = (struct pf_addr *)&h->ip_dst;
+ pd->ip_sum = &h->ip_sum;
+ pd->tot_len = ntohs(h->ip_len);
+ pd->tos = h->ip_tos;
+ }
switch (h->ip_p) {
case IPPROTO_TCP: {
@@ -5551,7 +5595,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
+
pd->src = (struct pf_addr *)&h->ip6_src;
pd->dst = (struct pf_addr *)&h->ip6_dst;
pd->sport = pd->dport = NULL;
@@ -5756,7 +5832,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 +5863,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 +5874,22 @@ 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) {
+ m = *m0;
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 +5971,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 +6004,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 +6088,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 +6120,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 +6131,22 @@ 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) {
+ m = *m0;
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 +6260,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 *);
