[client] Rewrite the SSH feature #4015

lixmal · 2025-06-19T17:33:36Z

Describe your changes

NetBird SSH Client

Port forwarding
Windows support
Non-interactive commands
Single command execution over SSH

SSH Server

SFTP
Port forwarding (without user switching)
PTY (interactive, non-interactive)
Non-PTY (commands)
Windows support

New Flags

--enable-ssh-local-port-forwarding
--enable-ssh-remote-port-forwarding
--enable-ssh-root
--enable-ssh-sftp

Changes

Default port changed to 22022
Redirect port 22 to 22022 when SSH server enabled
Remove implicit OpenSSH firewall port
Native SSH clients: known_hosts setup with peer IPs and hostnames

Issue ticket number and link

Stack

Checklist

Is it a bug fix
Is a typo/documentation fix
Is a feature enhancement
It is a refactor
Created tests that fail without the change (if possible)
Extended the README / documentation, if necessary

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

This reverts commit 26fc32f.

Copilot

Pull Request Overview

This PR overhauls the SSH feature by extending the management API with new SSH flags, updating dependencies for SFTP support, and refactoring the client UI into a tabbed settings interface that includes SSH options.

Added enableSSHRoot, enableSSHSFTP, enableSSHLocalPortForwarding, and enableSSHRemotePortForwarding flags to management.proto.
Bumped Go module dependencies (including github.com/pkg/sftp) in go.mod.
Refactored client/ui/client_ui.go to use a tabbed settings form and integrated SSH settings.
Removed platform-specific setWinSize stubs in client/ssh.

Reviewed Changes

Copilot reviewed 77 out of 79 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
management/proto/management.proto	Added four new SSH-related boolean flags
go.mod	Updated dependency versions and added SFTP package
client/ui/client_ui.go	Refactored settings UI into tabs and added SSH form
client/ssh/window_windows.go	Removed Windows PTY resize stub
client/ssh/window_unix.go	Removed Unix PTY resize stub
client/ssh/window_freebsd.go	Removed FreeBSD PTY resize stub

Comments suppressed due to low confidence (3)

go.mod:195

The github.com/kr/fs dependency is added but not referenced in the changes; consider removing this unused indirect dependency to keep the module clean.

	github.com/kr/fs v0.1.0 // indirect

client/ui/client_ui.go:377

[nitpick] The settings window height was reduced from 500 to 400, which may truncate the SSH tab content. Verify the new layout and adjust the window size as needed to prevent clipping.

	s.wSettings.Resize(fyne.NewSize(600, 400))

pappz · 2025-07-08T13:45:39Z

Why do we have multiple lines?

root@ubuntu2:/etc/ssh/ssh_known_hosts.d# cat 99-netbird 
# NetBird SSH known hosts
# Generated automatically - do not edit manually

100.108.5.104,ubuntu1.netbird.selfhosted,ubuntu1,[100.108.5.104]:22,[100.108.5.104]:22022 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBt+HdgXSos2HtJP74gwd5M+lNwrLkbaLhQmRl23/AAg
100.108.5.104,ubuntu1.netbird.selfhosted,ubuntu1,[100.108.5.104]:22,[100.108.5.104]:22022 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBt+HdgXSos2HtJP74gwd5M+lNwrLkbaLhQmRl23/AAg
100.108.5.104,ubuntu1.netbird.selfhosted,ubuntu1,[100.108.5.104]:22,[100.108.5.104]:22022 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBt+HdgXSos2HtJP74gwd5M+lNwrLkbaLhQmRl23/AAg
100.108.5.104,ubuntu1.netbird.selfhosted,ubuntu1,[100.108.5.104]:22,[100.108.5.104]:22022 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBt+HdgXSos2HtJP74gwd5M+lNwrLkbaLhQmRl23/AAg
100.108.5.104,ubuntu1.netbird.selfhosted,ubuntu1,[100.108.5.104]:22,[100.108.5.104]:22022 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBt+HdgXSos2HtJP74gwd5M+lNwrLkbaLhQmRl23/AAg
root@ubuntu2:/etc/ssh/ssh_known_hosts.d#

pappz · 2025-07-08T13:51:48Z

What is the proper way to disable this feature and start using the original sshd configuration?

pappz · 2025-07-09T13:55:32Z

client/ssh/server/port_forwarding.go

+func (s *Server) SetAllowRemotePortForwarding(allow bool) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.allowRemotePortForwarding = allow


Does allowRemotePortForwarding take effect after we run the Start() function? Is it modifiable at runtime?

It's currently static but the field would take effect if changed

lixmal · 2025-07-11T20:04:22Z

@pappz

What is the proper way to disable this feature and start using the original sshd configuration?

netbird up --allow-server-ssh=false or through the dashboard

Why do we have multiple lines?

Will fix

sonarqubecloud · 2025-07-14T20:03:33Z

Quality Gate passed

Issues
0 New issues
4 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.8% Duplication on New Code

See analysis details on SonarQube Cloud

pappz · 2025-07-15T11:50:41Z

