Skip to content

Commit

Permalink
Fix home dir for insecure_drop mode (#42660)
Browse files Browse the repository at this point in the history 
This change fixes a bug where file uploads/downloads in the web UI
would fail if the local user (provisioned by Teleport in
insecure-drop mode) didn't have a home directory.
  • Loading branch information
@atburke
atburke committed Jun 10, 2024
1 parent ba1db85 commit dc0009d
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 46 deletions.
77 changes: 54 additions & 23 deletions integration/hostuser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,26 @@ func TestRootHostUsersBackend(t *testing.T) {
SudoersPath: sudoersTestDir,
HostUUID: "hostuuid",
}
t.Cleanup(func() {
// cleanup users if they got left behind due to a failing test
host.UserDel(testuser)
cmd := exec.Command("groupdel", testgroup)
cmd.Run()
})

t.Run("Test CreateGroup", func(t *testing.T) {
t.Cleanup(cleanupUsersAndGroups(nil, []string{testgroup}))

err := usersbk.CreateGroup(testgroup, "")
require.NoError(t, err)
err = usersbk.CreateGroup(testgroup, "")
require.True(t, trace.IsAlreadyExists(err))

_, err = usersbk.LookupGroup(testgroup)
require.NoError(t, err)
})

t.Run("Test CreateUser and group", func(t *testing.T) {
err := usersbk.CreateUser(testuser, []string{testgroup}, "", "")
t.Cleanup(cleanupUsersAndGroups([]string{testuser}, []string{testgroup}))
err := usersbk.CreateGroup(testgroup, "")
require.NoError(t, err)

testHome := filepath.Join("/home", testuser)
err = usersbk.CreateUser(testuser, []string{testgroup}, testHome, "", "")
require.NoError(t, err)

tuser, err := usersbk.Lookup(testuser)
Expand All @@ -89,17 +93,26 @@ func TestRootHostUsersBackend(t *testing.T) {
require.NoError(t, err)
require.Contains(t, tuserGids, group.Gid)

err = usersbk.CreateUser(testuser, []string{}, "", "")
err = usersbk.CreateUser(testuser, []string{}, testHome, "", "")
require.True(t, trace.IsAlreadyExists(err))

require.NoFileExists(t, filepath.Join("/home", testuser))
err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid)
require.NoFileExists(t, testHome)
err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid)
require.NoError(t, err)
require.FileExists(t, filepath.Join("/home", testuser, ".bashrc"))
t.Cleanup(func() {
os.RemoveAll(testHome)
})
require.FileExists(t, filepath.Join(testHome, ".bashrc"))
})

t.Run("Test DeleteUser", func(t *testing.T) {
err := usersbk.DeleteUser(testuser)
t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))
err := usersbk.CreateUser(testuser, nil, "", "", "")
require.NoError(t, err)
_, err = usersbk.Lookup(testuser)
require.NoError(t, err)

err = usersbk.DeleteUser(testuser)
require.NoError(t, err)

