From 3e6c75573fd55d4bcdb6ad19f8c362d59bb1d9f0 Mon Sep 17 00:00:00 2001 From: Nate Brown Date: Wed, 23 Oct 2024 14:28:02 -0500 Subject: [PATCH] Fix static host map wrong responder situations, correct logging (#1259) --- e2e/handshakes_test.go | 115 +++++++++++++++++++++++++++++++++++++---- handshake_ix.go | 39 +++++++------- remote_list.go | 4 +- 3 files changed, 129 insertions(+), 29 deletions(-) diff --git a/e2e/handshakes_test.go b/e2e/handshakes_test.go index 6be94ad..f6069bf 100644 --- a/e2e/handshakes_test.go +++ b/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")) + + 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 { - h := &header.H{} 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) diff --git a/handshake_ix.go b/handshake_ix.go index 73e8541..3add83d 100644 --- a/handshake_ix.go +++ b/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) diff --git a/remote_list.go b/remote_list.go index fa14f42..94db8f2 100644 --- a/remote_list.go +++ b/remote_list.go @@ -576,7 +576,9 @@ func (r *RemoteList) unlockedCollect() { dnsAddrs := r.hr.GetIPs() for _, addr := range dnsAddrs { if r.shouldAdd == nil || r.shouldAdd(addr.Addr()) { - addrs = append(addrs, addr) + if !r.unlockedIsBad(addr) { + addrs = append(addrs, addr) + } } }