On Tue, May 03, 2022 at 02:12:36PM +0200, Claudio Jeker wrote:
> On Tue, May 03, 2022 at 02:08:33PM +0200, Alexandr Nedvedicky wrote:
> > Hello
> > 
> > On Tue, May 03, 2022 at 10:44:48AM +0200, Claudio Jeker wrote:
> > </snip>
> > > 
> > > The RFC does not use the usual MUST to enforce any of this.
> > > So yes, we should probably not be too strict because there is no way to
> > > force accept the packet when pf_walk_header() returns PF_DROP.
> > > 
> > > I agree that the TTL MUST be 1. Also the destination address must be a
> > > multicast address (IGMP to a unicast IP makes no sense).
> > >  
> > 
> >     updated diff below requires destination address to be multicast for 
> > IGMP.
> > 
> > thanks and
> > regards
> > sashan
> > 
> > --------8<----------------8<----------------8<----------------8<--------
> > diff --git a/sys/net/pf.c b/sys/net/pf.c
> > index f15e1ead8c0..feb7965c427 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)
> > +   if (pd->proto == IPPROTO_IGMP) {
> > +           /* According to RFC 1112 ttl must be set to 1. */
> > +           if ((h->ip_ttl != 1) || !IN_MULTICAST(h->ip_dst.s_addr)) {
> > +                   DPFPRINTF(LOG_NOTICE, "TTL in IGMP must be 1");
> 
> Maybe use "Invalid IGMP" here (which is similar to the IPv6 case).
> Apart from that OK claudio
> 

    will fix it before commit.

thanks and
regards
sashan

Reply via email to