Fix static host map wrong responder situations, correct logging (#1259)

e2e/handshakes_test.go

@@ -97,16 +97,10 @@ func TestGoodHandshake(t *testing.T) {
 func TestWrongResponderHandshake(t *testing.T) {
 	ca, _, caKey, _ := NewTestCaCert(time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{})
 
-	// The IPs here are chosen on purpose:
-	// The current remote handling will sort by preference, public, and then lexically.
-	// So we need them to have a higher address than evil (we could apply a preference though)
 	myControl, myVpnIpNet, myUdpAddr, _ := newSimpleServer(ca, caKey, "me", "10.128.0.100/24", nil)
 	theirControl, theirVpnIpNet, theirUdpAddr, _ := newSimpleServer(ca, caKey, "them", "10.128.0.99/24", nil)
 	evilControl, evilVpnIp, evilUdpAddr, _ := newSimpleServer(ca, caKey, "evil", "10.128.0.2/24", nil)
 
-	// Add their real udp addr, which should be tried after evil.
-	myControl.InjectLightHouseAddr(theirVpnIpNet.Addr(), theirUdpAddr)
-
 	// Put the evil udp addr in for their vpn Ip, this is a case of being lied to by the lighthouse.
 	myControl.InjectLightHouseAddr(theirVpnIpNet.Addr(), evilUdpAddr)
 
@@ -119,10 +113,114 @@ func TestWrongResponderHandshake(t *testing.T) {
 	theirControl.Start()
 	evilControl.Start()
 
-	t.Log("Start the handshake process, we will route until we see our cached packet get sent to them")
+	t.Log("Start the handshake process, we will route until we see the evil tunnel closed")
 	myControl.InjectTunUDPPacket(theirVpnIpNet.Addr(), 80, 80, []byte("Hi from me"))
-	r.RouteForAllExitFunc(func(p *udp.Packet, c *nebula.Control) router.ExitType {
+
 	h := &header.H{}
+	r.RouteForAllExitFunc(func(p *udp.Packet, c *nebula.Control) router.ExitType {
+		err := h.Parse(p.Data)
+		if err != nil {
+			panic(err)
+		}
+
+		if h.Type == header.CloseTunnel && p.To == evilUdpAddr {
+			return router.RouteAndExit
+		}
+
+		return router.KeepRouting
+	})
+
+	t.Log("Evil tunnel is closed, inject the correct udp addr for them")
+	myControl.InjectLightHouseAddr(theirVpnIpNet.Addr(), theirUdpAddr)
+	pendingHi := myControl.GetHostInfoByVpnIp(theirVpnIpNet.Addr(), true)
+	assert.NotContains(t, pendingHi.RemoteAddrs, evilUdpAddr)
+
+	t.Log("Route until we see the cached packet")
+	r.RouteForAllExitFunc(func(p *udp.Packet, c *nebula.Control) router.ExitType {
+		err := h.Parse(p.Data)
+		if err != nil {
+			panic(err)
+		}
+
+		if p.To == theirUdpAddr && h.Type == 1 {
+			return router.RouteAndExit
+		}
+
+		return router.KeepRouting
+	})
+
+	//TODO: Assert pending hostmap - I should have a correct hostinfo for them now
+
+	t.Log("My cached packet should be received by them")
+	myCachedPacket := theirControl.GetFromTun(true)
+	assertUdpPacket(t, []byte("Hi from me"), myCachedPacket, myVpnIpNet.Addr(), theirVpnIpNet.Addr(), 80, 80)
+
+	t.Log("Test the tunnel with them")
+	assertHostInfoPair(t, myUdpAddr, theirUdpAddr, myVpnIpNet.Addr(), theirVpnIpNet.Addr(), myControl, theirControl)
+	assertTunnel(t, myVpnIpNet.Addr(), theirVpnIpNet.Addr(), myControl, theirControl, r)
+
+	t.Log("Flush all packets from all controllers")
+	r.FlushAll()
+
+	t.Log("Ensure ensure I don't have any hostinfo artifacts from evil")
+	assert.Nil(t, myControl.GetHostInfoByVpnIp(evilVpnIp.Addr(), true), "My pending hostmap should not contain evil")
+	assert.Nil(t, myControl.GetHostInfoByVpnIp(evilVpnIp.Addr(), false), "My main hostmap should not contain evil")
+
+	//TODO: assert hostmaps for everyone
+	r.RenderHostmaps("Final hostmaps", myControl, theirControl, evilControl)
+	t.Log("Success!")
+	myControl.Stop()
+	theirControl.Stop()
+}
+
+func TestWrongResponderHandshakeStaticHostMap(t *testing.T) {
+	ca, _, caKey, _ := NewTestCaCert(time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{})
+
+	theirControl, theirVpnIpNet, theirUdpAddr, _ := newSimpleServer(ca, caKey, "them", "10.128.0.99/24", nil)
+	evilControl, evilVpnIp, evilUdpAddr, _ := newSimpleServer(ca, caKey, "evil", "10.128.0.2/24", nil)
+	o := m{
+		"static_host_map": m{
+			theirVpnIpNet.Addr().String(): []string{evilUdpAddr.String()},
+		},
+	}
+	myControl, myVpnIpNet, myUdpAddr, _ := newSimpleServer(ca, caKey, "me", "10.128.0.100/24", o)
+
+	// Put the evil udp addr in for their vpn addr, this is a case of a remote at a static entry changing its vpn addr.
+	myControl.InjectLightHouseAddr(theirVpnIpNet.Addr(), evilUdpAddr)
+
+	// Build a router so we don't have to reason who gets which packet
+	r := router.NewR(t, myControl, theirControl, evilControl)
+	defer r.RenderFlow()
+
+	// Start the servers
+	myControl.Start()
+	theirControl.Start()
+	evilControl.Start()
+
+	t.Log("Start the handshake process, we will route until we see the evil tunnel closed")
+	myControl.InjectTunUDPPacket(theirVpnIpNet.Addr(), 80, 80, []byte("Hi from me"))
+
+	h := &header.H{}
+	r.RouteForAllExitFunc(func(p *udp.Packet, c *nebula.Control) router.ExitType {
+		err := h.Parse(p.Data)
+		if err != nil {
+			panic(err)
+		}
+
+		if h.Type == header.CloseTunnel && p.To == evilUdpAddr {
+			return router.RouteAndExit
+		}
+
+		return router.KeepRouting
+	})
+
+	t.Log("Evil tunnel is closed, inject the correct udp addr for them")
+	myControl.InjectLightHouseAddr(theirVpnIpNet.Addr(), theirUdpAddr)
+	pendingHi := myControl.GetHostInfoByVpnIp(theirVpnIpNet.Addr(), true)
+	assert.NotContains(t, pendingHi.RemoteAddrs, evilUdpAddr)
+
+	t.Log("Route until we see the cached packet")
+	r.RouteForAllExitFunc(func(p *udp.Packet, c *nebula.Control) router.ExitType {
 		err := h.Parse(p.Data)
 		if err != nil {
 			panic(err)
@@ -151,7 +249,6 @@ func TestWrongResponderHandshake(t *testing.T) {
 	t.Log("Ensure ensure I don't have any hostinfo artifacts from evil")
 	assert.Nil(t, myControl.GetHostInfoByVpnIp(evilVpnIp.Addr(), true), "My pending hostmap should not contain evil")
 	assert.Nil(t, myControl.GetHostInfoByVpnIp(evilVpnIp.Addr(), false), "My main hostmap should not contain evil")
-	//NOTE: if evil lost the handshake race it may still have a tunnel since me would reject the handshake since the tunnel is complete
 
 	//TODO: assert hostmaps for everyone
 	r.RenderHostmaps("Final hostmaps", myControl, theirControl, evilControl)

handshake_ix.go

@@ -418,6 +418,21 @@ func ixHandshakeStage2(f *Interface, addr netip.AddrPort, via *ViaSender, hh *Ha
 	fingerprint := remoteCert.Fingerprint
 	issuer := remoteCert.Certificate.Issuer()
 
+	hostinfo.remoteIndexId = hs.Details.ResponderIndex
+	hostinfo.lastHandshakeTime = hs.Details.Time
+
+	// Store their cert and our symmetric keys
+	ci.peerCert = remoteCert
+	ci.dKey = NewNebulaCipherState(dKey)
+	ci.eKey = NewNebulaCipherState(eKey)
+
+	// Make sure the current udpAddr being used is set for responding
+	if addr.IsValid() {
+		hostinfo.SetRemote(addr)
+	} else {
+		hostinfo.relayState.InsertRelayTo(via.relayHI.vpnIp)
+	}
+
 	// Ensure the right host responded
 	if vpnIp != hostinfo.vpnIp {
 		f.l.WithField("intendedVpnIp", hostinfo.vpnIp).WithField("haveVpnIp", vpnIp).
@@ -435,10 +450,8 @@ func ixHandshakeStage2(f *Interface, addr netip.AddrPort, via *ViaSender, hh *Ha
 			newHH.hostinfo.remotes = hostinfo.remotes
 			newHH.hostinfo.remotes.BlockRemote(addr)
 
-			// Get the correct remote list for the host we did handshake with
-			hostinfo.remotes = f.lightHouse.QueryCache(vpnIp)
-
-			f.l.WithField("blockedUdpAddrs", newHH.hostinfo.remotes.CopyBlockedRemotes()).WithField("vpnIp", vpnIp).
+			f.l.WithField("blockedUdpAddrs", newHH.hostinfo.remotes.CopyBlockedRemotes()).
+				WithField("vpnIp", newHH.hostinfo.vpnIp).
 				WithField("remotes", newHH.hostinfo.remotes.CopyAddrs(f.hostMap.GetPreferredRanges())).
 				Info("Blocked addresses for handshakes")
 
@@ -446,6 +459,9 @@ func ixHandshakeStage2(f *Interface, addr netip.AddrPort, via *ViaSender, hh *Ha
 			newHH.packetStore = hh.packetStore
 			hh.packetStore = []*cachedPacket{}
 
+			// Get the correct remote list for the host we did handshake with
+			hostinfo.SetRemote(addr)
+			hostinfo.remotes = f.lightHouse.QueryCache(vpnIp)
 			// Finally, put the correct vpn ip in the host info, tell them to close the tunnel, and return true to tear down
 			hostinfo.vpnIp = vpnIp
 			f.sendCloseTunnel(hostinfo)
@@ -468,21 +484,6 @@ func ixHandshakeStage2(f *Interface, addr netip.AddrPort, via *ViaSender, hh *Ha
 		WithField("sentCachedPackets", len(hh.packetStore)).
 		Info("Handshake message received")
 
-	hostinfo.remoteIndexId = hs.Details.ResponderIndex
-	hostinfo.lastHandshakeTime = hs.Details.Time
-
-	// Store their cert and our symmetric keys
-	ci.peerCert = remoteCert
-	ci.dKey = NewNebulaCipherState(dKey)
-	ci.eKey = NewNebulaCipherState(eKey)
-
-	// Make sure the current udpAddr being used is set for responding
-	if addr.IsValid() {
-		hostinfo.SetRemote(addr)
-	} else {
-		hostinfo.relayState.InsertRelayTo(via.relayHI.vpnIp)
-	}
-
 	// Build up the radix for the firewall if we have subnets in the cert
 	hostinfo.CreateRemoteCIDR(remoteCert.Certificate)
 

remote_list.go

@@ -576,9 +576,11 @@ func (r *RemoteList) unlockedCollect() {
 	dnsAddrs := r.hr.GetIPs()
 	for _, addr := range dnsAddrs {
 		if r.shouldAdd == nil || r.shouldAdd(addr.Addr()) {
+			if !r.unlockedIsBad(addr) {
 				addrs = append(addrs, addr)
 			}
 		}
+	}
 
 	r.addrs = addrs
 	r.relays = relays