From 949ec7865335bfdf1c8977a22787f96873713205 Mon Sep 17 00:00:00 2001
From: Wade Simmons <wadey@slack-corp.com>
Date: Mon, 6 Dec 2021 14:09:05 -0500
Subject: [PATCH] don't set ConnectionState to nil (#590)

* don't set ConnectionState to nil

We might have packets processing in another thread, so we can't safely
just set this to nil. Since we removed it from the hostmaps, the next
packets to process should start the handshake over again.

I believe this comment is outdated or incorrect, since the next
handshake will start over with a new HostInfo, I don't think there is
any way a counter reuse could happen:

> We must null the connectionstate or a counter reuse may happen

Here is a panic we saw that I think is related:

    panic: runtime error: invalid memory address or nil pointer dereference
    [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x93a037]
    goroutine 59 [running, locked to thread]:
    github.com/slackhq/nebula.(*Firewall).Drop(...)
            github.com/slackhq/nebula/firewall.go:380
    github.com/slackhq/nebula.(*Interface).consumeInsidePacket(...)
            github.com/slackhq/nebula/inside.go:59
    github.com/slackhq/nebula.(*Interface).listenIn(...)
            github.com/slackhq/nebula/interface.go:233
    created by github.com/slackhq/nebula.(*Interface).run
            github.com/slackhq/nebula/interface.go:191

* use closeTunnel
---
 hostmap.go | 8 --------
 outside.go | 9 +++------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/hostmap.go b/hostmap.go
index 6b726c1..7646d23 100644
--- a/hostmap.go
+++ b/hostmap.go
@@ -397,10 +397,6 @@ func (hm *HostMap) Punchy(ctx context.Context, conn *udp.Conn) {
 	}
 }
 
-func (i *HostInfo) BindConnectionState(cs *ConnectionState) {
-	i.ConnectionState = cs
-}
-
 // TryPromoteBest handles re-querying lighthouses and probing for better paths
 // NOTE: It is an error to call this if you are a lighthouse since they should not roam clients!
 func (i *HostInfo) TryPromoteBest(preferredRanges []*net.IPNet, ifce *Interface) {
@@ -541,10 +537,6 @@ func (i *HostInfo) SetRemoteIfPreferred(hm *HostMap, newRemote *udp.Addr) bool {
 	return false
 }
 
-func (i *HostInfo) ClearConnectionState() {
-	i.ConnectionState = nil
-}
-
 func (i *HostInfo) RecvErrorExceeded() bool {
 	if i.recvError < 3 {
 		i.recvError += 1
diff --git a/outside.go b/outside.go
index a081ec0..e0dba7d 100644
--- a/outside.go
+++ b/outside.go
@@ -349,12 +349,9 @@ func (f *Interface) handleRecvError(addr *udp.Addr, h *header.H) {
 		return
 	}
 
-	// We delete this host from the main hostmap
-	f.hostMap.DeleteHostInfo(hostinfo)
-	// We also delete it from pending to allow for
-	// fast reconnect. We must null the connectionstate
-	// or a counter reuse may happen
-	hostinfo.ConnectionState = nil
+	f.closeTunnel(hostinfo, false)
+	// We also delete it from pending hostmap to allow for
+	// fast reconnect.
 	f.handshakeManager.DeleteHostInfo(hostinfo)
 }