Hello,
</snip>
> 
> Checking that the TTL equals 1 is a good thing.  We should prevent
> that someone is forwarding such packets.
> 
> The router alert is a hint to routers on the way to look at these
> packets.  If they are missing, no harm is done.  Maybe some multicast
> does not work.  But there is no security argument to filter these.
> 
> I have seen IGMP packets without router alert.  Our stack has fixed
> that in OpenBSD 5.6.  Don't believe what is written in RFCs.
> 
> ----------------------------
> revision 1.40
> date: 2014/05/12 09:15:00;  author: mpi;  state: Exp;  lines: +28 -5;
> Includes a router altert option (RAO) in IGMP packets.   Without this
> option, required by the RFC2236, some L3 switches do not examine the
> packets.
> 
> Based on FreeBSD's r14622 via Florian Riehm on tech@. ok bluhm@, jca@
> ----------------------------

    I see. new diff keeps check for ttl/hop-limit to be 1.
    it also keeps check for link-local source address in IPv6


OK ? or should I also drop a check for link-local source address
in IPv6?

thanks and
regards
sashan

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index f15e1ead8c0..2187d895749 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -6456,8 +6456,15 @@ pf_walk_header(struct pf_pdesc *pd, struct ip *h, 
u_short *reason)
        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);
+       if (pd->proto == IPPROTO_IGMP) {
+               /* According to RFC 1112 ttl must be set to 1. */
+               if (h->ip_ttl != 1) {
+                       DPFPRINTF(LOG_NOTICE, "TTL in IGMP must be 1");
+                       REASON_SET(reason, PFRES_IPOPTIONS);
+                       return (PF_DROP);
+               } else
+                       CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
+       }
        /* stop walking over non initial fragments */
        if ((h->ip_off & htons(IP_OFFMASK)) != 0)
                return (PF_PASS);
@@ -6698,7 +6705,21 @@ pf_walk_header6(struct pf_pdesc *pd, struct ip6_hdr *h, 
u_short *reason)
                        case MLD_LISTENER_REPORT:
                        case MLD_LISTENER_DONE:
                        case MLDV2_LISTENER_REPORT:
-                               CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
+                               /*
+                                * According to RFC 2710 all MLD messages are
+                                * sent with hop-limit (ttl) set to 1, and link
+                                * local source address.  If either one is
+                                * missing then MLD message is invalid and
+                                * should be discarded.
+                                */
+                               if ((h->ip6_hlim == 1) &&
+                                   IN6_IS_ADDR_LINKLOCAL(&h->ip6_src))
+                                       CLR(pd->badopts, PF_OPT_ROUTER_ALERT);
+                               else {
+                                       DPFPRINTF(LOG_NOTICE, "invalid MLD");
+                                       REASON_SET(reason, PFRES_IPOPTIONS);
+                                       return (PF_DROP);
+                               }
                                break;
                        }
                        return (PF_PASS);

Reply via email to