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

[Seblu]

On Sat, May 28, 2011 at 5:52 AM, Eric Bélanger <snowmaniscool at gmail.com> wrote:
> On Fri, May 27, 2011 at 10:19 AM, Seblu <seblu at seblu.net> wrote:
>> On Fri, May 27, 2011 at 11:42 AM, Eric Bélanger <snowmaniscool at gmail.com> 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 <snowmaniscool at gmail.com>
>>> ---
>>>  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?

> 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?


-- 
Sébastien Luttringer
www.seblu.net