Flavio Leitner <[email protected]> writes: > On Thu, Jun 08, 2017 at 03:46:24PM -0400, Aaron Conole wrote: >> Flavio Leitner <[email protected]> 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 <[email protected]> >> >> --- >> >> 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 >> >> [email protected] >> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
