Re: [arch-projects] [initscripts][PATCH 2/2] rc.d: Add check to cleanly abort rc.d script if user doesn't have root privileges

On Mon, May 30, 2011 at 6:23 PM, Seblu wrote:

On Sat, May 28, 2011 at 5:52 AM, Eric Bélanger wrote:

...

On Fri, May 27, 2011 at 10:19 AM, Seblu wrote:

...

On Fri, May 27, 2011 at 11:42 AM, Eric Bélanger wrote:

...

This implements FS#24095. The check is only made for the start, stop and restart actions of the daemon scripts. This allows regular user to use the help and list functionality of rc.d and also to use rc.d for actions that doesn't require root privileges, like the status action of some daemon scripts.

Signed-off-by: Eric Bélanger ---  rc.d |    4 ++++  1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/rc.d b/rc.d index 97f266a..2325623 100755 --- a/rc.d +++ b/rc.d @@ -43,6 +43,10 @@ case $1 in                ;;        *)                action=$1 +               if [[ "$EUID" != '0' ]] && [[ "$action" == 'start' || "$action" == 'stop' || "$action" == 'restart' ]] ; then +                   echo 'Error: this script must be run as root to use this functionality.' +                   exit 1 +               fi                shift                # set same environment variables as init                runlevel=$(/sbin/runlevel)

As i said in FS#24095, if we really want do this, we should not do this in rc.d script but in functions which is loaded by real rc scripts.

Increasingly, why choose start/stop/restart and not reload by example? By example, in virtualbox_bin we have fixusb, which must be run as root.

I only chose start/stop/restart because they are the only standard ones. There is a page describing arch standard, or at least, a namming convention?

The only one I can think of is the daemon prototype file which has a status function to display if the daemon is running or not. There are 2 packages that I maintain (or have maintained) that use the status for something else so it seem to be more standard than I thought. The reload action seem to be quite common and standard. Later tonight, I'll go through all the daemons in the repos and list what action they have (except start/stop/restart) and what these actions do. It'll give use a better idea if there is a genereal concensus on these various actions.

...

Not only all daemons script have them but they all do the same things: start/stop a binary before creating/removing a file contatining the PID in a directory that needs root privileges.  I am 100% sure that root privilege is required.  The other actions are not used by all daemons and what they do depends on the daemon itself. So we can't be really sure if root privileges are required or not.

I decided to play it safe and to treat everything else than start/stop/restart as edge cases. It makes sense, but I am forbidden to do

...

...

I think we should offer a check_root function which can be called in rc scripts to ensure rootitude. Be we cannot generically know if a rc need to be root or not.

That might be ideal.  But for the reason I mentionned above, the daemon scripts will probably need to be modified to indicate what privileges are needed for all actions.  Perhaps, demanding root privileges by default except when there is a NEED_ROOT=0 defined in the case, e.g. i agree. Idea is better.

...

 status)     NEED_ROOT=0     do stuff that don't need root privs...

Don't easy to implement in bash...

...

would reduce the workload on fixing the packages. That will surely take some time before it's implemented unless that can be done without modifying the daemon scripts. As said before, we can use functions scripts which is sourced by all rc.d scripts to make this check rather than doing it in rc.d

...

Until this is done, this proposed patch is a good compromise between the ideal situation and the current situation where no check is done at all.

Basically, there is maybe no problem to don't check. This is not done from the beginning. If you can, you can. If you need right it will fail. KISS.

...

 The majority of users will use rc.d to start/stop/restart daemons anyway.  Since you can use rc.d on many daemons at once, it's better to abort the rc.d script early so you can rerun it with sufficient privileges instead of having to wait for all the daemons to fail.  It would also be trivial to remove this check in rc.d once (or if) we have implemented something better.

I do not see the difference. If all daemons will need root privileges, they will fail.

I think a lots of users will still use /etc/rc.d/x start/stop/restart, and it's kind of sugar around if you can or not run a daemon.

I propose a patch following your recommendation here : https://github.com/seblu/arch-initscripts/commit/5a59a30

What do you think?

Your patch doesn't do what I was thinking mainly because my idea wasn't a good one. The problem is that the checking is done in /etc/rc.d/functions which is sourced at the begining of the script so the NEED_ROOT variable needs to be defined globally while I wanted a local approach that would depend on the items in the case statement. I don't know if it's clear but it doesn't matter because I found a better solution that works without needing the NEED_ROOT variable. We need to add in /etc/rc.d/functions your need_root function followed by: if [[ "$1" == 'start' || "$1" == 'stop' || "$1" == 'restart' ]]; then need_root fi I only have start/stop/restart in that code sample but we'll probably need to add more actions , like reload, depending on what I find in the daemon investigation I described above. I did tests with deamon scripts and rc.d and it works without any modifications needed to them. Couple drawbacks: - The list of actions to test for root is hard-coded. If I add a foobar action which requires root privileges to a daemon, it'll need to be added to the list in /etc/rc.d/functions. As this should happen often, it would be minimal maintenance. - It suppose that all daemon using a given action need the same privileges. If it's not the case, we'll need to either fix the packages, enforce root privileges or go with the majority depnding on the case. - When thinking about this, I only had the rc.d and daemon scripts in mind and I monly testes these. If there are other scripts which sources /etc/rc.d/functions, we'll need to check them to see if they still work fine with my change.

-- Sébastien Luttringer www.seblu.net