Hi Markos,

Markos Chandras <[email protected]> writes:

> Hi Aaron,
>
> On 07/05/2017 08:56 PM, Aaron Conole wrote:
>> After this commit, the fedora RPM will create the openvswitch user, from the
>> non-static pool, for use as an Open vSwitch daemon user.  This only happens
>> on install - not upgrade.  This will be the default user:group
>> combination for the openvswitch daemons.
>> 
>> Signed-off-by: Aaron Conole <[email protected]>
>> ---

Thanks for the review!

>>  rhel/openvswitch-fedora.spec.in                  | 13 +++++++++++++
>>  rhel/usr_lib_systemd_system_ovsdb-server.service |  1 +
>>  2 files changed, 14 insertions(+)
>> 
>> diff --git a/rhel/openvswitch-fedora.spec.in 
>> b/rhel/openvswitch-fedora.spec.in
>> index 88d4331..7c805b2 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -92,6 +92,9 @@ Requires: openssl hostname iproute module-init-tools
>>  #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
>>  #Requires: kernel >= 3.15.0-0
>>  
>> +Requires(post): /usr/bin/getent
>> +Requires(post): /usr/sbin/useradd
>> +Requires(post): /usr/bin/sed
>>  Requires(post): systemd-units
>>  Requires(preun): systemd-units
>>  Requires(postun): systemd-units
>> @@ -357,6 +360,16 @@ rm -rf $RPM_BUILD_ROOT
>>  %endif
>>  
>>  %post
>> +if [ $1 -eq 1 ]; then
>> +    getent passwd openvswitch >/dev/null || \
>> +        useradd -r -d / -s /sbin/nologin -c "Open vSwitch Daemons" 
>> openvswitch
>> +
>> +    sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
>
> I am a bit puzzled about this to be honest... I am wondering if it would
> be better to do it the other way around. For example, supply a sysconfig
> file with OVS_USER_ID commented out, but if it's an upgrade, then do the
> sed magic to switch to root:root so things keep working as before. Would
> that be better?

I prefer to do modifications only on install; that's the only time we
know for sure the exact state of the files being manipulated.  On
upgrade, I worry that a user can change the contents in such a way that
the script matches, but does the wrong thing.  Does it make sense, or
did I misunderstand?

>> +
>> +    # In the case of upgrade, this is not needed.
>> +    chown -R openvswitch:openvswitch /etc/openvswitch
>
> Should this be part of the systemd file in a ExecStartPre statement
> instead? Similar to what you do for the /var/run/openvswitch directory.

I thought about doing that in the systemd script, but it exposes a
vulnerability.  Assume that I have access to the openvswitch user (for a
moment).

  openvswitch /tmp$ gcc -o give_me_a_shell give_me_a_shell.c
  openvswitch /tmp$ chmod 4755 give_me_a_shell
  openvswitch /tmp$ cp give_me_a_shell /etc/openvswitch/
  openvswitch /tmp$ echo 'OVS_USER_ID=root:root' >> \
     /etc/sysconfig/openvswitch
  openvswitch /tmp$ # do something that makes the admin restart ovs
  openvswitch /tmp$ ls -lah /etc/openvswitch/give_me_a_shell
  srwxr-xr-x.   1 root root 5.5K Jul  5 13:42 give_me_a_shell
  openvswitch /tmp$ /etc/openvswitch/give_me_a_shell
  # id
  uid=0(root) gid=0(root) groups=0(root)

So, I left it out.  The alternative is to list every file we wish to
have chmod'ed, but I think that's probably a bit much.

-Aaron
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to