From 58ab66ec51f1963fbee302c75ad0017d81d37884 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 8 May 2023 09:42:31 -0700 Subject: [PATCH] ssh/tailssh: support LDAP users for Tailscale SSH Fixes #4945 Change-Id: Ie013cb47684cb87928a44f92c66352310bfe53f1 Signed-off-by: Brad Fitzpatrick --- ssh/tailssh/incubator.go | 22 +++---- ssh/tailssh/tailssh.go | 17 +----- ssh/tailssh/tailssh_test.go | 13 +++- ssh/tailssh/user.go | 116 ++++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 28 deletions(-) create mode 100644 ssh/tailssh/user.go diff --git a/ssh/tailssh/incubator.go b/ssh/tailssh/incubator.go index d4815213d..8171eb409 100644 --- a/ssh/tailssh/incubator.go +++ b/ssh/tailssh/incubator.go @@ -688,18 +688,14 @@ func (ss *sshSession) startWithStdPipes() (err error) { return nil } -func loginShell(u *user.User) string { +func loginShell(u *userMeta) string { + if u.LoginShell != "" { + // This field should be populated on Linux, at least, because + // func userLookup on Linux uses "getent" to look up the user + // and that populates it. + return u.LoginShell + } switch runtime.GOOS { - case "linux": - if distro.Get() == distro.Gokrazy { - return "/tmp/serial-busybox/ash" - } - out, _ := exec.Command("getent", "passwd", u.Uid).Output() - // out is "root:x:0:0:root:/root:/bin/bash" - f := strings.SplitN(string(out), ":", 10) - if len(f) > 6 { - return strings.TrimSpace(f[6]) // shell - } case "darwin": // Note: /Users/username is key, and not the same as u.HomeDir. out, _ := exec.Command("dscl", ".", "-read", filepath.Join("/Users", u.Username), "UserShell").Output() @@ -715,12 +711,12 @@ func loginShell(u *user.User) string { return "/bin/sh" } -func envForUser(u *user.User) []string { +func envForUser(u *userMeta) []string { return []string{ fmt.Sprintf("SHELL=" + loginShell(u)), fmt.Sprintf("USER=" + u.Username), fmt.Sprintf("HOME=" + u.HomeDir), - fmt.Sprintf("PATH=" + defaultPathForUser(u)), + fmt.Sprintf("PATH=" + defaultPathForUser(&u.User)), } } diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index 63e14b86d..e15d4d991 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -22,7 +22,6 @@ "net/url" "os" "os/exec" - "os/user" "path/filepath" "runtime" "strconv" @@ -45,7 +44,6 @@ "tailscale.com/util/clientmetric" "tailscale.com/util/mak" "tailscale.com/util/multierr" - "tailscale.com/version/distro" ) var ( @@ -222,7 +220,7 @@ type conn struct { finalActionErr error // set by doPolicyAuth or resolveNextAction info *sshConnInfo // set by setInfo - localUser *user.User // set by doPolicyAuth + localUser *userMeta // set by doPolicyAuth userGroupIDs []string // set by doPolicyAuth pubKey gossh.PublicKey // set by doPolicyAuth @@ -379,16 +377,7 @@ func (c *conn) doPolicyAuth(ctx ssh.Context, pubKey ssh.PublicKey) error { if a.Accept { c.finalAction = a } - if runtime.GOOS == "linux" && distro.Get() == distro.Gokrazy { - // Gokrazy is a single-user appliance with ~no userspace. - // There aren't users to look up (no /etc/passwd, etc) - // so rather than fail below, just hardcode root. - // TODO(bradfitz): fix os/user upstream instead? - c.userGroupIDs = []string{"0"} - c.localUser = &user.User{Uid: "0", Gid: "0", Username: "root"} - return nil - } - lu, err := user.Lookup(localUser) + lu, err := userLookup(localUser) if err != nil { c.logf("failed to look up %v: %v", localUser, err) ctx.SendAuthBanner(fmt.Sprintf("failed to look up %v\r\n", localUser)) @@ -970,7 +959,7 @@ func (c *conn) detachSession(ss *sshSession) { // handleSSHAgentForwarding starts a Unix socket listener and in the background // forwards agent connections between the listener and the ssh.Session. // On success, it assigns ss.agentListener. -func (ss *sshSession) handleSSHAgentForwarding(s ssh.Session, lu *user.User) error { +func (ss *sshSession) handleSSHAgentForwarding(s ssh.Session, lu *userMeta) error { if !ssh.AgentRequested(ss) || !ss.conn.finalAction.AllowAgentForwarding { return nil } diff --git a/ssh/tailssh/tailssh_test.go b/ssh/tailssh/tailssh_test.go index 5eb1a3f62..d124b49df 100644 --- a/ssh/tailssh/tailssh_test.go +++ b/ssh/tailssh/tailssh_test.go @@ -851,7 +851,11 @@ func TestSSH(t *testing.T) { if err != nil { t.Fatal(err) } - sc.localUser = u + um, err := userLookup(u.Uid) + if err != nil { + t.Fatal(err) + } + sc.localUser = um sc.info = &sshConnInfo{ sshUser: "test", src: netip.MustParseAddrPort("1.2.3.4:32342"), @@ -1135,3 +1139,10 @@ func TestPathFromPAMEnvLineOnNixOS(t *testing.T) { } t.Logf("success; got=%q", got) } + +func TestStdOsUserUserAssumptions(t *testing.T) { + v := reflect.TypeOf(user.User{}) + if got, want := v.NumField(), 5; got != want { + t.Errorf("os/user.User has %v fields; this package assumes %v", got, want) + } +} diff --git a/ssh/tailssh/user.go b/ssh/tailssh/user.go new file mode 100644 index 000000000..775caa795 --- /dev/null +++ b/ssh/tailssh/user.go @@ -0,0 +1,116 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build linux || (darwin && !ios) || freebsd || openbsd + +package tailssh + +import ( + "context" + "errors" + "log" + "os/exec" + "os/user" + "runtime" + "strings" + "time" + "unicode/utf8" + + "tailscale.com/version/distro" +) + +// userMeta is a wrapper around *user.User with extra fields. +type userMeta struct { + user.User + + // LoginShell is the user's login shell. + LoginShell string +} + +// GroupIds returns the list of group IDs that the user is a member of. +func (u *userMeta) GroupIds() ([]string, error) { + if runtime.GOOS == "linux" && distro.Get() == distro.Gokrazy { + // Gokrazy is a single-user appliance with ~no userspace. + // There aren't users to look up (no /etc/passwd, etc) + // so rather than fail below, just hardcode root. + // TODO(bradfitz): fix os/user upstream instead? + return []string{"0"}, nil + } + return u.User.GroupIds() +} + +// userLookup is like os/user.LookupId but it returns a *userMeta wrapper +// around a *user.User with extra fields. +func userLookup(uid string) (*userMeta, error) { + if runtime.GOOS != "linux" { + return userLookupStd(uid) + } + + // No getent on Gokrazy. So hard-code the login shell. + if distro.Get() == distro.Gokrazy { + um, err := userLookupStd(uid) + if err == nil { + um.LoginShell = "/tmp/serial-busybox/ash" + } + return um, err + } + + // On Linux, default to using "getent" to look up users so that + // even with static tailscaled binaries without cgo (as we distribute), + // we can still look up PAM/NSS users which the standard library's + // os/user without cgo won't get (because of no libc hooks). + // But if "getent" fails, userLookupGetent falls back to the standard + // library anyway. + return userLookupGetent(uid) +} + +func validUsername(uid string) bool { + if len(uid) > 32 || len(uid) == 0 { + return false + } + for _, r := range uid { + if r < ' ' || r == 0x7f || r == utf8.RuneError { // TODO(bradfitz): more? + return false + } + } + return true +} + +func userLookupGetent(uid string) (*userMeta, error) { + // Do some basic validation before passing this string to "getent", even though + // getent should do its own validation. + if !validUsername(uid) { + return nil, errors.New("invalid username") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + out, err := exec.CommandContext(ctx, "getent", "passwd", uid).Output() + if err != nil { + log.Printf("error calling getent for user %q: %v", uid, err) + return userLookupStd(uid) + } + // output is "alice:x:1001:1001:Alice Smith,,,:/home/alice:/bin/bash" + f := strings.SplitN(strings.TrimSpace(string(out)), ":", 10) + for len(f) < 7 { + f = append(f, "") + } + um := &userMeta{ + User: user.User{ + Username: f[0], + Uid: f[2], + Gid: f[3], + Name: f[4], + HomeDir: f[5], + }, + LoginShell: f[6], + } + return um, nil +} + +func userLookupStd(uid string) (*userMeta, error) { + u, err := user.LookupId(uid) + if err != nil { + return nil, err + } + return &userMeta{User: *u}, nil +}