[ovs-dev] [PATCH 5/6] redhat: dynamically allocate and reference ovs user

Aaron Conole aconole at redhat.com
Thu Jun 8 20:39:07 UTC 2017


Flavio Leitner <fbl at sysclose.org> writes:

> On Thu, Jun 08, 2017 at 03:46:24PM -0400, Aaron Conole wrote:
>> Flavio Leitner <fbl at sysclose.org> writes:
>> 
>> > On Sat, Jun 03, 2017 at 11:10:00AM -0400, 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.
>> >> 
>> >> To do this in a way that doesn't impact existing installations, the
>> >> /etc/openvswitch directory will be created during the installation,
>> >> rather than being provided as part of the rpm.
>> >
>> > In the previous patch you add the user configuration to the sysconfig
>> > file and here it adds the same info again to another file which the
>> > user might change, getting us back to state today but now with 2
>> > files.
>> 
>> Sortof.  The idea is the openvswitch-pre file is like the default.conf
>> you detail below.  I could remove the comment from the openvswitch
>> config file, if you think it's causing confusion.  More follows.
>
> /etc/sysconfig is meant to be changed by the user and the name
> implies that it is executed before something, which something?

Agreed.  Poor choice on my part.

>> > Perhaps we could adopt another approach that we have a default
>> > recommended configuration and then a file where the user can
>> > customize it?
>> >
>> > In this case we would create /etc/openvswitch/default.conf.
>> >
>> > If the user wants to change something, it replaces the variable in
>> > /etc/sysconfig/openvswitch as it works today.  Since default.conf is
>> > owned by the system, we can assume it's not edited by the user.
>> 
>> That's the assumption on openvswitch-pre, but I might have gotten the
>> %files section wrong for it (meaning, I assume a user will not edit
>> it).  I can call it /etc/openvswitch/default.conf if you want; that's
>> probably better, actually - so v2 I will do that, regardless what form
>> it takes.
>
> OK, I assume that since it's /etc/sysconfig, the user could change.
>
>> > Then we ship /etc/openvswitch/default.conf with
>> > OVS_USER_ID="openvswitch:openvswitch" by default, so new installations
>> > will have the file state correct in the rpmdb.
>> 
>> Is that not the case for the file I added?  I thought it was okay, but I
>> might misunderstand the way the file flags work.
>
> Sort of, because we have the config in three places now:
> One could edit the systemd services to change the line
> Environment="OVS_USER_ID=root:root", or the /etc/sysconfig/openvswitch
> or /etc/sysconfig/openvswitch-pre
>
>> > The %post appends to the end of /etc/sysconfig/openvswitch the
>> > variable replacing the default user id to root.
>> 
>> Here are the issues I can think of:
>> 
>>  - The rpmdb now thinks that the openvswitch file has been modified by
>>    the user (so the %config part will be flagged even though the user
>>    did nothing).
>
> Yup, then on the future upgrades, the config file should not be replaced
> and that would happen with /etc/sysconfig/openvswitch-pre, right?

Well, upgrades wouldn't write to openvswitch-pre; that only happens on
install.

>>  - The %post on upgrade will have to detect if the user has an
>>    OVS_USER_ID specified, and ignore it appropriately
>
> Right.
>
>>  - The file permissions have to change again (ie: we change them to in
>>    the %files section to be owned openvswitch:openvswitch, then have to
>>    change them in the %post for upgrade to be the correct
>>    user/group... but only if we know we are editing the ovs_user_id, I
>>    think).
>
> You mean /etc/openvswitch or another files?

Yes, /etc/openvswitch/ and all the files under it.  We have this problem
anyway, I guess.  The pains of transitioning :)

>> I'm willing to try and rework this series to go in that direction, but I
>> prefer doing it this way (setting the uid on new installs in %post)
>> because it's an easy binary decision.  Is it new?  Okay, there's no way
>> we can break something on the user, so change things around.  On the
>> other hand, working from the opposite, we have to detect properly that
>> the user's system won't be broken by the change.  Maybe there's a good
>> enough regular expression / other test we can design.  I'll take any
>> suggestion :)
>
> Wouldn't the rpm change the /etc/openvswitch permissions back to
> default when the user upgrades from a new installation?

I didn't notice this; I can look for it specifically to confirm.  Maybe
I missed it in my testing.

