On Tue, 16 Apr 2019 at 12:36, Ansis Atteka <ansisatt...@gmail.com> wrote: > > On Tue, 16 Apr 2019 at 10:46, Aaron Conole <acon...@redhat.com> wrote: > > > > Ansis Atteka <aatt...@ovn.org> writes: > > > > > Otherwise, Open vSwitch will fail to start with the following > > > error "libcap-ng is not configured at compile time" when it > > > attempts to downgrade to Open vSwitch user. > > > > > > Also, if packages were built in a way where processes are > > > supposed to be running only as root, then there is no point > > > in creating "openvswitch" user in the first place. > > > > > > Signed-off-by: Ansis Atteka <aatt...@ovn.org> > > > --- > > > > It seems strange to not provide a user-downgrade option just because we > > cannot drop capabilities via libcap-ng. > > I did not look too close in the daemon-unix.c, but I believe in > current design "linux capabilities" and "linux user downgrade" go hand > in hand (i.e. you either do both or neither). At least for Linux > Kernel datapath. Not sure about other datapaths. > > > > > Maybe it's possible instead to change daemon-unix.c to provide an > > alternative fallback when building on linux without libcapng? Something > > like the untested code here. I have no idea if all of the capabilities > > will get dropped when the user/group ids are changed, but we might be > > able to deal with that as well. WDYT? > > I can take a look at that and if there is a valid use-case I can send > another patch addressing that specific issue. However, I think we can > at this time proceed with this patch, because > 1. it fixes fatal bug when OVS was built without libcap-ng support; AND > 2. it does break anything when OVS was built with libcap-ng support.
meant to say "it does NOT break anything" > > Let me know what you think and if I am missing something? Is your main > concern that if there turns out to be a valid use case to not have > libcap-ng support but still user downgrade feature, then portions of > this patch will need to be reverted? If so, I am fine with that... > > > > diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c > > index 6169763c2..cd2f66295 100644 > > --- a/lib/daemon-unix.c > > +++ b/lib/daemon-unix.c > > @@ -859,9 +859,10 @@ daemon_become_new_user__(bool access_datapath) > > if (LIBCAPNG) { > > daemon_become_new_user_linux(access_datapath); > > } else { > > - VLOG_FATAL("%s: fail to downgrade user using libcap-ng. " > > - "(libcap-ng is not configured at compile time), " > > - "aborting.", pidfile); > > + VLOG_INFO("%s: fail to downgrade user using libcap-ng. " > > + "(libcap-ng is not configured at compile time).", > > + pidfile); > > + daemon_become_new_user_unix(); > > } > > } else { > > daemon_become_new_user_unix(); > > > > > > > poc/playbook-fedora-builder.yml | 6 +++--- > > > rhel/openvswitch-fedora.spec.in | 8 ++++++++ > > > 2 files changed, 11 insertions(+), 3 deletions(-) > > > > > > diff --git a/poc/playbook-fedora-builder.yml > > > b/poc/playbook-fedora-builder.yml > > > index 70f0b6ff2..b955714fc 100644 > > > --- a/poc/playbook-fedora-builder.yml > > > +++ b/poc/playbook-fedora-builder.yml > > > @@ -99,17 +99,17 @@ > > > - openvswitch-dkms.spec > > > > > > - name: Build Open vSwitch user space rpms > > > - command: rpmbuild -bb --without check rhel/openvswitch-fedora.spec > > > + command: rpmbuild -bb --without check --without libcapng > > > rhel/openvswitch-fedora.spec > > > args: > > > chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}" > > > > > > - name: Build Open vSwitch kmod rpm > > > - command: rpmbuild -bb --without check rhel/openvswitch-fedora.spec > > > + command: rpmbuild -bb --without check --without libcapng > > > rhel/openvswitch-fedora.spec > > > args: > > > chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}" > > > > > > - name: Build Open vSwitch dkms rpm > > > - command: rpmbuild -bb --without check rhel/openvswitch-dkms.spec > > > + command: rpmbuild -bb --without check --without libcapng > > > rhel/openvswitch-dkms.spec > > > args: > > > chdir: "{{SOURCE}}/openvswitch-{{version.stdout}}" > > > > > > diff --git a/rhel/openvswitch-fedora.spec.in > > > b/rhel/openvswitch-fedora.spec.in > > > index c1cd3f4c6..ce728b4f0 100644 > > > --- a/rhel/openvswitch-fedora.spec.in > > > +++ b/rhel/openvswitch-fedora.spec.in > > > @@ -350,6 +350,7 @@ rm -rf $RPM_BUILD_ROOT > > > %endif > > > > > > %pre > > > +%if %{with libcapng} > > > getent group openvswitch >/dev/null || groupadd -r openvswitch > > > getent passwd openvswitch >/dev/null || \ > > > useradd -r -g openvswitch -d / -s /sbin/nologin \ > > > @@ -359,9 +360,11 @@ getent passwd openvswitch >/dev/null || \ > > > getent group hugetlbfs >/dev/null || groupadd -r hugetlbfs > > > usermod -a -G hugetlbfs openvswitch > > > %endif > > > +%endif > > > exit 0 > > > > > > %post > > > +%if %{with libcapng} > > > if [ $1 -eq 1 ]; then > > > sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch > > > sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' > > > %{_sysconfdir}/logrotate.d/openvswitch > > > @@ -376,6 +379,7 @@ if [ $1 -eq 1 ]; then > > > chown -R openvswitch:openvswitch /etc/openvswitch > > > chown -R openvswitch:openvswitch /var/log/openvswitch > > > fi > > > +%endif > > > > > > %if 0%{?systemd_post:1} > > > %systemd_post %{name}.service > > > @@ -445,7 +449,11 @@ fi > > > %endif > > > > > > %files > > > +%if %{with libcapng} > > > %defattr(-,openvswitch,openvswitch) > > > +%else > > > +%defattr(-,root,root) > > > +%endif > > > %dir %{_sysconfdir}/openvswitch > > > %{_sysconfdir}/openvswitch/default.conf > > > %config %ghost %{_sysconfdir}/openvswitch/conf.db _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev