On Thu, Apr 21, 2022 at 07:08:38PM +0200, Alexander Bluhm wrote:
> Hi,
>
> IGMP and ICMP6 for multicast packets have router alert options.
> Per default pf drops all IP packets with options. Usually people
> ask what is wrong until someone points out that they have to use a
> pf rule with allow-opts.
>
> As this is normal behavior and our kernel generates such packets,
> the pf default is bad.
>
> Diff is untested, but otto@ and florian@ could try it.
Tested this with success on two hosts that hasd issues because their
outgoing icmp6 messages were blcoked.
> Currently it allows all options. Should I make it specific to
> router alert with IGMP or ICMP6?
To me it looks like the icmp6 case already is limited to MLD?
-Otto
>
> bluhm
>
> Index: net/pf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> retrieving revision 1.1126
> diff -u -p -r1.1126 pf.c
> --- net/pf.c 17 Mar 2022 18:27:55 -0000 1.1126
> +++ net/pf.c 21 Apr 2022 16:30:18 -0000
> @@ -6398,6 +6398,9 @@ pf_walk_header(struct pf_pdesc *pd, stru
> 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 = 0;
> /* stop walking over non initial fragments */
> if ((h->ip_off & htons(IP_OFFMASK)) != 0)
> return (PF_PASS);
> @@ -6494,6 +6497,7 @@ pf_walk_header6(struct pf_pdesc *pd, str
> {
> 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;
> @@ -6607,9 +6611,23 @@ pf_walk_header6(struct pf_pdesc *pd, str
> pd->off += (ext.ip6e_len + 1) * 8;
> pd->proto = ext.ip6e_nxt;
> break;
> + case IPPROTO_ICMPV6:
> + 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:
> + pd->badopts = 0;
> + break;
> + }
> + /* FALLTHROUGH */
> 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) :
>
