[FIXED/CHANGED] Reject server/cluster/gateway names with spaces (#5676)

Having cluster name with spaces in the context of a leaf node could
cause problems when a subscription on the leaf node would be propagated
in the cluster's hub.

Since when a gateway is specified the name of the gateway needs to match
the name of the cluster, the restriction of "no spaces" is added to
gateway name.

Finally, in the case of the leafnode, if no cluster name is specified
the server name is used, so also applying the restriction to the server
name.

Resolves #5633

Signed-off-by: Ivan Kozlovic <[email protected]>

Author: kozlovic

server/errors.go

@@ -153,6 +153,9 @@ var (
 	// Gateway's name.
 	ErrWrongGateway = errors.New("wrong gateway")
 
+	// ErrGatewayNameHasSpaces signals that the gateway name contains spaces, which is not allowed.
+	ErrGatewayNameHasSpaces = errors.New("gateway name cannot contain spaces")
+
 	// ErrNoSysAccount is returned when an attempt to publish or subscribe is made
 	// when there is no internal system account defined.
 	ErrNoSysAccount = errors.New("system account not setup")
@@ -163,6 +166,9 @@ var (
 	// ErrServerNotRunning is used to signal an error that a server is not running.
 	ErrServerNotRunning = errors.New("server is not running")
 
+	// ErrServerNameHasSpaces signals that the server name contains spaces, which is not allowed.
+	ErrServerNameHasSpaces = errors.New("server name cannot contain spaces")
+
 	// ErrBadMsgHeader signals the parser detected a bad message header
 	ErrBadMsgHeader = errors.New("bad message header detected")
 
@@ -180,6 +186,9 @@ var (
 	// ErrClusterNameRemoteConflict signals that a remote server has a different cluster name.
 	ErrClusterNameRemoteConflict = errors.New("cluster name from remote server conflicts")
 
+	// ErrClusterNameHasSpaces signals that the cluster name contains spaces, which is not allowed.
+	ErrClusterNameHasSpaces = errors.New("cluster name cannot contain spaces")
+
 	// ErrMalformedSubject is returned when a subscription is made with a subject that does not conform to subject rules.
 	ErrMalformedSubject = errors.New("malformed subject")
 

server/events_test.go

@@ -103,14 +103,14 @@ func runTrustedCluster(t *testing.T) (*Server, *Options, *Server, *Options, nkey
 	mr.Store(apub, jwt)
 
 	optsA := DefaultOptions()
-	optsA.Cluster.Name = "TEST CLUSTER 22"
+	optsA.Cluster.Name = "TEST_CLUSTER_22"
 	optsA.Cluster.Host = "127.0.0.1"
 	optsA.TrustedKeys = []string{pub}
 	optsA.AccountResolver = mr
 	optsA.SystemAccount = apub
 	optsA.ServerName = "A_SRV"
 	// Add in dummy gateway
-	optsA.Gateway.Name = "TEST CLUSTER 22"
+	optsA.Gateway.Name = "TEST_CLUSTER_22"
 	optsA.Gateway.Host = "127.0.0.1"
 	optsA.Gateway.Port = -1
 	optsA.gatewaysSolicitDelay = 30 * time.Second
@@ -1876,7 +1876,7 @@ func TestServerEventsStatsZ(t *testing.T) {
 	if m.Server.ID != sa.ID() {
 		t.Fatalf("Did not match IDs")
 	}
-	if m.Server.Cluster != "TEST CLUSTER 22" {
+	if m.Server.Cluster != "TEST_CLUSTER_22" {
 		t.Fatalf("Did not match cluster name")
 	}
 	if m.Server.Version != VERSION {
@@ -2978,11 +2978,11 @@ func TestServerEventsPingMonitorz(t *testing.T) {
 			[]string{"now", "routes"}},
 		{"ROUTEZ", json.RawMessage(`{"name":""}`), &Routez{},
 			[]string{"now", "routes"}},
-		{"ROUTEZ", json.RawMessage(`{"cluster":"TEST CLUSTER 22"}`), &Routez{},
+		{"ROUTEZ", json.RawMessage(`{"cluster":"TEST_CLUSTER_22"}`), &Routez{},
 			[]string{"now", "routes"}},
 		{"ROUTEZ", json.RawMessage(`{"cluster":"CLUSTER"}`), &Routez{},
 			[]string{"now", "routes"}},
-		{"ROUTEZ", json.RawMessage(`{"cluster":"TEST CLUSTER 22", "subscriptions":true}`), &Routez{},
+		{"ROUTEZ", json.RawMessage(`{"cluster":"TEST_CLUSTER_22", "subscriptions":true}`), &Routez{},
 			[]string{"now", "routes"}},
 
 		{"JSZ", nil, &JSzOptions{}, []string{"now", "disabled"}},
@@ -3083,8 +3083,8 @@ func TestGatewayNameClientInfo(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Could not parse INFO json: %v\n", err)
 	}
-	if info.Cluster != "TEST CLUSTER 22" {
-		t.Fatalf("Expected a cluster name of 'TEST CLUSTER 22', got %q", info.Cluster)
+	if info.Cluster != "TEST_CLUSTER_22" {
+		t.Fatalf("Expected a cluster name of 'TEST_CLUSTER_22', got %q", info.Cluster)
 	}
 }
 

server/gateway.go

@@ -18,12 +18,14 @@ import (
 	"crypto/sha256"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/rand"
 	"net"
 	"net/url"
 	"sort"
 	"strconv"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -299,17 +301,20 @@ func (r *RemoteGatewayOpts) clone() *RemoteGatewayOpts {
 
 // Ensure that gateway is properly configured.
 func validateGatewayOptions(o *Options) error {
-	if o.Gateway.Name == "" && o.Gateway.Port == 0 {
+	if o.Gateway.Name == _EMPTY_ && o.Gateway.Port == 0 {
 		return nil
 	}
-	if o.Gateway.Name == "" {
-		return fmt.Errorf("gateway has no name")
+	if o.Gateway.Name == _EMPTY_ {
+		return errors.New("gateway has no name")
+	}
+	if strings.Contains(o.Gateway.Name, " ") {
+		return ErrGatewayNameHasSpaces
 	}
 	if o.Gateway.Port == 0 {
 		return fmt.Errorf("gateway %q has no port specified (select -1 for random port)", o.Gateway.Name)
 	}
 	for i, g := range o.Gateway.Gateways {
-		if g.Name == "" {
+		if g.Name == _EMPTY_ {
 			return fmt.Errorf("gateway in the list %d has no name", i)
 		}
 		if len(g.URLs) == 0 {

server/leafnode.go

@@ -1288,6 +1288,13 @@ func (c *client) processLeafnodeInfo(info *Info) {
 			c.closeConnection(WrongPort)
 			return
 		}
+		// Reject a cluster that contains spaces.
+		if info.Cluster != _EMPTY_ && strings.Contains(info.Cluster, " ") {
+			c.mu.Unlock()
+			c.sendErrAndErr(ErrClusterNameHasSpaces.Error())
+			c.closeConnection(ProtocolViolation)
+			return
+		}
 		// Capture a nonce here.
 		c.nonce = []byte(info.Nonce)
 		if info.TLSRequired && didSolicit {
@@ -1779,6 +1786,13 @@ func (c *client) processLeafNodeConnect(s *Server, arg []byte, lang string) erro
 		return err
 	}
 
+	// Reject a cluster that contains spaces.
+	if proto.Cluster != _EMPTY_ && strings.Contains(proto.Cluster, " ") {
+		c.sendErrAndErr(ErrClusterNameHasSpaces.Error())
+		c.closeConnection(ProtocolViolation)
+		return ErrClusterNameHasSpaces
+	}
+
 	// Check for cluster name collisions.
 	if cn := s.cachedClusterName(); cn != _EMPTY_ && proto.Cluster != _EMPTY_ && proto.Cluster == cn {
 		c.sendErrAndErr(ErrLeafNodeHasSameClusterName.Error())

server/opts.go

@@ -917,7 +917,13 @@ func (o *Options) processConfigFileLine(k string, v any, errors *[]error, warnin
 	case "port":
 		o.Port = int(v.(int64))
 	case "server_name":
-		o.ServerName = v.(string)
+		sn := v.(string)
+		if strings.Contains(sn, " ") {
+			err := &configErr{tk, ErrServerNameHasSpaces.Error()}
+			*errors = append(*errors, err)
+			return
+		}
+		o.ServerName = sn
 	case "host", "net":
 		o.Host = v.(string)
 	case "debug":
@@ -1690,7 +1696,13 @@ func parseCluster(v any, opts *Options, errors *[]error, warnings *[]error) erro
 		tk, mv = unwrapValue(mv, &lt)
 		switch strings.ToLower(mk) {
 		case "name":
-			opts.Cluster.Name = mv.(string)
+			cn := mv.(string)
+			if strings.Contains(cn, " ") {
+				err := &configErr{tk, ErrClusterNameHasSpaces.Error()}
+				*errors = append(*errors, err)
+				continue
+			}
+			opts.Cluster.Name = cn
 		case "listen":
 			hp, err := parseListen(mv)
 			if err != nil {
@@ -1920,7 +1932,13 @@ func parseGateway(v any, o *Options, errors *[]error, warnings *[]error) error {
 		tk, mv = unwrapValue(mv, &lt)
 		switch strings.ToLower(mk) {
 		case "name":
-			o.Gateway.Name = mv.(string)
+			gn := mv.(string)
+			if strings.Contains(gn, " ") {
+				err := &configErr{tk, ErrGatewayNameHasSpaces.Error()}
+				*errors = append(*errors, err)
+				continue
+			}
+			o.Gateway.Name = gn
 		case "listen":
 			hp, err := parseListen(mv)
 			if err != nil {

server/server.go

@@ -1004,6 +1004,9 @@ func (s *Server) ClientURL() string {
 }
 
 func validateCluster(o *Options) error {
+	if o.Cluster.Name != _EMPTY_ && strings.Contains(o.Cluster.Name, " ") {
+		return ErrClusterNameHasSpaces
+	}
 	if o.Cluster.Compression.Mode != _EMPTY_ {
 		if err := validateAndNormalizeCompressionOption(&o.Cluster.Compression, CompressionS2Fast); err != nil {
 			return err
@@ -1013,8 +1016,9 @@ func validateCluster(o *Options) error {
 		return fmt.Errorf("cluster: %v", err)
 	}
 	// Check that cluster name if defined matches any gateway name.
-	if o.Gateway.Name != "" && o.Gateway.Name != o.Cluster.Name {
-		if o.Cluster.Name != "" {
+	// Note that we have already verified that the gateway name does not have spaces.
+	if o.Gateway.Name != _EMPTY_ && o.Gateway.Name != o.Cluster.Name {
+		if o.Cluster.Name != _EMPTY_ {
 			return ErrClusterNameConfigConflict
 		}
 		// Set this here so we do not consider it dynamic.
@@ -1055,6 +1059,9 @@ func validateOptions(o *Options) error {
 		return fmt.Errorf("max_payload (%v) cannot be higher than max_pending (%v)",
 			o.MaxPayload, o.MaxPending)
 	}
+	if o.ServerName != _EMPTY_ && strings.Contains(o.ServerName, " ") {
+		return errors.New("server name cannot contain spaces")
+	}
 	// Check that the trust configuration is correct.
 	if err := validateTrustedOperators(o); err != nil {
 		return err

server/server_test.go

@@ -2150,3 +2150,53 @@ func TestServerConfigLastLineComments(t *testing.T) {
 	require_NoError(t, err)
 	defer nc.Close()
 }
+
+func TestServerClusterAndGatewayNameNoSpace(t *testing.T) {
+	conf := createConfFile(t, []byte(`
+		port: -1
+		server_name: "my server"
+	`))
+	_, err := ProcessConfigFile(conf)
+	require_Error(t, err, ErrServerNameHasSpaces)
+
+	o := DefaultOptions()
+	o.ServerName = "my server"
+	_, err = NewServer(o)
+	require_Error(t, err, ErrServerNameHasSpaces)
+
+	conf = createConfFile(t, []byte(`
+		port: -1
+		server_name: "myserver"
+		cluster {
+			port: -1
+			name: "my cluster"
+		}
+	`))
+	_, err = ProcessConfigFile(conf)
+	require_Error(t, err, ErrClusterNameHasSpaces)
+
+	o = DefaultOptions()
+	o.Cluster.Name = "my cluster"
+	o.Cluster.Port = -1
+	_, err = NewServer(o)
+	require_Error(t, err, ErrClusterNameHasSpaces)
+
+	conf = createConfFile(t, []byte(`
+		port: -1
+		server_name: "myserver"
+		gateway {
+			port: -1
+			name: "my gateway"
+		}
+	`))
+	_, err = ProcessConfigFile(conf)
+	require_Error(t, err, ErrGatewayNameHasSpaces)
+
+	o = DefaultOptions()
+	o.Cluster.Name = _EMPTY_
+	o.Cluster.Port = 0
+	o.Gateway.Name = "my gateway"
+	o.Gateway.Port = -1
+	_, err = NewServer(o)
+	require_Error(t, err, ErrGatewayNameHasSpaces)
+}

test/leafnode_test.go

@@ -4426,3 +4426,27 @@ func TestLeafnodeHeaders(t *testing.T) {
 		t.Fatalf("leaf msg header is empty")
 	}
 }
+
+func TestLeafNodeClusterNameWithSpacesRejected(t *testing.T) {
+	s, opts := runLeafServer()
+	defer s.Shutdown()
+
+	lc := createLeafConn(t, opts.LeafNode.Host, opts.LeafNode.Port)
+	defer lc.Close()
+
+	checkInfoMsg(t, lc)
+	sendProto(t, lc, "CONNECT {\"cluster\":\"my cluster\"}\r\n")
+	expect := expectCommand(t, lc)
+	expect(errRe)
+	expectDisconnect(t, lc)
+
+	lc = createLeafConn(t, opts.LeafNode.Host, opts.LeafNode.Port)
+	defer lc.Close()
+	leafSend, leafExpect := setupLeaf(t, lc, 1)
+	// The accept side does expect an INFO from an handshake,
+	// so it will "ignore" the first one.
+	leafSend("INFO {\"server\":\"server\"}\r\n")
+	leafSend("INFO {\"cluster\":\"my cluster\"}\r\n")
+	leafExpect(errRe)
+	expectDisconnect(t, lc)
+}