volkszaehler / vzlogger

Logging utility for various meters & sensors
http://wiki.volkszaehler.org/software/controller/vzlogger
GNU General Public License v3.0
145 stars 123 forks source link

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

Open jscheidtmann opened 1 year ago

jscheidtmann commented 1 year ago

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:

OR

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 commented 1 year ago

We already recommend to solve this problem by using a different user: https://wiki.volkszaehler.org/software/controller/vzlogger/installation_cpp-version#vzlogger_als_anderer_benutzer_ausfuehren

(Also in english available: https://wiki.volkszaehler.org/software/controller/vzlogger/installation_en#run_vzlogger_with_different_user )

jscheidtmann commented 1 year ago

Ah, hadn't spotted that yet. Then:

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

r00t- commented 1 year ago

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?