[ovs-dev] [PATCH] rhel: if rpms were built without libcapng then let processrs to run as root

Aaron Conole aconole at redhat.com
Wed Apr 17 15:36:54 UTC 2019


Ansis Atteka <ansisatteka at gmail.com> writes:

> On Tue, 16 Apr 2019 at 12:36, Ansis Atteka <ansisatteka at gmail.com> wrote:
>>
>> On Tue, 16 Apr 2019 at 10:46, Aaron Conole <aconole at redhat.com> wrote:
>> >
>> > Ansis Atteka <aatteka at 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 <aatteka at 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list