From 214dd8280ca1918f257af21db60567840cf3ab4f Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 29 Apr 2022 08:58:49 +0000 Subject: [PATCH] IGMP and ICMP6 MLD packets always have the router alert option set. pf blocked IPv4 options and IPv6 option header by default. This forced users to set allow-opts in pf rules. Better let multicast work by default. Detect router alerts by parsing IP options and hop by hop headers. If the packet has only this option and is a multicast control packet, do not block it due to bad options. tested by otto@; OK sashan@ --- sys/net/pf.c | 136 ++++++++++++++++++++++++++++++++++++------- sys/net/pfvar_priv.h | 5 +- 2 files changed, 119 insertions(+), 22 deletions(-) diff --git a/sys/net/pf.c b/sys/net/pf.c index ffbdf790fe7..f15e1ead8c0 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf.c,v 1.1126 2022/03/17 18:27:55 sthen Exp $ */ +/* $OpenBSD: pf.c,v 1.1127 2022/04/29 08:58:49 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -227,6 +227,8 @@ u_int16_t pf_calc_mss(struct pf_addr *, sa_family_t, int, static __inline int pf_set_rt_ifp(struct pf_state *, struct pf_addr *, sa_family_t, struct pf_src_node **); struct pf_divert *pf_get_divert(struct mbuf *); +int pf_walk_option(struct pf_pdesc *, struct ip *, + int, int, u_short *); int pf_walk_header(struct pf_pdesc *, struct ip *, u_short *); int pf_walk_option6(struct pf_pdesc *, struct ip6_hdr *, @@ -3956,7 +3958,7 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm, rtable_l2(ctx.act.rtableid) != pd->rdomain) pd->destchg = 1; - if (r->action == PF_PASS && pd->badopts && ! r->allow_opts) { + if (r->action == PF_PASS && pd->badopts != 0 && ! r->allow_opts) { REASON_SET(&ctx.reason, PFRES_IPOPTIONS); #if NPFLOG > 0 pd->pflog |= PF_LOG_FORCE; @@ -6381,6 +6383,55 @@ pf_get_divert(struct mbuf *m) return ((struct pf_divert *)(mtag + 1)); } +int +pf_walk_option(struct pf_pdesc *pd, struct ip *h, int off, int end, + u_short *reason) +{ + uint8_t type, length, opts[15 * 4 - sizeof(struct ip)]; + + KASSERT(end - off <= sizeof(opts)); + m_copydata(pd->m, off, end - off, opts); + end -= off; + off = 0; + + while (off < end) { + type = opts[off]; + if (type == IPOPT_EOL) + break; + if (type == IPOPT_NOP) { + off++; + continue; + } + if (off + 2 > end) { + DPFPRINTF(LOG_NOTICE, "IP length opt"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + length = opts[off + 1]; + if (length < 2) { + DPFPRINTF(LOG_NOTICE, "IP short opt"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + if (off + length > end) { + DPFPRINTF(LOG_NOTICE, "IP long opt"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + switch (type) { + case IPOPT_RA: + SET(pd->badopts, PF_OPT_ROUTER_ALERT); + break; + default: + SET(pd->badopts, PF_OPT_OTHER); + break; + } + off += length; + } + + return (PF_PASS); +} + int pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short *reason) { @@ -6393,11 +6444,20 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short *reason) REASON_SET(reason, PFRES_SHORT); return (PF_DROP); } - if (hlen != sizeof(struct ip)) - pd->badopts++; + if (hlen != sizeof(struct ip)) { + if (pf_walk_option(pd, h, pd->off + sizeof(struct ip), + pd->off + hlen, reason) != PF_PASS) + return (PF_DROP); + /* header options which contain only padding is fishy */ + if (pd->badopts == 0) + SET(pd->badopts, PF_OPT_OTHER); + } end = pd->off + ntohs(h->ip_len); pd->off += hlen; pd->proto = h->ip_p; + /* IGMP packets have router alert options, allow them */ + if (pd->proto == IPPROTO_IGMP) + CLR(pd->badopts, PF_OPT_ROUTER_ALERT); /* stop walking over non initial fragments */ if ((h->ip_off & htons(IP_OFFMASK)) != 0) return (PF_PASS); @@ -6455,7 +6515,10 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end, return (PF_DROP); } switch (opt.ip6o_type) { + case IP6OPT_PADN: + break; case IP6OPT_JUMBO: + SET(pd->badopts, PF_OPT_JUMBO); if (pd->jumbolen != 0) { DPFPRINTF(LOG_NOTICE, "IPv6 multiple jumbo"); REASON_SET(reason, PFRES_IPOPTIONS); @@ -6480,7 +6543,11 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end, return (PF_DROP); } break; + case IP6OPT_ROUTER_ALERT: + SET(pd->badopts, PF_OPT_ROUTER_ALERT); + break; default: + SET(pd->badopts, PF_OPT_OTHER); break; } off += sizeof(opt) + opt.ip6o_len; @@ -6494,6 +6561,7 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason) { struct ip6_frag frag; struct ip6_ext ext; + struct icmp6_hdr icmp6; struct ip6_rthdr rthdr; u_int32_t end; int hdr_cnt, fraghdr_cnt = 0, rthdr_cnt = 0; @@ -6506,9 +6574,22 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason) for (hdr_cnt = 0; hdr_cnt < pf_hdr_limit; hdr_cnt++) { switch (pd->proto) { case IPPROTO_ROUTING: - case IPPROTO_HOPOPTS: case IPPROTO_DSTOPTS: - pd->badopts++; + SET(pd->badopts, PF_OPT_OTHER); + break; + case IPPROTO_HOPOPTS: + if (!pf_pull_hdr(pd->m, pd->off, &ext, sizeof(ext), + NULL, reason, AF_INET6)) { + DPFPRINTF(LOG_NOTICE, "IPv6 short exthdr"); + return (PF_DROP); + } + if (pf_walk_option6(pd, h, pd->off + sizeof(ext), + pd->off + (ext.ip6e_len + 1) * 8, reason) + != PF_PASS) + return (PF_DROP); + /* option header which contains only padding is fishy */ + if (pd->badopts == 0) + SET(pd->badopts, PF_OPT_OTHER); break; } switch (pd->proto) { @@ -6587,19 +6668,11 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason) /* reassembly needs the ext header before the frag */ if (pd->fragoff == 0) pd->extoff = pd->off; - if (pd->proto == IPPROTO_HOPOPTS && pd->fragoff == 0) { - if (pf_walk_option6(pd, h, - pd->off + sizeof(ext), - pd->off + (ext.ip6e_len + 1) * 8, reason) - != PF_PASS) - return (PF_DROP); - if (ntohs(h->ip6_plen) == 0 && - pd->jumbolen != 0) { - DPFPRINTF(LOG_NOTICE, - "IPv6 missing jumbo"); - REASON_SET(reason, PFRES_IPOPTIONS); - return (PF_DROP); - } + if (pd->proto == IPPROTO_HOPOPTS && pd->fragoff == 0 && + ntohs(h->ip6_plen) == 0 && pd->jumbolen != 0) { + DPFPRINTF(LOG_NOTICE, "IPv6 missing jumbo"); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); } if (pd->proto == IPPROTO_AH) pd->off += (ext.ip6e_len + 2) * 4; @@ -6607,9 +6680,30 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, u_short *reason) pd->off += (ext.ip6e_len + 1) * 8; pd->proto = ext.ip6e_nxt; break; + case IPPROTO_ICMPV6: + /* fragments may be short, ignore inner header then */ + if (pd->fragoff != 0 && end < pd->off + sizeof(icmp6)) { + pd->off = pd->fragoff; + pd->proto = IPPROTO_FRAGMENT; + return (PF_PASS); + } + if (!pf_pull_hdr(pd->m, pd->off, &icmp6, sizeof(icmp6), + NULL, reason, AF_INET6)) { + DPFPRINTF(LOG_NOTICE, "IPv6 short icmp6hdr"); + return (PF_DROP); + } + /* ICMP multicast packets have router alert options */ + switch (icmp6.icmp6_type) { + case MLD_LISTENER_QUERY: + case MLD_LISTENER_REPORT: + case MLD_LISTENER_DONE: + case MLDV2_LISTENER_REPORT: + CLR(pd->badopts, PF_OPT_ROUTER_ALERT); + break; + } + return (PF_PASS); case IPPROTO_TCP: case IPPROTO_UDP: - case IPPROTO_ICMPV6: /* fragments may be short, ignore inner header then */ if (pd->fragoff != 0 && end < pd->off + (pd->proto == IPPROTO_TCP ? sizeof(struct tcphdr) : @@ -7167,7 +7261,7 @@ done: if (action != PF_DROP) { if (s) { /* The non-state case is handled in pf_test_rule() */ - if (action == PF_PASS && pd.badopts && + if (action == PF_PASS && pd.badopts != 0 && !(s->state_flags & PFSTATE_ALLOWOPTS)) { action = PF_DROP; REASON_SET(&reason, PFRES_IPOPTIONS); diff --git a/sys/net/pfvar_priv.h b/sys/net/pfvar_priv.h index 092258d4159..ec8f61a800d 100644 --- a/sys/net/pfvar_priv.h +++ b/sys/net/pfvar_priv.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pfvar_priv.h,v 1.9 2022/04/08 18:17:24 bluhm Exp $ */ +/* $OpenBSD: pfvar_priv.h,v 1.10 2022/04/29 08:58:49 bluhm Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -180,6 +180,9 @@ struct pf_pdesc { u_int32_t fragoff; /* fragment header offset */ u_int32_t jumbolen; /* length from v6 jumbo header */ u_int32_t badopts; /* v4 options or v6 routing headers */ +#define PF_OPT_OTHER 0x0001 +#define PF_OPT_JUMBO 0x0002 +#define PF_OPT_ROUTER_ALERT 0x0004 u_int16_t rdomain; /* original routing domain */ u_int16_t virtual_proto; -- 2.20.1