Ansis Atteka <ansisatt...@gmail.com> writes: > 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.
I guess this is because we need cap_net_admin, cap_net_raw, etc., but it's possible to keep this even without using libcap-ng (or even needing programmatic idiom, iirc). For example, it is possible to attach these capabilities to the ovs-vswitchd binary on the filesystem (for filesystems which support) and thereby provide this functionality without needing libcap-ng. Then we retain the ability to run as a (mostly) unprivileged user, and still provide the control and slow-path for the kernel space side. >> > >> > 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... RHEL systems run as non-root user by default. I think it's a valid use-case. I agree, we need to address the issue when building without libcap-ng. Can you check if it is possible to update the RPM build so that the file capabilities are attached to the ovs-vswitchd executable directly? If it works, it would allow us to achieve the same end result in an alternative path. >> > >> > 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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev