The branch main has been updated by kp:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=8de7f8ed5eefe85b4df068c54059656e12539c4b

commit 8de7f8ed5eefe85b4df068c54059656e12539c4b
Author:     Kristof Provost <k...@freebsd.org>
AuthorDate: 2024-09-21 16:00:13 +0000
Commit:     Kristof Provost <k...@freebsd.org>
CommitDate: 2024-10-10 12:10:39 +0000

    pf: reduce IPv6 header parsing code duplication
    
    There were two loops in pf_setup_pdesc() and pf_normalize_ip6()
    walking over the IPv6 header chain.  Merge them into one loop,
    adjust some length checks and fix IPv6 jumbo option handling.  Also
    allow strange but legal IPv6 packets with plen=0 passing through
    pf.  IPv6 jumbo packets still get dropped.
    testing dhill@; ok mcbride@ henning@
    
    Obtained from:  OpenBSD, bluhm <bl...@openbsd.org>, d68283bbf0
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D46925
---
 sys/net/pfvar.h          |   6 +-
 sys/netpfil/pf/pf.c      | 261 ++++++++++++++++++++++++++++++++++-------------
 sys/netpfil/pf/pf_norm.c | 145 ++++++--------------------
 3 files changed, 225 insertions(+), 187 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index b5d56ab45ce7..1e28693b960d 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2361,8 +2361,10 @@ int      pf_normalize_ip(struct mbuf **, struct pfi_kkif 
*, u_short *,
 #endif /* INET */
 
 #ifdef INET6
-int    pf_normalize_ip6(struct mbuf **, struct pfi_kkif *, u_short *,
-           struct pf_pdesc *);
+int    pf_walk_header6(struct mbuf *, uint8_t *, int *, int *, uint32_t *,
+           u_short *);
+int    pf_normalize_ip6(struct mbuf **, struct pfi_kkif *, int,
+           u_short *, struct pf_pdesc *);
 void   pf_poolmask(struct pf_addr *, struct pf_addr*,
            struct pf_addr *, struct pf_addr *, sa_family_t);
 void   pf_addr_inc(struct pf_addr *, sa_family_t);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 7edf65ae4a09..a482e08dd6ac 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -363,6 +363,8 @@ static u_int16_t     pf_calc_mss(struct pf_addr *, 
sa_family_t,
                                int, u_int16_t);
 static int              pf_check_proto_cksum(struct mbuf *, int, int,
                            u_int8_t, sa_family_t);
+static int              pf_walk_option6(struct mbuf *, int, int, uint32_t *,
+                           u_short *);
 static void             pf_print_state_parts(struct pf_kstate *,
                            struct pf_state_key *, struct pf_state_key *);
 static void             pf_patch_8(struct mbuf *, u_int16_t *, u_int8_t *, 
u_int8_t,
@@ -8460,6 +8462,144 @@ pf_dummynet_route(struct pf_pdesc *pd, struct pf_kstate 
*s,
        return (0);
 }
 
+#ifdef INET6
+static int
+pf_walk_option6(struct mbuf *m, int off, int end, uint32_t *jumbolen,
+    u_short *reason)
+{
+       struct ip6_opt           opt;
+       struct ip6_opt_jumbo     jumbo;
+       struct ip6_hdr          *h = mtod(m, struct ip6_hdr *);
+
+       while (off < end) {
+               if (!pf_pull_hdr(m, off, &opt.ip6o_type, sizeof(opt.ip6o_type),
+                       NULL, reason, AF_INET6)) {
+                       DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short opt type"));
+                       return (PF_DROP);
+               }
+               if (opt.ip6o_type == IP6OPT_PAD1) {
+                       off++;
+                       continue;
+               }
+               if (!pf_pull_hdr(m, off, &opt, sizeof(opt), NULL, reason,
+                       AF_INET6)) {
+                       DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short opt"));
+                       return (PF_DROP);
+               }
+               if (off + sizeof(opt) + opt.ip6o_len > end) {
+                       DPFPRINTF(PF_DEBUG_MISC, ("IPv6 long opt"));
+                       REASON_SET(reason, PFRES_IPOPTIONS);
+                       return (PF_DROP);
+               }
+               switch (opt.ip6o_type) {
+               case IP6OPT_JUMBO:
+                       if (*jumbolen != 0) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 multiple 
jumbo"));
+                               REASON_SET(reason, PFRES_IPOPTIONS);
+                               return (PF_DROP);
+                       }
+                       if (ntohs(h->ip6_plen) != 0) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 bad jumbo 
plen"));
+                               REASON_SET(reason, PFRES_IPOPTIONS);
+                               return (PF_DROP);
+                       }
+                       if (!pf_pull_hdr(m, off, &jumbo, sizeof(jumbo), NULL,
+                               reason, AF_INET6)) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short jumbo"));
+                               return (PF_DROP);
+                       }
+                       memcpy(jumbolen, jumbo.ip6oj_jumbo_len,
+                           sizeof(*jumbolen));
+                       *jumbolen = ntohl(*jumbolen);
+                       if (*jumbolen < IPV6_MAXPACKET) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short 
jumbolen"));
+                               REASON_SET(reason, PFRES_IPOPTIONS);
+                               return (PF_DROP);
+                       }
+                       break;
+               default:
+                       break;
+               }
+               off += sizeof(opt) + opt.ip6o_len;
+       }
+
+       return (PF_PASS);
+}
+
+int
+pf_walk_header6(struct mbuf *m, uint8_t *nxt, int *off, int *extoff,
+    uint32_t *jumbolen, u_short *reason)
+{
+       struct ip6_ext           ext;
+       struct ip6_rthdr         rthdr;
+       struct ip6_hdr          *h = mtod(m, struct ip6_hdr *);
+       int                      rthdr_cnt = 0;
+
+       *nxt = h->ip6_nxt;
+       *off = sizeof(struct ip6_hdr);
+       *extoff = 0;
+       *jumbolen = 0;
+       for (;;) {
+               switch (*nxt) {
+               case IPPROTO_FRAGMENT:
+                       /* jumbo payload packets cannot be fragmented */
+                       if (*jumbolen != 0) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 fragmented 
jumbo"));
+                               REASON_SET(reason, PFRES_FRAG);
+                               return (PF_DROP);
+                       }
+                       return (PF_PASS);
+               case IPPROTO_ROUTING:
+                       if (rthdr_cnt++) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 multiple 
rthdr"));
+                               REASON_SET(reason, PFRES_IPOPTIONS);
+                               return (PF_DROP);
+                       }
+                       if (!pf_pull_hdr(m, *off, &rthdr, sizeof(rthdr), NULL,
+                               reason, AF_INET6)) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short rthdr"));
+                               return (PF_DROP);
+                       }
+                       if (rthdr.ip6r_type == IPV6_RTHDR_TYPE_0) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 rthdr0"));
+                               REASON_SET(reason, PFRES_IPOPTIONS);
+                               return (PF_DROP);
+                       }
+                       /* FALLTHROUGH */
+               case IPPROTO_AH:
+               case IPPROTO_HOPOPTS:
+               case IPPROTO_DSTOPTS:
+                       if (!pf_pull_hdr(m, *off, &ext, sizeof(ext), NULL,
+                               reason, AF_INET6)) {
+                               DPFPRINTF(PF_DEBUG_MISC, ("IPv6 short exthdr"));
+                               return (PF_DROP);
+                       }
+                       *extoff = *off;
+                       if (*nxt == IPPROTO_HOPOPTS) {
+                               if (pf_walk_option6(m, *off + sizeof(ext),
+                                       *off + (ext.ip6e_len + 1) * 8, jumbolen,
+                                       reason) != PF_PASS)
+                                       return (PF_DROP);
+                               if (ntohs(h->ip6_plen) == 0 && *jumbolen != 0) {
+                                       DPFPRINTF(PF_DEBUG_MISC,
+                                           ("IPv6 missing jumbo"));
+                                       REASON_SET(reason, PFRES_IPOPTIONS);
+                                       return (PF_DROP);
+                               }
+                       }
+                       if (*nxt == IPPROTO_AH)
+                               *off += (ext.ip6e_len + 2) * 4;
+                       else
+                               *off += (ext.ip6e_len + 1) * 8;
+                       *nxt = ext.ip6e_nxt;
+                       break;
+               default:
+                       return (PF_PASS);
+               }
+       }
+}
+#endif
+
 static void
 pf_init_pdesc(struct pf_pdesc *pd, struct mbuf *m)
 {
@@ -8554,7 +8694,9 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc 
*pd, struct mbuf **m0,
 #ifdef INET6
        case AF_INET6: {
                struct ip6_hdr *h;
-               int terminal = 0;
+               int extoff;
+               uint32_t jumbolen;
+               uint8_t nxt;
 
                if (__predict_false(m->m_len < sizeof(struct ip6_hdr)) &&
                    (m = *m0 = m_pullup(*m0, sizeof(struct ip6_hdr))) == NULL) {
@@ -8566,12 +8708,11 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc 
*pd, struct mbuf **m0,
                        return (-1);
                }
 
-               /* We do IP header normalization and packet reassembly here */
-               if (pf_normalize_ip6(m0, kif, reason, pd) != PF_PASS) {
+               if (pf_walk_header6(m, &nxt, off, &extoff, &jumbolen, reason)
+                   != PF_PASS) {
                        *action = PF_DROP;
                        return (-1);
                }
-               m = *m0;
 
                h = mtod(m, struct ip6_hdr *);
                pd->src = (struct pf_addr *)&h->ip6_src;
@@ -8584,76 +8725,51 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc 
*pd, struct mbuf **m0,
                pd->didx = (dir == PF_IN) ? 1 : 0;
                pd->tos = IPV6_DSCP(h);
                pd->tot_len = ntohs(h->ip6_plen) + sizeof(struct ip6_hdr);
-               *off = ((caddr_t)h - m->m_data) + sizeof(struct ip6_hdr);
                pd->virtual_proto = pd->proto = h->ip6_nxt;
                pd->act.rtableid = -1;
 
-               do {
-                       switch (pd->proto) {
-                       case IPPROTO_FRAGMENT:
-                               pd->virtual_proto = PF_VPROTO_FRAGMENT;
-                               if (kif == NULL || r == NULL) /* pflog */
-                                       *action = PF_DROP;
-                               else
-                                       *action = pf_test_rule(r, s, kif, m, 
*off,
-                                           pd, a, ruleset, inp,
-                                           *hdrlen);
-                               if (*action == PF_DROP)
-                                       REASON_SET(reason, PFRES_FRAG);
-                               return (-1);
-                       case IPPROTO_ROUTING: {
-                               struct ip6_rthdr rthdr;
+               /* We do IP header normalization and packet reassembly here */
+               if (pf_normalize_ip6(m0, kif, *off, reason, pd) !=
+                   PF_PASS) {
+                       *action = PF_DROP;
+                       return (-1);
+               }
+               m = *m0;
+               if (m == NULL) {
+                       /* packet sits in reassembly queue, no error */
+                       *action = PF_PASS;
+                       return (-1);
+               }
 
-                               if (pd->badopts++) {
-                                       DPFPRINTF(PF_DEBUG_MISC,
-                                           ("pf: IPv6 more than one rthdr"));
-                                       *action = PF_DROP;
-                                       REASON_SET(reason, PFRES_IPOPTIONS);
-                                       return (-1);
-                               }
-                               if (!pf_pull_hdr(m, *off, &rthdr, sizeof(rthdr),
-                                       NULL, reason, pd->af)) {
-                                       DPFPRINTF(PF_DEBUG_MISC,
-                                           ("pf: IPv6 short rthdr"));
-                                       *action = PF_DROP;
-                                       REASON_SET(reason, PFRES_SHORT);
-                                       return (-1);
-                               }
-                               if (rthdr.ip6r_type == IPV6_RTHDR_TYPE_0) {
-                                       DPFPRINTF(PF_DEBUG_MISC,
-                                           ("pf: IPv6 rthdr0"));
-                                       *action = PF_DROP;
-                                       REASON_SET(reason, PFRES_IPOPTIONS);
-                                       return (-1);
-                               }
-                               /* FALLTHROUGH */
-                       }
-                       case IPPROTO_AH:
-                       case IPPROTO_HOPOPTS:
-                       case IPPROTO_DSTOPTS: {
-                               /* get next header and header length */
-                               struct ip6_ext opt6;
-
-                               if (!pf_pull_hdr(m, *off, &opt6, sizeof(opt6),
-                                       NULL, reason, pd->af)) {
-                                       DPFPRINTF(PF_DEBUG_MISC,
-                                           ("pf: IPv6 short opt"));
-                                       *action = PF_DROP;
-                                       return (-1);
-                               }
-                               if (pd->proto == IPPROTO_AH)
-                                       *off += (opt6.ip6e_len + 2) * 4;
-                               else
-                                       *off += (opt6.ip6e_len + 1) * 8;
-                               pd->virtual_proto = pd->proto = opt6.ip6e_nxt;
-                               /* goto the next header */
-                               break;
-                       }
-                       default:
-                               terminal++;
-                               break;
-                       }
-               } while (!terminal);
+               /*
+                * Reassembly may have changed the next protocol from fragment
+                * to something else, so update.
+                */
+               h = mtod(m, struct ip6_hdr *);
+               pd->virtual_proto = pd->proto = h->ip6_nxt;
+
+               /* recalc offset, refetch header, then update pd */
+               if (pf_walk_header6(m, &nxt, off, &extoff, &jumbolen, reason) !=
+                   PF_PASS) {
+                       *action = PF_DROP;
+                       return (-1);
+               }
+
+               if (pd->proto == IPPROTO_FRAGMENT) {
+                       /*
+                        * handle fragments that aren't reassembled by
+                        * normalization
+                        */
+                       pd->virtual_proto = PF_VPROTO_FRAGMENT;
+                       if (kif == NULL || r == NULL) /* pflog */
+                               *action = PF_DROP;
+                       else
+                               *action = pf_test_rule(r, s, kif, m, *off,
+                                   pd, a, ruleset, NULL /* XXX TODO */, 
*hdrlen);
+                       if (*action != PF_PASS)
+                               REASON_SET(reason, PFRES_FRAG);
+                       return (-1);
+               }
 
                break;
        }
@@ -9160,8 +9276,12 @@ pf_test(sa_family_t af, int dir, int pflags, struct 
ifnet *ifp, struct mbuf **m0
        }
 
 done:
+       m = *m0;
        PF_RULES_RUNLOCK();
 
+       if (m == NULL)
+               goto eat_pkt;
+
        if (action == PF_PASS && pd.badopts &&
            !((s && s->state_flags & PFSTATE_ALLOWOPTS) || r->allow_opts)) {
                action = PF_DROP;
@@ -9355,6 +9475,7 @@ done:
                break;
        }
 
+eat_pkt:
        SDT_PROBE4(pf, ip, test, done, action, reason, r, s);
 
        if (s && action != PF_DROP) {
diff --git a/sys/netpfil/pf/pf_norm.c b/sys/netpfil/pf/pf_norm.c
index 18a9cff2a5c2..a159528c1756 100644
--- a/sys/netpfil/pf/pf_norm.c
+++ b/sys/netpfil/pf/pf_norm.c
@@ -1210,25 +1210,25 @@ pf_normalize_ip(struct mbuf **m0, struct pfi_kkif *kif, 
u_short *reason,
 #ifdef INET6
 int
 pf_normalize_ip6(struct mbuf **m0, struct pfi_kkif *kif,
-    u_short *reason, struct pf_pdesc *pd)
+    int off, u_short *reason, struct pf_pdesc *pd)
 {
-       struct mbuf             *m = *m0;
+       struct mbuf             *m;
        struct pf_krule         *r;
-       struct ip6_hdr          *h = mtod(m, struct ip6_hdr *);
-       int                      extoff;
-       int                      off;
-       struct ip6_ext           ext;
-       struct ip6_opt           opt;
        struct ip6_frag          frag;
-       u_int32_t                plen;
-       int                      optend;
-       int                      ooff;
-       u_int8_t                 proto;
-       int                      terminal;
+       int                      extoff;
+       uint32_t                 jumbolen;
+       uint8_t                  nxt;
        bool                     scrub_compat;
 
        PF_RULES_RASSERT();
 
+again:
+       m = *m0;
+
+       if (pf_walk_header6(m, &nxt, &off, &extoff, &jumbolen, reason)
+           != PF_PASS)
+               return (PF_DROP);
+
        r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_SCRUB].active.ptr);
        /*
         * Check if there are any scrub rules, matching or not.
@@ -1247,16 +1247,14 @@ pf_normalize_ip6(struct mbuf **m0, struct pfi_kkif *kif,
                        r = r->skip[PF_SKIP_DIR];
                else if (r->af && r->af != AF_INET6)
                        r = r->skip[PF_SKIP_AF];
-#if 0 /* header chain! */
-               else if (r->proto && r->proto != h->ip6_nxt)
+               else if (r->proto && r->proto != pd->proto)
                        r = r->skip[PF_SKIP_PROTO];
-#endif
                else if (PF_MISMATCHAW(&r->src.addr,
-                   (struct pf_addr *)&h->ip6_src, AF_INET6,
+                   (struct pf_addr *)&pd->src, AF_INET6,
                    r->src.neg, kif, M_GETFIB(m)))
                        r = r->skip[PF_SKIP_SRC_ADDR];
                else if (PF_MISMATCHAW(&r->dst.addr,
-                   (struct pf_addr *)&h->ip6_dst, AF_INET6,
+                   (struct pf_addr *)&pd->dst, AF_INET6,
                    r->dst.neg, NULL, M_GETFIB(m)))
                        r = r->skip[PF_SKIP_DST_ADDR];
                else
@@ -1276,112 +1274,29 @@ pf_normalize_ip6(struct mbuf **m0, struct pfi_kkif 
*kif,
                pf_rule_to_actions(r, &pd->act);
        }
 
-       /* Check for illegal packets */
-       if (sizeof(struct ip6_hdr) + IPV6_MAXPACKET < m->m_pkthdr.len)
-               goto drop;
-
-again:
-       h = mtod(m, struct ip6_hdr *);
-       plen = ntohs(h->ip6_plen);
-       /* jumbo payload option not supported */
-       if (plen == 0)
-               goto drop;
-
-       extoff = 0;
-       off = sizeof(struct ip6_hdr);
-       proto = h->ip6_nxt;
-       terminal = 0;
-       do {
-               switch (proto) {
-               case IPPROTO_FRAGMENT:
-                       goto fragment;
-                       break;
-               case IPPROTO_AH:
-               case IPPROTO_ROUTING:
-               case IPPROTO_DSTOPTS:
-                       if (!pf_pull_hdr(m, off, &ext, sizeof(ext), NULL,
-                           NULL, AF_INET6))
-                               goto shortpkt;
-                       extoff = off;
-                       if (proto == IPPROTO_AH)
-                               off += (ext.ip6e_len + 2) * 4;
-                       else
-                               off += (ext.ip6e_len + 1) * 8;
-                       proto = ext.ip6e_nxt;
-                       break;
-               case IPPROTO_HOPOPTS:
-                       if (!pf_pull_hdr(m, off, &ext, sizeof(ext), NULL,
-                           NULL, AF_INET6))
-                               goto shortpkt;
-                       extoff = off;
-                       optend = off + (ext.ip6e_len + 1) * 8;
-                       ooff = off + sizeof(ext);
-                       do {
-                               if (!pf_pull_hdr(m, ooff, &opt.ip6o_type,
-                                   sizeof(opt.ip6o_type), NULL, NULL,
-                                   AF_INET6))
-                                       goto shortpkt;
-                               if (opt.ip6o_type == IP6OPT_PAD1) {
-                                       ooff++;
-                                       continue;
-                               }
-                               if (!pf_pull_hdr(m, ooff, &opt, sizeof(opt),
-                                   NULL, NULL, AF_INET6))
-                                       goto shortpkt;
-                               if (ooff + sizeof(opt) + opt.ip6o_len > optend)
-                                       goto drop;
-                               if (opt.ip6o_type == IP6OPT_JUMBO)
-                                       goto drop;
-                               ooff += sizeof(opt) + opt.ip6o_len;
-                       } while (ooff < optend);
-
-                       off = optend;
-                       proto = ext.ip6e_nxt;
-                       break;
-               default:
-                       terminal = 1;
-                       break;
-               }
-       } while (!terminal);
-
-       if (sizeof(struct ip6_hdr) + plen > m->m_pkthdr.len)
-               goto shortpkt;
-
-       return (PF_PASS);
-
- fragment:
-       if (pd->flags & PFDESC_IP_REAS)
+       if (!pf_pull_hdr(m, off, &frag, sizeof(frag), NULL, reason, AF_INET6))
                return (PF_DROP);
-       if (sizeof(struct ip6_hdr) + plen > m->m_pkthdr.len)
-               goto shortpkt;
-
-       if (!pf_pull_hdr(m, off, &frag, sizeof(frag), NULL, NULL, AF_INET6))
-               goto shortpkt;
 
        /* Offset now points to data portion. */
        off += sizeof(frag);
 
-       /* Returns PF_DROP or *m0 is NULL or completely reassembled mbuf. */
-       if (pf_reassemble6(m0, &frag, off, extoff, reason) != PF_PASS)
-               return (PF_DROP);
-       m = *m0;
-       if (m == NULL)
-               return (PF_DROP);
+       if (nxt == IPPROTO_FRAGMENT) {
+               if (pd->flags & PFDESC_IP_REAS)
+                       return (PF_DROP);
 
-       pd->flags |= PFDESC_IP_REAS;
-       goto again;
+               /* Returns PF_DROP or *m0 is NULL or completely reassembled
+                * mbuf. */
+               if (pf_reassemble6(m0, &frag, off, extoff, reason) != PF_PASS)
+                       return (PF_DROP);
 
- shortpkt:
-       REASON_SET(reason, PFRES_SHORT);
-       if (r != NULL && r->log)
-               PFLOG_PACKET(kif, m, PF_DROP, *reason, r, NULL, NULL, pd, 1);
-       return (PF_DROP);
+               pd->flags |= PFDESC_IP_REAS;
+               m = *m0;
+               if (m == NULL)
+                       return (PF_DROP);
+               goto again;
+       }
 
- drop:
-       REASON_SET(reason, PFRES_NORM);
-       if (r != NULL && r->log)
-               PFLOG_PACKET(kif, m, PF_DROP, *reason, r, NULL, NULL, pd, 1);
-       return (PF_DROP);
+       return (PF_PASS);
 }
 #endif /* INET6 */
 

Reply via email to