Repository: nats-io/nats-server (go)

Issue:
Time duration for `handshake_first` option is not respected for leafnodes
### Observed behavior

According to this [ADR](https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-40.md) passing a time duration for the `handshake_first` option, makes the server backward compatible because after the timeout the server is supposed to use the standard TLS flow.

This works for standard connections (from clients) but not for leafnodes.
If the leafnode doesn't declare the `handshake_first` field in the `remotes` section, its connection will be never accepted by the server

I'm able to connect to my server using `nats-cli` with or without the `--tlsfirst` flag (because of the timeout) but the leafnode can connect to the server only if it use the field in the remote section.

### Expected behavior

A leafnode connection without `handshake_first` is accepted (standard TLS flow) after the initial timeout

### Server and client version

nats-server: v2.10.18 (both server and leafnode)
nats-cli: 0.1.5

### Host environment

_No response_

### Steps to reproduce

Main server has this configuration
```
TLS = {
  cert_file: $CERT_FILE
  key_file: $KEY_FILE
  ca_file: $CA_FILE
  verify_and_map: true
  timeout: 2
  handshake_first: 500ms
}

tls : $TLS

leafnodes {
    port: 20023
    tls: $TLS
}
```

The leafnode has this one
```
TLS = {
  cert_file: $CERT_FILE
  key_file: $KEY_FILE
  ca_file: $CA_FILE
  verify_and_map: true
  handshake_first: 500ms
}

tls : $TLS

leafnodes {
  remotes: [
    {
      url: $SERVER
      tls {
         cert_file: $CERT_FILE
         key_file: $KEY_FILE
         ca_file: $CA_FILE
         handshake_first: true
      }
      account: $MY_ACCOUNT
    }
  ] 
}
```

In this case works, but if I remove the `handshake_first` from `remotes` section it doesn't

```
TLS = {
  cert_file: $CERT_FILE
  key_file: $KEY_FILE
  ca_file: $CA_FILE
  verify_and_map: true
  handshake_first: 500ms
}

tls : $TLS

leafnodes {
  remotes: [
    {
      url: $SERVER
      tls {
         cert_file: $CERT_FILE
         key_file: $KEY_FILE
         ca_file: $CA_FILE
      }
      account: $FILEWAVE_ACCOUNT
    }
  ] 
}
```

Pull request:
(2.11) [ADDED] LeafNode: Support for TLS handshake_first duration
It will now be possible to configure the hub's `handshake_first` field in the `tls{}` block as `auto` or a duration (e.g. `"250ms"`).

When that is the case, if the server does not receive data within this time frame, it will revert to send the `INFO` protocol in plaintext because it expects the remote LeafNode to perform the TLS handshake only after receiving it.

Resolves #5772.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>


Patch:
diff --git a/server/leafnode.go b/server/leafnode.go
index 16cbb4ca..5f0834da 100644
--- a/server/leafnode.go
+++ b/server/leafnode.go
@@ -20,6 +20,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"io"
 	"math/rand"
 	"net"
 	"net/http"
@@ -987,8 +988,11 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 	c.initClient()
 	c.Noticef("Leafnode connection created%s %s", remoteSuffix, c.opts.Name)
 
-	var tlsFirst bool
-	var infoTimeout time.Duration
+	var (
+		tlsFirst         bool
+		tlsFirstFallback time.Duration
+		infoTimeout      time.Duration
+	)
 	if remote != nil {
 		solicited = true
 		remote.Lock()
@@ -1006,6 +1010,10 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 		if ws != nil {
 			c.Debugf("Leafnode compression=%v", c.ws.compress)
 		}
+		tlsFirst = opts.LeafNode.TLSHandshakeFirst
+		if f := opts.LeafNode.TLSHandshakeFirstFallback; f > 0 {
+			tlsFirstFallback = f
+		}
 	}
 	c.mu.Unlock()
 
