On Mon, May 30, 2011 at 6:23 PM, Seblu <[email protected]> wrote: > On Sat, May 28, 2011 at 5:52 AM, Eric Bélanger <[email protected]> > wrote: >> On Fri, May 27, 2011 at 10:19 AM, Seblu <[email protected]> wrote: >>> On Fri, May 27, 2011 at 11:42 AM, Eric Bélanger <[email protected]> >>> 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 <[email protected]> >>>> --- >>>> 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 >
