From 8b62f7beb88bd36d45f1f417f3c8fcba47cea944 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 18 Apr 2022 18:25:19 -0700 Subject: [PATCH] ssh/tailssh: start moving auth checks earlier, adding banner Updates #3802 Change-Id: I243952f10f439387dc56b01d03b13657ddf25069 Signed-off-by: Brad Fitzpatrick --- ssh/tailssh/tailssh.go | 33 ++++++++++++++++------ ssh/tailssh/tailssh_test.go | 47 ++++++++++++++++--------------- tempfork/gliderlabs/ssh/server.go | 6 ---- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index cffd7016b..add5a8d61 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -87,7 +87,7 @@ func init() { // HandleSSHConn handles a Tailscale SSH connection from c. func (srv *server) HandleSSHConn(c net.Conn) error { - ss, err := srv.newSSHServer() + ss, err := srv.newSSHServer(c) if err != nil { return err } @@ -109,8 +109,28 @@ func (srv *server) OnPolicyChange() { } } -func (srv *server) newSSHServer() (*ssh.Server, error) { +func (srv *server) newSSHServer(c net.Conn) (*ssh.Server, error) { ss := &ssh.Server{ + ServerConfigCallback: func(ctx ssh.Context) *gossh.ServerConfig { + conf := &gossh.ServerConfig{ + NoClientAuth: true, + NoClientAuthCallback: func(m gossh.ConnMetadata) (*gossh.Permissions, error) { + if srv.requiresPubKey(m.User(), toIPPort(m.LocalAddr()), toIPPort(m.RemoteAddr())) { + return nil, errors.New("public key required") // any non-nil error will do + } + return nil, nil + }, + BannerCallback: func(m gossh.ConnMetadata) string { + // TODO(bradfitz): make this be a "you are rejected, contact + // your Tailnet admin etc etc" message or or the + // SSHAction.Message from the rejecting SSHAction if + // matched. + return "# Tailscale SSH server\n" + }, + } + return conf + }, + Handler: srv.handleSSH, RequestHandlers: map[string]ssh.RequestHandler{}, SubsystemHandlers: map[string]ssh.SubsystemHandler{}, @@ -122,12 +142,9 @@ func (srv *server) newSSHServer() (*ssh.Server, error) { }, Version: "SSH-2.0-Tailscale", LocalPortForwardingCallback: srv.mayForwardLocalPortTo, - NoClientAuthCallback: func(m gossh.ConnMetadata) (*gossh.Permissions, error) { - if srv.requiresPubKey(m.User(), toIPPort(m.LocalAddr()), toIPPort(m.RemoteAddr())) { - return nil, errors.New("public key required") // any non-nil error will do - } - return nil, nil - }, + + // TODO(bradfitz,maisem): don't register this hook if the policy doesn't + // involve any pubkey stuff at all for the user identified by c. PublicKeyHandler: func(ctx ssh.Context, key ssh.PublicKey) bool { if srv.acceptPubKey(ctx.User(), toIPPort(ctx.LocalAddr()), toIPPort(ctx.RemoteAddr()), key) { srv.logf("accepting SSH public key %s", bytes.TrimSpace(gossh.MarshalAuthorizedKey(key))) diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 7fed58c05..09d6ed6f6 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -211,34 +211,11 @@ func TestSSH(t *testing.T) { dir := t.TempDir() lb.SetVarRoot(dir) - srv := &server{ - lb: lb, - logf: logf, - } - ss, err := srv.newSSHServer() - if err != nil { - t.Fatal(err) - } - u, err := user.Current() if err != nil { t.Fatal(err) } - ci := &sshConnInfo{ - sshUser: "test", - src: netaddr.MustParseIPPort("1.2.3.4:32342"), - dst: netaddr.MustParseIPPort("1.2.3.5:22"), - node: &tailcfg.Node{}, - uprof: &tailcfg.UserProfile{}, - } - - ss.Handler = func(s ssh.Session) { - ss := srv.newSSHSession(s, ci, u) - ss.action = &tailcfg.SSHAction{Accept: true} - ss.run() - } - ln, err := net.Listen("tcp4", "127.0.0.1:0") if err != nil { t.Fatal(err) @@ -255,6 +232,30 @@ func TestSSH(t *testing.T) { } return } + + srv := &server{ + lb: lb, + logf: logf, + } + ss, err := srv.newSSHServer(c) + if err != nil { + t.Error(err) + return + } + + ci := &sshConnInfo{ + sshUser: "test", + src: netaddr.MustParseIPPort("1.2.3.4:32342"), + dst: netaddr.MustParseIPPort("1.2.3.5:22"), + node: &tailcfg.Node{}, + uprof: &tailcfg.UserProfile{}, + } + + ss.Handler = func(s ssh.Session) { + ss := srv.newSSHSession(s, ci, u) + ss.action = &tailcfg.SSHAction{Accept: true} + ss.run() + } go ss.HandleConn(c) } }() diff --git a/tempfork/gliderlabs/ssh/server.go b/tempfork/gliderlabs/ssh/server.go index c9372c43c..934139e2c 100644 --- a/tempfork/gliderlabs/ssh/server.go +++ b/tempfork/gliderlabs/ssh/server.go @@ -38,8 +38,6 @@ type Server struct { HostSigners []Signer // private keys for the host key, must have at least one Version string // server version to be sent before the initial handshake - NoClientAuthCallback func(gossh.ConnMetadata) (*gossh.Permissions, error) - KeyboardInteractiveHandler KeyboardInteractiveHandler // keyboard-interactive authentication handler PasswordHandler PasswordHandler // password authentication handler PublicKeyHandler PublicKeyHandler // public key authentication handler @@ -131,10 +129,6 @@ func (srv *Server) config(ctx Context) *gossh.ServerConfig { if srv.PasswordHandler == nil && srv.PublicKeyHandler == nil && srv.KeyboardInteractiveHandler == nil { config.NoClientAuth = true } - if srv.NoClientAuthCallback != nil { - config.NoClientAuth = true - config.NoClientAuthCallback = srv.NoClientAuthCallback - } if srv.Version != "" { config.ServerVersion = "SSH-2.0-" + srv.Version }