_, err = user.Lookup(testuser)
Expand All @@ -108,13 +121,10 @@ func TestRootHostUsersBackend(t *testing.T) {

t.Run("Test GetAllUsers", func(t *testing.T) {
checkUsers := []string{"teleport-usera", "teleport-userb", "teleport-userc"}
t.Cleanup(func() {
for _, u := range checkUsers {
usersbk.DeleteUser(u)
}
})
t.Cleanup(cleanupUsersAndGroups(checkUsers, nil))

for _, u := range checkUsers {
err := usersbk.CreateUser(u, []string{}, "", "")
err := usersbk.CreateUser(u, []string{}, "", "", "")
require.NoError(t, err)
}

Expand Down Expand Up @@ -143,7 +153,8 @@ func TestRootHostUsersBackend(t *testing.T) {
})

t.Run("Test CreateHomeDirectory does not follow symlinks", func(t *testing.T) {
err := usersbk.CreateUser(testuser, []string{testgroup}, "", "")
t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))
err := usersbk.CreateUser(testuser, nil, "", "", "")
require.NoError(t, err)

tuser, err := usersbk.Lookup(testuser)
Expand All @@ -154,14 +165,15 @@ func TestRootHostUsersBackend(t *testing.T) {
bashrcPath := filepath.Join("/home", testuser, ".bashrc")
require.NoFileExists(t, bashrcPath)

require.NoError(t, os.MkdirAll(filepath.Join("/home", testuser), 0o700))
testHome := filepath.Join("/home", testuser)
require.NoError(t, os.MkdirAll(testHome, 0o700))

require.NoError(t, os.Symlink("/tmp/ignoreme", bashrcPath))
require.NoFileExists(t, "/tmp/ignoreme")

err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid)
err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid)
t.Cleanup(func() {
os.RemoveAll(filepath.Join("/home", testuser))
os.RemoveAll(testHome)
})
require.NoError(t, err)
require.NoFileExists(t, "/tmp/ignoreme")
Expand Down Expand Up @@ -216,7 +228,7 @@ func TestRootHostUsers(t *testing.T) {
u, err := user.Lookup(testuser)
require.NoError(t, err)
requireUserInGroups(t, u, testGroups)
require.NoDirExists(t, u.HomeDir)
require.Equal(t, string(os.PathSeparator), u.HomeDir)

require.NoError(t, closer.Close())
_, err = user.Lookup(testuser)
Expand Down Expand Up @@ -256,6 +268,25 @@ func TestRootHostUsers(t *testing.T) {
require.Equal(t, err, user.UnknownUserError(testuser))
})

t.Run("test create permanent user", func(t *testing.T) {
users := srv.NewHostUsers(context.Background(), presence, "host_uuid")
expectedHome := filepath.Join("/home", testuser)
require.NoDirExists(t, expectedHome)

closer, err := users.CreateUser(testuser, &services.HostUsersInfo{Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP})
require.NoError(t, err)
require.Nil(t, closer)
t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))

u, err := user.Lookup(testuser)
require.NoError(t, err)
require.Equal(t, expectedHome, u.HomeDir)
require.DirExists(t, expectedHome)
t.Cleanup(func() {
os.RemoveAll(expectedHome)
})
})

