Bug 553381 - clvmd initscript [LSB|FedoraGuidelines] compliance
clvmd initscript [LSB|FedoraGuidelines] compliance
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: lvm2 (Show other bugs)
6.0
All Linux
low Severity medium
: rc
: ---
Assigned To: Peter Rajnoha
Cluster QE
:
Depends On: 533247
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-07 14:10 EST by Lon Hohberger
Modified: 2016-04-26 11:19 EDT (History)
11 users (show)

See Also:
Fixed In Version: lvm2-2.02.82-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 533247
Environment:
Last Closed: 2011-05-19 10:25:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
New clvmd init script (3.98 KB, text/plain)
2010-02-26 08:10 EST, Fabio Massimo Di Nitto
no flags Details
proposed patch (639 bytes, patch)
2010-07-07 07:46 EDT, Fabio Massimo Di Nitto
no flags Details | Diff

  None (edit)
Description Lon Hohberger 2010-01-07 14:10:00 EST
+++ This bug was initially created as a clone of Bug #533247 +++

Description of problem:

This is a list of issues for the clvmd init script when compared with the criteria in 
https://fedoraproject.org/wiki/Packaging/SysVInitScript

Version-Release number of selected component (if applicable):
lvm2-cluster-2.02.53-2.fc12.i686


LSB Header is missing Description: section

Missing required action force-reload

Missing required action try-restart and condrestart

`service clvmd` should have exit status 2, currently 1.

Exit status for an invalid action should be 2, currently 1.


Questions:
 Should clvmd start up if the locking method is set to "1" for local locking?

--- Additional comment from nstraz@redhat.com on 2009-11-11 17:41:42 EDT ---

Should the clvmd init script require cman is already started?  If so, it should be included in the LSB header.

--- Additional comment from fedora-triage-list@redhat.com on 2009-11-16 10:09:43 EDT ---


This bug appears to have been reported against 'rawhide' during the Fedora 12 development cycle.
Changing version to '12'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 1 RHEL Product and Program Management 2010-01-07 14:24:52 EST
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.
Comment 4 Fabio Massimo Di Nitto 2010-02-26 08:10:46 EST
Created attachment 396535 [details]
New clvmd init script

New init script as commited to upstream CVS
Comment 6 Nate Straz 2010-06-08 17:16:35 EDT
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
Comment 10 Fabio Massimo Di Nitto 2010-07-07 07:46:37 EDT
Created attachment 430041 [details]
proposed patch

this patch addresses the last 2 remaining bits to have LSB complaint init script.
Comment 17 Peter Rajnoha 2010-07-28 11:11:21 EDT
The lvm2-2.02.72-2.el6 build should now address the problems mentioned in comment #6.
Comment 19 Nate Straz 2010-08-11 15:43:05 EDT
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 $?
...
Comment 20 Fabio Massimo Di Nitto 2010-08-12 02:18:50 EDT
(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 $?
> ...
Comment 21 Fabio Massimo Di Nitto 2010-08-12 04:31:13 EDT
(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.
Comment 22 Fabio Massimo Di Nitto 2010-08-12 04:51:10 EDT
After further investigation, daemon function does indeed mask return codes from the real daemon.
Comment 23 Fabio Massimo Di Nitto 2010-08-12 07:49:32 EDT
Back to POST, patch has been committed upstream.
Comment 29 Nate Straz 2010-08-12 11:03:03 EDT
(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.
Comment 30 Fabio Massimo Di Nitto 2010-08-12 11:12:17 EDT
(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.
Comment 31 Alasdair Kergon 2010-08-12 13:42:34 EDT
Can we please be clear where this requirement comes from?  E.g. Reference to LSB?  Or is it only a Fedora thing?
Comment 32 Nate Straz 2010-08-12 14:19:00 EDT
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.
Comment 33 Alasdair Kergon 2010-08-12 14:29:48 EDT
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.
Comment 34 Nate Straz 2010-08-12 14:33:22 EDT
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.
Comment 35 Alasdair Kergon 2010-08-12 14:52:44 EDT
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.
Comment 36 Fabio Massimo Di Nitto 2010-08-12 15:03:19 EDT
(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.
Comment 37 Nate Straz 2010-08-12 15:40:55 EDT
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
Comment 38 Fabio Massimo Di Nitto 2010-08-12 16:38:00 EDT
(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.
Comment 40 Alasdair Kergon 2010-11-22 16:26:31 EST
What happened with this one?

- What else is still to be done for 6.1?
- Did anything get into GA despite 'FailedQA' ?
Comment 41 Peter Rajnoha 2010-11-23 08:09:26 EST
(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).
Comment 42 Peter Rajnoha 2010-11-23 08:21:44 EST
(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.
Comment 44 Alasdair Kergon 2010-11-23 08:34:35 EST
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?
Comment 45 Peter Rajnoha 2010-11-23 12:01:24 EST
(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
Comment 47 Alasdair Kergon 2010-11-24 19:18:24 EST
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.)
Comment 48 Milan Broz 2011-01-25 07:21:13 EST
> 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.)
Comment 50 Nate Straz 2011-03-17 10:35:12 EDT
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
:: [   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  ]
Comment 51 errata-xmlrpc 2011-05-19 10:25:49 EDT
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

Note You need to log in before you can comment on or make changes to this bug.