From 26f7ed858dc6f746a1e8a12cbf792f8676c301f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Sun, 9 Aug 2026 11:28:56 +0200 Subject: [PATCH] [android] ask for an SSH password only when the server needs one Connect() reports a password-required marker instead of a raw handshake error when a regular SSH server turns down the NetBird key, so the caller can prompt and retry as often as the user needs. NetBird servers are excluded: they authenticate with a JWT or the NetBird key, so a failure there is genuine. The marker is a string because gomobile flattens errors to their message across the binding. Errors that reach the terminal are unwrapped to their root cause, so a dial failure reads "i/o timeout" rather than repeating every layer that added context; the full chain still goes to the log. A normal shell exit no longer surfaces as "EOF". Reset() lets a closed client back a reconnect, which keeps the Java-side session and its scrollback alive across a drop, and the JWT flow now reports that it is waiting on the browser instead of blocking silently. --- client/android/ssh_client.go | 97 ++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/client/android/ssh_client.go b/client/android/ssh_client.go index 04fb12ddd..b1cc80d48 100644 --- a/client/android/ssh_client.go +++ b/client/android/ssh_client.go @@ -9,6 +9,7 @@ import ( "io" "net" "strconv" + "strings" "sync" "time" @@ -27,6 +28,13 @@ const ( sshDetectionTimeout = 5 * time.Second ) +// PasswordRequiredMarker tells Java to prompt for a password and retry. It is +// a string because gomobile flattens errors to their message, so a sentinel +// value would not survive the binding. +const PasswordRequiredMarker = "netbird-ssh-password-required" + +var errPasswordRequired = errors.New(PasswordRequiredMarker) + // SSHTerminalListener receives SSH session events. It is implemented in Java. // // All callbacks are invoked from goroutines and may run concurrently with each @@ -120,12 +128,47 @@ func (s *SSHClient) Connect(host string, port int, user, password string) error HostKeyCallback: hostKeyCallback, Timeout: sshDialTimeout, } - return s.dialAndHandshake(host, port, clientConfig) + err = s.dialAndHandshake(host, port, clientConfig) + + // A regular server may still accept a password, so let the caller ask for + // one instead of failing. NetBird servers never use a password, so a + // failure there is genuine. + if err != nil && serverType != detection.ServerTypeNetBirdJWT && + serverType != detection.ServerTypeNetBirdNoJWT && isAuthFailure(err) { + return errPasswordRequired + } + if err != nil { + log.Infof("SSH: connect to %s:%d failed: %v", host, port, err) + return rootCause(err) + } + return nil +} + +// isAuthFailure distinguishes credential rejection from dial, timeout and +// host-key errors, which retrying with a password would not fix. +func isAuthFailure(err error) bool { + if errors.Is(err, errPasswordRequired) { + return true + } + var partial *gossh.PartialSuccessError + if errors.As(err, &partial) { + return true + } + return strings.Contains(err.Error(), "unable to authenticate") } // StartSession requests a PTY and starts an interactive shell. Output from // the session is forwarded to the listener via OnData. func (s *SSHClient) StartSession(cols, rows int) error { + err := s.startSession(cols, rows) + if err != nil { + log.Infof("SSH: start session failed: %v", err) + return rootCause(err) + } + return nil +} + +func (s *SSHClient) startSession(cols, rows int) error { log.Debugf("SSH: starting session %dx%d", cols, rows) s.mu.Lock() sshClient := s.sshClient @@ -286,7 +329,9 @@ func (s *SSHClient) buildAuth(cfg *profilemanager.Config, engine *internal.Engin })) } if len(auths) == 0 { - return nil, nil, errors.New("no auth method available: provide a password or configure NetBird SSH key") + // Nothing to offer at all: ask for a password rather than failing, + // so the caller can retry once the user supplies one. + return nil, nil, errPasswordRequired } return auths, gossh.InsecureIgnoreHostKey(), nil // nolint:gosec // TOFU not yet implemented } @@ -315,6 +360,10 @@ func (s *SSHClient) requestJWTToken(cfg *profilemanager.Config) (string, error) go urlOpener.Open(flowInfo.VerificationURIComplete, flowInfo.UserCode) + // WaitToken blocks for as long as the browser round-trip takes, so say so + // rather than leaving the terminal blank. + s.notifyStatus("Waiting for browser authentication...") + tokenInfo, err := flow.WaitToken(ctx, flowInfo) if err != nil { return "", fmt.Errorf("wait for token: %w", err) @@ -375,15 +424,53 @@ func (s *SSHClient) readLoop(r io.Reader, name string) { } } if err != nil { - if !errors.Is(err, io.EOF) { - log.Debugf("ssh %s read: %v", name, err) + // EOF is a normal shell exit, so report it without a reason. + if errors.Is(err, io.EOF) { + s.notifyClose("") + return } - s.notifyClose(err.Error()) + log.Debugf("ssh %s read: %v", name, err) + s.notifyClose(rootCause(err).Error()) return } } } +// rootCause returns the innermost error of a %w chain, so the terminal shows +// "i/o timeout" rather than every layer that added context on the way up. +func rootCause(err error) error { + for { + // A joined error has no single root, so keep it as-is. + if _, ok := err.(interface{ Unwrap() []error }); ok { + return err + } + next := errors.Unwrap(err) + if next == nil { + return err + } + err = next + } +} + +// Reset makes a closed client usable for another Connect: Close leaves the +// one-shot guard set, and clearing it lets the same client back a reconnect. +func (s *SSHClient) Reset() { + s.mu.Lock() + defer s.mu.Unlock() + s.closed = false +} + +// notifyStatus writes a progress line to the terminal through the normal +// output path, so long steps are visible while nothing else is arriving. +func (s *SSHClient) notifyStatus(text string) { + s.mu.Lock() + listener := s.listener + s.mu.Unlock() + if listener != nil { + listener.OnData([]byte("\r\n\x1b[33m" + text + "\x1b[0m\r\n")) + } +} + func (s *SSHClient) notifyClose(reason string) { s.mu.Lock() if s.closed {