[FIXED] Added defensive code for handling of leafnode connection

This is a port of PR #1652/#1660. The code in the 2.1.x branch
is not sensitive to the issue fixed in these PRs because marking
the connection as closed (for instance due to a TCP error in
sendProtoNow) will not set `c.nc` to nil, so there won't be
the nil dereference issue that was found in the main branch.
However, porting the code for extra safety.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

server/leafnode.go

@@ -652,6 +652,24 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 	// Grab lock
 	c.mu.Lock()
 
+	// If connection has been closed, this function will unlock and call
+	// closeConnection() to ensure proper clean-up.
+	isClosedUnlock := func() bool {
+		if c.isClosed() {
+			c.mu.Unlock()
+			c.closeConnection(WriteError)
+			return true
+		}
+		return false
+	}
+
+	// I don't think that the connection can be closed this early (since it isn't
+	// registered anywhere and doesn't have read/write loops running), but let's
+	// check in case code is changed in the future and there is such possibility.
+	if isClosedUnlock() {
+		return nil
+	}
+
 	if solicited {
 		// We need to wait here for the info, but not for too long.
 		c.nc.SetReadDeadline(time.Now().Add(DEFAULT_LEAFNODE_INFO_WAIT))
@@ -684,6 +702,12 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 			return nil
 		}
 
+		// Not sure that can happen, but in case the connection was marked
+		// as closed during the call to parse...
+		if isClosedUnlock() {
+			return nil
+		}
+
 		// Do TLS here as needed.
 		tlsRequired := remote.TLS || remote.TLSConfig != nil
 		if tlsRequired {
@@ -747,6 +771,11 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 
 			// Re-Grab lock
 			c.mu.Lock()
+
+			// Timeout may have closed the connection while the lock was released.
+			if isClosedUnlock() {
+				return nil
+			}
 		}
 
 		c.sendLeafConnect(tlsRequired)
@@ -767,6 +796,12 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 		// this before it can initiate the TLS handshake..
 		c.sendProtoNow(bytes.Join(pcs, []byte(" ")))
 
+		// The above call could have marked the connection as closed (due to
+		// TCP error), so if that is the case, bail out here.
+		if isClosedUnlock() {
+			return nil
+		}
+
 		// Check to see if we need to spin up TLS.
 		if info.TLSRequired {
 			c.Debugf("Starting TLS leafnode server handshake")
@@ -793,14 +828,17 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 
 			// Indicate that handshake is complete (used in monitoring)
 			c.flags.set(handshakeComplete)
+
+			// Timeout may have closed the connection while the lock was released.
+			if isClosedUnlock() {
+				return nil
+			}
 		}
 
 		// Leaf nodes will always require a CONNECT to let us know
 		// when we are properly bound to an account.
 		// The connection may have been closed
-		if !c.isClosed() {
-			c.setAuthTimer(secondsToDuration(opts.LeafNode.AuthTimeout))
-		}
+		c.setAuthTimer(secondsToDuration(opts.LeafNode.AuthTimeout))
 	}
 
 	// Keep track in case server is shutdown before we can successfully register.

test/leafnode_test.go

@@ -3555,3 +3555,36 @@ func TestLeafNodeQueueSubscriberUnsubscribe(t *testing.T) {
 	// Make sure we receive nothing...
 	expectNothing(t, lc)
 }
+
+func TestLeafNodeNoPanicOnTLSSolicit(t *testing.T) {
+	content := `
+	listen: "127.0.0.1:-1"
+	leafnodes {
+		listen: "127.0.0.1:-1"
+		tls {
+			cert_file: "./configs/certs/server-cert.pem"
+			key_file: "./configs/certs/server-key.pem"
+			timeout: 2.0
+		}
+	}
+	`
+	conf := createConfFile(t, []byte(content))
+	defer os.Remove(conf)
+
+	s, opts := RunServerWithConfig(conf)
+	defer s.Shutdown()
+
+	lc, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", opts.LeafNode.Port))
+	if err != nil {
+		t.Fatalf("Unable to connect: %v", err)
+	}
+
+	// Then close right away
+	lc.Close()
+
+	// Check server does not crash...
+	time.Sleep(250 * time.Millisecond)
+	if s.ID() == "" {
+		t.Fatalf("should not happen")
+	}
+}