diff --git a/cmd/ssh-auth-none-demo/ssh-auth-none-demo.go b/cmd/ssh-auth-none-demo/ssh-auth-none-demo.go index dccfc3a25..f92c80ae4 100644 --- a/cmd/ssh-auth-none-demo/ssh-auth-none-demo.go +++ b/cmd/ssh-auth-none-demo/ssh-auth-none-demo.go @@ -65,8 +65,10 @@ func main() { ServerConfigCallback: func(ctx ssh.Context) *gossh.ServerConfig { start := time.Now() return &gossh.ServerConfig{ - ImplicitAuthMethod: "tailscale", - NoClientAuth: true, // required for the NoClientAuthCallback to run + NextAuthMethodCallback: func(conn gossh.ConnMetadata, prevErrors []error) []string { + return []string{"tailscale"} + }, + NoClientAuth: true, // required for the NoClientAuthCallback to run NoClientAuthCallback: func(cm gossh.ConnMetadata) (*gossh.Permissions, error) { cm.SendAuthBanner(fmt.Sprintf("# Banner: doing none auth at %v\r\n", time.Since(start))) diff --git a/go.mod b/go.mod index 00683171a..7e6ddfd61 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( github.com/tailscale/certstore v0.1.1-0.20220316223106-78d6e1c49d8d github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502 github.com/tailscale/goexpect v0.0.0-20210902213824-6e8c725cea41 - github.com/tailscale/golang-x-crypto v0.0.0-20221009170451-62f465106986 + github.com/tailscale/golang-x-crypto v0.0.0-20221011214003-2ffa11beee90 github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05 github.com/tailscale/hujson v0.0.0-20220630195928-54599719472f github.com/tailscale/mkctr v0.0.0-20220601142259-c0b937af2e89 diff --git a/go.sum b/go.sum index fa88ce39f..ccac75d61 100644 --- a/go.sum +++ b/go.sum @@ -1082,8 +1082,8 @@ github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502 h1:34icjjmqJ2HP github.com/tailscale/depaware v0.0.0-20210622194025-720c4b409502/go.mod h1:p9lPsd+cx33L3H9nNoecRRxPssFKUwwI50I3pZ0yT+8= github.com/tailscale/goexpect v0.0.0-20210902213824-6e8c725cea41 h1:/V2rCMMWcsjYaYO2MeovLw+ClP63OtXgCF2Y1eb8+Ns= github.com/tailscale/goexpect v0.0.0-20210902213824-6e8c725cea41/go.mod h1:/roCdA6gg6lQyw/Oz6gIIGu3ggJKYhF+WC/AQReE5XQ= -github.com/tailscale/golang-x-crypto v0.0.0-20221009170451-62f465106986 h1:jWSwTR9CY13oa2oxhR3FInk1ybqC1NbF9cFeoWrrx+E= -github.com/tailscale/golang-x-crypto v0.0.0-20221009170451-62f465106986/go.mod h1:95n9fbUCixVSI4QXLEvdKJjnYK2eUlkTx9+QwLPXFKU= +github.com/tailscale/golang-x-crypto v0.0.0-20221011214003-2ffa11beee90 h1:Vw3TVi00T2/J3yU22807VB0K0Fo8lNMUBEo2gL0L1bM= +github.com/tailscale/golang-x-crypto v0.0.0-20221011214003-2ffa11beee90/go.mod h1:95n9fbUCixVSI4QXLEvdKJjnYK2eUlkTx9+QwLPXFKU= github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05 h1:4chzWmimtJPxRs2O36yuGRW3f9SYV+bMTTvMBI0EKio= github.com/tailscale/goupnp v1.0.1-0.20210804011211-c64d0f06ea05/go.mod h1:PdCqy9JzfWMJf1H5UJW2ip33/d4YkoKN0r67yKH1mG8= github.com/tailscale/hujson v0.0.0-20220630195928-54599719472f h1:n4r/sJ92cBSBHK8n9lR1XLFr0OiTVeGfN5TR+9LaN7E= diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index 22ba54334..480fa4231 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -48,6 +48,13 @@ var ( sshVerboseLogging = envknob.RegisterBool("TS_DEBUG_SSH_VLOG") ) +const ( + // forcePasswordSuffix is the suffix at the end of a username that forces + // Tailscale SSH into password authentication mode to work around buggy SSH + // clients that get confused by successful replies to auth type "none". + forcePasswordSuffix = "+password" +) + // ipnLocalBackend is the subset of ipnlocal.LocalBackend that we use. // It is used for testing. type ipnLocalBackend interface { @@ -197,7 +204,10 @@ type conn struct { idH string connID string // ID that's shared with control - noPubKeyPolicyAuthError error // set by BannerCallback + // anyPasswordIsOkay is whether the client is authorized but has requested + // password-based auth to work around their buggy SSH client. When set, we + // accept any password in the PasswordHandler. + anyPasswordIsOkay bool // set by NoClientAuthCallback action0 *tailcfg.SSHAction // set by doPolicyAuth; first matching action currentAction *tailcfg.SSHAction // set by doPolicyAuth, updated by resolveNextAction @@ -273,7 +283,43 @@ func (c *conn) NoClientAuthCallback(ctx ssh.Context) error { if err := c.doPolicyAuth(ctx, nil /* no pub key */); err != nil { return err } - return c.isAuthorized(ctx) + if err := c.isAuthorized(ctx); err != nil { + return err + } + + // Let users specify a username ending in +password to force password auth. + // This exists for buggy SSH clients that get confused by success from + // "none" auth. + if strings.HasSuffix(ctx.User(), forcePasswordSuffix) { + c.anyPasswordIsOkay = true + return errors.New("any password please") // not shown to users + } + return nil +} + +func (c *conn) nextAuthMethodCallback(cm gossh.ConnMetadata, prevErrors []error) (nextMethod []string) { + switch { + case c.anyPasswordIsOkay: + nextMethod = append(nextMethod, "password") + case len(prevErrors) > 0 && prevErrors[len(prevErrors)-1] == errPubKeyRequired: + nextMethod = append(nextMethod, "publickey") + } + + // The fake "tailscale" method is always appended to next so OpenSSH renders + // that in parens as the final failure. (It also shows up in "ssh -v", etc) + nextMethod = append(nextMethod, "tailscale") + return +} + +// fakePasswordHandler is our implementation of the PasswordHandler hook that +// checks whether the user's password is correct. But we don't actually use +// passwords. This exists only for when the user's username ends in "+password" +// to signal that their SSH client is buggy and gets confused by auth type +// "none" succeeding and they want our SSH server to require a dummy password +// prompt instead. We then accept any password since we've already authenticated +// & authorized them. +func (c *conn) fakePasswordHandler(ctx ssh.Context, password string) bool { + return c.anyPasswordIsOkay } // PublicKeyHandler implements ssh.PublicKeyHandler is called by the @@ -345,9 +391,8 @@ func (c *conn) doPolicyAuth(ctx ssh.Context, pubKey ssh.PublicKey) error { // ServerConfig implements ssh.ServerConfigCallback. func (c *conn) ServerConfig(ctx ssh.Context) *gossh.ServerConfig { return &gossh.ServerConfig{ - // OpenSSH presents this on failure as `Permission denied (tailscale).` - ImplicitAuthMethod: "tailscale", - NoClientAuth: true, // required for the NoClientAuthCallback to run + NoClientAuth: true, // required for the NoClientAuthCallback to run + NextAuthMethodCallback: c.nextAuthMethodCallback, } } @@ -370,6 +415,7 @@ func (srv *server) newConn() (*conn, error) { NoClientAuthHandler: c.NoClientAuthCallback, PublicKeyHandler: c.PublicKeyHandler, + PasswordHandler: c.fakePasswordHandler, Handler: c.handleSessionPostSSHAuth, LocalPortForwardingCallback: c.mayForwardLocalPortTo, @@ -495,7 +541,7 @@ func (c *conn) setInfo(ctx ssh.Context) error { return nil } ci := &sshConnInfo{ - sshUser: ctx.User(), + sshUser: strings.TrimSuffix(ctx.User(), forcePasswordSuffix), src: toIPPort(ctx.RemoteAddr()), dst: toIPPort(ctx.LocalAddr()), } diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 7259d4cba..2a3156358 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -335,10 +335,12 @@ func TestSSHAuthFlow(t *testing.T) { }) tests := []struct { - name string - state *localState - wantBanners []string - authErr bool + name string + sshUser string // defaults to alice + state *localState + wantBanners []string + usesPassword bool + authErr bool }{ { name: "no-policy", @@ -410,6 +412,16 @@ func TestSSHAuthFlow(t *testing.T) { wantBanners: []string{"First", "Go Away!"}, authErr: true, }, + { + name: "force-password-auth", + sshUser: "alice+password", + state: &localState{ + sshEnabled: true, + matchingRule: acceptRule, + }, + usesPassword: true, + wantBanners: []string{"Welcome to Tailscale SSH!"}, + }, } s := &server{ logf: logger.Discard, @@ -420,9 +432,24 @@ func TestSSHAuthFlow(t *testing.T) { t.Run(tc.name, func(t *testing.T) { sc, dc := nettest.NewTCPConn(src, dst, 1024) s.lb = tc.state + sshUser := "alice" + if tc.sshUser != "" { + sshUser = tc.sshUser + } + var passwordUsed atomic.Bool cfg := &gossh.ClientConfig{ - User: "alice", + User: sshUser, HostKeyCallback: gossh.InsecureIgnoreHostKey(), + Auth: []gossh.AuthMethod{ + gossh.PasswordCallback(func() (secret string, err error) { + if !tc.usesPassword { + t.Error("unexpected use of PasswordCallback") + return "", errors.New("unexpected use of PasswordCallback") + } + passwordUsed.Store(true) + return "any-pass", nil + }), + }, BannerCallback: func(message string) error { if len(tc.wantBanners) == 0 { t.Errorf("unexpected banner: %q", message)