t.Run("test create sudoers enabled users", func(t *testing.T) {
if _, err := exec.LookPath("visudo"); err != nil {
t.Skip("Visudo not found on path")
Expand Down
18 changes: 13 additions & 5 deletions lib/srv/usermgmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ type HostUsersBackend interface {
// CreateGroup creates a group on a host.
CreateGroup(group string, gid string) error
// CreateUser creates a user on a host.
CreateUser(name string, groups []string, uid, gid string) error
CreateUser(name string, groups []string, home, uid, gid string) error
// DeleteUser deletes a user from a host.
DeleteUser(name string) error
// CreateHomeDirectory creates the users home directory and copies in /etc/skel
CreateHomeDirectory(user string, uid, gid string) error
CreateHomeDirectory(userHome string, uid, gid string) error
}

type userCloser struct {
Expand Down Expand Up @@ -298,6 +298,14 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
return nil, trace.WrapWithMessage(err, "error while creating groups")
}

var home string
if ui.Mode != types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP {
home, err = readDefaultHome(name)
if err != nil {
return nil, trace.Wrap(err)
}
}

err = u.doWithUserLock(func(_ types.SemaphoreLease) error {
if err := u.storage.UpsertHostUserInteractionTime(u.ctx, name, time.Now()); err != nil {
return trace.Wrap(err)
Expand All @@ -310,7 +318,7 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
}
}

err = u.backend.CreateUser(name, groups, ui.UID, ui.GID)
err = u.backend.CreateUser(name, groups, home, ui.UID, ui.GID)
if err != nil && !trace.IsAlreadyExists(err) {
return trace.WrapWithMessage(err, "error while creating user")
}
Expand All @@ -320,8 +328,8 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
return trace.Wrap(err)
}

if ui.Mode != types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP {
if err := u.backend.CreateHomeDirectory(name, user.Uid, user.Gid); err != nil {
if home != "" {
if err := u.backend.CreateHomeDirectory(home, user.Uid, user.Gid); err != nil {
return trace.Wrap(err)
}
}
Expand Down
15 changes: 3 additions & 12 deletions lib/srv/usermgmt_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,8 @@ func (*HostUsersProvisioningBackend) CreateGroup(name string, gid string) error
}

// CreateUser creates a user on a host
func (*HostUsersProvisioningBackend) CreateUser(name string, groups []string, uid, gid string) error {
home, err := readDefaultHome(name)
if err != nil {
return trace.Wrap(err)
}
_, err = host.UserAdd(name, groups, home, uid, gid)
func (*HostUsersProvisioningBackend) CreateUser(name string, groups []string, home, uid, gid string) error {
_, err := host.UserAdd(name, groups, home, uid, gid)
return trace.Wrap(err)
}

Expand Down Expand Up @@ -214,7 +210,7 @@ func readDefaultSkel() (string, error) {
return skel, trace.Wrap(err)
}

func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gidS string) error {
func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS string) error {
uid, err := strconv.Atoi(uidS)
if err != nil {
return trace.Wrap(err)
Expand All @@ -224,11 +220,6 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gi
return trace.Wrap(err)
}

userHome, err := readDefaultHome(user)
if err != nil {
return trace.Wrap(err)
}

err = os.Mkdir(userHome, 0o700)
if err != nil {
if os.IsExist(err) {
Expand Down
4 changes: 4 additions & 0 deletions lib/srv/usermgmt_other.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ func newHostUsersBackend() (HostUsersBackend, error) {
func newHostSudoersBackend(_ string) (HostSudoersBackend, error) {
return nil, trace.NotImplemented("Host user creation management is only supported on linux")
}

func readDefaultHome(user string) (string, error) {
return "", trace.NotImplemented("readDefaultHome is only supported on linux")
}
8 changes: 4 additions & 4 deletions lib/srv/usermgmt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (tm *testHostUserBackend) CreateGroup(group, gid string) error {
return nil
}

func (tm *testHostUserBackend) CreateUser(user string, groups []string, uid, gid string) error {
func (tm *testHostUserBackend) CreateUser(user string, groups []string, home, uid, gid string) error {
_, ok := tm.users[user]
if ok {
return trace.AlreadyExists("Group %q, already exists", user)
Expand Down Expand Up @@ -194,7 +194,7 @@ func TestUserMgmt_CreateTemporaryUser(t *testing.T) {
require.NotContains(t, backend.users, "bob")

backend.CreateGroup("testgroup", "")
backend.CreateUser("simon", []string{}, "", "")
backend.CreateUser("simon", []string{}, "", "", "")

// try to create a temporary user for simon
closer, err = users.CreateUser("simon", userinfo)
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestUserMgmtSudoers_CreateTemporaryUser(t *testing.T) {
}
// test user already exists but teleport-service group has not yet
// been created
backend.CreateUser("testuser", nil, "", "")
backend.CreateUser("testuser", nil, "", "", "")
_, err := users.CreateUser("testuser", &services.HostUsersInfo{
Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP,
})
Expand Down Expand Up @@ -288,7 +288,7 @@ func TestUserMgmt_DeleteAllTeleportSystemUsers(t *testing.T) {
if slices.Contains(user.groups, types.TeleportServiceGroup) {
users.CreateUser(user.user, &services.HostUsersInfo{Groups: user.groups})
} else {
mgmt.CreateUser(user.user, user.groups, "", "")
mgmt.CreateUser(user.user, user.groups, "", "", "")
}
}
require.NoError(t, users.DeleteAllUsers())
Expand Down
17 changes: 15 additions & 2 deletions lib/utils/host/hostusers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ package host
import (
"bytes"
"errors"
"os"
"os/exec"
"os/user"
"strings"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -65,7 +67,8 @@ func UserAdd(username string, groups []string, home, uid, gid string) (exitCode
}

if home == "" {
return -1, trace.BadParameter("home is a required parameter")
// Users without a home directory should land at the root, to match OpenSSH behavior.
home = string(os.PathSeparator)
}

// useradd ---no-create-home (username) (groups)...
Expand Down Expand Up @@ -111,8 +114,18 @@ func UserDel(username string) (exitCode int, err error) {
if err != nil {
return -1, trace.Wrap(err, "cant find userdel binary")
}
u, err := user.Lookup(username)
if err != nil {
return -1, trace.Wrap(err)
}
args := make([]string, 0, 2)
// Only remove the home dir if it exists and isn't the root.
if u.HomeDir != "" && u.HomeDir != string(os.PathSeparator) {
args = append(args, "--remove")
}
args = append(args, username)
// userdel --remove (remove home) username
cmd := exec.Command(userdelBin, "--remove", username)
cmd := exec.Command(userdelBin, args...)
output, err := cmd.CombinedOutput()
log.Debugf("%s output: %s", cmd.Path, string(output))
return cmd.ProcessState.ExitCode(), trace.Wrap(err)
Expand Down

0 comments on commit dc0009d

Please sign in to comment.