prometheus-community / ipmi_exporter

Remote IPMI exporter for Prometheus
MIT License
471 stars 133 forks source link

Will be supported by non-system administrator execution #32

Closed cawamata closed 5 years ago

cawamata commented 5 years ago

Hi. Recently, I found this exporter for monitoring H/W status via freeipmi commands. However, this exporter is based on running by system administrator because of executing freeipmi commands.

I think this is not good from security, because if this process was cracked, cracker will get system administrator authority.

My proposal is the followings not to execute by system administrator

What do you think about this?

ektich commented 5 years ago

Depending on sudo becomes too much "installation" specific: what about systems that don't use sudo for privilege separation?

I think documenting how this can be achieved with permissions under /dev/ might be a better idea.

another option to explore is setuid/setcap on freeipmi binary, instead of executing it via sudo, and not changing anything under /dev.

cawamata commented 5 years ago

@ektich Thank you for answer your idea. I will try to use 'setuid/setcap on freeipmi binary' because the permission investigation under /dev/ take a time than this method.

Do you have a plan to update README to run this daemon by non-administrator? Should I create Pull Request for this?

dswarbrick commented 5 years ago

I think optionally supporting sudo execution would be preferable, since making the freeipmi binary setuid / setcap opens up a pretty big security hole. Remember that the tools can be used to change power state on the local host - do you want non-root users to be able to do that?

ektich commented 5 years ago

Good point. To stop non-root users from running freeimpi its permissions should be changed so that only members of a "trusted" group are allowed to execute it (unless you absolutely need ordinary users to execute freeimpi. In that case passwordless sudo is the only option).

ektich commented 5 years ago

I've been looking at impi-sensors as installed by freeipmi-tools package in Debian buster, and it looks like it does not support capabilities at all, and it also has a hard-coded check for the uid and it refuses to run as any other user (unless I'm missing something in the configuration of freeipmi). So tweaking permissions in /dev/ is not going to help. The only to options remaining are calling freeipmi tools via sudo, or setting setuid bit on freeipmi tools, changing group ownership to a group to which the user that will be running ipmitools beliongs to, and changing permissions of tools themselves to "750" (remove execute permission from "all users")

dswarbrick commented 5 years ago

For reference / curiosity, the getuid check is in common/toolcommon/tool-util-common.c:

int
ipmi_is_root ()
{
#if IPMI_DONT_CHECK_FOR_ROOT
  return (1);
#else /* !IPMI_DONT_CHECK_FOR_ROOT */
  uid_t uid = getuid ();
  if (uid == 0)
    return (1);
  return (0);
#endif /* !IPMI_DONT_CHECK_FOR_ROOT */
}

That function is called by the ipmi_open function which appears to be fairly central to all the tools in freeipmi.

IMHO, the sudo approach is the slightly more secure approach, since you can explicitly specify the arguments that it is allowed to be called with in the sudoers config (e.g. restrict it to reading sensors only). Having the binaries setuid / setcap doesn't offer that level of granularity.

bitfehler commented 5 years ago

Hi, thank you for your interest in IPMI exporter. I understand that it might be undesirable to run the exporter as root. However, as other people have already pointed out in this thread, running IPMI tools as non-root can be tricky. Getting this to work is very system- and environment-specific, and we don't think the exporter should concern itself with this, as it would make this simple tool much too complicated if we were to support all possible scenarios.

That said, there is already a possibility to use sudo by means of a wrapper script. The wrapper script could look like this:

#!/bin/sh
sudo /usr/sbin/$(basename $0) "$@"

Create three executable files somewhere NOT in $PATH with the names ipmimonitoring, bmc-info, and ipmi-dcmi that are either a copy of this script or links to one instance of it. Then you can have the exporter use these scripts by calling it with --freeipmi.path=/path/to/scripts. This is the gist of it, I hope you get the idea, you could of course adjust the scripts as needed.

Please let me know if this would take care of your concerns. I could add a brief section to the README describing this.

cawamata commented 5 years ago

@bitfehler Thank you for answer. I confirmed your suggestion works as I expected.

Can you add this information to README?