Hi guys, I have a question, because I am facing a performance issue, but I don’t know how to solve it.
Basically, because at some point I need to run few commands as root during the installation phase of a software, my package manager ISM need to use Process.run to elevate temporary the privileges.
But the problem is, when I use Process.run instead of the standard crystal function to copy a file, I am facing performance issue. Specially when I need to copy a big amount of file.
def installFile(target : String, path : String, user : String, group : String, mode : String)
if !Ism.stillHaveSudoAccess && systemHandleUserAccess
Ism.printInstallFileSecurityNotification
end
requestedCommands = <<-CMD
#{systemHandleUserAccess ? "sudo" : ""} install -o #{user} -g #{group} -m #{mode} \"#{target}\" \"#{path}\"
CMD
process = Process.run(requestedCommands, shell: true)
if !process.success?
Ism.notifyOfRunSystemCommandError(requestedCommands)
Ism.exitProgram
end
end
Do you know if there is any way to do better ?
But to be honest will be better if I stick on the use of the command install, because this command is designed to handle gracefully file updates too.
The File methods for chmod and chown depend on integer uid and gid, but you can get those via the System::User and System::Group classes.
require "system/user"
require "system/group"
def installFile(target : String, path : String, user : String, group : String, mode : String)
sys_user = System::User.find_by(name: user)
sys_grp = System::Group.find_by(id: sys_user.group_id)
installFile target, path,
uid: sys_user.id.to_i,
gid: sys_grp.id.to_i,
permissions: File::Permissions.new(mode.to_i(8))
end
def installFile(target : String, path : String, uid : Int, gid : Int, permissions : File::Permissions)
File.copy target, path
File.chmod path, permissions
File.chown path, uid, gid
end
I haven’t run this, but it might be a good starting point for you.
Thank you very much jgaskins for your answer. But maybe I didn’t explain properly, but I am not looking for another way to set permissions, I am looking for a way to improve the performance of Process.run, because I have to run this command as root via sudo.
Basically, at some point, my program will request sudo access
The hard part is that you’re asking how to make something fast that is inherently slow. If performance is critical, moving away from Process.run. Spawning a new process is implemented as fork + exec on POSIX systems, and that’s a heavy pair of syscalls. It’s less slow than it used to be, but it still involves significant overhead.
Depending on how much performance you need, you have a few options. In descending order of performance impact:
- don’t use
Process.run at all and do the work in Crystal instead, using setuid for sudo access
- this requires that the program’s file be owned by
root and have the setuid flag set on it with chmod u+s
- make sure you drop root privileges when you no longer need them
- I’ve provided some code below that lets you run
sudo as a Crystal block, so the block is executed as root and normal user privileges are restored after the block completes — the tradeoff here is that setuid is process-wide so you probably don’t want to do non-root work concurrently in other fibers
- shell out to
sudo install only once, passing all of the file paths to it instead of one path at a time
- this amortizes the
Process.run overhead
- The easiest route, though certainly still the slowest, would be to use
shell: false to avoid calling /bin/sh in between because that’s yet another fork + exec for each Process.run call. If you do this, keep in mind that you’ll have to construct the args yourself in Crystal code because the shell won’t do it for you.
Here is some code that will use setuid appropriately for you:
pp LibC.getuid
sudo { pp LibC.getuid }
pp LibC.getuid
def sudo(&)
uid = LibC.getuid
result = LibC.setuid 0 # root
if result.negative?
raise InvalidPrivileges.new("Could not setuid to root, make sure executable is owned by `root` and has setuid flag set with `chmod u+s`")
end
begin
yield
ensure
LibC.setuid uid
end
end
class InvalidPrivileges < Exception
end
Remember that the compiled program must be owned by root and must have chmod u+s set in order for the sudo block to work. On my machine, this is the output, showing my normal uid, then a root escalation inside the block, then back to my normal uid after the block completes:
501
0
501
2 Likes
Thanks a lot ! I am very interested of your code you wrote.
I just worry for one thing because I am not completely familiar with suid.
I always heard it is better to avoid any program with the bit suid set, because it can bring a lot of security concern. So what should I keep in mind/ remember to don’t get security concern with this ?
What’s happen exactly when you set the bit suid ?
So I guess basically a program with this bit setted can switch to any user ID ?
Not any uid, just the owner of the file. root is definitely a big hammer, so if there is another user you can chown the file to, that would be ideal.
“Always/never do X” rules are rarely as generally applicable as the people that say them want them to sound. For example, Docker avoided using setuid for years, but then we had to run containers the Docker daemon as root which was arguably worse.
In order for setuid to be exploited, you would need to be vulnerable to ACE attacks while the program is running. If you’re prompting the user for sudo access, you’re opening the user to the same things after they auth unless they have sudo configured to ask for their password every time.
Usually these attacks happen via buffer overflows or use-after-free situations. In C programs, these can be common and that’s probably why you’ve heard that advice. But these vulnerabilities are exceedingly rare in Crystal because we generally don’t manage our own heap memory and, when we have to (for example, when integrating with a C library), there are excellent patterns that are trivial to implement that avoid these situations.
One thing you do need to watch out for, which I mentioned above, is that you don’t want to run anything concurrently while that sudo block is running. Don’t spawn other fibers inside the block and, if you already spawned other fibers before the block, wait for them to complete or have them pause their work while the sudo block is running. Every file they write, every socket they open, etc will all be done as the setuid user process-wide. Same advice for when the block ends — everything returns to the original user’s permissions at that point, even if it was in a fiber that was spawned inside the block.
Everything you said make really sens.
I will really think of this option. Sound really amazing
A slightly simpler change is to keep using a separate process but use a process that does all the “installs” at once rather than launch one process for each.
I am fairly sure there are such things already but writing one is very simple too.
I have one last question about this jgaskins. If I use this way with the suid bit, I would like one more thing. It’s when I temporary run a privileged command, I would like it ask for a password . Because I would like still a barrier if anyone have a physical access .
Is it possible ?
I find how to do ! Thank you for your help @jgaskins !
So now I am facing a problem now.
Now I get an error because my function can return Process::Status OR nil:
def runChrootTasks(chrootTasks, quiet = false, asRoot = false) : Process::Status
quietMode = (quiet ? Process::Redirect::Close : Process::Redirect::Inherit)
File.write(@settings.rootPath+ISM::Default::Path::TemporaryDirectory+ISM::Default::Filename::Task, chrootTasks)
process = Process.run( "chmod +x #{@settings.rootPath}#{ISM::Default::Path::TemporaryDirectory}#{ISM::Default::Filename::Task}",
output: quietMode,
error: quietMode,
shell: true)
process = runAsRoot {
Process.run("HOME=/var/lib/ism chroot #{asRoot ? "" : "--userspec=#{systemId}:#{systemId}"} #{@settings.rootPath} ./#{ISM::Default::Path::TemporaryDirectory}#{ISM::Default::Filename::Task}",
output: quietMode,
error: quietMode,
shell: true)
}
File.delete(@settings.rootPath+ISM::Default::Path::TemporaryDirectory+ISM::Default::Filename::Task)
return process
rescue error
printSystemCallErrorNotification(error)
exitProgram
end
How can I make sure the method return always Process::Status ? 
It’s the rescue block. What does the exitProgram method look like?