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.

client/cmd/root.go

@@ -32,6 +32,7 @@ const (
 	interfaceNameFlag      = "interface-name"
 	wireguardPortFlag      = "wireguard-port"
 	disableAutoConnectFlag = "disable-auto-connect"
+	serverSSHAllowedFlag = "allow-server-ssh"
 )
 
 var (
@@ -55,6 +56,7 @@ var (
 	natExternalIPs          []string
 	customDNSAddress        string
 	rosenpassEnabled        bool
+	serverSSHAllowed        bool
 	interfaceName           string
 	wireguardPort           uint16
 	autoConnectDisabled     bool
@@ -128,6 +130,7 @@ func init() {
 			`E.g. --dns-resolver-address 127.0.0.1:5053 or --dns-resolver-address ""`,
 	)
 	upCmd.PersistentFlags().BoolVar(&rosenpassEnabled, enableRosenpassFlag, false, "[Experimental] Enable Rosenpass feature. If enabled, the connection will be post-quantum secured via Rosenpass.")
+	upCmd.PersistentFlags().BoolVar(&serverSSHAllowed, serverSSHAllowedFlag, false, "Allow SSH server on peer. If enabled, the SSH server will be permitted")
 	upCmd.PersistentFlags().BoolVar(&autoConnectDisabled, disableAutoConnectFlag, false, "Disables auto-connect feature. If enabled, then the client won't connect automatically when the service starts.")
 }
 
@@ -179,7 +182,7 @@ func FlagNameToEnvVar(cmdFlag string, prefix string) string {
 	return prefix + upper
 }
 
-// DialClientGRPCServer returns client connection to the dameno server.
+// DialClientGRPCServer returns client connection to the daemon server.
 func DialClientGRPCServer(ctx context.Context, addr string) (*grpc.ClientConn, error) {
 	ctx, cancel := context.WithTimeout(ctx, time.Second*3)
 	defer cancel()

client/cmd/up.go

@@ -94,6 +94,10 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
 		ic.RosenpassEnabled = &rosenpassEnabled
 	}
 
+	if cmd.Flag(serverSSHAllowedFlag).Changed {
+		ic.ServerSSHAllowed = &serverSSHAllowed
+	}
+
 	if cmd.Flag(interfaceNameFlag).Changed {
 		if err := parseInterfaceName(interfaceName); err != nil {
 			return err
@@ -192,6 +196,10 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error {
 		loginRequest.RosenpassEnabled = &rosenpassEnabled
 	}
 
+	if cmd.Flag(serverSSHAllowedFlag).Changed {
+		loginRequest.ServerSSHAllowed = &serverSSHAllowed
+	}
+
 	if cmd.Flag(disableAutoConnectFlag).Changed {
 		loginRequest.DisableAutoConnect = &autoConnectDisabled
 	}

client/internal/config.go

@@ -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,13 +291,24 @@ func update(input ConfigInput) (*Config, error) {
 
 	if input.RosenpassEnabled != nil {
 		config.RosenpassEnabled = *input.RosenpassEnabled
+		refresh = true
 	}
 
 	if input.DisableAutoConnect != nil {
 		config.DisableAutoConnect = *input.DisableAutoConnect
 		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 {

client/internal/connect.go

@@ -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 != "" {

client/internal/engine.go

@@ -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")
-		}
-	} 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)
+				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
 		}
-		e.sshServer = nil
+		return nil
+
 	}
-	return nil
 }
 
 func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error {

client/internal/engine_test.go

@@ -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{