chris-huxtable / user_group.cr

Adds System Users and Groups.
ISC License
4 stars 1 forks source link

Missing getuid and setresuid #1

Closed didactic-drunk closed 4 years ago

didactic-drunk commented 5 years ago

Will you accept a PR to add the following:

Basically this with the names conforming to your project.

I got tired of fighting for crystal user/group PR acceptance and would prefer to not maintain my own shard.

chris-huxtable commented 5 years ago

I wouldn't say its missing many of these.

while

I would be open to adding a become_user and become_group method with named parameters for effective, and saved that then call setresuid and setresgid. Provided that it is compatible with UNIX systems that DO NOT support setresuid or setresgid such as macOS.

My goal with these wrappings is not to just be a simple mapping to c functions. I wanted it to be easier to use and more conceptually object oriented.

didactic-drunk commented 5 years ago

I need this to work in a chroot environment as a setuid application without using getpwnam switching euid/uid back and forth and also comparing xuid against files ownership. The group is never changed.

I would normally implement this as a low level and high level API (pardon the shorthand and incorrect types): The high level API become would handle cross platform and I guess the usual cases. The low level API would provide full POSIX.

# High level API for a permanent switch.
def become(user_id : User, group : Group?)
  group_id = group.try &.group_id
  become user.user_id, group_id
end
def become(user_id : UserIdT, group_id : GroupIdT?)
  # Use the low level API as the implementation.
  setresgid(...) if group_id
  setresuid(...)
end

# Full POSIX control.
#
def setresuid(...)
  # Error checking
  LibC.setresuid(...)
end

This is one of those times where I need POSIX functions. I don't think windows supports setuid programs. Googling suggests not. Attempting to abstract the low level API for windows seems like it would lead to NotImplemented.

If you would like to label the low level in a certain way, say "use become instead" or :nodoc: it I'm ok with that.

MacOS is my main test platform. It works. NotImplemented is raised if saved_id is set to a value it can't comply with similar to what you've done with saved_id.

chris-huxtable commented 5 years ago

Check out https://github.com/chris-huxtable/restrict.cr

If that doesn't work let me know and I will think about it again.

didactic-drunk commented 5 years ago

Also, I'm actively working to get these functions as standards in 7822. After seeing what you went through I'm not sure how long that will take which is why I came here. They don't seem to like shorthand euid either.

chris-huxtable commented 5 years ago

euid and the others are an artifact of simpler more constrained times. Inertia isn't a good reason to keep them (IMO).

Not to mention they aren't self descriptive. People aren't very good at following directions or reading manuals on a good day. I think its best to put training wheels on so its at least a bit harder to screw up.

didactic-drunk commented 5 years ago

restrict.cr only meets the needs of of a single call from the program below. It's great for single unidirectional switching but I need to swap euid/ruid multiple times within a program.

My shortened post from 7829:

Most of the proposed high level API's seems to assume the program only wants to switch user's once for privilege dropping.

Process.setresuid is intended to be used as the underlying method for such an operation by supplying all user id's with the same parameter. A higher level API can build on that.

Process.setresuid is necessary for privilege switching back and forth between root and other user's, for setuid/setgid programs with similar switching behaviors or perhaps for other uses.

Setuid example (similar usage to visudo or crontab):

uid = Process.uid
suid = Process.euid

# open file as suid user
Process.setresuid uid, suid
lock_file = File.new lock_file

# open file as original user
Process.setresuid suid, uid
user_file = File::Tempfile.new

fork do
  # drop all privileges to original user
  Process.become user # calls setgroups, setresgid, setresuid in the correct order.
  # spawn editor
end.wait

# parse and validate file
File.delete user_file

# back to suid user to save in a non user accessible location
Process.setresuid uid, suid
# save changes to suid owned location
File.delete lock_file
didactic-drunk commented 5 years ago

The names aren't important to me. I need the functionality.

chris-huxtable commented 5 years ago

Your above example would appear to work with my original proposal provided you saved the Users instead of the id's. Please excuse any mistakes, I haven't used this shard or Crystal is quite a few months.

user = Process.user
euser = Process.effective_user

# open file as suid user
Process.become_user(user, effective: euser)
lock_file = File.new lock_file

# open file as original user
Process.become_user(euser, effective: user)
user_file = File::Tempfile.new

fork do
  # drop all privileges to original user
  Process.become user # calls setgroups, setresgid, setresuid in the correct order.
  # spawn editor
end.wait

# parse and validate file
File.delete user_file

# back to suid user to save in a non user accessible location
Process.become_user(user, effective: euser)

# save changes to suid owned location
File.delete lock_file
jrester commented 4 years ago

Wrapper for setting the uid and gid would be great for docker environments, where the current API fails because the uid and gid aren't associated with any username. Therefore something like Process.set_uid and Process.set_gid would be great or something similar.

chris-huxtable commented 4 years ago

Closing. Resolved using @jrester's PR.