This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.

Created attachment 396535 [details]
New clvmd init script

New init script as commited to upstream CVS

There are still a few problems with the clvmd initscript outstanding

1) clvmd does not generate a pid file
:: [   LOG    ] :: >>>>>>>>> pid file
:: [   FAIL   ] :: File /var/run/clvmd.pid should exist

2) initscript should check for insufficient rights and return status 4
:: [   LOG    ] :: >>>>>>>>> insufficient rights
:: [   PASS   ] ::  Starting service for restarting under nonpriv user
:: [   FAIL   ] :: Insufficient rights, restarting resrvice under nonprivileged user must fail (Expected 4, got 1)

[root@morph-01 tmp]# su - testmonkey service clvmd restart
Restarting clvmd:                                          [  OK  ]
touch: cannot touch `/var/lock/subsys/clvmd': Permission denied
[root@morph-01 tmp]# echo $?
1

Created attachment 430041 [details]
proposed patch

this patch addresses the last 2 remaining bits to have LSB complaint init script.

The lvm2-2.02.72-2.el6 build should now address the problems mentioned in comment #6.

We're not quite there yet

:: [   LOG    ] :: Assertions: 26 good, 1 bad

:: [   LOG    ] :: >>>>>>>>> insufficient rights
:: [   PASS   ] ::  Starting service for restarting under nonpriv user
:: [   FAIL   ] :: Insufficient rights, restarting resrvice under nonprivileged user must fail (Expected 4, got 1)

The daemon is now returning 4, but the init script isn't handling that return code correctly:

[testmonkey@dash-01 ~]$ /etc/init.d/clvmd restart
Restarting clvmd:    Cannot run as a non-root user.
                                                           [FAILED]
Signaling clvmd to exit /etc/rc.d/init.d/functions: line 536: kill: (29461) - Operation not permitted
                                                           [FAILED]
[testmonkey@dash-01 ~]$ echo $?
1

/etc/rc.d/init.d/clvmd:

        # Try to get clvmd to restart itself. This will preserve
        # exclusive LV locks
        action "Restarting $DAEMON: " $DAEMON -S

        # If that fails then do a normal stop & restart
        if  [ $? != 0 ]; then
            stop && start
            return $?
...

(In reply to comment #19)
> We're not quite there yet
> 
> :: [   LOG    ] :: Assertions: 26 good, 1 bad
> 
> :: [   LOG    ] :: >>>>>>>>> insufficient rights
> :: [   PASS   ] ::  Starting service for restarting under nonpriv user
> :: [   FAIL   ] :: Insufficient rights, restarting resrvice under nonprivileged
> user must fail (Expected 4, got 1)
> 
> The daemon is now returning 4, but the init script isn't handling that return
> code correctly:

I will fix both in the init script, but you need to consider the option to file a bug for initscripts component too. clvmd uses daemon function from /etc/rc.d/init.d/functions and that function is masking the return codes from the daemon. Doesn´t sound right to me.

> 
> [testmonkey@dash-01 ~]$ /etc/init.d/clvmd restart
> Restarting clvmd:    Cannot run as a non-root user.
>                                                            [FAILED]
> Signaling clvmd to exit /etc/rc.d/init.d/functions: line 536: kill: (29461) -
> Operation not permitted
>                                                            [FAILED]
> [testmonkey@dash-01 ~]$ echo $?
> 1
> 
> /etc/rc.d/init.d/clvmd:
> 
>         # Try to get clvmd to restart itself. This will preserve
>         # exclusive LV locks
>         action "Restarting $DAEMON: " $DAEMON -S
> 
>         # If that fails then do a normal stop & restart
>         if  [ $? != 0 ]; then
>             stop && start
>             return $?
> ...

(In reply to comment #20)
> (In reply to comment #19)
> > We're not quite there yet
> > 
> > :: [   LOG    ] :: Assertions: 26 good, 1 bad
> > 
> > :: [   LOG    ] :: >>>>>>>>> insufficient rights
> > :: [   PASS   ] ::  Starting service for restarting under nonpriv user
> > :: [   FAIL   ] :: Insufficient rights, restarting resrvice under nonprivileged
> > user must fail (Expected 4, got 1)
> > 
> > The daemon is now returning 4, but the init script isn't handling that return
> > code correctly:
> 
> I will fix both in the init script, but you need to consider the option to file
> a bug for initscripts component too. clvmd uses daemon function from
> /etc/rc.d/init.d/functions and that function is masking the return codes from
> the daemon. Doesn´t sound right to me.

Sorry, disregard my last comment, I misread the output from the testtool.

After further investigation, daemon function does indeed mask return codes from the real daemon.

Back to POST, patch has been committed upstream.

http://sources.redhat.com/git/gitweb.cgi?p=lvm2.git;a=commit;h=0cce8cfbaa661df3af1cbb6f190bcae3ed8bc18e

^ Upstream patch

(In reply to comment #24)
> http://sources.redhat.com/git/gitweb.cgi?p=lvm2.git;a=commit;h=0cce8cfbaa661df3af1cbb6f190bcae3ed8bc18e
> 
> ^ Upstream patch    

I don't think this is the best way to fix the bug.  Now the init script can't be used at all by a non-privileged user, not even for usage or status.  It would be better to check the return code of `clvmd -S` since that now returns 4 when a non-root user runs it.

(In reply to comment #29)
> (In reply to comment #24)
> > http://sources.redhat.com/git/gitweb.cgi?p=lvm2.git;a=commit;h=0cce8cfbaa661df3af1cbb6f190bcae3ed8bc18e
> > 
> > ^ Upstream patch    
> 
> I don't think this is the best way to fix the bug.  Now the init script can't
> be used at all by a non-privileged user, not even for usage or status.  It
> would be better to check the return code of `clvmd -S` since that now returns 4
> when a non-root user runs it.    

Are usage or status supposed to be usable by all? If so where is it documented?

Read also my comment in CVS commit and post to lvm-devel.

The init script is complex and does several operations, not just start/stop clvmd.

Clvmd itself is OK as it returns 4 when executed as non-root user, but the problem remains for all the other tools that do not respect this convention. This means having to script around N commands in a way that is incredibly fragile.

So unless there is a real need to be able to use status and usage as normal user, this patch will do fine.

Can we please be clear where this requirement comes from?  E.g. Reference to LSB?  Or is it only a Fedora thing?

I don't think there is any explicit requirement stated anywhere.  Init scripts are required to be 0755 which implies that everyone should be able to execute the script.  All other init scripts on the system will run as a non-root user.

If there is no LSB requirement for it then where did Fedora's "exit with code 4" come from?  I fully agree that scripts that are executable by others should be of some use to non-root, and if they aren't, instead of returning 4, the script permissions should be changed to deny others executability.

I think we didn't agree on what "the requirement" referred to.

The return codes are part of LSB:

http://refspecs.freestandards.org/LSB_3.1.1/LSB-Core-generic/LSB-Core-generic/iniscrptact.html

The requirement that users be able to run init scripts to get usage and status is not specified anywhere that I can find.

Ah.  Reading that, the ability for other users to run the script is implied by the requirement to exit with code 4 when hitting a permission failure.

(In reply to comment #32)
> I don't think there is any explicit requirement stated anywhere.  Init scripts
> are required to be 0755 which implies that everyone should be able to execute
> the script.  All other init scripts on the system will run as a non-root user.    

Actually they won't. Based on the last comments (c33/34/35), most of the scripts implementing "exit 4" don't do it correctly.Perhaps you should consider filing bugs for those as well.

Actually they do.  Try running all init script as a non-root user without args.  Except for the cluster and lvm2 related scripts, you get a usage message.  Now run them all with "status."  You'll get status for most and "status unknown due to insufficient privileges."  For the cluster scripts we get the message along the lines of "only root can execute this script."

Data collected with:

[testmonkey@dash-01 init.d]$ for i in *; do echo -n "$i: "; sh $i; done
[testmonkey@dash-01 init.d]$ for i in *; do echo -n "$i: "; sh $i status; done

(In reply to comment #37)
> Actually they do.  Try running all init script as a non-root user without args.
>  Except for the cluster and lvm2 related scripts, you get a usage message.  Now
> run them all with "status."  You'll get status for most and "status unknown due
> to insufficient privileges."  For the cluster scripts we get the message along
> the lines of "only root can execute this script."
> 
> Data collected with:
> 
> [testmonkey@dash-01 init.d]$ for i in *; do echo -n "$i: "; sh $i; done
> [testmonkey@dash-01 init.d]$ for i in *; do echo -n "$i: "; sh $i status; done    

Try this other bit instead so it's easier to see what fails without getting lost in the noise:

for i in *; do echo -n "$i: "; sh $i ; echo; done
auditd: 
irqbalance: 
restorecond: 
rpcgssd: 
rpcsvcgssd: 
sendmail: 
single: 

[fabbione@gundam init.d]$ for i in *; do echo -n "$i: "; sh $i status; done
bluetooth: Usage: bluetooth {start|stop}
firstboot: Usage: firstboot {start|stop}
halt: Usage: halt {start}
hsqldb: Only 'root' may use this init script
ip6tables: ip6tables: line 53: /etc/sysconfig/ip6tables-config: Permission denied
iptables: iptables: line 53: /etc/sysconfig/iptables-config: Permission denied
irqbalance: killall: Usage: killall {start}
rpcsvcgssd: rsyslog: rsyslogd status unknown due to insufficient privileges.

Clearly this varies from the dataset, and I think we should take this specific bit somewhere else.

You also want to notice another thing.

Most init scripts use status() from /etc/rc.d/init.d/functions to determine if a daemon is running. status uses __pid_of_var that does indeed return 4 if the user is not allowed to read the lock/pid file. Being able to get a status back is dependent strictly on the daemon paranoia and not a given fact.

As for the cluster scripts, I am happy for only root to be able to run them, since they do use different tools that requires root even to check if stuff is running.

As for clvmd, I'll let the team decide what they want and I'll just fix it as long as we can finish with this bug that's becoming a soap opera.

What happened with this one?

- What else is still to be done for 6.1?
- Did anything get into GA despite 'FailedQA' ?

(In reply to comment #40)
> - What else is still to be done for 6.1?

I think we still need to decide on what to do and what seems to be the correct behaviour in case the script is run by non-root user (...controversy in comment #37 and comment #38).

> - Did anything get into GA despite 'FailedQA' ?

Everything should be in GA except the controversial "run by non-root user and return code" part (based on comment #27, this did not get further and was postponed to 6.1).

(In reply to comment #41)
> Everything should be in GA except the controversial "run by non-root user..."

..meaing, the patch mentioned in comment #24 is not included in rhel6 yet.

So I'm still lost.  Please would someone summarise?

What does the script in RHEL6.0 still do "wrong" today?
(How does it currently behave?)

What are the choices as to how to fix it?

(In reply to comment #44)
> (How does it currently behave?)

(running as a non-root user, current rhel6)

[peter@localhost ~]$ /etc/init.d/clvmd
Usage: /etc/init.d/clvmd {start|stop|status|restart|condrestart|try-restart|reload|force-reload}

[peter@localhost ~]$ echo $?
2

[peter@localhost ~]$ /etc/init.d/clvmd status
clvmd is stopped

[peter@localhost ~]$ echo $?
3

[peter@localhost ~]$ /etc/init.d/clvmd restart
Restarting clvmd: Cannot run as a non-root user.   [FAILED]
Starting clvmd: Cannot run as a non-root user.     [FAILED]

[peter@localhost ~]$ echo $?
1

------------------------

(running as a non-root user, with the additional patch from comment #24, not currently included in rhel6)

[peter@localhost ~]$ /etc/init.d/clvmd
clvmd init script can only be executed as root user

[peter@localhost ~]$ echo $?
4

[peter@localhost ~]$ /etc/init.d/clvmd status
clvmd init script can only be executed as root user

[peter@localhost ~]$ echo $?
4

[peter@localhost ~]$ /etc/init.d/clvmd restart
clvmd init scrupt can only be executed as root user

[peter@localhost ~]$ echo $?
4

So starting from where we are today, minimum change approach:

(In reply to comment #45)
> (running as a non-root user, current rhel6)
> 
> [peter@localhost ~]$ /etc/init.d/clvmd
> Usage: /etc/init.d/clvmd
> {start|stop|status|restart|condrestart|try-restart|reload|force-reload}
> 
> [peter@localhost ~]$ echo $?
> 2

That's OK

> [peter@localhost ~]$ /etc/init.d/clvmd status
> clvmd is stopped

That's OK

(assuming it really is stopped)

> [peter@localhost ~]$ echo $?
> 3

> [peter@localhost ~]$ /etc/init.d/clvmd restart
> Restarting clvmd: Cannot run as a non-root user.   [FAILED]
> Starting clvmd: Cannot run as a non-root user.     [FAILED]
> 
> [peter@localhost ~]$ echo $?
> 1

That's wrong - needs to return 4.

> ------------------------
> 
> (running as a non-root user, with the additional patch from comment #24, not
> currently included in rhel6)
> 
> [peter@localhost ~]$ /etc/init.d/clvmd
> clvmd init script can only be executed as root user
> 
> [peter@localhost ~]$ echo $?
> 4
> 
> [peter@localhost ~]$ /etc/init.d/clvmd status
> clvmd init script can only be executed as root user
> 
> [peter@localhost ~]$ echo $?
> 4
> 
> [peter@localhost ~]$ /etc/init.d/clvmd restart
> clvmd init scrupt can only be executed as root user
> 
> [peter@localhost ~]$ echo $?
> 4

I'd be less happy with that.
The principle should be that permissions in the filesystem or running daemon comms control this information, not the script itself.

I.e. the script itself should apply no security restrictions of its own, but just reflect restrictions already present elsewhere on the system itself.  (Sending signal to daemon fails, viewing 'ps' or pidfile fails etc.)

> I.e. the script itself should apply no security restrictions of its own, but
> just reflect restrictions already present elsewhere on the system itself. 

The check for root is there also because some tests inside script like vgdisplay fails.

All this is wrong in multi level policy system anyway, so I would prefer to not rewrite it again (we have the same check now in RHEL4/5).

And solve it later properly by assigning capabilities for actions (but that requires also kernel changes).
(Restarting clvmd require different permission than activation or displaying info about some LVs etc.)

::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
:: [   LOG    ] :: Test description
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::

Cannot find the PURPOSE file of this test. Could be a missing, or
rlInitializeJournal wasn't called from appropriate location

::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
:: [   LOG    ] :: Test
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::

:: [   LOG    ] :: >>>>>>>>> service start
:: [   PASS   ] ::  Service must start without problem
:: [   PASS   ] ::  Then Status command
:: [   PASS   ] ::  Already started service
:: [   PASS   ] ::  Again status command
:: [   LOG    ] :: >>>>>>>>> service restart
:: [   PASS   ] ::  Restarting of service
:: [   PASS   ] ::  Status command
:: [   LOG    ] :: >>>>>>>>> service stop
:: [   PASS   ] ::  Stopping service
:: [   PASS   ] ::  Status of stopped service
:: [   PASS   ] ::  Stopping service again
:: [   PASS   ] ::  Status of stopped service
:: [   LOG    ] :: >>>>>>>>> pid file
:: [   PASS   ] :: File /var/run/clvmd.pid should exist
:: [   PASS   ] :: Running 'echo 666666 > /var/run/clvmd.pid'
:: [   PASS   ] ::  Existing pid file, but service not started
:: [   LOG    ] :: >>>>>>>>> lock file
:: [   PASS   ] :: File /var/lock/subsys/clvmd should exist
:: [   PASS   ] :: Running 'touch /var/lock/subsys/clvmd'
:: [   PASS   ] ::  Existing lock file, but service not started
:: [   LOG    ] :: >>>>>>>>> insufficient rights
:: [   PASS   ] ::  Starting service for restarting under nonpriv user
:: [   PASS   ] :: Insufficient rights, restarting resrvice under nonprivileged user must fail
:: [   LOG    ] :: >>>>>>>>> operations
:: [   PASS   ] ::  Service have to implement start function
:: [   PASS   ] ::  Service have to implement restart function
:: [   PASS   ] ::  Service have to implement status function
:: [   PASS   ] ::  Service have to implement condrestart function
:: [   PASS   ] ::  Service have to implement try-restart function
:: [   PASS   ] ::  Service have to implement reload function
:: [   PASS   ] ::  Service have to implement force-reload function
:: [   LOG    ] :: >>>>>>>>> nonexist operations
:: [   PASS   ] ::  Testing proper return code when nonexisting function
:: [   LOG    ] :: >>>>>>>>> invalid arguments
:: [   PASS   ] ::  When no arguments added to service, it must fail with proper return code
:: [   LOG    ] :: Duration: 12s
:: [   LOG    ] :: Assertions: 27 good, 0 bad
:: [   PASS   ] :: RESULT: Test
/usr/share/rhts-library//journal.sh: line 91: $OUTPUTFILE: ambiguous redirect
:: [09:34:46] ::  JOURNAL: /tmp/tmp.YSUAjcJlCD/journal.xml
:: [09:34:46] ::  OUTPUTFILE:
Signaling clvmd to exit                                    [  OK  ]
clvmd terminated                                           [  OK  ]

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0772.html