Another round of cleanup

control.go

@@ -130,6 +130,7 @@ func (c *Control) ListHostmapIndexes(pendingMap bool) []ControlHostInfo {
 }
 
 // GetHostInfoByVpnIp returns a single tunnels hostInfo, or nil if not found
+// Caller should take care to Unmap() any 4in6 addresses prior to calling.
 func (c *Control) GetHostInfoByVpnIp(vpnIp netip.Addr, pending bool) *ControlHostInfo {
 	var hl controlHostLister
 	if pending {
@@ -148,6 +149,7 @@ func (c *Control) GetHostInfoByVpnIp(vpnIp netip.Addr, pending bool) *ControlHos
 }
 
 // SetRemoteForTunnel forces a tunnel to use a specific remote
+// Caller should take care to Unmap() any 4in6 addresses prior to calling.
 func (c *Control) SetRemoteForTunnel(vpnIp netip.Addr, addr netip.AddrPort) *ControlHostInfo {
 	hostInfo := c.f.hostMap.QueryVpnIp(vpnIp)
 	if hostInfo == nil {
@@ -160,6 +162,7 @@ func (c *Control) SetRemoteForTunnel(vpnIp netip.Addr, addr netip.AddrPort) *Con
 }
 
 // CloseTunnel closes a fully established tunnel. If localOnly is false it will notify the remote end as well.
+// Caller should take care to Unmap() any 4in6 addresses prior to calling.
 func (c *Control) CloseTunnel(vpnIp netip.Addr, localOnly bool) bool {
 	hostInfo := c.f.hostMap.QueryVpnIp(vpnIp)
 	if hostInfo == nil {

firewall.go

@@ -863,13 +863,8 @@ func (fr *FirewallRule) match(p firewall.Packet, c *cert.NebulaCertificate) bool
 	}
 
 	matched := false
-	//TODO: bart does not support this, we can do it for every prefix though :(
-	// https://github.com/gaissmai/bart/issues/61
-	//fr.CIDR.EachContains(p.RemoteIP, func(_ netip.Prefix, flc *firewallLocalCIDR) bool {
-	//	matched = flc.match(p, c)
-	//	return !matched
-	//})
-	fr.CIDR.All(func(prefix netip.Prefix, val *firewallLocalCIDR) bool {
+	prefix := netip.PrefixFrom(p.RemoteIP, p.RemoteIP.BitLen())
+	fr.CIDR.EachLookupPrefix(prefix, func(prefix netip.Prefix, val *firewallLocalCIDR) bool {
 		if prefix.Contains(p.RemoteIP) && val.match(p, c) {
 			matched = true
 			return false

go.mod

@@ -41,7 +41,7 @@ require (
 	github.com/bits-and-blooms/bitset v1.13.0 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/gaissmai/bart v0.10.0 // indirect
+	github.com/gaissmai/bart v0.11.1 // indirect
 	github.com/google/btree v1.1.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect

go.sum

@@ -28,6 +28,8 @@ github.com/flynn/noise v1.1.0 h1:KjPQoQCEFdZDiP03phOvGi11+SVVhBG2wOWAorLsstg=
 github.com/flynn/noise v1.1.0/go.mod h1:xbMo+0i6+IGbYdJhF31t2eR1BIU0CYc12+BNAKwUTag=
 github.com/gaissmai/bart v0.10.0 h1:yCZCYF8xzcRnqDe4jMk14NlJjL1WmMsE7ilBzvuHtiI=
 github.com/gaissmai/bart v0.10.0/go.mod h1:KHeYECXQiBjTzQz/om2tqn3sZF1J7hw9m6z41ftj3fg=
+github.com/gaissmai/bart v0.11.1 h1:5Uv5XwsaFBRo4E5VBcb9TzY8B7zxFf+U7isDxqOrRfc=
+github.com/gaissmai/bart v0.11.1/go.mod h1:KHeYECXQiBjTzQz/om2tqn3sZF1J7hw9m6z41ftj3fg=
 github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
 github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
 github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=

hostmap.go

@@ -132,18 +132,6 @@ func (rs *RelayState) CopyRelayForIdxs() []uint32 {
 	return ret
 }
 
-func (rs *RelayState) RemoveRelay(localIdx uint32) (netip.Addr, bool) {
-	rs.Lock()
-	defer rs.Unlock()
-	r, ok := rs.relayForByIdx[localIdx]
-	if !ok {
-		return netip.Addr{}, false
-	}
-	delete(rs.relayForByIdx, localIdx)
-	delete(rs.relayForByIp, r.PeerIp)
-	return r.PeerIp, true
-}
-
 func (rs *RelayState) CompleteRelayByIP(vpnIp netip.Addr, remoteIdx uint32) bool {
 	rs.Lock()
 	defer rs.Unlock()

lighthouse.go

@@ -951,7 +951,6 @@ func (lhh *LightHouseHandler) handleHostQuery(n *NebulaMeta, vpnIp netip.Addr, a
 	reqVpnIp := n.Details.VpnIp
 
 	//TODO: IPV6-WORK
-	//TODO: this all looks suspect to me, triple check it
 	b := [4]byte{}
 	binary.BigEndian.PutUint32(b[:], n.Details.VpnIp)
 	queryVpnIp := netip.AddrFrom4(b)
@@ -984,7 +983,7 @@ func (lhh *LightHouseHandler) handleHostQuery(n *NebulaMeta, vpnIp netip.Addr, a
 		n = lhh.resetMeta()
 		n.Type = NebulaMeta_HostPunchNotification
 		//TODO: IPV6-WORK
-		b := vpnIp.As4()
+		b = vpnIp.As4()
 		n.Details.VpnIp = binary.BigEndian.Uint32(b[:])
 		lhh.coalesceAnswers(c, n)
 
@@ -1003,7 +1002,6 @@ func (lhh *LightHouseHandler) handleHostQuery(n *NebulaMeta, vpnIp netip.Addr, a
 	lhh.lh.metricTx(NebulaMeta_HostPunchNotification, 1)
 
 	//TODO: IPV6-WORK
-	b = [4]byte{}
 	binary.BigEndian.PutUint32(b[:], reqVpnIp)
 	sendTo := netip.AddrFrom4(b)
 	w.SendMessageToVpnIp(header.LightHouse, 0, sendTo, lhh.pb[:ln], lhh.nb, lhh.out[:0])
@@ -1049,13 +1047,12 @@ func (lhh *LightHouseHandler) handleHostQueryReply(n *NebulaMeta, vpnIp netip.Ad
 	//TODO: IPV6-WORK
 	b := [4]byte{}
 	binary.BigEndian.PutUint32(b[:], n.Details.VpnIp)
-	am := lhh.lh.unlockedGetRemoteList(netip.AddrFrom4(b))
+	certVpnIp := netip.AddrFrom4(b)
+	am := lhh.lh.unlockedGetRemoteList(certVpnIp)
 	am.Lock()
 	lhh.lh.Unlock()
 
 	//TODO: IPV6-WORK
-	binary.BigEndian.PutUint32(b[:], n.Details.VpnIp)
-	certVpnIp := netip.AddrFrom4(b)
 	am.unlockedSetV4(vpnIp, certVpnIp, n.Details.Ip4AndPorts, lhh.lh.unlockedShouldAddV4)
 	am.unlockedSetV6(vpnIp, certVpnIp, n.Details.Ip6AndPorts, lhh.lh.unlockedShouldAddV6)
 

main.go

@@ -71,8 +71,12 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg
 	ones, _ := certificate.Details.Ips[0].Mask.Size()
 	addr, ok := netip.AddrFromSlice(certificate.Details.Ips[0].IP)
 	if !ok {
-		//TODO: IPV6-WORK
-		// should never happen but if it did we should log it
+		err = util.NewContextualError(
+			"Invalid ip address in certificate",
+			m{"vpnIp": certificate.Details.Ips[0].IP},
+			nil,
+		)
+		return nil, err
 	}
 	tunCidr := netip.PrefixFrom(addr, ones)
 

overlay/route.go

@@ -24,7 +24,7 @@ type Route struct {
 // Equal determines if a route that could be installed in the system route table is equal to another
 // Via is ignored since that is only consumed within nebula itself
 func (r Route) Equal(t Route) bool {
-	if r.Cidr == t.Cidr {
+	if r.Cidr != t.Cidr {
 		return false
 	}
 	if r.Metric != t.Metric {
@@ -117,8 +117,7 @@ func parseRoutes(c *config.C, network netip.Prefix) ([]Route, error) {
 			return nil, fmt.Errorf("entry %v.route in tun.routes failed to parse: %v", i+1, err)
 		}
 
-		//TODO: IPV6-WORK, this is a little different than before, make sure its still cool
-		if !network.Contains(r.Cidr.Addr()) {
+		if !network.Contains(r.Cidr.Addr()) || r.Cidr.Bits() < network.Bits() {
 			return nil, fmt.Errorf(
 				"entry %v.route in tun.routes is not contained within the network attached to the certificate; route: %v, network: %v",
 				i+1,
@@ -201,7 +200,6 @@ func parseUnsafeRoutes(c *config.C, network netip.Prefix) ([]Route, error) {
 
 		viaVpnIp, err := netip.ParseAddr(via)
 		if err != nil {
-			//TODO: could include the error
 			return nil, fmt.Errorf("entry %v.via in tun.unsafe_routes failed to parse address: %v", i+1, err)
 		}
 

overlay/tun_darwin.go

@@ -189,7 +189,7 @@ func (t *tun) Activate() error {
 	var addr, mask [4]byte
 
 	if !t.cidr.Addr().Is4() {
-		//TODO:
+		//TODO: IPV6-WORK
 		panic("need ipv6")
 	}
 
@@ -393,7 +393,7 @@ func (t *tun) addRoutes(logErrors bool) error {
 			continue
 		}
 
-		if r.Cidr.Addr().Is6() {
+		if !r.Cidr.Addr().Is4() {
 			//TODO: implement ipv6
 			panic("Cant handle ipv6 routes yet")
 		}
@@ -451,7 +451,6 @@ func (t *tun) removeRoutes(routes []Route) error {
 		}
 
 		routeAddr.IP = r.Cidr.Addr().As4()
-		//TODO: we could avoid the copy
 		copy(maskAddr.IP[:], prefixToMask(r.Cidr))
 
 		err := delRoute(routeSock, routeAddr, maskAddr, t.linkAddr)

overlay/tun_freebsd.go

@@ -83,8 +83,6 @@ func newTunFromFd(_ *config.C, _ *logrus.Logger, _ int, _ netip.Prefix) (*tun, e
 }
 
 func newTun(c *config.C, l *logrus.Logger, cidr netip.Prefix, _ bool) (*tun, error) {
-	//TODO: IPV6-WORK might need to unmap cidr here
-
 	// Try to open existing tun device
 	var file *os.File
 	var err error

overlay/tun_linux.go

@@ -516,9 +516,14 @@ func (t *tun) updateRoutes(r netlink.RouteUpdate) {
 	}
 
 	//TODO: IPV6-WORK what if not ok?
-	gwAddr, _ := netip.AddrFromSlice(r.Gw)
+	gwAddr, ok := netip.AddrFromSlice(r.Gw)
+	if !ok {
+		t.l.WithField("route", r).Debug("Ignoring route update, invalid gateway address")
+		return
+	}
+
 	gwAddr = gwAddr.Unmap()
-	if !t.cidr.Contains(gwAddr.Unmap()) {
+	if !t.cidr.Contains(gwAddr) {
 		// Gateway isn't in our overlay network, ignore
 		t.l.WithField("route", r).Debug("Ignoring route update, not in our network")
 		return
@@ -530,19 +535,22 @@ func (t *tun) updateRoutes(r netlink.RouteUpdate) {
 		return
 	}
 
-	newTree := t.routeTree.Load().Clone()
-	//TODO: IPV6-WORK might need to unmap here
-	//TODO: IPV6-WORK what if not ok
-	dstAddr, _ := netip.AddrFromSlice(r.Dst.IP)
+	dstAddr, ok := netip.AddrFromSlice(r.Dst.IP)
+	if !ok {
+		t.l.WithField("route", r).Debug("Ignoring route update, invalid destination address")
+		return
+	}
+
 	ones, _ := r.Dst.Mask.Size()
 	dst := netip.PrefixFrom(dstAddr, ones)
 
+	newTree := t.routeTree.Load().Clone()
+
 	if r.Type == unix.RTM_NEWROUTE {
 		t.l.WithField("destination", r.Dst).WithField("via", r.Gw).Info("Adding route")
 		newTree.Insert(dst, gwAddr)
 
 	} else {
-		//TODO: IPV6-WORK verify this works, we could also get and check the gw is the same
 		newTree.Delete(dst)
 		t.l.WithField("destination", r.Dst).WithField("via", r.Gw).Info("Removing route")
 	}

overlay/tun_wintun_windows.go

@@ -156,7 +156,7 @@ func (t *winTun) addRoutes(logErrors bool) error {
 		}
 
 		// Add our unsafe route
-		err := luid.AddRoute(t.cidr, r.Via, uint32(r.Metric))
+		err := luid.AddRoute(r.Cidr, r.Via, uint32(r.Metric))
 		if err != nil {
 			retErr := util.NewContextualError("Failed to add route", map[string]interface{}{"route": r}, err)
 			if logErrors {
@@ -170,8 +170,7 @@ func (t *winTun) addRoutes(logErrors bool) error {
 		}
 
 		if !foundDefault4 {
-			//TODO: IPV6-WORK verify this is correct
-			if r.Cidr.Bits() == 0 {
+			if r.Cidr.Bits() == 0 && r.Cidr.Addr().BitLen() == 32 {
 				foundDefault4 = true
 			}
 		}

relay_manager.go

@@ -374,7 +374,3 @@ func (rm *relayManager) handleCreateRelayRequest(h *HostInfo, f *Interface, m *N
 		}
 	}
 }
-
-func (rm *relayManager) RemoveRelay(localIdx uint32) {
-	rm.hostmap.RemoveRelay(localIdx)
-}