Re: [Xen-devel] [PATCH 7/7] tools/hotplug: add wrapper to start xenstored

[George Dunlap <dunlapg@xxxxxxxxx>]

On Thu, Sep 10, 2015 at 4:01 PM, M A Young <m.a.young@xxxxxxxxxxxx> wrote:
> On Thu, 10 Sep 2015, Wei Liu wrote:
>
>> On Thu, Sep 10, 2015 at 03:19:35PM +0100, George Dunlap wrote:
>> > On Wed, Jan 7, 2015 at 9:40 AM, Olaf Hering <olaf@xxxxxxxxx> wrote:
>> > > On Tue, Jan 06, Ian Campbell wrote:
>> > >
>> > >> On Fri, 2014-12-19 at 12:25 +0100, Olaf Hering wrote:
>> > >> > The shell wrapper in xenstored.service does not handle XENSTORE_TRACE.
>> > >> >
>> > >> > Create a separate wrapper script which is used in the sysv runlevel
>> > >> > script and in systemd xenstored.service. It preserves existing
>> > >> > behaviour by handling the XENSTORE_TRACE boolean. It also implements
>> > >> > the handling of XENSTORED_ARGS=. This variable has to be added to
>> > >> > sysconfig/xencommons.
>> > >>
>> > >> Why don't we just drop XENSTORED_* in favour of XENSTORED_ARGS (with an
>> > >> example in the sysconfig file of enabling tracing if you like)?
>> > >
>> > > After having two weeks to think about this I came to the same
>> > > conclusion. I think whatever the outcome is, the boolean should be
>> > > removed. The sysconfig file should get a XENSTORED_ARGS="" along with a
>> > > help text which mentions "-T /path" and "xenstored --help" to get other
>> > > options because there is no man page.
>> > >
>> > >> Going to a wrapper script just to make some fairly uncommon debugging
>> > >> option marginally more convenient seems like overkill to me, plus
>> > >> XENSTORED_ARGS would allow for passing other useful options to
>> > >> xenstored.
>> > >
>> > > If I recall correctly the point of the current 'sh -c "exec ..."' stunt
>> > > was to expand the XENSTORE variable from the sysconfig file. But this
>> > > approach leads to failures with SELinux because the socket passing does
>> > > not work this way. Up to now I have not seen a success report for
>> > > selinux+systemd+xenstored. Maybe its already somewhere in the other
>> > > unread mails.
>> > >
>> > > In my cover letter I provided some possible ways to handle
>> > > selinux+systemd+xenstored. Ideally the approach "Exec=/usr/bin/env
>> > > $XENSTORED --no-fork $XENSTORED_ARGS" works because it means its
>> > > possible to select a binary via the sysconfig file. But it also means
>> > > the XENSTORE_TRACE boolean has to be removed in favour of the plain
>> > > XENSTORED_ARGS= approach mentioned above. Finally this would avoid the
>> > > need for a wrapper script.
>> > >
>> > > Hopefully someone with access to a SELinux enabled system will report
>> > > which approach actually works.
>> >
>> > I can confirm that
>> >
>> > ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS"
>> >
>> > does not work on a CentOS7 system with selinux enabled, but that
>> >
>> > ExecStart=/usr/bin/env $XENSTORED --no-fork $XENSTORED_ARGS
>> >
>>
>> And the difference between these two approaches is /usr/bin/env
>> preserves envars while sh -c doesn't?
>
> /bin/sh doesn't give the right selinux permissions so xenstored doesn't
> start. Presumably /usr/bin/env does.

Or to be more precise, /bin/sh exec from that context doesn't allow
xenstored to make the accept() system call, so you get a log full of
these:

type=PROCTITLE msg=audit(1441892166.173:22242):
proctitle=2F7573722F7362696E2F78656E73746F726564002D2D6E6F2D666F726B
type=AVC msg=audit(1441892166.173:22243): avc:  denied  { accept } for
 pid=615 comm="xenstored" path="/run/xenstored/socket"
scontext=system_u:system_r:xenstored_t:s0
tcontext=system_u:system_r:initrc_t:s0 tclass=unix_stream_socket
permissive=0
type=SYSCALL msg=audit(1441892166.173:22243): arch=c000003e syscall=43
success=no exit=-13 a0=3 a1=0 a2=0 a3=0 items=0 ppid=1 pid=615
auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
fsgid=0 tty=(none) ses=4294967295 comm="xenstored"
exe="/usr/sbin/xenstored" subj=system_u:system_r:xenstored_t:s0
key=(null)

At least, I assume the denied {accept} means the accept system call...
I haven't verified that syscall 43 is in fact accept.

I assume that "env" since was designed to execute other programs,
selinux on CentOS 7 has been programmed to retain the context.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel