From 8ee044ea4a8a7710b298fa4b077b787319f92bb3 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 15 Apr 2022 13:19:13 -0700 Subject: [PATCH] ssh/tailssh: make the SSH server a singleton, register with LocalBackend Remove the weird netstack -> tailssh dependency and instead have tailssh register itself with ipnlocal when linked. This makes tailssh.server a singleton, so we can have a global map of all sessions. Updates #3802 Change-Id: Iad5caec3a26a33011796878ab66b8e7b49339f29 Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/depaware.txt | 2 +- cmd/tailscaled/ssh.go | 12 ++++ ipn/ipnlocal/local.go | 28 ++++++++ ssh/tailssh/tailssh.go | 68 +++++++++---------- .../tailscaled_deps_test_darwin.go | 1 + .../integration/tailscaled_deps_test_linux.go | 1 + wgengine/netstack/netstack.go | 5 +- wgengine/netstack/ssh.go | 14 ---- 8 files changed, 78 insertions(+), 53 deletions(-) create mode 100644 cmd/tailscaled/ssh.go delete mode 100644 wgengine/netstack/ssh.go diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 411861d37..b2e977957 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -231,7 +231,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/portlist from tailscale.com/ipn/ipnlocal tailscale.com/safesocket from tailscale.com/client/tailscale+ tailscale.com/smallzstd from tailscale.com/ipn/ipnserver+ - LD 💣 tailscale.com/ssh/tailssh from tailscale.com/wgengine/netstack + LD 💣 tailscale.com/ssh/tailssh from tailscale.com/cmd/tailscaled tailscale.com/syncs from tailscale.com/control/controlknobs+ tailscale.com/tailcfg from tailscale.com/client/tailscale+ LD tailscale.com/tempfork/gliderlabs/ssh from tailscale.com/ssh/tailssh diff --git a/cmd/tailscaled/ssh.go b/cmd/tailscaled/ssh.go new file mode 100644 index 000000000..e389535a4 --- /dev/null +++ b/cmd/tailscaled/ssh.go @@ -0,0 +1,12 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build linux || darwin +// +build linux darwin + +package main + +// Force registration of tailssh with LocalBackend. +import _ "tailscale.com/ssh/tailssh" + diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 1d7a63673..23e1489f5 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -73,6 +73,20 @@ func getControlDebugFlags() []string { return nil } +// SSHServer is the interface of the conditionally linked ssh/tailssh.server. +type SSHServer interface { + HandleSSHConn(net.Conn) error +} + +type newSSHServerFunc func(logger.Logf, *LocalBackend) (SSHServer, error) + +var newSSHServer newSSHServerFunc // or nil + +// RegisterNewSSHServer lets the conditionally linked ssh/tailssh package register itself. +func RegisterNewSSHServer(fn newSSHServerFunc) { + newSSHServer = fn +} + // LocalBackend is the glue between the major pieces of the Tailscale // network software: the cloud control plane (via controlclient), the // network data plane (via wgengine), and the user-facing UIs and CLIs @@ -103,6 +117,7 @@ type LocalBackend struct { newDecompressor func() (controlclient.Decompressor, error) varRoot string // or empty if SetVarRoot never called sshAtomicBool syncs.AtomicBool + sshServer SSHServer // or nil filterHash deephash.Sum @@ -205,6 +220,12 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, diale gotPortPollRes: make(chan struct{}), loginFlags: loginFlags, } + if newSSHServer != nil { + b.sshServer, err = newSSHServer(logf, b) + if err != nil { + return nil, fmt.Errorf("newSSHServer: %w", err) + } + } // Default filter blocks everything and logs nothing, until Start() is called. b.setFilter(filter.NewAllowNone(logf, &netaddr.IPSet{})) @@ -3225,3 +3246,10 @@ func (b *LocalBackend) DoNoiseRequest(req *http.Request) (*http.Response, error) } return cc.DoNoiseRequest(req) } + +func (b *LocalBackend) HandleSSHConn(c net.Conn) error { + if b.sshServer == nil { + return errors.New("no SSH server") + } + return b.sshServer.HandleSSHConn(c) +} diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index b37bd3476..c30f7e961 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -42,27 +42,41 @@ import ( "tailscale.com/types/logger" ) -var sshVerboseLogging = envknob.Bool("TS_DEBUG_SSH_VLOG") +var ( + debugPolicyFile = envknob.String("TS_DEBUG_SSH_POLICY_FILE") + debugIgnoreTailnetSSHPolicy = envknob.Bool("TS_DEBUG_SSH_IGNORE_TAILNET_POLICY") + sshVerboseLogging = envknob.Bool("TS_DEBUG_SSH_VLOG") +) -// TODO(bradfitz): this is all very temporary as code is temporarily -// being moved around; it will be restructured and documented in -// following commits. +type server struct { + lb *ipnlocal.LocalBackend + logf logger.Logf + tailscaledPath string -// Handle handles an SSH connection from c. -func Handle(logf logger.Logf, lb *ipnlocal.LocalBackend, c net.Conn) error { - tsd, err := os.Executable() - if err != nil { - return err - } - // TODO(bradfitz): make just one server for the whole process. rearrange - // netstack's hooks to be a constructor given a lb instead. Then the *server - // will have a HandleTailscaleConn method. - srv := &server{ - lb: lb, - logf: logf, - tailscaledPath: tsd, - } - ss, err := srv.newSSHServer() + // mu protects activeSessions. + mu sync.Mutex + activeSessionByH map[string]*sshSession // ssh.SessionID (DH H) => that session + activeSessionBySharedID map[string]*sshSession // yyymmddThhmmss-XXXXX => session +} + +func init() { + ipnlocal.RegisterNewSSHServer(func(logf logger.Logf, lb *ipnlocal.LocalBackend) (ipnlocal.SSHServer, error) { + tsd, err := os.Executable() + if err != nil { + return nil, err + } + srv := &server{ + lb: lb, + logf: logf, + tailscaledPath: tsd, + } + return srv, nil + }) +} + +// HandleSSHConn handles a Tailscale SSH connection from c. +func (s *server) HandleSSHConn(c net.Conn) error { + ss, err := s.newSSHServer() if err != nil { return err } @@ -121,22 +135,6 @@ func (srv *server) newSSHServer() (*ssh.Server, error) { return ss, nil } -type server struct { - lb *ipnlocal.LocalBackend - logf logger.Logf - tailscaledPath string - - // mu protects activeSessions. - mu sync.Mutex - activeSessionByH map[string]*sshSession // ssh.SessionID (DH H) => that session - activeSessionBySharedID map[string]*sshSession // yyymmddThhmmss-XXXXX => session -} - -var ( - debugPolicyFile = envknob.String("TS_DEBUG_SSH_POLICY_FILE") - debugIgnoreTailnetSSHPolicy = envknob.Bool("TS_DEBUG_SSH_IGNORE_TAILNET_POLICY") -) - // mayForwardLocalPortTo reports whether the ctx should be allowed to port forward // to the specified host and port. // TODO(bradfitz/maisem): should we have more checks on host/port? diff --git a/tstest/integration/tailscaled_deps_test_darwin.go b/tstest/integration/tailscaled_deps_test_darwin.go index 947a4e2f1..daf5b92fa 100644 --- a/tstest/integration/tailscaled_deps_test_darwin.go +++ b/tstest/integration/tailscaled_deps_test_darwin.go @@ -33,6 +33,7 @@ import ( _ "tailscale.com/net/tstun" _ "tailscale.com/paths" _ "tailscale.com/safesocket" + _ "tailscale.com/ssh/tailssh" _ "tailscale.com/tailcfg" _ "tailscale.com/tsweb" _ "tailscale.com/types/flagtype" diff --git a/tstest/integration/tailscaled_deps_test_linux.go b/tstest/integration/tailscaled_deps_test_linux.go index 947a4e2f1..daf5b92fa 100644 --- a/tstest/integration/tailscaled_deps_test_linux.go +++ b/tstest/integration/tailscaled_deps_test_linux.go @@ -33,6 +33,7 @@ import ( _ "tailscale.com/net/tstun" _ "tailscale.com/paths" _ "tailscale.com/safesocket" + _ "tailscale.com/ssh/tailssh" _ "tailscale.com/tailcfg" _ "tailscale.com/tsweb" _ "tailscale.com/types/flagtype" diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index bb7fe96d9..852c2a720 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -662,9 +662,8 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) { c := gonet.NewTCPConn(&wq, ep) if ns.lb != nil { - if reqDetails.LocalPort == 22 && ns.processSSH() && ns.isLocalIP(dialIP) && handleSSH != nil { - ns.logf("handling SSH connection....") - if err := handleSSH(ns.logf, ns.lb, c); err != nil { + if reqDetails.LocalPort == 22 && ns.processSSH() && ns.isLocalIP(dialIP) { + if err := ns.lb.HandleSSHConn(c); err != nil { ns.logf("ssh error: %v", err) } return diff --git a/wgengine/netstack/ssh.go b/wgengine/netstack/ssh.go deleted file mode 100644 index 36a17097c..000000000 --- a/wgengine/netstack/ssh.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build linux || (darwin && !ios) -// +build linux darwin,!ios - -package netstack - -import "tailscale.com/ssh/tailssh" - -func init() { - handleSSH = tailssh.Handle -}