From 43618e8b20602e7c511826b43882cebac263cf08 Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Mon, 11 Sep 2023 16:59:26 +0100 Subject: [PATCH 1/5] Allow sudoer files to be created without host users --- integration/hostuser_test.go | 47 ++++++++++--------- lib/services/access_checker.go | 86 ++++++++++++++++++++++++---------- lib/services/role_test.go | 46 +----------------- lib/srv/ctx.go | 4 ++ lib/srv/forward/sshserver.go | 6 +++ lib/srv/mock.go | 5 ++ lib/srv/regular/sshserver.go | 39 ++++++++++++--- lib/srv/sess.go | 74 ++++++++++++++++++++++++++++- lib/srv/usermgmt.go | 81 ++++++++++++++++++++++++-------- lib/srv/usermgmt_linux.go | 27 +++++++---- lib/srv/usermgmt_other.go | 7 ++- lib/srv/usermgmt_test.go | 22 +++++---- 12 files changed, 304 insertions(+), 140 deletions(-) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 7fb538ea959a2..157e6c9fe4d2e 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -54,7 +54,8 @@ func requireRoot(t *testing.T) { func TestRootHostUsersBackend(t *testing.T) { requireRoot(t) sudoersTestDir := t.TempDir() - backend := srv.HostUsersProvisioningBackend{ + usersbk := srv.HostUsersProvisioningBackend{} + sudoersbk := srv.HostSudoersProvisioningBackend{ SudoersPath: sudoersTestDir, HostUUID: "hostuuid", } @@ -66,27 +67,27 @@ func TestRootHostUsersBackend(t *testing.T) { }) t.Run("Test CreateGroup", func(t *testing.T) { - err := backend.CreateGroup(testgroup, "") + err := usersbk.CreateGroup(testgroup, "") require.NoError(t, err) - err = backend.CreateGroup(testgroup, "") + err = usersbk.CreateGroup(testgroup, "") require.True(t, trace.IsAlreadyExists(err)) }) t.Run("Test CreateUser and group", func(t *testing.T) { - err := backend.CreateUser(testuser, []string{testgroup}, "", "") + err := usersbk.CreateUser(testuser, []string{testgroup}, "", "") require.NoError(t, err) - tuser, err := backend.Lookup(testuser) + tuser, err := usersbk.Lookup(testuser) require.NoError(t, err) - group, err := backend.LookupGroup(testgroup) + group, err := usersbk.LookupGroup(testgroup) require.NoError(t, err) tuserGids, err := tuser.GroupIds() require.NoError(t, err) require.Contains(t, tuserGids, group.Gid) - err = backend.CreateUser(testuser, []string{}, "", "") + err = usersbk.CreateUser(testuser, []string{}, "", "") require.True(t, trace.IsAlreadyExists(err)) require.NoFileExists(t, filepath.Join("/home", testuser)) @@ -96,7 +97,7 @@ func TestRootHostUsersBackend(t *testing.T) { }) t.Run("Test DeleteUser", func(t *testing.T) { - err := backend.DeleteUser(testuser) + err := usersbk.DeleteUser(testuser) require.NoError(t, err) _, err = user.Lookup(testuser) @@ -107,15 +108,15 @@ func TestRootHostUsersBackend(t *testing.T) { checkUsers := []string{"teleport-usera", "teleport-userb", "teleport-userc"} t.Cleanup(func() { for _, u := range checkUsers { - backend.DeleteUser(u) + usersbk.DeleteUser(u) } }) for _, u := range checkUsers { - err := backend.CreateUser(u, []string{}, "", "") + err := usersbk.CreateUser(u, []string{}, "", "") require.NoError(t, err) } - users, err := backend.GetAllUsers() + users, err := usersbk.GetAllUsers() require.NoError(t, err) require.Subset(t, users, append(checkUsers, "root")) }) @@ -125,16 +126,16 @@ func TestRootHostUsersBackend(t *testing.T) { t.Skip("visudo not found on path") } validSudoersEntry := []byte("root ALL=(ALL) ALL") - err := backend.CheckSudoers(validSudoersEntry) + err := sudoersbk.CheckSudoers(validSudoersEntry) require.NoError(t, err) invalidSudoersEntry := []byte("yipee i broke sudo!!!!") - err = backend.CheckSudoers(invalidSudoersEntry) + err = sudoersbk.CheckSudoers(invalidSudoersEntry) require.Contains(t, err.Error(), "visudo: invalid sudoers file") // test sudoers entry containing . or ~ - require.NoError(t, backend.WriteSudoersFile("user.name", validSudoersEntry)) + require.NoError(t, sudoersbk.WriteSudoersFile("user.name", validSudoersEntry)) _, err = os.Stat(filepath.Join(sudoersTestDir, "teleport-hostuuid-user_name")) require.NoError(t, err) - require.NoError(t, backend.RemoveSudoersFile("user.name")) + require.NoError(t, sudoersbk.RemoveSudoersFile("user.name")) _, err = os.Stat(filepath.Join(sudoersTestDir, "teleport-hostuuid-user_name")) require.True(t, os.IsNotExist(err)) }) @@ -260,6 +261,7 @@ func TestRootHostUsers(t *testing.T) { } uuid := "host_uuid" users := srv.NewHostUsers(context.Background(), presence, uuid) + sudoers := srv.NewHostSudoers(uuid) sudoersPath := func(username, uuid string) string { return fmt.Sprintf("/etc/sudoers.d/teleport-%s-%s", uuid, username) @@ -271,26 +273,25 @@ func TestRootHostUsers(t *testing.T) { }) closer, err := users.CreateUser(testuser, &services.HostUsersInfo{ - Sudoers: []string{"ALL=(ALL) ALL"}, - Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, + Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, }) require.NoError(t, err) + err = sudoers.WriteSudoers(testuser, []string{"ALL=(ALL) ALL"}) + require.NoError(t, err) _, err = os.Stat(sudoersPath(testuser, uuid)) require.NoError(t, err) // delete the user and ensure the sudoers file got deleted require.NoError(t, closer.Close()) + require.NoError(t, sudoers.RemoveSudoers(testuser)) _, err = os.Stat(sudoersPath(testuser, uuid)) require.True(t, os.IsNotExist(err)) // ensure invalid sudoers entries dont get written - closer, err = users.CreateUser(testuser, - &services.HostUsersInfo{ - Sudoers: []string{"badsudoers entry!!!"}, - Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, - }) + err = sudoers.WriteSudoers(testuser, + []string{"badsudoers entry!!!"}, + ) require.Error(t, err) - defer closer.Close() _, err = os.Stat(sudoersPath(testuser, uuid)) require.True(t, os.IsNotExist(err)) }) diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index afdf53090bc3f..1b0bac5eefdfd 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -201,6 +201,9 @@ type AccessChecker interface { // a role disallows host user creation HostUsers(types.Server) (*HostUsersInfo, error) + // HostSudoers returns host sudoers entries matching a server + HostSudoers(types.Server) ([]string, error) + // DesktopGroups returns the desktop groups a user is allowed to create or an access denied error if a role disallows desktop user creation DesktopGroups(types.WindowsDesktop) ([]string, error) @@ -833,8 +836,6 @@ func (a *accessChecker) DesktopGroups(s types.WindowsDesktop) ([]string, error) type HostUsersInfo struct { // Groups is the list of groups to include host users in Groups []string - // Sudoers is a list of entries for a users sudoers file - Sudoers []string // Mode determines if a host user should be deleted after a session // ends or not. Mode types.CreateHostUserMode @@ -848,7 +849,6 @@ type HostUsersInfo struct { // a role disallows host user creation func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { groups := make(map[string]struct{}) - var sudoers []string var mode types.CreateHostUserMode roleSet := make([]types.Role, len(a.RoleSet)) @@ -857,7 +857,6 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { return strings.Compare(a.GetName(), b.GetName()) }) - seenSudoers := make(map[string]struct{}) for _, role := range roleSet { result, _, err := checkRoleLabelsMatch(types.Allow, role, a.info.Traits, s, false) if err != nil { @@ -894,6 +893,62 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { for _, group := range role.GetHostGroups(types.Allow) { groups[group] = struct{}{} } + } + + for _, role := range roleSet { + result, _, err := checkRoleLabelsMatch(types.Deny, role, a.info.Traits, s, false) + if err != nil { + return nil, trace.Wrap(err) + } + if !result { + continue + } + for _, group := range role.GetHostGroups(types.Deny) { + delete(groups, group) + } + } + + traits := a.Traits() + var gid string + gidL := traits[constants.TraitHostUserGID] + if len(gidL) >= 1 { + gid = gidL[0] + } + var uid string + uidL := traits[constants.TraitHostUserUID] + if len(uidL) >= 1 { + uid = uidL[0] + } + + return &HostUsersInfo{ + Groups: utils.StringsSliceFromSet(groups), + Mode: mode, + UID: uid, + GID: gid, + }, nil +} + +// HostSudoers returns host sudoers entries matching a server +func (a *accessChecker) HostSudoers(s types.Server) ([]string, error) { + var sudoers []string + + roleSet := make([]types.Role, len(a.RoleSet)) + copy(roleSet, a.RoleSet) + slices.SortStableFunc(roleSet, func(a types.Role, b types.Role) int { + return strings.Compare(a.GetName(), b.GetName()) + }) + + seenSudoers := make(map[string]struct{}) + for _, role := range roleSet { + result, _, err := checkRoleLabelsMatch(types.Allow, role, a.info.Traits, s, false) + if err != nil { + return nil, trace.Wrap(err) + } + // skip nodes that dont have matching labels + if !result { + continue + } + for _, sudoer := range role.GetHostSudoers(types.Allow) { if _, ok := seenSudoers[sudoer]; ok { continue @@ -912,9 +967,6 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { if !result { continue } - for _, group := range role.GetHostGroups(types.Deny) { - delete(groups, group) - } outer: for _, sudoer := range sudoers { @@ -931,25 +983,7 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { sudoers = finalSudoers } - traits := a.Traits() - var gid string - gidL := traits[constants.TraitHostUserGID] - if len(gidL) >= 1 { - gid = gidL[0] - } - var uid string - uidL := traits[constants.TraitHostUserUID] - if len(uidL) >= 1 { - uid = uidL[0] - } - - return &HostUsersInfo{ - Groups: utils.StringsSliceFromSet(groups), - Sudoers: sudoers, - Mode: mode, - UID: uid, - GID: gid, - }, nil + return sudoers, nil } // AccessInfoFromLocalCertificate returns a new AccessInfo populated from the diff --git a/lib/services/role_test.go b/lib/services/role_test.go index c1d65974ffd7b..97554f578bbf0 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -6894,9 +6894,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { sudoers: []string{"%sudo ALL=(ALL) ALL"}, roles: NewRoleSet(&types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"%sudo ALL=(ALL) ALL"}, @@ -6920,9 +6917,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "a", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"sudoers entry 1"}, @@ -6933,9 +6927,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "b", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, HostSudoers: []string{"sudoers entry 2"}, @@ -6943,9 +6934,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { }, }, &types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"fail": []string{"abc"}}, HostSudoers: []string{"not present sudoers entry"}, @@ -6966,9 +6954,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { sudoers: nil, roles: NewRoleSet(&types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"%sudo ALL=(ALL) ALL"}, @@ -6976,9 +6961,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { }, }, &types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Deny: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"*"}, @@ -6999,9 +6981,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { sudoers: []string{"%sudo ALL=(ALL) ALL"}, roles: NewRoleSet(&types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{ @@ -7012,9 +6991,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { }, }, &types.RoleV6{ Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Deny: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"removed entry"}, @@ -7038,9 +7014,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "a", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"sudoers entry 1"}, @@ -7051,9 +7024,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "c", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, HostSudoers: []string{"sudoers entry 4", "sudoers entry 1"}, @@ -7064,9 +7034,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "b", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, HostSudoers: []string{"sudoers entry 2", "sudoers entry 3"}, @@ -7090,9 +7057,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "a", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"sudoers entry 1"}, @@ -7103,9 +7067,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "d", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Deny: types.RoleConditions{ NodeLabels: types.Labels{"success": []string{"abc"}}, HostSudoers: []string{"sudoers entry 1"}, @@ -7116,9 +7077,6 @@ func TestHostUsers_HostSudoers(t *testing.T) { Name: "c", }, Spec: types.RoleSpecV6{ - Options: types.RoleOptions{ - CreateHostUser: types.NewBoolOption(true), - }, Allow: types.RoleConditions{ NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, HostSudoers: []string{"sudoers entry 1", "sudoers entry 2"}, @@ -7137,9 +7095,9 @@ func TestHostUsers_HostSudoers(t *testing.T) { } { t.Run(tc.test, func(t *testing.T) { accessChecker := makeAccessCheckerWithRoleSet(tc.roles) - info, err := accessChecker.HostUsers(tc.server) + info, err := accessChecker.HostSudoers(tc.server) require.NoError(t, err) - require.Equal(t, tc.sudoers, info.Sudoers) + require.Equal(t, tc.sudoers, info) }) } } diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index af468ce89114d..6e52cea14aa3c 100644 --- a/lib/srv/ctx.go +++ b/lib/srv/ctx.go @@ -180,6 +180,10 @@ type Server interface { // host user provisioning GetHostUsers() HostUsers + // GetHostSudoers returns the HostSudoers instance being used to manage + // sudoer file provisioning + GetHostSudoers() HostSudoers + // TargetMetadata returns metadata about the session target node. TargetMetadata() apievents.ServerMetadata } diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index c31bc5f869e76..fc1593c0a672c 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -486,6 +486,12 @@ func (s *Server) GetHostUsers() srv.HostUsers { return nil } +// GetHostSudoers returns the HostSudoers instance being used to manage +// sudoer file provisioning, unimplemented for the forwarder server. +func (s *Server) GetHostSudoers() srv.HostSudoers { + return nil +} + // GetRestrictedSessionManager returns a NOP manager since for a // forwarding server it makes no sense (it has to run on the actual // node). diff --git a/lib/srv/mock.go b/lib/srv/mock.go index 02cb589a52c9f..622b165c484f1 100644 --- a/lib/srv/mock.go +++ b/lib/srv/mock.go @@ -284,6 +284,11 @@ func (m *mockServer) GetHostUsers() HostUsers { return nil } +// GetHostSudoers +func (m *mockServer) GetHostSudoers() HostSudoers { + return nil +} + // Implementation of ssh.Conn interface. type mockSSHConn struct { remoteAddr net.Addr diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index b95df2d676502..7e7c9b78d7a65 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -216,6 +216,9 @@ type Server struct { // users is used to start the automatic user deletion loop users srv.HostUsers + // sudoers is used to manage sudoers file provisioning + sudoers srv.HostSudoers + // tracerProvider is used to create tracers capable // of starting spans. tracerProvider oteltrace.TracerProvider @@ -311,6 +314,12 @@ func (s *Server) GetHostUsers() srv.HostUsers { return s.users } +// GetHostSudoers returns the HostSudoers instance being used to manage +// sudoers file provisioning +func (s *Server) GetHostSudoers() srv.HostSudoers { + return s.sudoers +} + // ServerOption is a functional option passed to the server type ServerOption func(s *Server) error @@ -362,6 +371,11 @@ func (s *Server) Start() error { return trace.Wrap(err) } } + // clean up sudoers before starting, in case any were left behind + // due to a crash for example + if err := s.sudoers.CleanupSudoers(); err != nil { + return trace.Wrap(err) + } // Heartbeat should start only after s.srv.Start. // If the server is configured to listen on port 0 (such as in tests), // it'll only populate its actual listening address during s.srv.Start. @@ -373,6 +387,11 @@ func (s *Server) Start() error { // Serve servers service on started listener func (s *Server) Serve(l net.Listener) error { + // clean up sudoers before starting, in case any were left behind + // due to a crash for example + if err := s.sudoers.CleanupSudoers(); err != nil { + return trace.Wrap(err) + } s.startPeriodicOperations() return trace.Wrap(s.srv.Serve(l)) } @@ -385,7 +404,7 @@ func (s *Server) startPeriodicOperations() { } // If the server allows host user provisioning, this will start an // automatic cleanup process for any temporary leftover users. - if s.users != nil { + if s.GetCreateHostUser() && s.users != nil { go s.users.UserCleanup() } if s.cloudLabels != nil { @@ -805,13 +824,10 @@ func New( trace.ComponentFields: logrus.Fields{}, }) - if s.createHostUser { - users := srv.NewHostUsers(ctx, s.storage, s.ID()) - if err != nil { - return nil, trace.Wrap(err) - } - s.users = users + if s.GetCreateHostUser() { + s.users = srv.NewHostUsers(ctx, s.storage, s.ID()) } + s.sudoers = srv.NewHostSudoers(s.ID()) s.reg, err = srv.NewSessionRegistry(srv.SessionRegistryConfig{ Srv: s, @@ -1673,6 +1689,9 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, if err := s.termHandlers.SessionRegistry.TryCreateHostUser(serverContext); err != nil { return trace.Wrap(err) } + if err := s.termHandlers.SessionRegistry.TryWriteSudoersFile(serverContext); err != nil { + return trace.Wrap(err) + } return s.termHandlers.HandleExec(ctx, ch, req, serverContext) case sshutils.PTYRequest: return s.termHandlers.HandlePTYReq(ctx, ch, req, serverContext) @@ -1680,6 +1699,9 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, if err := s.termHandlers.SessionRegistry.TryCreateHostUser(serverContext); err != nil { return trace.Wrap(err) } + if err := s.termHandlers.SessionRegistry.TryWriteSudoersFile(serverContext); err != nil { + return trace.Wrap(err) + } return s.termHandlers.HandleShell(ctx, ch, req, serverContext) case constants.InitiateFileTransfer: return s.termHandlers.HandleFileTransferRequest(ctx, ch, req, serverContext) @@ -1710,6 +1732,9 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, s.Logger.Warn(err) return nil } + if err := s.termHandlers.SessionRegistry.TryWriteSudoersFile(serverContext); err != nil { + return trace.Wrap(err) + } // to maintain interoperability with OpenSSH, agent forwarding requests // should never fail, all errors should be logged and we should continue diff --git a/lib/srv/sess.go b/lib/srv/sess.go index d4ec736d46770..7d193779c8f61 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -99,6 +99,31 @@ type SessionRegistry struct { // users is used for automatic user creation when new sessions are // started users HostUsers + + // sudoers is used to create sudoers files at session start + sudoers HostSudoers + sessionsByUser *userSessions +} + +type userSessions struct { + sessionsByUser map[string]int + m sync.Mutex +} + +func (us *userSessions) add(user string) { + us.m.Lock() + defer us.m.Unlock() + count := us.sessionsByUser[user] + us.sessionsByUser[user] = count + 1 +} + +func (us *userSessions) del(user string) int { + us.m.Lock() + defer us.m.Unlock() + count := us.sessionsByUser[user] + count -= 1 + us.sessionsByUser[user] = count + return count } type SessionRegistryConfig struct { @@ -146,6 +171,10 @@ func NewSessionRegistry(cfg SessionRegistryConfig) (*SessionRegistry, error) { }), sessions: make(map[rsession.ID]*session), users: cfg.Srv.GetHostUsers(), + sudoers: cfg.Srv.GetHostSudoers(), + sessionsByUser: &userSessions{ + sessionsByUser: make(map[string]int), + }, }, nil } @@ -185,9 +214,50 @@ func (s *SessionRegistry) Close() { s.log.Debug("Closing Session Registry.") } +type sudoersCloser struct { + username string + userSessions *userSessions + cleanup func(name string) error +} + +func (sc *sudoersCloser) Close() error { + count := sc.userSessions.del(sc.username) + if count != 0 { + return nil + } + if err := sc.cleanup(sc.username); err != nil { + return trace.Wrap(err) + } + return nil +} + +func (s *SessionRegistry) TryWriteSudoersFile(ctx *ServerContext) error { + sudoers, err := ctx.Identity.AccessChecker.HostSudoers(ctx.srv.GetInfo()) + if err != nil { + return trace.Wrap(err) + } + if len(sudoers) == 0 { + // not an error, sudoers may not be configured. + return nil + } + if err := s.sudoers.WriteSudoers(ctx.Identity.Login, sudoers); err != nil { + return trace.Wrap(err) + } + + s.sessionsByUser.add(ctx.Identity.Login) + ctx.AddCloser(&sudoersCloser{ + username: ctx.Identity.Login, + userSessions: s.sessionsByUser, + cleanup: s.sudoers.RemoveSudoers, + }) + + return nil +} + func (s *SessionRegistry) TryCreateHostUser(ctx *ServerContext) error { - if !ctx.srv.GetCreateHostUser() || s.users == nil { - return nil // not an error to not be able to create a host user + if !ctx.srv.GetCreateHostUser() { + s.log.Debugf("not creating host users: node has disabled host user creation") + return nil // not an error to not be able to create host users } ui, err := ctx.Identity.AccessChecker.HostUsers(ctx.srv.GetInfo()) diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index 23bd0c79eef6c..98a0d47678a98 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -40,8 +40,8 @@ import ( func NewHostUsers(ctx context.Context, storage *local.PresenceService, uuid string) HostUsers { // newHostUsersBackend statically returns a valid backend or an error, // resulting in a staticcheck linter error on darwin - backend, err := newHostUsersBackend(uuid) //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. - if err != nil { //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. + backend, err := newHostUsersBackend() //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. + if err != nil { //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. log.Warnf("Error making new HostUsersBackend: %s", err) return nil } @@ -55,6 +55,30 @@ func NewHostUsers(ctx context.Context, storage *local.PresenceService, uuid stri } } +func NewHostSudoers(uuid string) HostSudoers { + // newHostSudoersBackend statically returns a valid backend or an error, + // resulting in a staticcheck linter error on darwin + backend, err := newHostSudoersBackend(uuid) //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. + if err != nil { //nolint:staticcheck // linter fails on non-linux system as only linux implementation returns useful values. + log.Warnf("Error making new HostUsersBackend: %s", err) + return nil + } + return &HostSudoersManagement{ + backend: backend, + } +} + +type HostSudoersBackend interface { + // CheckSudoers ensures that a sudoers file to be written is valid + CheckSudoers(contents []byte) error + // WriteSudoersFile creates the user's sudoers file. + WriteSudoersFile(user string, entries []byte) error + // RemoveSudoersFile deletes a user's sudoers file. + RemoveSudoersFile(user string) error + // RemoveAllSudoersFiles removes all presnt teleport managed sudoers files + RemoveAllSudoersFiles() error +} + type HostUsersBackend interface { // GetAllUsers returns all host users on a node. GetAllUsers() ([]string, error) @@ -72,12 +96,6 @@ type HostUsersBackend interface { CreateUser(name string, groups []string, uid, gid string) error // DeleteUser deletes a user from a host. DeleteUser(name string) error - // CheckSudoers ensures that a sudoers file to be written is valid - CheckSudoers(contents []byte) error - // WriteSudoersFile creates the user's sudoers file. - WriteSudoersFile(user string, entries []byte) error - // RemoveSudoersFile deletes a user's sudoers file. - RemoveSudoersFile(user string) error // CreateHomeDirectory creates the users home directory and copies in /etc/skel CreateHomeDirectory(user string, uid, gid string) error } @@ -101,6 +119,15 @@ func (u *userCloser) Close() error { var ErrUserLoggedIn = errors.New("User logged in error") +type HostSudoers interface { + // WriteSudoers creates a temporary Teleport user in the TeleportServiceGroup + WriteSudoers(name string, sudoers []string) error + // RemoveSudoers removes the users sudoer file + RemoveSudoers(name string) error + // CleanupSudoers removes all sudoers in /etc/sudoers.d/teleport-$HOSTUUID-* + CleanupSudoers() error +} + type HostUsers interface { // CreateUser creates a temporary Teleport user in the TeleportServiceGroup CreateUser(name string, hostRoleInfo *services.HostUsersInfo) (io.Closer, error) @@ -136,7 +163,12 @@ type HostUserManagement struct { userGrace time.Duration } +type HostSudoersManagement struct { + backend HostSudoersBackend +} + var _ HostUsers = &HostUserManagement{} +var _ HostSudoers = &HostSudoersManagement{} // Under the section "Including other files from within sudoers": // @@ -154,6 +186,28 @@ func sanitizeSudoersName(username string) string { return sudoersSanitizationMatcher.ReplaceAllString(username, "_") } +// WriteSudoers creates a sudoers file for a user from a list of entries +func (u *HostSudoersManagement) WriteSudoers(name string, sudoers []string) error { + var sudoersOut strings.Builder + for _, entry := range sudoers { + sudoersOut.WriteString(fmt.Sprintf("%s %s\n", name, entry)) + } + err := u.backend.WriteSudoersFile(name, []byte(sudoersOut.String())) + return trace.Wrap(err) +} + +// CleanupSudoers will remove all sudoers files managed by teleport +func (u *HostSudoersManagement) CleanupSudoers() error { + return trace.Wrap(u.backend.RemoveAllSudoersFiles()) +} + +func (u *HostSudoersManagement) RemoveSudoers(name string) error { + if err := u.backend.RemoveSudoersFile(name); err != nil { + return trace.Wrap(err) + } + return nil +} + // CreateUser creates a temporary Teleport user in the TeleportServiceGroup func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo) (io.Closer, error) { if ui.Mode == types.CreateHostUserMode_HOST_USER_MODE_UNSPECIFIED { @@ -264,14 +318,6 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo) return nil, trace.Wrap(err) } - if len(ui.Sudoers) != 0 { - var sudoers strings.Builder - for _, entry := range ui.Sudoers { - sudoers.WriteString(fmt.Sprintf("%s %s\n", name, entry)) - } - err = u.backend.WriteSudoersFile(name, []byte(sudoers.String())) - } - if ui.Mode == types.CreateHostUserMode_HOST_USER_MODE_KEEP { return nil, trace.Wrap(err) } @@ -392,9 +438,6 @@ func (u *HostUserManagement) DeleteUser(username string, gid string) error { return trace.Wrap(err) } - if err := u.backend.RemoveSudoersFile(username); err != nil { - return trace.Wrap(err) - } return nil } } diff --git a/lib/srv/usermgmt_linux.go b/lib/srv/usermgmt_linux.go index 44eca6f26741f..775fd9e3bfe19 100644 --- a/lib/srv/usermgmt_linux.go +++ b/lib/srv/usermgmt_linux.go @@ -34,16 +34,25 @@ import ( // HostUsersProvisioningBackend is used to implement HostUsersBackend type HostUsersProvisioningBackend struct { - // SudoersPath is the path to write sudoers files to. - SudoersPath string +} + +// HostSudoersProvisioningBackend is used to implement HostSudoersBackend +type HostSudoersProvisioningBackend struct { // HostUUID is the UUID of the running host HostUUID string + // SudoersPath is the path to write sudoers files to. + SudoersPath string } // newHostUsersBackend initializes a new OS specific HostUsersBackend -func newHostUsersBackend(uuid string) (HostUsersBackend, error) { - return &HostUsersProvisioningBackend{ - SudoersPath: "/etc/sudoers.d", +func newHostUsersBackend() (HostUsersBackend, error) { + return &HostUsersProvisioningBackend{}, nil +} + +// newHostUsersBackend initializes a new OS specific HostUsersBackend +func newHostSudoersBackend(uuid string) (HostSudoersBackend, error) { + return &HostSudoersProvisioningBackend{ + SudoersPath: "/etc/sudoers.d/", HostUUID: uuid, }, nil } @@ -101,7 +110,7 @@ func (*HostUsersProvisioningBackend) DeleteUser(name string) error { } // CheckSudoers ensures that a sudoers file to be written is valid -func (*HostUsersProvisioningBackend) CheckSudoers(contents []byte) error { +func (*HostSudoersProvisioningBackend) CheckSudoers(contents []byte) error { err := host.CheckSudoers(contents) if err != nil { return trace.Wrap(err) @@ -128,7 +137,7 @@ func writeSudoersFile(root, name string, data []byte) (string, error) { } // WriteSudoersFile creates the user's sudoers file. -func (u *HostUsersProvisioningBackend) WriteSudoersFile(username string, contents []byte) error { +func (u *HostSudoersProvisioningBackend) WriteSudoersFile(username string, contents []byte) error { if err := u.CheckSudoers(contents); err != nil { return trace.Wrap(err) } @@ -148,7 +157,7 @@ func (u *HostUsersProvisioningBackend) WriteSudoersFile(username string, content } // RemoveSudoersFile deletes a user's sudoers file. -func (u *HostUsersProvisioningBackend) RemoveSudoersFile(username string) error { +func (u *HostSudoersProvisioningBackend) RemoveSudoersFile(username string) error { fileUsername := sanitizeSudoersName(username) sudoersFilePath := filepath.Join(u.SudoersPath, fmt.Sprintf("teleport-%s-%s", u.HostUUID, fileUsername)) if _, err := os.Stat(sudoersFilePath); os.IsNotExist(err) { @@ -256,4 +265,4 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gi } return nil -} + diff --git a/lib/srv/usermgmt_other.go b/lib/srv/usermgmt_other.go index 3b7b22094be3a..1288fec0a7dc2 100644 --- a/lib/srv/usermgmt_other.go +++ b/lib/srv/usermgmt_other.go @@ -24,6 +24,11 @@ import ( ) //nolint:staticcheck // intended to always return an error for non-linux builds -func newHostUsersBackend(_ string) (HostUsersBackend, error) { +func newHostUsersBackend() (HostUsersBackend, error) { + return nil, trace.NotImplemented("Host user creation management is only supported on linux") +} + +//nolint:staticcheck // intended to always return an error for non-linux builds +func newHostSudoersBackend(_ string) (HostSudoersBackend, error) { return nil, trace.NotImplemented("Host user creation management is only supported on linux") } diff --git a/lib/srv/usermgmt_test.go b/lib/srv/usermgmt_test.go index 692dc18b8a155..853d916c2ff41 100644 --- a/lib/srv/usermgmt_test.go +++ b/lib/srv/usermgmt_test.go @@ -149,7 +149,12 @@ func (tm *testHostUserBackend) WriteSudoersFile(user string, entries []byte) err return nil } +func (*testHostUserBackend) RemoveAllSudoersFiles() error { + return nil +} + var _ HostUsersBackend = &testHostUserBackend{} +var _ HostSudoersBackend = &testHostUserBackend{} func TestUserMgmt_CreateTemporaryUser(t *testing.T) { t.Parallel() @@ -206,25 +211,24 @@ func TestUserMgmtSudoers_CreateTemporaryUser(t *testing.T) { backend: backend, storage: pres, } + sudoers := HostSudoersManagement{ + backend: backend, + } closer, err := users.CreateUser("bob", &services.HostUsersInfo{ - Groups: []string{"hello", "sudo"}, - Sudoers: []string{"validsudoers"}, - Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, + Groups: []string{"hello", "sudo"}, + Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, }) require.NoError(t, err) require.NotNil(t, closer) + require.Empty(t, backend.sudoers) + sudoers.WriteSudoers("bob", []string{"validsudoers"}) require.Equal(t, map[string]string{"bob": "bob validsudoers"}, backend.sudoers) + sudoers.RemoveSudoers("bob") require.NoError(t, closer.Close()) require.Empty(t, backend.sudoers) - _, err = users.CreateUser("bob", &services.HostUsersInfo{ - Groups: []string{"hello", "sudo"}, - Sudoers: []string{"invalid"}, - Mode: types.CreateHostUserMode_HOST_USER_MODE_DROP, - }) - require.Error(t, err) t.Run("no teleport-service group", func(t *testing.T) { backend := newTestUserMgmt() From 5bb2eee9f979ca179aec723bbf01f3ea14adeaeb Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Fri, 15 Sep 2023 13:14:31 +0100 Subject: [PATCH 2/5] Dont sort when fetching HostUsers --- lib/services/access_checker.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 1b0bac5eefdfd..0f851adb1ac7d 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -851,13 +851,7 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { groups := make(map[string]struct{}) var mode types.CreateHostUserMode - roleSet := make([]types.Role, len(a.RoleSet)) - copy(roleSet, a.RoleSet) - slices.SortStableFunc(roleSet, func(a types.Role, b types.Role) int { - return strings.Compare(a.GetName(), b.GetName()) - }) - - for _, role := range roleSet { + for _, role := range a.RoleSet { result, _, err := checkRoleLabelsMatch(types.Allow, role, a.info.Traits, s, false) if err != nil { return nil, trace.Wrap(err) @@ -895,7 +889,7 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { } } - for _, role := range roleSet { + for _, role := range a.RoleSet { result, _, err := checkRoleLabelsMatch(types.Deny, role, a.info.Traits, s, false) if err != nil { return nil, trace.Wrap(err) From aba2489e9f85d78ec6bae729efd986f5ca5108d8 Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Fri, 15 Sep 2023 13:24:19 +0100 Subject: [PATCH 3/5] Dont cleanup at session end --- lib/srv/regular/sshserver.go | 13 ++----------- lib/srv/sess.go | 2 +- lib/srv/usermgmt.go | 9 --------- lib/srv/usermgmt_linux.go | 2 +- 4 files changed, 4 insertions(+), 22 deletions(-) diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 7e7c9b78d7a65..c188680ce85b7 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -371,11 +371,6 @@ func (s *Server) Start() error { return trace.Wrap(err) } } - // clean up sudoers before starting, in case any were left behind - // due to a crash for example - if err := s.sudoers.CleanupSudoers(); err != nil { - return trace.Wrap(err) - } // Heartbeat should start only after s.srv.Start. // If the server is configured to listen on port 0 (such as in tests), // it'll only populate its actual listening address during s.srv.Start. @@ -387,11 +382,6 @@ func (s *Server) Start() error { // Serve servers service on started listener func (s *Server) Serve(l net.Listener) error { - // clean up sudoers before starting, in case any were left behind - // due to a crash for example - if err := s.sudoers.CleanupSudoers(); err != nil { - return trace.Wrap(err) - } s.startPeriodicOperations() return trace.Wrap(s.srv.Serve(l)) } @@ -1733,7 +1723,8 @@ func (s *Server) dispatch(ctx context.Context, ch ssh.Channel, req *ssh.Request, return nil } if err := s.termHandlers.SessionRegistry.TryWriteSudoersFile(serverContext); err != nil { - return trace.Wrap(err) + s.Logger.Warn(err) + return nil } // to maintain interoperability with OpenSSH, agent forwarding requests diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 7d193779c8f61..3d1b18d0b959c 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -256,7 +256,7 @@ func (s *SessionRegistry) TryWriteSudoersFile(ctx *ServerContext) error { func (s *SessionRegistry) TryCreateHostUser(ctx *ServerContext) error { if !ctx.srv.GetCreateHostUser() { - s.log.Debugf("not creating host users: node has disabled host user creation") + s.log.Debug("Not creating host user: node has disabled host user creation.") return nil // not an error to not be able to create host users } diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index 98a0d47678a98..dda1278a71ac7 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -75,8 +75,6 @@ type HostSudoersBackend interface { WriteSudoersFile(user string, entries []byte) error // RemoveSudoersFile deletes a user's sudoers file. RemoveSudoersFile(user string) error - // RemoveAllSudoersFiles removes all presnt teleport managed sudoers files - RemoveAllSudoersFiles() error } type HostUsersBackend interface { @@ -124,8 +122,6 @@ type HostSudoers interface { WriteSudoers(name string, sudoers []string) error // RemoveSudoers removes the users sudoer file RemoveSudoers(name string) error - // CleanupSudoers removes all sudoers in /etc/sudoers.d/teleport-$HOSTUUID-* - CleanupSudoers() error } type HostUsers interface { @@ -196,11 +192,6 @@ func (u *HostSudoersManagement) WriteSudoers(name string, sudoers []string) erro return trace.Wrap(err) } -// CleanupSudoers will remove all sudoers files managed by teleport -func (u *HostSudoersManagement) CleanupSudoers() error { - return trace.Wrap(u.backend.RemoveAllSudoersFiles()) -} - func (u *HostSudoersManagement) RemoveSudoers(name string) error { if err := u.backend.RemoveSudoersFile(name); err != nil { return trace.Wrap(err) diff --git a/lib/srv/usermgmt_linux.go b/lib/srv/usermgmt_linux.go index 775fd9e3bfe19..7b926c705b655 100644 --- a/lib/srv/usermgmt_linux.go +++ b/lib/srv/usermgmt_linux.go @@ -265,4 +265,4 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gi } return nil - +} From 92fb0f4416552dcef783fd4f01a357bf2646b3af Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Mon, 18 Sep 2023 13:41:45 +0100 Subject: [PATCH 4/5] Resolve comments --- lib/services/access_checker.go | 2 +- lib/srv/ctx.go | 2 +- lib/srv/forward/sshserver.go | 4 ++-- lib/srv/mock.go | 4 ++-- lib/srv/regular/sshserver.go | 4 ++-- lib/srv/sess.go | 6 +++++- lib/srv/usermgmt.go | 6 ++++-- 7 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 0f851adb1ac7d..abb9ae1451f76 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -928,7 +928,7 @@ func (a *accessChecker) HostSudoers(s types.Server) ([]string, error) { roleSet := make([]types.Role, len(a.RoleSet)) copy(roleSet, a.RoleSet) - slices.SortStableFunc(roleSet, func(a types.Role, b types.Role) int { + slices.SortFunc(roleSet, func(a types.Role, b types.Role) int { return strings.Compare(a.GetName(), b.GetName()) }) diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index 6e52cea14aa3c..74db58922ae2b 100644 --- a/lib/srv/ctx.go +++ b/lib/srv/ctx.go @@ -182,7 +182,7 @@ type Server interface { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoer file provisioning - GetHostSudoers() HostSudoers + GetHostSudoers() (HostSudoers, error) // TargetMetadata returns metadata about the session target node. TargetMetadata() apievents.ServerMetadata diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index fc1593c0a672c..a919e3d737f22 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -488,8 +488,8 @@ func (s *Server) GetHostUsers() srv.HostUsers { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoer file provisioning, unimplemented for the forwarder server. -func (s *Server) GetHostSudoers() srv.HostSudoers { - return nil +func (s *Server) GetHostSudoers() (srv.HostSudoers, error) { + return nil, trace.NotImplemented("not implemented for forwarding server") } // GetRestrictedSessionManager returns a NOP manager since for a diff --git a/lib/srv/mock.go b/lib/srv/mock.go index 622b165c484f1..92ad427c5875e 100644 --- a/lib/srv/mock.go +++ b/lib/srv/mock.go @@ -285,8 +285,8 @@ func (m *mockServer) GetHostUsers() HostUsers { } // GetHostSudoers -func (m *mockServer) GetHostSudoers() HostSudoers { - return nil +func (m *mockServer) GetHostSudoers() (HostSudoers, error) { + return nil, nil } // Implementation of ssh.Conn interface. diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index c188680ce85b7..0aabe097a5ba4 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -316,8 +316,8 @@ func (s *Server) GetHostUsers() srv.HostUsers { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoers file provisioning -func (s *Server) GetHostSudoers() srv.HostSudoers { - return s.sudoers +func (s *Server) GetHostSudoers() (srv.HostSudoers, error) { + return s.sudoers, nil } // ServerOption is a functional option passed to the server diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 3d1b18d0b959c..0d6ff02cc5d7b 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -164,6 +164,10 @@ func NewSessionRegistry(cfg SessionRegistryConfig) (*SessionRegistry, error) { return nil, trace.Wrap(err) } + sudoers, err := cfg.Srv.GetHostSudoers() + if err != nil { + return nil, trace.Wrap(err) + } return &SessionRegistry{ SessionRegistryConfig: cfg, log: log.WithFields(log.Fields{ @@ -171,7 +175,7 @@ func NewSessionRegistry(cfg SessionRegistryConfig) (*SessionRegistry, error) { }), sessions: make(map[rsession.ID]*session), users: cfg.Srv.GetHostUsers(), - sudoers: cfg.Srv.GetHostSudoers(), + sudoers: sudoers, sessionsByUser: &userSessions{ sessionsByUser: make(map[string]int), }, diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index dda1278a71ac7..9f80141c1f3bf 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -163,8 +163,10 @@ type HostSudoersManagement struct { backend HostSudoersBackend } -var _ HostUsers = &HostUserManagement{} -var _ HostSudoers = &HostSudoersManagement{} +var ( + _ HostUsers = &HostUserManagement{} + _ HostSudoers = &HostSudoersManagement{} +) // Under the section "Including other files from within sudoers": // From c353601813f2989c675690e5a1c7900d0042fad0 Mon Sep 17 00:00:00 2001 From: Alex McGrath Date: Thu, 21 Sep 2023 13:34:57 +0100 Subject: [PATCH 5/5] Resolve comments, switch to a notimplemented host sudoers --- integration/hostuser_test.go | 8 ++++---- lib/services/access_checker.go | 3 +-- lib/srv/ctx.go | 2 +- lib/srv/forward/sshserver.go | 4 ++-- lib/srv/mock.go | 4 ++-- lib/srv/regular/sshserver.go | 7 +++++-- lib/srv/sess.go | 9 +++++---- lib/srv/usermgmt.go | 12 ++++++++++++ 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 157e6c9fe4d2e..fbe6957e44124 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -91,7 +91,7 @@ func TestRootHostUsersBackend(t *testing.T) { require.True(t, trace.IsAlreadyExists(err)) require.NoFileExists(t, filepath.Join("/home", testuser)) - err = backend.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid) + err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid) require.NoError(t, err) require.FileExists(t, filepath.Join("/home", testuser, ".bashrc")) }) @@ -141,10 +141,10 @@ func TestRootHostUsersBackend(t *testing.T) { }) t.Run("Test CreateHomeDirectory does not follow symlinks", func(t *testing.T) { - err := backend.CreateUser(testuser, []string{testgroup}, "", "") + err := usersbk.CreateUser(testuser, []string{testgroup}, "", "") require.NoError(t, err) - tuser, err := backend.Lookup(testuser) + tuser, err := usersbk.Lookup(testuser) require.NoError(t, err) require.NoError(t, os.WriteFile("/etc/skel/testfile", []byte("test\n"), 0o700)) @@ -157,7 +157,7 @@ func TestRootHostUsersBackend(t *testing.T) { require.NoError(t, os.Symlink("/tmp/ignoreme", bashrcPath)) require.NoFileExists(t, "/tmp/ignoreme") - err = backend.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid) + err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid) t.Cleanup(func() { os.RemoveAll(filepath.Join("/home", testuser)) }) diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index abb9ae1451f76..bf627b29cd6ff 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -926,8 +926,7 @@ func (a *accessChecker) HostUsers(s types.Server) (*HostUsersInfo, error) { func (a *accessChecker) HostSudoers(s types.Server) ([]string, error) { var sudoers []string - roleSet := make([]types.Role, len(a.RoleSet)) - copy(roleSet, a.RoleSet) + roleSet := slices.Clone(a.RoleSet) slices.SortFunc(roleSet, func(a types.Role, b types.Role) int { return strings.Compare(a.GetName(), b.GetName()) }) diff --git a/lib/srv/ctx.go b/lib/srv/ctx.go index 74db58922ae2b..6e52cea14aa3c 100644 --- a/lib/srv/ctx.go +++ b/lib/srv/ctx.go @@ -182,7 +182,7 @@ type Server interface { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoer file provisioning - GetHostSudoers() (HostSudoers, error) + GetHostSudoers() HostSudoers // TargetMetadata returns metadata about the session target node. TargetMetadata() apievents.ServerMetadata diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go index a919e3d737f22..42c596ea0df8a 100644 --- a/lib/srv/forward/sshserver.go +++ b/lib/srv/forward/sshserver.go @@ -488,8 +488,8 @@ func (s *Server) GetHostUsers() srv.HostUsers { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoer file provisioning, unimplemented for the forwarder server. -func (s *Server) GetHostSudoers() (srv.HostSudoers, error) { - return nil, trace.NotImplemented("not implemented for forwarding server") +func (s *Server) GetHostSudoers() srv.HostSudoers { + return &srv.HostSudoersNotImplemented{} } // GetRestrictedSessionManager returns a NOP manager since for a diff --git a/lib/srv/mock.go b/lib/srv/mock.go index 92ad427c5875e..26b8bbd87911b 100644 --- a/lib/srv/mock.go +++ b/lib/srv/mock.go @@ -285,8 +285,8 @@ func (m *mockServer) GetHostUsers() HostUsers { } // GetHostSudoers -func (m *mockServer) GetHostSudoers() (HostSudoers, error) { - return nil, nil +func (m *mockServer) GetHostSudoers() HostSudoers { + return &HostSudoersNotImplemented{} } // Implementation of ssh.Conn interface. diff --git a/lib/srv/regular/sshserver.go b/lib/srv/regular/sshserver.go index 0aabe097a5ba4..42c413850edfe 100644 --- a/lib/srv/regular/sshserver.go +++ b/lib/srv/regular/sshserver.go @@ -316,8 +316,11 @@ func (s *Server) GetHostUsers() srv.HostUsers { // GetHostSudoers returns the HostSudoers instance being used to manage // sudoers file provisioning -func (s *Server) GetHostSudoers() (srv.HostSudoers, error) { - return s.sudoers, nil +func (s *Server) GetHostSudoers() srv.HostSudoers { + if s.sudoers == nil { + return &srv.HostSudoersNotImplemented{} + } + return s.sudoers } // ServerOption is a functional option passed to the server diff --git a/lib/srv/sess.go b/lib/srv/sess.go index 0d6ff02cc5d7b..882c9d5046897 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -164,10 +164,7 @@ func NewSessionRegistry(cfg SessionRegistryConfig) (*SessionRegistry, error) { return nil, trace.Wrap(err) } - sudoers, err := cfg.Srv.GetHostSudoers() - if err != nil { - return nil, trace.Wrap(err) - } + sudoers := cfg.Srv.GetHostSudoers() return &SessionRegistry{ SessionRegistryConfig: cfg, log: log.WithFields(log.Fields{ @@ -236,6 +233,10 @@ func (sc *sudoersCloser) Close() error { } func (s *SessionRegistry) TryWriteSudoersFile(ctx *ServerContext) error { + if s.sudoers == nil { + return nil + } + sudoers, err := ctx.Identity.AccessChecker.HostSudoers(ctx.srv.GetInfo()) if err != nil { return trace.Wrap(err) diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index 9f80141c1f3bf..248acef6d1702 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -124,6 +124,18 @@ type HostSudoers interface { RemoveSudoers(name string) error } +type HostSudoersNotImplemented struct{} + +// WriteSudoers creates a temporary Teleport user in the TeleportServiceGroup +func (*HostSudoersNotImplemented) WriteSudoers(string, []string) error { + return trace.NotImplemented("host sudoers functionality not implemented on this platform") +} + +// RemoveSudoers removes the users sudoer file +func (*HostSudoersNotImplemented) RemoveSudoers(name string) error { + return trace.NotImplemented("host sudoers functionality not implemented on this platform") +} + type HostUsers interface { // CreateUser creates a temporary Teleport user in the TeleportServiceGroup CreateUser(name string, hostRoleInfo *services.HostUsersInfo) (io.Closer, error)