@@ -1069,7 +1077,33 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 		info.Nonce = bytesToString(c.nonce)
 		info.CID = c.cid
 		proto := generateInfoJSON(info)
-		if !opts.LeafNode.TLSHandshakeFirst {
+
+		var pre []byte
+		// We need first to check for "TLS First" fallback delay.
+		if tlsFirstFallback > 0 {
+			// We wait and see if we are getting any data. Since we did not send
+			// the INFO protocol yet, only clients that use TLS first should be
+			// sending data (the TLS handshake). We don't really check the content:
+			// if it is a rogue agent and not an actual client performing the
+			// TLS handshake, the error will be detected when performing the
+			// handshake on our side.
+			pre = make([]byte, 4)
+			c.nc.SetReadDeadline(time.Now().Add(tlsFirstFallback))
+			n, _ := io.ReadFull(c.nc, pre[:])
+			c.nc.SetReadDeadline(time.Time{})
+			// If we get any data (regardless of possible timeout), we will proceed
+			// with the TLS handshake.
+			if n > 0 {
+				pre = pre[:n]
+			} else {
+				// We did not get anything so we will send the INFO protocol.
+				pre = nil
+				// Set the boolean to false for the rest of the function.
+				tlsFirst = false
+			}
+		}
+
+		if !tlsFirst {
 			// We have to send from this go routine because we may
 			// have to block for TLS handshake before we start our
 			// writeLoop go routine. The other side needs to receive
@@ -1086,6 +1120,10 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 
 		// Check to see if we need to spin up TLS.
 		if !c.isWebsocket() && info.TLSRequired {
+			// If we have a prebuffer create a multi-reader.
+			if len(pre) > 0 {
+				c.nc = &tlsMixConn{c.nc, bytes.NewBuffer(pre)}
+			}
 			// Perform server-side TLS handshake.
 			if err := c.doTLSServerHandshake(tlsHandshakeLeaf, opts.LeafNode.TLSConfig, opts.LeafNode.TLSTimeout, opts.LeafNode.TLSPinnedCerts); err != nil {
 				c.mu.Unlock()
@@ -1095,7 +1133,8 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
 
 		// If the user wants the TLS handshake to occur first, now that it is
 		// done, send the INFO protocol.
-		if opts.LeafNode.TLSHandshakeFirst {
+		if tlsFirst {
+			c.flags.set(didTLSFirst)
 			c.sendProtoNow(proto)
 			if c.isClosed() {
 				c.mu.Unlock()
diff --git a/server/opts.go b/server/opts.go
index 677686b6..38f0d181 100644
--- a/server/opts.go
+++ b/server/opts.go
@@ -145,22 +145,33 @@ type RemoteGatewayOpts struct {
 
 // LeafNodeOpts are options for a given server to accept leaf node connections and/or connect to a remote cluster.
 type LeafNodeOpts struct {
-	Host              string        `json:"addr,omitempty"`
-	Port              int           `json:"port,omitempty"`
-	Username          string        `json:"-"`
-	Password          string        `json:"-"`
-	Nkey              string        `json:"-"`
-	Account           string        `json:"-"`
-	Users             []*User       `json:"-"`
-	AuthTimeout       float64       `json:"auth_timeout,omitempty"`
-	TLSConfig         *tls.Config   `json:"-"`
-	TLSTimeout        float64       `json:"tls_timeout,omitempty"`
-	TLSMap            bool          `json:"-"`
-	TLSPinnedCerts    PinnedCertSet `json:"-"`
-	TLSHandshakeFirst bool          `json:"-"`
-	Advertise         string        `json:"-"`
-	NoAdvertise       bool          `json:"-"`
-	ReconnectInterval time.Duration `json:"-"`
+	Host           string        `json:"addr,omitempty"`
+	Port           int           `json:"port,omitempty"`
+	Username       string        `json:"-"`
+	Password       string        `json:"-"`
+	Nkey           string        `json:"-"`
+	Account        string        `json:"-"`
+	Users          []*User       `json:"-"`
+	AuthTimeout    float64       `json:"auth_timeout,omitempty"`
+	TLSConfig      *tls.Config   `json:"-"`
+	TLSTimeout     float64       `json:"tls_timeout,omitempty"`
+	TLSMap         bool          `json:"-"`
+	TLSPinnedCerts PinnedCertSet `json:"-"`
+	// When set to true, the server will perform the TLS handshake before
+	// sending the INFO protocol. For remote leafnodes that are not configured
+	// with a similar option, their connection will fail with some sort
+	// of timeout or EOF error since they are expecting to receive an
+	// INFO protocol first.
+	TLSHandshakeFirst bool `json:"-"`
+	// If TLSHandshakeFirst is true and this value is strictly positive,
+	// the server will wait for that amount of time for the TLS handshake
+	// to start before falling back to previous behavior of sending the
+	// INFO protocol first. It allows for a mix of newer remote leafnodes
+	// that can require a TLS handshake first, and older that can't.
+	TLSHandshakeFirstFallback time.Duration `json:"-"`
+	Advertise                 string        `json:"-"`
+	NoAdvertise               bool          `json:"-"`
+	ReconnectInterval         time.Duration `json:"-"`
 
 	// Compression options
 	Compression CompressionOpts `json:"-"`
@@ -2453,6 +2464,7 @@ func parseLeafNodes(v any, opts *Options, errors *[]error, warnings *[]error) er
 			opts.LeafNode.TLSMap = tc.Map
 			opts.LeafNode.TLSPinnedCerts = tc.PinnedCerts
 			opts.LeafNode.TLSHandshakeFirst = tc.HandshakeFirst
+			opts.LeafNode.TLSHandshakeFirstFallback = tc.FallbackDelay
 			opts.LeafNode.tlsConfigOpts = tc
 		case "leafnode_advertise", "advertise":
 			opts.LeafNode.Advertise = mv.(string)
diff --git a/server/reload.go b/server/reload.go
index 63e9367a..f0663fb7 100644
--- a/server/reload.go
+++ b/server/reload.go
@@ -847,6 +847,7 @@ func (l *leafNodeOption) Apply(s *Server) {
 	opts := s.getOpts()
 	if l.tlsFirstChanged {
 		s.Noticef("Reloaded: LeafNode TLS HandshakeFirst value is: %v", opts.LeafNode.TLSHandshakeFirst)
+		s.Noticef("Reloaded: LeafNode TLS HandshakeFirstFallback value is: %v", opts.LeafNode.TLSHandshakeFirstFallback)
 		for _, r := range opts.LeafNode.Remotes {
 			s.Noticef("Reloaded: LeafNode Remote to %v TLS HandshakeFirst value is: %v", r.URLs, r.TLSHandshakeFirst)
 		}
@@ -1368,14 +1369,16 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) {
 			tmpNew.TLSConfig = nil
 			tmpOld.tlsConfigOpts = nil
 			tmpNew.tlsConfigOpts = nil
-			// We will allow TLSHandshakeFirst to me config reloaded. First,
+			// We will allow TLSHandshakeFirst to be config reloaded. First,
 			// we just want to detect if there was a change in the leafnodes{}
 			// block, and if not, we will check the remotes.
-			handshakeFirstChanged := tmpOld.TLSHandshakeFirst != tmpNew.TLSHandshakeFirst
+			handshakeFirstChanged := tmpOld.TLSHandshakeFirst != tmpNew.TLSHandshakeFirst ||
+				tmpOld.TLSHandshakeFirstFallback != tmpNew.TLSHandshakeFirstFallback
 			// If changed, set them (in the temporary variables) to false so that the
 			// rest of the comparison does not fail.
 			if handshakeFirstChanged {
 				tmpOld.TLSHandshakeFirst, tmpNew.TLSHandshakeFirst = false, false
+				tmpOld.TLSHandshakeFirstFallback, tmpNew.TLSHandshakeFirstFallback = 0, 0
 			} else if len(tmpOld.Remotes) == len(tmpNew.Remotes) {
 				// Since we don't support changes in the remotes, we will do a
 				// simple pass to see if there was a change of this field.


Test patch:
diff --git a/server/leafnode_test.go b/server/leafnode_test.go
index 3207ba2f..14088926 100644
--- a/server/leafnode_test.go
+++ b/server/leafnode_test.go
@@ -4954,6 +4954,59 @@ func TestLeafNodeTLSHandshakeFirstVerifyNoInfoSent(t *testing.T) {
 	}
 }
 
+func TestLeafNodeTLSHandshakeFirstFallbackDelayConfigValues(t *testing.T) {
+	tmpl := `
+		listen: "127.0.0.1:-1"
+		leafnodes {
+			listen: "127.0.0.1:-1"
+			tls {
+				cert_file: 	"../test/configs/certs/server-cert.pem"
+				key_file:  	"../test/configs/certs/server-key.pem"
+				timeout: 	1
+				first: 		%s
+			}
+		}
+	`
+	for _, test := range []struct {
+		name  string
+		val   string
+		first bool
+		delay time.Duration
+	}{
+		{"first as boolean true", "true", true, 0},
+		{"first as boolean false", "false", false, 0},
+		{"first as string true", "\"true\"", true, 0},
+		{"first as string false", "\"false\"", false, 0},
+		{"first as string on", "on", true, 0},
+		{"first as string off", "off", false, 0},
+		{"first as string auto", "auto", true, DEFAULT_TLS_HANDSHAKE_FIRST_FALLBACK_DELAY},
+		{"first as string auto_fallback", "auto_fallback", true, DEFAULT_TLS_HANDSHAKE_FIRST_FALLBACK_DELAY},
+		{"first as fallback duration", "300ms", true, 300 * time.Millisecond},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			conf := createConfFile(t, []byte(fmt.Sprintf(tmpl, test.val)))
+			s, o := RunServerWithConfig(conf)
+			defer s.Shutdown()
+
+			if test.first {
+				if !o.LeafNode.TLSHandshakeFirst {
+					t.Fatal("Expected tls first to be true, was not")
+				}
+				if test.delay != o.LeafNode.TLSHandshakeFirstFallback {
+					t.Fatalf("Expected fallback delay to be %v, got %v", test.delay, o.LeafNode.TLSHandshakeFirstFallback)
+				}
+			} else {
+				if o.LeafNode.TLSHandshakeFirst {
+					t.Fatal("Expected tls first to be false, was not")
+				}
+				if o.LeafNode.TLSHandshakeFirstFallback != 0 {
+					t.Fatalf("Expected fallback delay to be 0, got %v", o.LeafNode.TLSHandshakeFirstFallback)
+				}
+			}
+		})
+	}
+}
+
 func TestLeafNodeTLSHandshakeFirst(t *testing.T) {
 	tmpl1 := `
 		port : -1
@@ -5049,6 +5102,18 @@ func TestLeafNodeTLSHandshakeFirst(t *testing.T) {
 	// Reload again with "true"
 	reloadUpdateConfig(t, s1, confHub, fmt.Sprintf(tmpl1, "true"))
 	checkLeafNodeConnected(t, s2)
+
+	// Shutdown the remote
+	s2.Shutdown()
+
+	// Reload the hub with handshake_first with a duration
+	reloadUpdateConfig(t, s1, confHub, fmt.Sprintf(tmpl1, `"250ms"`))
+
+	// Now start s2 with a remote that does not use handshake_first (set to false)
+	confSpoke = createConfFile(t, []byte(fmt.Sprintf(tmpl2, o1.LeafNode.Port, "false")))
+	s2, _ = RunServerWithConfig(confSpoke)
+	defer s2.Shutdown()
+	checkLeafNodeConnected(t, s2)
 }
 
 func TestLeafNodeTLSHandshakeEvenForRemoteWithNoTLSBlock(t *testing.T) {