Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion client/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const (
interfaceNameFlag = "interface-name"
wireguardPortFlag = "wireguard-port"
disableAutoConnectFlag = "disable-auto-connect"
serverSSHAllowedFlag = "allow-server-ssh"
)

var (
Expand All @@ -55,6 +56,7 @@ var (
natExternalIPs []string
customDNSAddress string
rosenpassEnabled bool
serverSSHAllowed bool
interfaceName string
wireguardPort uint16
autoConnectDisabled bool
Expand Down Expand Up @@ -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.")
}

Expand Down Expand Up @@ -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()
Expand Down
8 changes: 8 additions & 0 deletions client/cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
33 changes: 26 additions & 7 deletions client/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
//
Expand All @@ -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
Expand All @@ -93,6 +95,7 @@ func ReadConfig(configPath string) (*Config, error) {
if _, err := util.ReadJson(configPath, config); err != nil {
return nil, err
}

return config, nil
}

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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

bcmmbaga marked this conversation as resolved.
Show resolved Hide resolved
if refresh {
// since we have new management URL, we need to update config file
if err := util.WriteJson(input.ConfigPath, config); err != nil {
Expand Down
2 changes: 2 additions & 0 deletions client/internal/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 != "" {
Expand Down
76 changes: 43 additions & 33 deletions client/internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions client/internal/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
Loading
Loading