On Thu, Jun 2, 2011 at 2:30 AM, Eric Bélanger <[email protected]> wrote: > On Tue, May 31, 2011 at 7:29 PM, Eric Bélanger <[email protected]> > wrote: >> 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. >> > > Hi, > > Here's my findings. Basically, all daemons actions need root > privileges except a few of them: > > - iflist) and rtlist) from network daemon > - reload) action for the dbus daemon. As all other daemon require > root privileges for reload, it will be best to enforce it for the > reload action. > - status). A majority of daemon don't need root privilege for it but, > again, there are exceptions: > - ufw: looks like a bug in its daemon file. I'll submit a bug report. > - laptop-mode-tools: With user privs, you get some information > about the system except the ones that need > direct access to the hard drive devices. Not really worth > fixing. Let's ignore it. > - apcupsd: I can't test this one as I don't have a UPS.I get the > same error message whether I have root > privileges or not so it might not require them. > > Long story short: I suggest enforcing root privileges for all daemon > actions except: iflist, rtlist and status. So we have: > > > need_root() { > (( $EUID != 0 )) && printf 'You need to be root.\n' && exit 1 > } > > if [[ "$1" == 'attach' || "$1" == 'careless_start' || "$1" == 'check' || \ > "$1" == 'clean' || "$1" == 'condrestart' || "$1" == 'down' || \ > "$1" == 'force-quit' || "$1" == 'force-reload' || "$1" == > 'force-restart' || \ > "$1" == 'ifdown' || "$1" == 'ifup' || "$1" == 'load' || \ > "$1" == 'logrotate' || "$1" == 'purge' || "$1" == 'reload' || \ > "$1" == 'restart' || "$1" == 'resume' || "$1" == 'rtdown' || > "$1" == 'rtup' || \ > "$1" == 'save' || "$1" == 'setup' || "$1" == 'start' || "$1" == > 'stop' || \ > "$1" == 'suspend' || "$1" == 'unload' || "$1" == 'up' ]]; then > need_root > fi > Hi,
Thanks for crawling initscripts to find problematic options. You probably miss some case in aur, where rc.d scripts implement "strange" options which need root priv, for example virtualbox_bin which have a fixsub. I really don't like this idea of hardcode which options should or should not needs root priv in initscripts, because, I think we should let rc.d scripts depends of initscripts package, and not initscripts package depends of ufw, laptop-tools or others. I also think we should be able to resume by a clear and simple policy like : By default initscripts need to be root (Thomas in FS#24095). If you want bypass this assumption to offer a by options (or by what you want) right management, just disable and do what you want. need_root is just a proposed generic helper to enforce root policy. But, scripts maintainer can check if user who ask something is member of a group to allow his script. Intelligence is in the script, we should not try to predict what it will do a in special cases (options). Let script maintainers choose. Cheers, -- Sébastien Luttringer www.seblu.net
