| Message ID | 20170603151001.19716-6-aconole@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
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. 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. 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. The %post appends to the end of /etc/sysconfig/openvswitch the variable replacing the default user id to root. 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. fbl > > Signed-off-by: Aaron Conole <aconole@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Flavio Leitner <fbl@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. > 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. > 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. > 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). - The %post on upgrade will have to detect if the user has an OVS_USER_ID specified, and ignore it appropriately - 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). 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 :) > 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? > fbl > > >> >> Signed-off-by: Aaron Conole <aconole@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@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Jun 08, 2017 at 03:46:24PM -0400, Aaron Conole wrote: > Flavio Leitner <fbl@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? > > 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? > - 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? > 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? > > 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. fbl > > > fbl > > > > > >> > >> Signed-off-by: Aaron Conole <aconole@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@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Flavio Leitner <fbl@sysclose.org> writes: > On Thu, Jun 08, 2017 at 03:46:24PM -0400, Aaron Conole wrote: >> Flavio Leitner <fbl@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@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@openvswitch.org >> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Jun 08, 2017 at 04:39:07PM -0400, Aaron Conole wrote: > >> 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. Well, I haven't put much thought on it so feel free to push back. > In the meantime, I will resubmit patches 2/6 and 3/6 as a separate > series since they are independent of this work. Sounds good! Thanks Aaron,
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} \
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. Signed-off-by: Aaron Conole <aconole@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(-)