On 21 December 2015 at 15:46, Ansis Atteka <ansisatt...@gmail.com> wrote:
>
>
> On 27 November 2015 at 05:35, Flavio Leitner <f...@sysclose.org> wrote:
>
>> On Fri, Nov 20, 2015 at 03:33:20AM -0800, Andy Zhou wrote:
>> > Make RHEL systemd distributions start OVS and OVN daemons under user
>> > ovs. The 'ovs' user and group will be created at the openvswitch RPM
>> > installtion time.
>> >
>>
> Hi Flavio. I have taken over the development of this patch from Andy. See
> my comments inline.
>
> Also, while reviewing and trying to fix issues you pointed out in this
> patch I have thought about another possible implementation. Basically, what
> you say that user-switch would happen at run-time with something like:
>
> # ovs-vsctl migrate-to-user <user>
>
Sorry, I meant ovs-ctl or maybe even some other utility that is not subject
to current SElinux policy that ships on Fedora.
>
> This might have few benefits:
> 1. there wouldn't anymore be scattered and hardcoded chmod, chown
> invocations from the packaging hooks and init scripts. I am afraid that if
> chowns are invoked from various places it might become hard to maintain
> project.
> 2. instead of hardcoding --user in each ovs/ovn init scripts the user
> could be derived from the /etc/default/openvswitch file. We don't seem to
> use this file on RHEL, but we do have it on Debian.
> 3. RHEL and Fedora are distributions that on upgrade require admin to
> explicitly stop and start Openv vSwitch service. So it seems that natural
> place where this user-switch should happen is between service openvswitch
> stop and service openvswitch start commands that admin has to invoke.
> Otherwise, If we don't follow this approach then there could be corner case
> where ovs running as root reverts back some changes made by packaging hooks.
> 4. Admin could also specify arbitrary user if he does not want to use
> "openvswitch".
>
> However, the downsides of this approach are:
> 1. that Fedora spec file would have to become user agnostic (ie %files
> section should have "-" everywhere and rpm -V would not be that useful for
> audits).
> 2. since admin would have to invoke "ovs-ctl migrate-to-user <user>"
> before "service openvswitch start", then there could be two openvswitch
> environments out there - one where ovs runs under "root" and one where ovs
> runs under "openvswitch" user. Though we could always automate this from
> the service startup scripts.
>
> I am still thinking about this approach, but I was curious if you see a
> reason why this would or would not work?
>
>
>
>> > Signed-off-by: Andy Zhou <az...@ovn.org>
>> > Acked-by: Ben Pfaff <b...@ovn.org>
>> > ---
>> > rhel/openvswitch-fedora.spec.in | 18
>> ++++++++----------
>> > ...sr_lib_systemd_system_openvswitch-nonetwork.service | 4 ++--
>> > .../usr_lib_systemd_system_ovn-controller-vtep.service | 2 +-
>> > rhel/usr_lib_systemd_system_ovn-controller.service | 2 +-
>> > rhel/usr_lib_systemd_system_ovn-northd.service | 2 +-
>> > 5 files changed, 13 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
>> openvswitch-fedora.spec.in
>> > index be22e87..b91f1b3 100644
>> > --- a/rhel/openvswitch-fedora.spec.in
>> > +++ b/rhel/openvswitch-fedora.spec.in
>> > @@ -13,10 +13,6 @@
>> >
>> > #%define kernel 2.6.40.4-5.fc15.x86_64
>> >
>> > -# If libcap-ng isn't available and there is no need for running OVS
>> > -# as regular user, specify the '--without libcapng'
>> > -%bcond_without libcapng
>> > -
>>
>> People building small build roots with openvswitch could use that option,
>> but I don't know for sure if anyone is actually doing that. OK, let's
>> remove that and if anyone complains we can easily bring it back.
>>
>>
>> > # Enable PIE, bz#955181
>> > %global _hardened_build 1
>> >
>> > @@ -46,9 +42,7 @@ BuildRequires: desktop-file-utils
>> > BuildRequires: groff graphviz
>> > # make check dependencies
>> > BuildRequires: procps-ng
>> > -%if %{with libcapng}
>> > BuildRequires: libcap-ng libcap-ng-devel
>> > -%endif
>> >
>> > Requires: openssl iproute module-init-tools
>> > #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
>> > @@ -112,11 +106,7 @@ overlays and security groups.
>> >
>> > %build
>> > %configure \
>> > -%if %{with libcapng}
>> > --enable-libcapng \
>> > -%else
>> > - --disable-libcapng \
>> > -%endif
>> > --enable-ssl \
>> > --with-pkidir=%{_sharedstatedir}/openvswitch/pki
>> >
>> > @@ -162,6 +152,11 @@ install -d -m 0755
>> $RPM_BUILD_ROOT/%{_sharedstatedir}/openvswitch
>> > touch $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch/conf.db
>> > touch $RPM_BUILD_ROOT%{_sysconfdir}/openvswitch/system-id.conf
>> >
>> > +%pre
>> > +# Add the "ovs" user and group
>> > +/usr/sbin/useradd -c "Openvswitch Daemons" -s /sbin/nologin -r \
>> > + -d %{_rundir}/openvswitch ovs 2> /dev/null || :
>> > +
>>
>> I suggest to rename the user and group to 'openvswitch'.
>>
>
> Ok. I renamed user and group to "openvswitch". Also for Debian.
>
>
>>
>> Redirecting all errors to /dev/null can hide tricky bugs during
>> installations. I suggest to do something like:
>>
>> if ! getent passwd openvswitch >/dev/null; then
>> useradd -c "Openvswitch Daemons" -s /sbin/nologin -r \
>> -d %{_rundir}/openvswitch openvswitch
>> fi
>> exit 0
>>
> Ok
>
>>
>>
>>
>> > %check
>> > %if %{with check}
>> > if make check TESTSUITEFLAGS='%{_smp_mflags}' ||
>> > @@ -204,6 +199,8 @@ rm -rf $RPM_BUILD_ROOT
>> > %endif
>> >
>> > %post
>> > +chown -R ovs:ovs /etc/openvswitch #OVS DB files
>> > +chown -R ovs:ovs %{_rundir}/openvswitch
>>
>> This breaks rpm -V. You need to change file permissions in the
>> %file sections otherwise the filesystem and rpmdb won't match.
>>
>
> I think this "chown <rundir> " invocation should be moved out from
> packaging hooks because it might not exist in the first place on fresh
> installs (another option would be to suppress errors).
>
>
>> Also, %{_rundir}/openvswitch is marked as %ghost which means that
>> directory isn't packaged. It is created by systemd when the service
>> is initializing (RuntimeDirectory). Here we have a problem because
>> systemd will set rundir ownership to User= and Group= specified in
>> the service (which we don't specify, so root:root is assumed) and we
>> can't package the directory because /run is a tmpfs.
>>
>> Since you fix the %{_rundir}/openvswitch in the script ovs-lib, it seems
>> enough to just patch the line below:
>>
>> - %ghost %attr(755,root,root) %{_rundir}/openvswitch
>> + %ghost %attr(755,openvswitch,openvswitch) %{_rundir}/openvswitch
>
>
> To me it seems that rpm -V does not complain about %ghost directories.
>
> [root@localhost x86_64]# chown root:root /var/run/openvswitch/
> [root@localhost x86_64]# chown root:root /run/openvswitch/
> [root@localhost x86_64]# chown root:root /etc/openvswitch/
> [root@localhost x86_64]# rpm -V openvswitch-2.5.90-1.fc23.x86_64
> .....UG.. /etc/openvswitch
> [root@localhost x86_64]# chown openvswitch:openvswitch /etc/openvswitch/
> [root@localhost x86_64]# chown openvswitch:openvswitch /run/openvswitch
> [root@localhost x86_64]# chown openvswitch:openvswitch
> /var/run/openvswitch
> [root@localhost x86_64]# rpm -V openvswitch-2.5.90-1.fc23.x86_64
> [root@localhost x86_64]#
>
>
> Just curious if I am doing something wrong or if this is an issue with
> "rpm"?
>
>
>> See:
>> http://www.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html
>>
>>
>> > %if 0%{?systemd_post:1}
>> > %systemd_post %{name}.service
>> > %else
>> > @@ -214,6 +211,7 @@ rm -rf $RPM_BUILD_ROOT
>> > %endif
>> >
>> > %post ovn
>> > +chown -R ovs:ovs /var/lib/openvswitch #OVN DB files
>>
>> breaks rpmdb too.
>>
> True, fixed it.
>
>
>>
>>
>> > %if 0%{?systemd_post:1}
>> > %systemd_post ovn-controller.service
>> > %systemd_post ovn-controller-vtep.service
>> > diff --git a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> > index e4c2a66..f32ba24 100644
>> > --- a/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> > +++ b/rhel/usr_lib_systemd_system_openvswitch-nonetwork.service
>> > @@ -9,7 +9,7 @@ Type=oneshot
>> > RemainAfterExit=yes
>> > EnvironmentFile=-/etc/sysconfig/openvswitch
>> > ExecStart=/usr/share/openvswitch/scripts/ovs-ctl start \
>> > - --system-id=random $OPTIONS
>> > + --system-id=random --user=ovs:ovs $OPTIONS
>> > ExecStop=/usr/share/openvswitch/scripts/ovs-ctl stop
>> > RuntimeDirectory=openvswitch
>> > -RuntimeDirectoryMode=0755
>> > +RuntimeDirectoryMode=0775
>>
>> You need to sync this with the %attr above and the
>> ovs-lib (proposed 755)
>>
>
> ok
>
>>
>> Thanks,
>> fbl
>>
>> > diff --git a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
>> b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
>> > index 867a906..994bd77 100644
>> > --- a/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
>> > +++ b/rhel/usr_lib_systemd_system_ovn-controller-vtep.service
>> > @@ -27,4 +27,4 @@ Environment=VTEP_DB=unix:%t/openvswitch/db.sock
>> > ExecStart=/usr/bin/ovn-controller-vtep -vconsole:emer -vsyslog:err
>> -vfile:info \
>> > --log-file=/var/log/openvswitch/ovn-controller-vtep.log \
>> > --no-chdir --pidfile=${OVS_RUNDIR}/ovn-controller-vtep.pid \
>> > - --ovnsb-db=${OVN_DB} --vtep-db=${VTEP_DB}
>> > + --user ovs:ovs --ovnsb-db=${OVN_DB} --vtep-db=${VTEP_DB}
>> > diff --git a/rhel/usr_lib_systemd_system_ovn-controller.service
>> b/rhel/usr_lib_systemd_system_ovn-controller.service
>> > index 6b53ced..b01a804 100644
>> > --- a/rhel/usr_lib_systemd_system_ovn-controller.service
>> > +++ b/rhel/usr_lib_systemd_system_ovn-controller.service
>> > @@ -18,5 +18,5 @@ Type=simple
>> > Environment=OVS_RUNDIR=%t/openvswitch
>> > Environment=OVS_DB=unix:%t/openvswitch/db.sock
>> > ExecStart=/usr/bin/ovn-controller -vconsole:emer -vsyslog:err
>> -vfile:info \
>> > - --log-file=/var/log/openvswitch/ovn-controller.log \
>> > + --log-file=/var/log/openvswitch/ovn-controller.log --user
>> ovs:ovs \
>> > --no-chdir --pidfile=${OVS_RUNDIR}/ovn-controller.pid
>> ${OVS_DB}
>> > diff --git a/rhel/usr_lib_systemd_system_ovn-northd.service
>> b/rhel/usr_lib_systemd_system_ovn-northd.service
>> > index 5b3b03a..1abb8b3 100644
>> > --- a/rhel/usr_lib_systemd_system_ovn-northd.service
>> > +++ b/rhel/usr_lib_systemd_system_ovn-northd.service
>> > @@ -8,5 +8,5 @@ After=openvswitch.service
>> > Type=oneshot
>> > RemainAfterExit=yes
>> > Environment=OVS_RUNDIR=%t/openvswitch OVS_DBDIR=/var/lib/openvswitch
>> > -ExecStart=/usr/share/openvswitch/scripts/ovn-ctl start_northd
>> > +ExecStart=/usr/share/openvswitch/scripts/ovn-ctl --user=ovs:ovs
>> start_northd
>> > ExecStop=/usr/share/openvswitch/scripts/ovn-ctl stop_northd
>> > --
>> > 1.8.3.1
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
