Merge pull request #1652 from nats-io/fix_leafnode_accept_panic

[FIXED] Possible panic when server accepts TLS leafnode connection

server/leafnode.go

@@ -617,6 +617,17 @@ func (s *Server) createLeafNode(conn net.Conn, remote *leafNodeCfg) *client {
 	// Grab lock
 	c.mu.Lock()
 
+	// If client has been closed, this function will unlock and make sure
+	// the the connection is closed for proper clean-up.
+	isClosedUnlockAndReturn := func() bool {
+		if c.isClosed() {
+			c.mu.Unlock()
+			c.closeConnection(WriteError)
+			return true
+		}
+		return false
+	}
+
 	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))
@@ -649,6 +660,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 isClosedUnlockAndReturn() {
+			return nil
+		}
+
 		// Do TLS here as needed.
 		tlsRequired := remote.TLS || remote.TLSConfig != nil
 		if tlsRequired {
@@ -712,6 +729,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 isClosedUnlockAndReturn() {
+				return nil
+			}
 		}
 
 		if err := c.sendLeafConnect(clusterName, tlsRequired); err != nil {
@@ -736,6 +758,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 isClosedUnlockAndReturn() {
+			return nil
+		}
+
 		// Check to see if we need to spin up TLS.
 		if info.TLSRequired {
 			c.Debugf("Starting TLS leafnode server handshake")
@@ -762,14 +790,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 isClosedUnlockAndReturn() {
+				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

@@ -1204,6 +1204,39 @@ func TestLeafNodeTLS(t *testing.T) {
 	checkLeafNodeConnected(t, s)
 }
 
+func TestLeafNodeTLSConnCloseEarly(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")
+	}
+}
+
 type captureLeafNodeErrLogger struct {
 	dummyLogger
 	ch chan string