ssh/tailssh: start moving auth checks earlier, adding banner

Updates #3802

Change-Id: I243952f10f439387dc56b01d03b13657ddf25069
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

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)))

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)
 		}
 	}()

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
 	}