>> > Then on new installations we have /etc/openvswitch/default.conf with
>> > the recommended system options, nothing on /etc/sysconfig/openvswitch,
>> > no need to add root userid to the services.
>> >
>> > On upgrades, there will be the default.conf recommending to run as
>> > user, and /etc/sysconfig/openvswitch changing to root which the
>> > admin can comment out to move on, and system services are ok.
>> >
>> > What do you think? I am sure I missed something.
>> 
>> In one sense, it's probably more of a six-of-one and
>> half-dozen-of-the-other thing.  Whichever we choose, the requirements,
>> in my mind, are simple:
>> 
>> 1. Don't break existing users who are upgrading.
>> 2. Provide new installs with non-root users, because it's a good
>>    security practice.
>> 
>> Did I make sense?
>
> We share the same requirements.

Awesome!  Okay, I'll respin with your suggestions folded in.

In the meantime, I will resubmit patches 2/6 and 3/6 as a separate
series since they are independent of this work.

Thanks, Flavio!

> fbl
>
>
>> 
>> > fbl
>> >
>> >
>> >> 
>> >> Signed-off-by: Aaron Conole <aconole at redhat.com>
>> >> ---
>> >>  rhel/openvswitch-fedora.spec.in                  | 15 ++++++++++++++-
>> >>  rhel/usr_lib_systemd_system_ovs-vswitchd.service |  1 +
>> >>  rhel/usr_lib_systemd_system_ovsdb-server.service |  2 ++
>> >>  3 files changed, 17 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>> >> index fe6f15f..f4da735 100644
>> >> --- a/rhel/openvswitch-fedora.spec.in
>> >> +++ b/rhel/openvswitch-fedora.spec.in
>> >> @@ -92,6 +92,8 @@ 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): systemd-units
>> >>  Requires(preun): systemd-units
>> >>  Requires(postun): systemd-units
>> >> @@ -354,6 +356,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
>> >> +    echo "OVS_USER_ID=openvswitch:openvswitch" > \
>> >> +         %{_sysconfdir}/sysconfig/openvswitch-pre
>> >> +
>> >> +    # In the case of upgrade, this is not needed.
>> >> +    install -d -m 0755 -o openvswitch -g openvswitch /etc/openvswitch
>> >> +fi
>> >> +
>> >>  %if 0%{?systemd_post:1}
>> >>      %systemd_post %{name}.service
>> >>  %else
>> >> @@ -480,7 +492,8 @@ fi
>> >>  %defattr(-,root,root)
>> >>  %{_sysconfdir}/bash_completion.d/ovs-appctl-bashcomp.bash
>> >>  %{_sysconfdir}/bash_completion.d/ovs-vsctl-bashcomp.bash
>> >> -%dir %{_sysconfdir}/openvswitch
>> >> +%ghost %{_sysconfdir}/openvswitch
>> >> +%ghost %{_sysconfdir}/sysconfig/openvswitch-pre
>> >>  %config %ghost %{_sysconfdir}/openvswitch/conf.db
>> >>  %ghost %{_sysconfdir}/openvswitch/.conf.db.~lock~
>> >>  %config %ghost %{_sysconfdir}/openvswitch/system-id.conf
>> >> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
>> >> index d63bf4d..0434d20 100644
>> >> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service
>> >> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service
>> >> @@ -11,6 +11,7 @@ PartOf=openvswitch.service
>> >>  Type=forking
>> >>  Restart=on-failure
>> >>  Environment="OVS_USER_ID=root:root"
>> >> +EnvironmentFile=-/etc/sysconfig/openvswitch-pre
>> >>  EnvironmentFile=-/etc/sysconfig/openvswitch
>> >>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>> >>            --no-ovsdb-server --no-monitor --system-id=random \
>> >> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> index 67b50c8..8354087 100644
>> >> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> >> @@ -9,7 +9,9 @@ PartOf=openvswitch.service
>> >>  Type=forking
>> >>  Restart=on-failure
>> >>  Environment="OVS_USER_ID=root:root"
>> >> +EnvironmentFile=-/etc/sysconfig/openvswitch-pre
>> >>  EnvironmentFile=-/etc/sysconfig/openvswitch
>> >> +ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
>> >>  ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>> >>            --no-ovs-vswitchd --no-monitor --system-id=random \
>> >>            --ovs-user=${OVS_USER_ID} \
>> >> -- 
>> >> 2.9.4
>> >> 
>> >> _______________________________________________
>> >> dev mailing list
>> >> dev at openvswitch.org
>> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list