Index | Thread | Search

From:
Bjorn Ketelaars <bket@openbsd.org>
Subject:
Re: Fwd: Re: PPPoE assigns link local fe80::200:0:0:1 instead of fe80::1
To:
Stefan Sperling <stsp@stsp.name>
Cc:
tech@openbsd.org
Date:
Tue, 21 Jul 2026 07:08:50 +0200

Download raw body.

Thread
On Mon 20/07/2026 22:07, Stefan Sperling wrote:
> On Mon, Jul 20, 2026 at 09:26:01PM +0200, Bjorn Ketelaars wrote:
> > Bringing this from misc@ to tech@ as I'm looking for comments/OK.
> > 
> > 
> > netinet6: keep negotiated IPv6CP IFID unchanged for link-local address
> > 
> > When sppp/PPPoE installs a peer-suggested IPv6CP interface identifier,
> > in6_ifattach_linklocal() forced EUI64 G/U bit adjustments on the
> > provided IFID. This incorrectly rewrote a valid negotiated IFID such as
> > ::1 into ::200:0:0:1, yielding fe80::200:0:0:1 instead of fe80::1.
> > 
> > Do not apply EUI64 bit mangling when an explicit IFID is supplied; use
> > the negotiated value as-is (RFC 5072). Other callers are unaffected
> > since they pass NULL and continue using in6_get_ifid().
> > 
> > Reported and tested by Martin Crossley <admin AT crossleys DOT biz>.
> > Tested by me on a connection that allows an explicit IFID.
> 
> I am fine with this change in principle. I am not sure if we can safely
> assume that any potential future callers passing an ifid won't need EUI64
> bit mangling.
> 
> Would it make sense to guard EUI64 bit mangling with a check for
> point-to-point interfaces? Something like this?
> 
> 	if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
> 		ifra.ifra_addr.sin6_addr.s6_addr[8] &= ~EUI64_GBIT;
> 		ifra.ifra_addr.sin6_addr.s6_addr[8] |= EUI64_UBIT;
> 	}
>  
> > diff --git sys/netinet6/in6_ifattach.c sys/netinet6/in6_ifattach.c
> > index 975ccbf5056..c8c7033c058 100644
> > --- sys/netinet6/in6_ifattach.c
> > +++ sys/netinet6/in6_ifattach.c
> > @@ -244,8 +244,6 @@ in6_ifattach_linklocal(struct ifnet *ifp, struct in6_addr *ifid)
> >  		ifra.ifra_addr.sin6_addr.s6_addr16[0] = htons(0xfe80);
> >  		ifra.ifra_addr.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
> >  		ifra.ifra_addr.sin6_addr.s6_addr32[1] = 0;
> > -		ifra.ifra_addr.sin6_addr.s6_addr[8] &= ~EUI64_GBIT;
> > -		ifra.ifra_addr.sin6_addr.s6_addr[8] |= EUI64_UBIT;
> >  	} else
> >  		in6_get_ifid(ifp, &ifra.ifra_addr.sin6_addr);
> >  

Agree, checking for IFF_POINTOPOINT is more conservative and ensures we
don't inadvertently alter behaviour for non-P2P interfaces if another
caller passes an explicit IFID in the future.

Updated diff below.

OK?


diff --git sys/netinet6/in6_ifattach.c sys/netinet6/in6_ifattach.c
index 975ccbf5056..b1ab5f3437f 100644
--- sys/netinet6/in6_ifattach.c
+++ sys/netinet6/in6_ifattach.c
@@ -244,8 +244,12 @@ in6_ifattach_linklocal(struct ifnet *ifp, struct in6_addr *ifid)
 		ifra.ifra_addr.sin6_addr.s6_addr16[0] = htons(0xfe80);
 		ifra.ifra_addr.sin6_addr.s6_addr16[1] = htons(ifp->if_index);
 		ifra.ifra_addr.sin6_addr.s6_addr32[1] = 0;
-		ifra.ifra_addr.sin6_addr.s6_addr[8] &= ~EUI64_GBIT;
-		ifra.ifra_addr.sin6_addr.s6_addr[8] |= EUI64_UBIT;
+
+		/* RFC5072: Use negotiated P2P ifid as-is. */
+		if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
+			ifra.ifra_addr.sin6_addr.s6_addr[8] &= ~EUI64_GBIT;
+			ifra.ifra_addr.sin6_addr.s6_addr[8] |= EUI64_UBIT;
+		}
 	} else
 		in6_get_ifid(ifp, &ifra.ifra_addr.sin6_addr);