Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: RFC 6724 rule 5.5
To:
tech <tech@openbsd.org>
Date:
Thu, 18 Apr 2024 22:11:05 +0200

Download raw body.

Thread
On Thu, Apr 18, 2024 at 12:45:41PM +0200, Florian Obser wrote:
> On 2024-04-16 19:22 +02, Alexander Bluhm <bluhm@openbsd.org> wrote:
> > On Tue, Apr 16, 2024 at 06:23:07PM +0200, Florian Obser wrote:
> >> On 2024-04-16 14:28 +02, Florian Obser <florian@openbsd.org> wrote:
> >> > Disregard please, I'll get rid of the nonsensical IFF_LOOPBACK case
> >> > first I think that will make this slightly easier / compacter.
> >> 
> >> OK?
> >
> > The following chunk is wrong and not necessary.  It you remove that
> > part from the diff, it is OK bluhm@
> 
> Next try. I'll get this in, even if it kills me.
> This brings back the IFF_LOOPBACK cases and passes the regress tests
> that blew up for anton.
> 
> OK?

OK bluhm@

> diff --git sbin/slaacd/engine.c sbin/slaacd/engine.c
> index 000a9fcf413..72798552824 100644
> --- sbin/slaacd/engine.c
> +++ sbin/slaacd/engine.c
> @@ -2130,6 +2130,7 @@ configure_address(struct address_proposal *addr_proposal)
>  
>  	address.if_index = addr_proposal->if_index;
>  	memcpy(&address.addr, &addr_proposal->addr, sizeof(address.addr));
> +	memcpy(&address.gw, &addr_proposal->from, sizeof(address.gw));
>  	memcpy(&address.mask, &addr_proposal->mask, sizeof(address.mask));
>  	address.vltime = addr_proposal->vltime;
>  	address.pltime = addr_proposal->pltime;
> diff --git sbin/slaacd/engine.h sbin/slaacd/engine.h
> index 7a8551d2c50..079ef9a64e0 100644
> --- sbin/slaacd/engine.h
> +++ sbin/slaacd/engine.h
> @@ -19,6 +19,7 @@
>  struct imsg_configure_address {
>  	uint32_t		 if_index;
>  	struct sockaddr_in6	 addr;
> +	struct sockaddr_in6	 gw;
>  	struct in6_addr		 mask;
>  	uint32_t		 vltime;
>  	uint32_t		 pltime;
> diff --git sbin/slaacd/slaacd.c sbin/slaacd/slaacd.c
> index 4d1786361f7..d7fcec2601b 100644
> --- sbin/slaacd/slaacd.c
> +++ sbin/slaacd/slaacd.c
> @@ -632,6 +632,8 @@ configure_interface(struct imsg_configure_address *address)
>  
>  	memcpy(&in6_addreq.ifra_addr, &address->addr,
>  	    sizeof(in6_addreq.ifra_addr));
> +	memcpy(&in6_addreq.ifra_dstaddr, &address->gw,
> +	    sizeof(in6_addreq.ifra_dstaddr));
>  	memcpy(&in6_addreq.ifra_prefixmask.sin6_addr, &address->mask,
>  	    sizeof(in6_addreq.ifra_prefixmask.sin6_addr));
>  	in6_addreq.ifra_prefixmask.sin6_family = AF_INET6;
> diff --git sys/netinet6/icmp6.c sys/netinet6/icmp6.c
> index 2ec50eb06a2..e3c302969f1 100644
> --- sys/netinet6/icmp6.c
> +++ sys/netinet6/icmp6.c
> @@ -1164,7 +1164,7 @@ icmp6_reflect(struct mbuf **mp, size_t off, struct sockaddr *sa)
>  			rtfree(rt);
>  			goto bad;
>  		}
> -		ia6 = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
> +		ia6 = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid, rt);
>  		if (ia6 != NULL)
>  			src = &ia6->ia_addr.sin6_addr;
>  		if (src == NULL)
> diff --git sys/netinet6/in6.c sys/netinet6/in6.c
> index 50bceb006cb..62d5e674fbb 100644
> --- sys/netinet6/in6.c
> +++ sys/netinet6/in6.c
> @@ -562,13 +562,19 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
>  		return (EINVAL);
>  
>  	/*
> -	 * The destination address for a p2p link must have a family
> -	 * of AF_UNSPEC or AF_INET6.
> +	 * The destination address for a p2p link or the address of the
> +	 * announcing router for an autoconf address must have a family of
> +	 * AF_UNSPEC or AF_INET6.
>  	 */
> -	if ((ifp->if_flags & IFF_POINTOPOINT) != 0 &&
> -	    ifra->ifra_dstaddr.sin6_family != AF_INET6 &&
> -	    ifra->ifra_dstaddr.sin6_family != AF_UNSPEC)
> -		return (EAFNOSUPPORT);
> +	if ((ifp->if_flags & IFF_POINTOPOINT) ||
> +	    (ifp->if_flags & IFF_LOOPBACK) ||
> +	    (ifra->ifra_flags & IN6_IFF_AUTOCONF)) {
> +		if (ifra->ifra_dstaddr.sin6_family != AF_INET6 &&
> +		    ifra->ifra_dstaddr.sin6_family != AF_UNSPEC)
> +			return (EAFNOSUPPORT);
> +
> +	} else if (ifra->ifra_dstaddr.sin6_family != AF_UNSPEC)
> +			return (EINVAL);
>  
>  	/*
>  	 * validate ifra_prefixmask.  don't check sin6_family, netmask
> @@ -597,27 +603,15 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
>  		 */
>  		plen = in6_mask2len(&ia6->ia_prefixmask.sin6_addr, NULL);
>  	}
> -	/*
> -	 * If the destination address on a p2p interface is specified,
> -	 * and the address is a scoped one, validate/set the scope
> -	 * zone identifier.
> -	 */
> +
>  	dst6 = ifra->ifra_dstaddr;
> -	if ((ifp->if_flags & (IFF_POINTOPOINT|IFF_LOOPBACK)) != 0 &&
> -	    (dst6.sin6_family == AF_INET6)) {
> +	if (dst6.sin6_family == AF_INET6) {
>  		error = in6_check_embed_scope(&dst6, ifp->if_index);
>  		if (error)
>  			return error;
> -	}
> -	/*
> -	 * The destination address can be specified only for a p2p or a
> -	 * loopback interface.  If specified, the corresponding prefix length
> -	 * must be 128.
> -	 */
> -	if (ifra->ifra_dstaddr.sin6_family == AF_INET6) {
> -		if ((ifp->if_flags & (IFF_POINTOPOINT|IFF_LOOPBACK)) == 0)
> -			return (EINVAL);
> -		if (plen != 128)
> +
> +		if (((ifp->if_flags & IFF_POINTOPOINT) ||
> +		    (ifp->if_flags & IFF_LOOPBACK)) && plen != 128)
>  			return (EINVAL);
>  	}
>  	/* lifetime consistency check */
> @@ -652,7 +646,8 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
>  		ia6->ia_addr.sin6_family = AF_INET6;
>  		ia6->ia_addr.sin6_len = sizeof(ia6->ia_addr);
>  		ia6->ia6_updatetime = getuptime();
> -		if ((ifp->if_flags & (IFF_POINTOPOINT | IFF_LOOPBACK)) != 0) {
> +		if ((ifp->if_flags & IFF_POINTOPOINT) ||
> +		    (ifp->if_flags & IFF_LOOPBACK)) {
>  			/*
>  			 * XXX: some functions expect that ifa_dstaddr is not
>  			 * NULL for p2p interfaces.
> @@ -686,10 +681,10 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
>  
>  	/*
>  	 * If a new destination address is specified, scrub the old one and
> -	 * install the new destination.  Note that the interface must be
> -	 * p2p or loopback (see the check above.)
> +	 * install the new destination.
>  	 */
> -	if ((ifp->if_flags & IFF_POINTOPOINT) && dst6.sin6_family == AF_INET6 &&
> +	if (((ifp->if_flags & IFF_POINTOPOINT)  ||
> +	    (ifp->if_flags & IFF_LOOPBACK)) && dst6.sin6_family == AF_INET6 &&
>  	    !IN6_ARE_ADDR_EQUAL(&dst6.sin6_addr, &ia6->ia_dstaddr.sin6_addr)) {
>  		struct ifaddr *ifa = &ia6->ia_ifa;
>  
> @@ -706,6 +701,13 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq *ifra,
>  		ia6->ia_dstaddr = dst6;
>  	}
>  
> +	if ((ifra->ifra_flags & IN6_IFF_AUTOCONF) &&
> +	    dst6.sin6_family == AF_INET6 &&
> +	    !IN6_ARE_ADDR_EQUAL(&dst6.sin6_addr, &ia6->ia_gwaddr.sin6_addr)) {
> +		/* Set or update announcing router */
> +		ia6->ia_gwaddr = dst6;
> +	}
> +
>  	/*
>  	 * Set lifetimes.  We do not refer to ia6t_expire and ia6t_preferred
>  	 * to see if the address is deprecated or invalidated, but initialize
> @@ -1329,13 +1331,21 @@ in6_prefixlen2mask(struct in6_addr *maskp, int len)
>   * return the best address out of the same scope
>   */
>  struct in6_ifaddr *
> -in6_ifawithscope(struct ifnet *oifp, struct in6_addr *dst, u_int rdomain)
> +in6_ifawithscope(struct ifnet *oifp, struct in6_addr *dst, u_int rdomain,
> +    struct rtentry *rt)
>  {
>  	int dst_scope =	in6_addrscope(dst), src_scope, best_scope = 0;
>  	int blen = -1;
>  	struct ifaddr *ifa;
>  	struct ifnet *ifp;
>  	struct in6_ifaddr *ia6_best = NULL;
> +	struct in6_addr *gw6 = NULL;
> +
> +	if (rt) {
> +		if (rt->rt_gateway != NULL &&
> +		    rt->rt_gateway->sa_family == AF_INET6)
> +			gw6 = &(satosin6(rt->rt_gateway)->sin6_addr);
> +	}
>  
>  	if (oifp == NULL) {
>  		printf("%s: output interface is not specified\n", __func__);
> @@ -1460,8 +1470,16 @@ in6_ifawithscope(struct ifnet *oifp, struct in6_addr *dst, u_int rdomain)
>  			/*
>  			 * Rule 5.5: Prefer addresses in a prefix advertised
>  			 * by the next-hop.
> -			 * We do not track this information.
>  			 */
> +			if (gw6) {
> +				struct in6_addr *in6_bestgw, *in6_newgw;
> +
> +				in6_bestgw = &ia6_best->ia_gwaddr.sin6_addr;
> +				in6_newgw = &ifatoia6(ifa)->ia_gwaddr.sin6_addr;
> +				if (!IN6_ARE_ADDR_EQUAL(in6_bestgw, gw6) &&
> +				    IN6_ARE_ADDR_EQUAL(in6_newgw, gw6))
> +					goto replace;
> +			}
>  
>  			/*
>  			 * Rule 6: Prefer matching label.
> diff --git sys/netinet6/in6.h sys/netinet6/in6.h
> index 642a24b1999..db5896beb79 100644
> --- sys/netinet6/in6.h
> +++ sys/netinet6/in6.h
> @@ -404,6 +404,7 @@ struct sockaddr_in6;
>  struct ifaddr;
>  struct in6_ifaddr;
>  struct ifnet;
> +struct rtentry;
>  
>  void	ipv6_input(struct ifnet *, struct mbuf *);
>  struct mbuf *
> @@ -413,7 +414,8 @@ int	in6_cksum(struct mbuf *, uint8_t, uint32_t, uint32_t);
>  void	in6_proto_cksum_out(struct mbuf *, struct ifnet *);
>  int	in6_localaddr(struct in6_addr *);
>  int	in6_addrscope(struct in6_addr *);
> -struct	in6_ifaddr *in6_ifawithscope(struct ifnet *, struct in6_addr *, u_int);
> +struct	in6_ifaddr *in6_ifawithscope(struct ifnet *, struct in6_addr *, u_int,
> +	    struct rtentry *);
>  int	in6_mask2len(struct in6_addr *, u_char *);
>  int	in6_nam2sin6(const struct mbuf *, struct sockaddr_in6 **);
>  int	in6_sa2sin6(struct sockaddr *, struct sockaddr_in6 **);
> diff --git sys/netinet6/in6_src.c sys/netinet6/in6_src.c
> index d6163d254c0..db123c22a2b 100644
> --- sys/netinet6/in6_src.c
> +++ sys/netinet6/in6_src.c
> @@ -162,7 +162,7 @@ in6_pcbselsrc(const struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>  		if (ifp == NULL)
>  			return (ENXIO); /* XXX: better error? */
>  
> -		ia6 = in6_ifawithscope(ifp, dst, rtableid);
> +		ia6 = in6_ifawithscope(ifp, dst, rtableid, NULL);
>  		if_put(ifp);
>  
>  		if (ia6 == NULL)
> @@ -192,7 +192,7 @@ in6_pcbselsrc(const struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>  	if (rt != NULL) {
>  		ifp = if_get(rt->rt_ifidx);
>  		if (ifp != NULL) {
> -			ia6 = in6_ifawithscope(ifp, dst, rtableid);
> +			ia6 = in6_ifawithscope(ifp, dst, rtableid, rt);
>  			if_put(ifp);
>  		}
>  		if (ia6 == NULL) /* xxx scope error ?*/
> @@ -256,7 +256,7 @@ in6_selectsrc(const struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>  		if (ifp == NULL)
>  			return (ENXIO); /* XXX: better error? */
>  
> -		ia6 = in6_ifawithscope(ifp, dst, rtableid);
> +		ia6 = in6_ifawithscope(ifp, dst, rtableid, NULL);
>  		if_put(ifp);
>  
>  		if (ia6 == NULL)
> @@ -280,7 +280,7 @@ in6_selectsrc(const struct in6_addr **in6src, struct sockaddr_in6 *dstsock,
>  			ifp = if_get(htons(dstsock->sin6_scope_id));
>  
>  		if (ifp) {
> -			ia6 = in6_ifawithscope(ifp, dst, rtableid);
> +			ia6 = in6_ifawithscope(ifp, dst, rtableid, NULL);
>  			if_put(ifp);
>  
>  			if (ia6 == NULL)
> diff --git sys/netinet6/in6_var.h sys/netinet6/in6_var.h
> index 1323eee209a..a97a1d463f7 100644
> --- sys/netinet6/in6_var.h
> +++ sys/netinet6/in6_var.h
> @@ -93,6 +93,7 @@ struct	in6_ifaddr {
>  #define	ia_flags	ia_ifa.ifa_flags
>  
>  	struct	sockaddr_in6 ia_addr;	/* interface address */
> +	struct	sockaddr_in6 ia_gwaddr; /* router we learned address from */
>  	struct	sockaddr_in6 ia_dstaddr; /* space for destination addr */
>  	struct	sockaddr_in6 ia_prefixmask; /* prefix mask */
>  	TAILQ_ENTRY(in6_ifaddr) ia_list;	/* list of IP6 addresses */
> 
> 
> -- 
> In my defence, I have been left unsupervised.