Skip to content
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

protocol "exec" should drop elevated privileges instead of suggesting to recompile to allow popen as root #583

Open
jscheidtmann opened this issue Mar 12, 2023 · 3 comments

Comments

@jscheidtmann
Copy link

jscheidtmann commented Mar 12, 2023

Protocol "exec" denies to execute any programs, if running as root, see MeterExec.cpp - L123 ff. The option offered in the error message is to recompile vzlogger to allow this using a compile time configuration setting, that allows to call external programs as root.

Instead Exec should:

  • Drop elevated privileges, if executed as root, before running the external script.
  • Add a Username or UID parameter that specifies which uid to execute the command as in the configuration.
  • In addition popen uses the default shell, which seems to potentially be open for attacks (reading the cookbook cited below).

OR

  • at least recommend using protocol "exec" only when invoking vzlogger as a normal user.

Example code for dropping elevated privileges can be found in "Secure Programming Cookbook for C and C++ by John Viega, Matt Messier", Recipes 1.3, 1.6, 1.7. (but I am not sure this fully applies).

@J-A-U
Copy link
Collaborator

J-A-U commented Mar 13, 2023

@jscheidtmann
Copy link
Author

Ah, hadn't spotted that yet. Then:

  • vzlogger should recommend doing that instead of recommending to recompile with exec as root.
  • In addition I believe it would be a nice improvement to drop privileges for the exec.

I am willing to work on pull requests to do both. Will take some time, though.

@r00t- r00t- changed the title protocol "exec" should drop elevated privileges instead of recompiling to allow popen as root protocol "exec" should drop elevated privileges instead of suggesting to recompile to allow popen as root Mar 17, 2023
@r00t-
Copy link
Contributor

r00t- commented Mar 17, 2023

i don't think this is a real issue.
the default behaviour is safe, and recompiling to change behaviour is out of scope of any security considerations.
(but indeed we should probably recommend to avoid running as root instead of recommending to recompile.)
but improvements are always welcome of course!

also, the more critical improvement might be to provide the infrastructure (systemd service file, udev rules) to simply avoid running as root?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants