Re: IPv6: Invalid nd6 entry created for an RA without an lladdr

2019-10-02 Thread Ryan Stone
Ah, that’s my mistake. I originally saw the issue on an older FreeBSD release 
and missed that the code had changed subtly when I looked up the version in 
head. r328552 fixed this issue already

Thanks for the sanity check. 

Sent from my iPhone

> On Oct 2, 2019, at 6:51 PM, 神明達哉  wrote:
> 
> At Wed, 2 Oct 2019 17:04:23 -0400,
> Ryan Stone  wrote:
> > 
> > At work, our product is putting through an IPv6 conformance test and
> > it's found an issue in our handling of Routing Advertisements (RAs).
> > If we receive an RA that does not specify an lladdr, then
> > nd6_cache_lladdr() is called with lladdr NULL:
> > 
> > https://svnweb.freebsd.org/base/head/sys/netinet6/nd6.c?revision=347984=markup#l1961
> > 
> > In this case, the linkhdr cache is never initialized, but we still put
> > the entry in the STALE state at line 2032.
> 
> If lladdr is NULL shouldn't it reach line 2032?
> 
> if (lladdr != NULL) { /* (7) */
> nd6_llinfo_setstate(ln, ND6_LLINFO_STALE);
> EVENTHANDLER_INVOKE(lle_event, ln,
>LLENTRY_RESOLVED);
> }
> 
> In any case, if a host receives an RA without a link-layer address
> option and no neighbor cache entry exists for the RA sender at that
> point, it shouldn't set the state of a newly created cache entry to
> STALE.  While RFC4861 is not so explicit about this particular
> condition, I believe it's pretty clear from Section 6.3.4 of the RFC
> (it may even be questionable just to create an entry in this case, but
> that's probably within the scope of acceptable implementation choices
> as long as the entry is a mere placeholder).  I also believe FreeBSD
> used to not do this (correctly), so if it currently sets it to STALE
> it's quite likely to be some regression.
> 
> --
> JINMEI, Tatuya
___
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"


Re: IPv6: Invalid nd6 entry created for an RA without an lladdr

2019-10-02 Thread 神明達哉
At Wed, 2 Oct 2019 17:04:23 -0400,
Ryan Stone  wrote:
>
> At work, our product is putting through an IPv6 conformance test and
> it's found an issue in our handling of Routing Advertisements (RAs).
> If we receive an RA that does not specify an lladdr, then
> nd6_cache_lladdr() is called with lladdr NULL:
>
>
https://svnweb.freebsd.org/base/head/sys/netinet6/nd6.c?revision=347984=markup#l1961
>
> In this case, the linkhdr cache is never initialized, but we still put
> the entry in the STALE state at line 2032.

If lladdr is NULL shouldn't it reach line 2032?

if (lladdr != NULL) { /* (7) */
nd6_llinfo_setstate(ln, ND6_LLINFO_STALE);
EVENTHANDLER_INVOKE(lle_event, ln,
   LLENTRY_RESOLVED);
}

In any case, if a host receives an RA without a link-layer address
option and no neighbor cache entry exists for the RA sender at that
point, it shouldn't set the state of a newly created cache entry to
STALE.  While RFC4861 is not so explicit about this particular
condition, I believe it's pretty clear from Section 6.3.4 of the RFC
(it may even be questionable just to create an entry in this case, but
that's probably within the scope of acceptable implementation choices
as long as the entry is a mere placeholder).  I also believe FreeBSD
used to not do this (correctly), so if it currently sets it to STALE
it's quite likely to be some regression.

--
JINMEI, Tatuya
___
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"


IPv6: Invalid nd6 entry created for an RA without an lladdr

2019-10-02 Thread Ryan Stone
At work, our product is putting through an IPv6 conformance test and
it's found an issue in our handling of Routing Advertisements (RAs).
If we receive an RA that does not specify an lladdr, then
nd6_cache_lladdr() is called with lladdr NULL:

https://svnweb.freebsd.org/base/head/sys/netinet6/nd6.c?revision=347984=markup#l1961

In this case, the linkhdr cache is never initialized, but we still put
the entry in the STALE state at line 2032.

Because the entry is in the STALE state, nd6_resolve_slow() will
happily return the uninitialized data to callers, causing us to send
packets with a garbage link-layer header:

https://svnweb.freebsd.org/base/head/sys/netinet6/nd6.c?revision=347984=markup#l2410


I'm unsure what the standard says is the right behaviour in this
situation and before I start digging through RFCs, I was wondering if
anybody knew what the right thing to do is.  I've played with not
putting the nd6 entry into the STALE state if we don't have an lladdr,
and while it seems to work I'm unsure if it's the right thing to do.


If people are curious, the RA-handling code that calls into
nd6_cache_lladdr can be found here:

https://svnweb.freebsd.org/base/head/sys/netinet6/nd6_rtr.c?revision=348121=markup#l185
___
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"