[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