From 50b4b8b2c60000175b36e45e6e196a8bb7576edf Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Wed, 13 Apr 2022 15:41:04 -0700 Subject: [PATCH] ipn/ipnlocal: make peerIPs return a sorted slice Currently peerIPs doesn't do any sorting of the routes it returns. This is typically fine, however imagine the case of an HA subnet router failover. When a route R moves from peer A to peer B, the output of peerIPs changes. This in turn causes all the deephash check inside wgengine to fail as the hashed value of [R1, R2] is different than the hashed value of [R2, R1]. When the hash check failes, it causes wgengine to reconfigure all routes in the OS. This is especially problematic for macOS and iOS where we use the NetworkExtension. This commit makes it that the peerIPs are always sorted when returned, thus making the hash be consistent as long as the list of routes remains static. Signed-off-by: Maisem Ali --- ipn/ipnlocal/local.go | 11 +++++++++++ ipn/ipnlocal/local_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index af69d00b5..81a516073 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2397,9 +2397,20 @@ func peerRoutes(peers []wgcfg.Peer, cgnatThreshold int) (routes []netaddr.IPPref } else { routes = append(routes, cgNATIPs...) } + + sort.Slice(routes, func(i, j int) bool { + return ipPrefixLess(routes[i], routes[j]) + }) return routes } +func ipPrefixLess(ri, rj netaddr.IPPrefix) bool { + if ri.IP() == rj.IP() { + return ri.Bits() < rj.Bits() + } + return ri.IP().Less(rj.IP()) +} + // routerConfig produces a router.Config from a wireguard config and IPN prefs. func (b *LocalBackend) routerConfig(cfg *wgcfg.Config, prefs *ipn.Prefs) *router.Config { rs := &router.Config{ diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index cd8d8e004..8a0df2867 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -302,8 +302,31 @@ func TestPeerRoutes(t *testing.T) { }, }, want: []netaddr.IPPrefix{ - pp("fd7a:115c:a1e0::/48"), pp("100.64.0.0/10"), + pp("fd7a:115c:a1e0::/48"), + }, + }, + { + name: "output-should-be-sorted", + peers: []wgcfg.Peer{ + { + AllowedIPs: []netaddr.IPPrefix{ + pp("100.64.0.2/32"), + pp("10.0.0.0/16"), + }, + }, + { + AllowedIPs: []netaddr.IPPrefix{ + pp("100.64.0.1/32"), + pp("10.0.0.0/8"), + }, + }, + }, + want: []netaddr.IPPrefix{ + pp("10.0.0.0/8"), + pp("10.0.0.0/16"), + pp("100.64.0.1/32"), + pp("100.64.0.2/32"), }, }, }