Skip to content

Commit

Permalink
Order sudoers file lines by role name (#23760)
Browse files Browse the repository at this point in the history 
* Order sudoers lines by role name

* Add a warning in the docs to explain order sudoers roles are written as

* Resolve comments

* Resolve comments

* fix lints
  • Loading branch information
@lxea
lxea committed Apr 19, 2023
1 parent 61c65f1 commit 7b8b748
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 10 deletions.
7 changes: 7 additions & 0 deletions docs/pages/server-access/guides/host-user-creation.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ will be disabled. Roles that do not match the Node will not be checked.

</Admonition>

<Admonition type="warning">

When multiple roles contain `host_sudoers` entries, the sudoers file
will have the entries written to it ordered by role name

</Admonition>

If a role includes a `deny` rule that sets `host_sudoers` to `'*'`, the user will
have all sudoers entries removed when accessing matching Nodes, otherwise `deny`
rules are matched literally when filtering:
Expand Down
41 changes: 31 additions & 10 deletions lib/services/role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2707,9 +2707,17 @@ func (set RoleSet) EnhancedRecordingSet() map[string]bool {
// a role disallows host user creation
func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
groups := make(map[string]struct{})
sudoers := make(map[string]struct{})
var sudoers []string
serverLabels := s.GetAllLabels()
for _, role := range set {

roleSet := make([]types.Role, len(set))
copy(roleSet, set)
slices.SortStableFunc(roleSet, func(a types.Role, b types.Role) bool {
return strings.Compare(a.GetName(), b.GetName()) == -1
})

seenSudoers := make(map[string]struct{})
for _, role := range roleSet {
result, _, err := MatchLabels(role.GetNodeLabels(types.Allow), serverLabels)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2728,10 +2736,16 @@ func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
groups[group] = struct{}{}
}
for _, sudoer := range role.GetHostSudoers(types.Allow) {
sudoers[sudoer] = struct{}{}
if _, ok := seenSudoers[sudoer]; ok {
continue
}
seenSudoers[sudoer] = struct{}{}
sudoers = append(sudoers, sudoer)
}
}
for _, role := range set {

var finalSudoers []string
for _, role := range roleSet {
result, _, err := MatchLabels(role.GetNodeLabels(types.Deny), serverLabels)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2742,18 +2756,25 @@ func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
for _, group := range role.GetHostGroups(types.Deny) {
delete(groups, group)
}
for _, sudoer := range role.GetHostSudoers(types.Deny) {
if sudoer == "*" {
sudoers = nil
break

outer:
for _, sudoer := range sudoers {
for _, deniedSudoer := range role.GetHostSudoers(types.Deny) {
if deniedSudoer == "*" {
finalSudoers = nil
break outer
}
if sudoer != deniedSudoer {
finalSudoers = append(finalSudoers, sudoer)
}
}
delete(sudoers, sudoer)
}
sudoers = finalSudoers
}

return &HostUsersInfo{
Groups: utils.StringsSliceFromSet(groups),
Sudoers: utils.StringsSliceFromSet(sudoers),
Sudoers: sudoers,
}, nil
}

Expand Down
108 changes: 108 additions & 0 deletions lib/services/role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5955,6 +5955,9 @@ func TestHostUsers_HostSudoers(t *testing.T) {
test: "multiple roles, one not matching",
sudoers: []string{"sudoers entry 1", "sudoers entry 2"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
Name: "a",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
Expand All @@ -5965,6 +5968,9 @@ func TestHostUsers_HostSudoers(t *testing.T) {
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
Name: "b",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
Expand Down Expand Up @@ -6060,6 +6066,108 @@ func TestHostUsers_HostSudoers(t *testing.T) {
},
},
},
{
test: "multiple roles, order preserved by role name",
sudoers: []string{"sudoers entry 1", "sudoers entry 2", "sudoers entry 3", "sudoers entry 4"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
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"},
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
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"},
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
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"},
},
},
}),
server: &types.ServerV2{
Metadata: types.Metadata{
Labels: map[string]string{
"success": "abc",
},
},
},
},
{
test: "duplication handled",
sudoers: []string{"sudoers entry 2"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
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"},
},
},
}, &types.RoleV6{ // DENY sudoers entry 1
Metadata: types.Metadata{
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"},
},
},
}, &types.RoleV6{ // duplicate sudoers entry 1 case also gets removed
Metadata: types.Metadata{
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"},
},
},
}),
server: &types.ServerV2{
Metadata: types.Metadata{
Labels: map[string]string{
"success": "abc",
},
},
},
},
} {
t.Run(tc.test, func(t *testing.T) {
info, err := tc.roles.HostUsers(tc.server)
Expand Down

0 comments on commit 7b8b748

Please sign in to comment.