Disable SSH server by default on client side and add the flag --allow-server-ssh to enable it (#1508)

This changes the default behavior for new peers, by requiring the agent to be executed with allow-server-ssh set to true in order for the management configuration to take effect.
2026-10-04 04:29:06 +02:00 · 2024-02-20 11:13:27 +01:00
parent 8fd4166c53
commit d5338c09dc
10 changed files with 296 additions and 199 deletions
@@ -35,10 +35,11 @@ var defaultInterfaceBlacklist = []string{iface.WgInterfaceDefault, "wt", "utun",

 // ConfigInput carries configuration changes to the client
 type ConfigInput struct {
-	ManagementURL      string
-	AdminURL           string
-	ConfigPath         string
-	PreSharedKey       *string
+	ManagementURL    string
+	AdminURL         string
+	ConfigPath       string
+	PreSharedKey     *string
+	ServerSSHAllowed *bool
 	NATExternalIPs     []string
 	CustomDNSAddress   []byte
 	RosenpassEnabled   *bool
@@ -59,8 +60,9 @@ type Config struct {
 	IFaceBlackList       []string
 	DisableIPv6Discovery bool
 	RosenpassEnabled     bool
+	ServerSSHAllowed     *bool
 	// SSHKey is a private SSH key in a PEM format
-	SSHKey string
+	SSHKey               string

 	// ExternalIP mappings, if different from the host interface IP
 	//
@@ -77,9 +79,9 @@ type Config struct {
 	//      "12.34.56.78/eth0"     => IPv4 assigned to interface eth0 will be mapped to external IP of 12.34.56.78
 	//      "12.34.56.78/10.1.2.3" => interface IP 10.1.2.3 will be mapped to external IP of 12.34.56.78

-	NATExternalIPs []string
+	NATExternalIPs       []string
 	// CustomDNSAddress sets the DNS resolver listening address in format ip:port
-	CustomDNSAddress string
+	CustomDNSAddress     string

 	// DisableAutoConnect determines whether the client should not start with the service
 	// it's set to false by default due to backwards compatibility
@@ -93,6 +95,7 @@ func ReadConfig(configPath string) (*Config, error) {
 		if _, err := util.ReadJson(configPath, config); err != nil {
 			return nil, err
 		}
+
 		return config, nil
 	}

@@ -157,6 +160,7 @@ func createNewConfig(input ConfigInput) (*Config, error) {
 		DisableIPv6Discovery: false,
 		NATExternalIPs:       input.NATExternalIPs,
 		CustomDNSAddress:     string(input.CustomDNSAddress),
+		ServerSSHAllowed:     util.False(),
 		DisableAutoConnect:   false,
 	}

@@ -192,6 +196,10 @@ func createNewConfig(input ConfigInput) (*Config, error) {
 		config.RosenpassEnabled = *input.RosenpassEnabled
 	}

+	if input.ServerSSHAllowed != nil {
+		config.ServerSSHAllowed = input.ServerSSHAllowed
+	}
+
 	defaultAdminURL, err := parseURL("Admin URL", DefaultAdminURL)
 	if err != nil {
 		return nil, err
@@ -283,6 +291,7 @@ func update(input ConfigInput) (*Config, error) {

 	if input.RosenpassEnabled != nil {
 		config.RosenpassEnabled = *input.RosenpassEnabled
+		refresh = true
 	}

 	if input.DisableAutoConnect != nil {
@@ -290,6 +299,16 @@ func update(input ConfigInput) (*Config, error) {
 		refresh = true
 	}

+	if input.ServerSSHAllowed != nil {
+		config.ServerSSHAllowed = input.ServerSSHAllowed
+		refresh = true
+	}
+
+	if config.ServerSSHAllowed == nil {
+		config.ServerSSHAllowed = util.True()
+		refresh = true
+	}
+
 	if refresh {
 		// since we have new management URL, we need to update config file
 		if err := util.WriteJson(input.ConfigPath, config); err != nil {
@@ -23,6 +23,7 @@ import (
 	mgm "github.com/netbirdio/netbird/management/client"
 	mgmProto "github.com/netbirdio/netbird/management/proto"
 	signal "github.com/netbirdio/netbird/signal/client"
+	"github.com/netbirdio/netbird/util"
 	"github.com/netbirdio/netbird/version"
 )

@@ -283,6 +284,7 @@ func createEngineConfig(key wgtypes.Key, config *Config, peerConfig *mgmProto.Pe
 		NATExternalIPs:       config.NATExternalIPs,
 		CustomDNSAddress:     config.CustomDNSAddress,
 		RosenpassEnabled:     config.RosenpassEnabled,
+		ServerSSHAllowed:     util.ReturnBoolWithDefaultTrue(config.ServerSSHAllowed),
 	}

 	if config.PreSharedKey != "" {
@@ -80,6 +80,8 @@ type EngineConfig struct {
 	CustomDNSAddress string

 	RosenpassEnabled bool
+
+	ServerSSHAllowed bool
 }

 // Engine is a mechanism responsible for reacting on Signal and Management stream events and managing connections to the remote peers.
@@ -482,44 +484,52 @@ func isNil(server nbssh.Server) bool {
 }

 func (e *Engine) updateSSH(sshConf *mgmProto.SSHConfig) error {
-	if sshConf.GetSshEnabled() {
-		if runtime.GOOS == "windows" {
-			log.Warnf("running SSH server on Windows is not supported")
-			return nil
-		}
-		// start SSH server if it wasn't running
-		if isNil(e.sshServer) {
-			// nil sshServer means it has not yet been started
-			var err error
-			e.sshServer, err = e.sshServerFunc(e.config.SSHKey,
-				fmt.Sprintf("%s:%d", e.wgInterface.Address().IP.String(), nbssh.DefaultSSHPort))
-			if err != nil {
-				return err
+	
+	if !e.config.ServerSSHAllowed {
+		log.Warnf("running SSH server is not permitted")
+		return nil
+	} else {
+	
+		if sshConf.GetSshEnabled() {
+			if runtime.GOOS == "windows" {
+				log.Warnf("running SSH server on Windows is not supported")
+				return nil
 			}
-			go func() {
-				// blocking
-				err = e.sshServer.Start()
+			// start SSH server if it wasn't running
+			if isNil(e.sshServer) {
+				// nil sshServer means it has not yet been started
+				var err error
+				e.sshServer, err = e.sshServerFunc(e.config.SSHKey,
+					fmt.Sprintf("%s:%d", e.wgInterface.Address().IP.String(), nbssh.DefaultSSHPort))
 				if err != nil {
-					// will throw error when we stop it even if it is a graceful stop
-					log.Debugf("stopped SSH server with error %v", err)
+					return err
 				}
-				e.syncMsgMux.Lock()
-				defer e.syncMsgMux.Unlock()
-				e.sshServer = nil
-				log.Infof("stopped SSH server")
-			}()
-		} else {
-			log.Debugf("SSH server is already running")
+				go func() {
+					// blocking
+					err = e.sshServer.Start()
+					if err != nil {
+						// will throw error when we stop it even if it is a graceful stop
+						log.Debugf("stopped SSH server with error %v", err)
+					}
+					e.syncMsgMux.Lock()
+					defer e.syncMsgMux.Unlock()
+					e.sshServer = nil
+					log.Infof("stopped SSH server")
+				}()
+			} else {
+				log.Debugf("SSH server is already running")
+			}
+		} else if !isNil(e.sshServer) {
+			// Disable SSH server request, so stop it if it was running
+			err := e.sshServer.Stop()
+			if err != nil {
+				log.Warnf("failed to stop SSH server %v", err)
+			}
+			e.sshServer = nil
 		}
-	} else if !isNil(e.sshServer) {
-		// Disable SSH server request, so stop it if it was running
-		err := e.sshServer.Stop()
-		if err != nil {
-			log.Warnf("failed to stop SSH server %v", err)
-		}
-		e.sshServer = nil
+		return nil
+
 	}
-	return nil
 }

 func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error {
@@ -74,6 +74,7 @@ func TestEngine_SSH(t *testing.T) {
 		WgAddr:       "100.64.0.1/24",
 		WgPrivateKey: key,
 		WgPort:       33100,
+		ServerSSHAllowed: true,
 	}, MobileDependency{}, peer.NewRecorder("https://mgm"))

 	engine.dnsServer = &dns.MockServer{