-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFD: Add Automatic User Provisioning #11077
Conversation
746be5b
to
236fe62
Compare
236fe62
to
bf827ff
Compare
```yaml | ||
ssh_service: | ||
# when disabled, takes precedence over the role setting | ||
disable_auto_create_user: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean it will default to false, making this an opt-out
feature? My main concern here is unless we're dropping PAM support, we'll need to ensure that they play nicely together. Maybe this will end up being trivial, or we could turn off auto_create_user
when PAM is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think this and PAM should interact, teleport will be operating separately to create users, if the user has a pam script/module also creating users then there could be problems but the PAM script could be doing other things too.
This is defaulting to false, just so on a per node level user creation can be disabled
was created by teleport and that its safe for it to be deleted. The | ||
groups will be created via `groupadd` at startup if they do not already | ||
exist and users will be added to groups via `usermod -aG <list of groups> <username>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to also de-provision groups? I'm not sure how we could discern which groups were created by teleport other than keeping an internal record, so if we want this it may make sense to alter the clean-up process. Or maybe created groups could have a -teleport
suffix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was originally just going to just create groups at startup and leave them around
I'm not sure how group deletion would work, it doesnt seem to cause an error if a group gets deleted while a user in that group has a running process. Maybe it could work by checking that there are no users in the teleport-system group anymore and then deleting groups made by teleport.
I think having a -teleport suffix would work but might make it a bit unclear when configuring software, why users have some groups with -teleport and some groups that already existed dont have a suffix,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lxea In addition to making the changes we already discussed, can you please add a separate UX section describing the following use-cases and how the configuration would look like from Teleport administrator perspective:
- Teleport admin wants each user has a dedicated SSH account defined in their Okta attributes.
- Teleport admin wants to define which Linux groups each auto-created user will be added to.
- Teleport admin wants to make each auto-created user a sudoer.
- Teleport admin wants to define particular commands user will be able to run as root.
- Teleport admin wants to prohibit some nodes from auto-creating users.
In addition, can you also add a section addressing the security concern with preventing a user who was auto-added to sudoers from privilege escalation (e.g. modifying the sudoers file to their liking).
bf827ff
to
66f6179
Compare
I've updated the document for this however it requires a command allow list, i dont really think there is a sensible way to stop the user from editing the sudoers file if they have anything like |
66f6179
to
73c7ab1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xinding33 @stevenGravy Could you folks please take a look at this as well from the UX perspective?
|
||
### User creation | ||
|
||
In order to create users `useradd` will be executed from teleport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth mentioning that useradd
, compared to adduser
, does not provision home directory. I think it should be fine for dynamic users. Any there any other differences between these 2 commands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adduser
is distro specific so its sometimes not available or has different command sets available.
useradd
can create home directories, i think it just doesnt by default, its flag is --create-home
spec: | ||
options: | ||
# Controls whether this role supports auto provisioning of users. | ||
auto_create_user: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a user has multiple roles attached and only some (or one) of them enable the auto_create_user
- what would the behavior be? I think that it should only take effect if all roles matching the node you're accessing (by labels
) enable it. Could you add a section describing this scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a example to the UX section with my understanding of how this should work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
73c7ab1
to
afc181d
Compare
Teleport. This will require that names containing invalid characters | ||
have those characters removed/replaced. Information on the valid | ||
characters between Linux distros is available [here](https://systemd.io/USER_NAMES/). | ||
The common core of valid characters is `^[a-z][a-z0-9-]{0,30}$`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this configurable and start with this. This doesn't have @
, _
which are pretty common. Also GCP will create users with their personalized emails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm dont know how to make this configurable or what the ux should be like for such a thing, I intended to replace any characters that dont fit with -
.
I was operating under the impression that the users unix username would need to match the teleport user however I think it would make sense that it be any user available in the logins trait, in which case it could be up to the user to provide an external trait -- something like {{external.linix_username}}
, which conforms to the requirements of whatever Linux distro is being used and have Teleport try to blindly create users and just log an error when it fails
### User creation | ||
|
||
In order to create users `useradd` will be executed from teleport | ||
after a user has tried to access a Teleport SSH node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would agent recycle users that are inactive automatically? Same for sudoers lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by recycle inactive users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After sessions end the user and their sudoers files are deleted, if they're leftover (due to, for example a running process remaining) they and their sudoers file will be removed also.
Automatic user provisioning will require that all roles matching a | ||
node via `labels` have `auto_create_user=true` | ||
|
||
## UX Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Props for UX examples, makes it much easier to understand
afc181d
to
77c36b9
Compare
👍
Clarified in document -- if a user has multiple roles with separate sudoers specifications they will all be written to a /etc/sudoers.d/teleport-$unix_username` with each on a separate line
This is specified in the
Replied in comment |
create_host_user: true | ||
allow: | ||
# make it so this specific user can execute `/bin/ ` | ||
host_sudoers: ["{{internal.logins}} ALL = (root) NOPASSWD: /usr/bin/systemctl"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we prohibit arbitrary additions to a sudoers file and manually prepend the actual unix username to the lines in host_sudoers
instead? Or perhaps add another escape for the unix username that's being used to log in, as {{internal.logins}}
can have a lot of usernames, some of which might belong to other users in the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also not sure about using a naked systemctl
as an example in our docs, as that can be used to escalate to arbitrary command exec pretty easily (just edit
and restart
some existing service). Perhaps something along the lines of this?
host_sudoers: ["{{internal.logins}} ALL = (root) NOPASSWD: /usr/bin/systemctl"] | |
host_sudoers: ["{{internal.logins}} ALL = (root) NOPASSWD: /usr/bin/systemctl restart nginx.service"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we prohibit arbitrary additions to a sudoers file and manually prepend the actual unix username to the lines in host_sudoers instead? Or perhaps add another escape for the unix username that's being used to log in, as {{internal.logins}} can have a lot of usernames, some of which might belong to other users in the system.
I'm not sure, I think teleport should do as little as possible that isnt explicitly set by the user for sudoers file entries,
After all of a users sessions are logged out the created user and any | ||
`sudoers` files that were created for that user will be deleted if | ||
that user is also a member of the `teleport-system` group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we deal with leftover files? Are we ok with the UID being reused by some other user (real or ephemeral) in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it stands I expected UIDs to be reused, but perhaps it could make sense to have user deletion set the shell to /bin/nologin or something, this way UIDs wont clash
77c36b9
to
8b48a45
Compare
# add users to the wheel group | ||
groups: [wheel] | ||
# make it so users in the wheel group will be able to execute sudoers commands without a password | ||
host_sudoers: ["%wheel ALL=(ALL) NOPASSWD: ALL"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this field an array of entries for the sudoers file? Just want to make sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, each entry will be a new line
8b48a45
to
4c5029b
Compare
4c5029b
to
e0c4dcd
Compare
@espadolini @klizhentas Are there outstanding issues with this RFD that need to be addressed, or is this in an okay state to be merged? Implementation is here #11830 and sudoers is here #12061 |
I've already voiced my doubts around automatic user cleanup with regards to race conditions, background processes, UID reuse and home directory cleanup/reuse - we addressed some of it on the implementation side, but some flaws are part of the design, IMO. A different question: should we add some way to specify which host users can be automatically provisioned for a given user/role? AIUI with the current design there's no way to allow a user to have a handful of login names that can be used for existing host users and only some login names for autoprovisioning, so any (teleport) user with permissions for host user creation must be restricted to only the intended autoprovisioning logins. Perhaps the users that should be autoprovisioned should only come from the roles that allow autoprovisioning, similarly to how the check for |
e0c4dcd
to
233eb24
Compare
233eb24
to
4d8eecc
Compare
This adds an RFD with requirements for the implementation of automatic user provisioning on Teleport SSH nodes.
Related issue: #10486