client/ssh/client/client.go

+}
+
+// setupSessionIO connects session streams to local terminal
+func (c *Client) setupSessionIO(ctx context.Context, session *ssh.Session) {


pappz · 2025-07-15T12:07:01Z

client/ssh/client/terminal_windows.go

+	return e.Err
+}
+
+var (


Const, vars preferred first, and after the types, functions in the source code

pappz · 2025-07-15T13:28:46Z

client/ssh/server/server.go

+}
+
+// initializeServerState sets up server state after successful setup
+func (s *Server) initializeServerState(ln net.Listener, sshServer *ssh.Server) {


I do not see any reason for this function. Maybe it would be better to eliminate.

pappz · 2025-07-15T13:30:54Z

client/ssh/server/server.go

+	s.initializeServerState(ln, sshServer)
+	log.Infof("SSH server started on %s", addrDesc)
+
+	go s.serve(ln, sshServer)


Why do you pass ln and sshServer as parameters if the serve function is not static?

pappz · 2025-07-15T13:31:18Z

client/ssh/server/server.go

+	}
+
+	s.sshServer = nil
+	s.listener = nil


Who closes the s.listener?

if it is not required to close, then s.listener variable is unused

pappz · 2025-07-15T13:34:08Z

client/internal/engine_ssh.go

+		return errors.New("invalid local NetBird address")
+	}
+
+	if err := e.firewall.AddInboundDNAT(localAddr, firewallManager.ProtocolTCP, 22, 22022); err != nil {


Are these ports not configurable by command line?

lixmal added 16 commits June 17, 2025 02:48

Implement dns routes for Android

bb74e90

Fix test

49bbd90

Fix lint issues

50ac3d4

Remove some allocations

631b77d

Rename test files

8e94d85

Add tests

8684981

Improve nat perf

7cd44a9

Fix lint

2952669

Merge branch 'main' into android-dns-routes

51b9e93

Swap tracking and nat order

1a3b04d

Rename methods

471f90e

Fix spelling

d47c6b6

Remove nil checks

f51ce7c

Extract static error

9468e69

Set up fake ip route only if the dns feature flag is enabled

306d75f

Merge branch 'main' into android-dns-routes

a79cb1c

lixmal force-pushed the ssh-rewrite branch from 59b4dbf to 741da39 Compare June 19, 2025 17:44

lixmal marked this pull request as ready for review June 20, 2025 15:25

Copilot AI review requested due to automatic review settings June 20, 2025 15:25

This comment was marked as outdated.

Sign in to view

lixmal marked this pull request as draft June 22, 2025 14:49

lixmal added 9 commits June 25, 2025 15:03

Fix errorf

26fc32f

Revert "Fix errorf"

c788403

This reverts commit 26fc32f.

Display domains properly

5fc95d4

Use r

11bdf5b

Merge branch 'main' into android-dns-routes

5299e9f

Fix route notificaiton

0f79a89

Remove implicit inbound ssh firewall rules and change default port

520f2cf

Refactor ssh server and client

6ed846a

Tidy mod

f56075c

lixmal force-pushed the ssh-rewrite branch from 95dc81d to ff362b6 Compare July 3, 2025 13:17

Fix username validation and skip ci tests properly

0889566

lixmal force-pushed the ssh-rewrite branch from ff362b6 to 0889566 Compare July 3, 2025 13:36

lixmal marked this pull request as ready for review July 3, 2025 14:17

lixmal requested review from pappz and Copilot July 3, 2025 14:17

This comment was marked as outdated.

Sign in to view

lixmal requested a review from Copilot July 4, 2025 09:05

Copilot AI reviewed Jul 4, 2025

View reviewed changes

Base automatically changed from android-dns-routes to main July 4, 2025 14:43

lixmal added 2 commits July 4, 2025 16:53

Merge branch 'main' into ssh-rewrite

a4b0675

Remove empty file

e4e0b8f

pappz reviewed Jul 9, 2025

View reviewed changes

lixmal added 6 commits July 11, 2025 22:05

Merge branch 'main' into ssh-rewrite

cdded8c

Fix client ssh file

9a7daa1

Fix proto

ac71208

Fix build

fa893aa

Fix known hosts entries

d93b7c2

Generate ssh_config independently of ssh server

758a97c

pappz reviewed Jul 15, 2025

View reviewed changes

pappz self-requested a review July 21, 2025 13:17

Uh oh!

[client] Rewrite the SSH feature #4015

Are you sure you want to change the base?

[client] Rewrite the SSH feature #4015

Uh oh!

Conversation

lixmal commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

NetBird SSH Client

SSH Server

New Flags

Changes

Issue ticket number and link

Stack

Checklist

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

pappz commented Jul 8, 2025

Uh oh!

pappz commented Jul 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lixmal Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lixmal commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Jul 14, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lixmal commented Jun 19, 2025 •

edited

Loading

lixmal Jul 11, 2025 •

edited

Loading

lixmal commented Jul 11, 2025 •

edited

Loading