From nobody Wed Jul 23 14:23:48 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4bnGb867wgz62P7B; Wed, 23 Jul 2025 14:23:48 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4bnGb84K9Zz3pTb; Wed, 23 Jul 2025 14:23:48 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753280628; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=yA6Zc5de70LXAx9oiMfzH5+5fB4DX2gb8mzAGpV6GYM=; b=ewXcK7kguRLz3Fc3ifrawnFWtcdwAqjz48rX35IKhz+TcfVqhq3iDvpfsOobTQ0wE6afWh LQqJsHfQlw2XYJ1xE8QS/yWnDbrDYI8eMAEBwmD+yaUfrGYpTwffYasiWs8+6G8Oz/6bGc SwkP0BzFrOCY2/KYR3+iE6490ITPqmaTaw2EXwl+2PN/yF1XfKkoT0pjZKWCeC0zuHTaTU rwlJSfah4XWCEBKeGDq6oPr6NYp0DSzWE0eKh9xxwLprnqgkjbWGjv0NR/DICphRoKpqIP aE8yNZYO7VFY4Fp/r/cFbZ6Zm243Ta29Qr2wzWXXxPrVsP11h22jDMTCZ2+7Sg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1753280628; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=yA6Zc5de70LXAx9oiMfzH5+5fB4DX2gb8mzAGpV6GYM=; b=JPe2Mm2nf09rlRqNMXLP1k+7gEfoufZYPDkzWSRlGi5dY19uaOdxAsbr1LZhnzWbVl+dLd lmkXxfvcc+bqxXH1Bt7FSkSA4mbyePS8HQX7Bf8SQFLRLWPDuXK1Tg6R0Bi3E1fTGEg6dW 90lDZYQPzt/ygkOV+YjElABDzYxIKvBMOYcbjJWQXWKixoNu+dGSjmqmLcf9HGmoGFGC78 SmLPksHAfg0R0QMMc35GMY1RoLdx2vSVBsAQvEUfvkkhZAos7PeON9ey38KYwXurx+4o09 zHTVDIjSVtfN6ZtbHv9rcg84Mqab8CG+1aZBcktst/XH9EIDrEP5FE+AwCt3lg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1753280628; a=rsa-sha256; cv=none; b=TGxwbz6TfBMGI4kgxglrY66hv160HIssBtQ1CBNrW2vbvJnm0RiKwj9PeeUn5UnphVMz3u yUgUAjzL5Af+CxAloQd6f4fOXLhemOBL4Ljkr2FBjFmiAYgnJbm83iR/i9WK9mcj9fknCl PrXtMnl5sJjmw2DeIEoqlQlCxjMoQlrOMF8wLZCZ7SwSnDayGsfZEQc/jcTn+c69ZUhLMh movOXGHiPk+z5lJaNA/YZeRPJy+gUZdEOMU53s3N1Juphrq30OMPzGJh5f1Zzkcn9ei081 ELeonxyyu8rkvlWaypq9AQTw37BOikUgGTU24ErFuWe1JYIvXP4DnMD0vRnbGA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4bnGb83tRJz228; Wed, 23 Jul 2025 14:23:48 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 56NENmHb031180; Wed, 23 Jul 2025 14:23:48 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 56NENmn6031177; Wed, 23 Jul 2025 14:23:48 GMT (envelope-from git) Date: Wed, 23 Jul 2025 14:23:48 GMT Message-Id: <202507231423.56NENmn6031177@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 2eddb410f789 - main - pf: accept IP options for IGMP and ICMP6 MLD List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 2eddb410f7892ff637bbf058b58f2644f329e8d9 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=2eddb410f7892ff637bbf058b58f2644f329e8d9 commit 2eddb410f7892ff637bbf058b58f2644f329e8d9 Author: Kristof Provost AuthorDate: 2025-07-16 12:40:05 +0000 Commit: Kristof Provost CommitDate: 2025-07-23 13:35:44 +0000 pf: accept IP options for IGMP and ICMP6 MLD 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@ Relnotes: yes Obtained from: OpenBSD, bluhm , 214dd8280c Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/net/pfvar.h | 3 ++ sys/netpfil/pf/pf.c | 132 +++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 117 insertions(+), 18 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index 452a8eb4024b..efc884398e7b 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -1676,6 +1676,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 *ip_sum; u_int16_t flags; /* Let SCRUB trigger behavior in diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 1310445c4063..0a951815656e 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -375,6 +375,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_option(struct pf_pdesc *, struct ip *, + int, int, u_short *); static int pf_walk_header(struct pf_pdesc *, struct ip *, u_short *); #ifdef INET6 static int pf_walk_option6(struct pf_pdesc *, struct ip6_hdr *, @@ -9797,6 +9799,55 @@ pf_dummynet_route(struct pf_pdesc *pd, struct pf_kstate *s, return (0); } +static 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)]; + + MPASS(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(PF_DEBUG_MISC, ("IP length opt\n")); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + length = opts[off + 1]; + if (length < 2) { + DPFPRINTF(PF_DEBUG_MISC, ("IP short opt\n")); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + if (off + length > end) { + DPFPRINTF(PF_DEBUG_MISC, ("IP long opt\n")); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); + } + switch (type) { + case IPOPT_RA: + pd->badopts |= PF_OPT_ROUTER_ALERT; + break; + default: + pd->badopts |= PF_OPT_OTHER; + break; + } + off += length; + } + + return (PF_PASS); +} + static int pf_walk_header(struct pf_pdesc *pd, struct ip *h, u_short *reason) { @@ -9809,11 +9860,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) + 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) + pd->badopts &= ~PF_OPT_ROUTER_ALERT; /* stop walking over non initial fragments */ if ((h->ip_off & htons(IP_OFFMASK)) != 0) return (PF_PASS); @@ -9870,7 +9930,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: + pd->badopts |= PF_OPT_JUMBO; if (pd->jumbolen != 0) { DPFPRINTF(PF_DEBUG_MISC, ("IPv6 multiple jumbo")); REASON_SET(reason, PFRES_IPOPTIONS); @@ -9895,7 +9958,11 @@ pf_walk_option6(struct pf_pdesc *pd, struct ip6_hdr *h, int off, int end, return (PF_DROP); } break; + case IP6OPT_ROUTER_ALERT: + pd->badopts |= PF_OPT_ROUTER_ALERT; + break; default: + pd->badopts |= PF_OPT_OTHER; break; } off += sizeof(opt) + opt.ip6o_len; @@ -9909,6 +9976,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; uint32_t end; int hdr_cnt, fraghdr_cnt = 0, rthdr_cnt = 0; @@ -9920,9 +9988,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++; + 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(PF_DEBUG_MISC, ("IPv6 short exthdr\n")); + 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) + pd->badopts |= PF_OPT_OTHER; break; } switch (pd->proto) { @@ -10001,18 +10082,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(PF_DEBUG_MISC, - ("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(PF_DEBUG_MISC, ("IPv6 missing jumbo\n")); + REASON_SET(reason, PFRES_IPOPTIONS); + return (PF_DROP); } if (pd->proto == IPPROTO_AH) pd->off += (ext.ip6e_len + 2) * 4; @@ -10020,10 +10094,32 @@ 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(PF_DEBUG_MISC, + ("IPv6 short icmp6hdr\n")); + 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: + pd->badopts &= ~PF_OPT_ROUTER_ALERT; + break; + } + return (PF_PASS); case IPPROTO_TCP: case IPPROTO_UDP: case IPPROTO_SCTP: - 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) : @@ -10733,7 +10829,7 @@ done: if (s) memcpy(&pd.act, &s->act, sizeof(s->act)); - if (action == PF_PASS && pd.badopts && !pd.act.allow_opts) { + if (action == PF_PASS && pd.badopts != 0 && !pd.act.allow_opts) { action = PF_DROP; REASON_SET(&reason, PFRES_IPOPTIONS); pd.act.log = PF_LOG_FORCE;