ovn-ctl and ovn-lib currently chown the OVN log directory recursively when services start. That can undo ownership set by distribution tooling such as tmpfiles.d and logrotate. In particular, Debian and Ubuntu need ovn-controller.log to remain root:adm so rsyslog can read it, but a later OVN service restart may change it back to the OVN daemon user/group.
Add OVN tmpfiles.d and sysusers.d templates for the log directory, ovn-controller.log and the configured log owner/group. The log user, log group, tmpfiles.d installation directory and sysusers.d installation directory are configurable so downstream packages can choose their local policy. Debian configures the log owner as root:adm and installs the generated sysusers.d file for the adm group. Fedora/RHEL keeps openvswitch:openvswitch for log ownership, but leaves the openvswitch user to the Open vSwitch package's sysusers.d configuration. Stop recursively changing the OVN log directory ownership from ovn-ctl and ovn-lib at service startup. Runtime, database and configuration paths are still chowned according to --ovn-user, preserving the existing privilege-drop behavior for non-log state. Reported-at: https://github.com/ovn-org/ovn/issues/310 Signed-off-by: Zhang Hua <[email protected]> --- Submitted-at: https://github.com/ovn-org/ovn/pull/311 v2: - Added the trailing dot required by checkpatch to the subject. v3: - Added sysusers.d support alongside tmpfiles.d. - Added --with-sysusersdir configure option. - Generate sysusers.d entries from the configured log user/group. - Install the generated sysusers.d file in Debian packaging. - Do not package the generated sysusers.d file for Fedora/RHEL because the openvswitch user is managed by the Open vSwitch package there. Testing: - Ran utilities/checkpatch.py -1. - Ran make check TESTSUITEFLAGS="-j$(nproc)". Makefile.am | 8 ++++++ configure.ac | 4 +++ debian/ovn-common.install | 2 ++ debian/rules | 2 +- m4/ovn.m4 | 50 ++++++++++++++++++++++++++++++++++ rhel/ovn-fedora.spec.in | 7 +++++ utilities/automake.mk | 8 ++++++ utilities/ovn-ctl | 6 ++-- utilities/ovn-lib.in | 1 - utilities/ovn-sysusers.conf.in | 2 ++ utilities/ovn-tmpfiles.conf.in | 2 ++ 11 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 utilities/ovn-sysusers.conf.in create mode 100644 utilities/ovn-tmpfiles.conf.in diff --git a/Makefile.am b/Makefile.am index 0f2389b25..324cdfd8c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -131,6 +131,8 @@ noinst_PROGRAMS = noinst_SCRIPTS = OVSIDL_BUILT = pkgdata_DATA = +tmpfiles_DATA = +sysusers_DATA = sbin_SCRIPTS = scripts_SCRIPTS = completion_SCRIPTS = @@ -148,6 +150,8 @@ endif scriptsdir = $(pkgdatadir)/scripts completiondir = $(sysconfdir)/bash_completion.d pkgconfigdir = $(libdir)/pkgconfig +tmpfilesdir = @TMPFILESDIR@ +sysusersdir = @SYSUSERSDIR@ # This ensures that files added to EXTRA_DIST are always distributed, # even if they are inside an Automake if...endif conditional block that is @@ -166,6 +170,10 @@ SUFFIXES += .in sed \ -e 's,[@]PKIDIR[@],$(PKIDIR),g' \ -e 's,[@]LOGDIR[@],$(LOGDIR),g' \ + -e 's,[@]LOGUSER[@],$(LOGUSER),g' \ + -e 's,[@]LOGGROUP[@],$(LOGGROUP),g' \ + -e 's,[@]SYSUSERS_GROUP_LINE[@],$(SYSUSERS_GROUP_LINE),g' \ + -e 's,[@]SYSUSERS_USER_LINE[@],$(SYSUSERS_USER_LINE),g' \ -e 's,[@]DBDIR[@],$(DBDIR),g' \ -e 's,[@]PYTHON3[@],$(PYTHON3),g' \ -e 's,[@]OVN_RUNDIR[@],$(OVN_RUNDIR),g' \ diff --git a/configure.ac b/configure.ac index cfa4cc386..9de1b4d6f 100644 --- a/configure.ac +++ b/configure.ac @@ -88,6 +88,10 @@ OVS_CHECK_NETLINK OVS_CHECK_LINUX_NETLINK OVS_CHECK_OPENSSL OVN_CHECK_LOGDIR +OVN_CHECK_LOGUSER +OVN_CHECK_LOGGROUP +OVN_CHECK_TMPFILESDIR +OVN_CHECK_SYSUSERSDIR OVN_CHECK_PYTHON3 OVN_CHECK_FLAKE8 OVN_CHECK_SPHINX diff --git a/debian/ovn-common.install b/debian/ovn-common.install index fc48f07e4..7ae59e86a 100644 --- a/debian/ovn-common.install +++ b/debian/ovn-common.install @@ -12,4 +12,6 @@ usr/share/ovn/scripts/ovn-lib usr/share/ovn/scripts/ovn-bugtool-nbctl-show usr/share/ovn/scripts/ovn-bugtool-sbctl-lflow-list usr/share/ovn/scripts/ovn-bugtool-sbctl-show +usr/lib/tmpfiles.d/ovn-tmpfiles.conf +usr/lib/sysusers.d/ovn-sysusers.conf usr/lib/*/libovn*.so.* diff --git a/debian/rules b/debian/rules index b25a0b48e..0d5da5b26 100755 --- a/debian/rules +++ b/debian/rules @@ -30,7 +30,7 @@ override_dh_autoreconf: dh_autoreconf $(DH_AS_NEEDED) override_dh_auto_configure: - dh_auto_configure -- --enable-ssl --enable-shared --with-ovs-source=${OVSDIR} $(EXTRA_CONFIGURE_OPTS) + dh_auto_configure -- --enable-ssl --enable-shared --with-ovs-source=${OVSDIR} --with-log-user=root --with-log-group=adm $(EXTRA_CONFIGURE_OPTS) override_dh_auto_test: ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS))) diff --git a/m4/ovn.m4 b/m4/ovn.m4 index 22ad1a27f..1c7d432d9 100644 --- a/m4/ovn.m4 +++ b/m4/ovn.m4 @@ -127,6 +127,56 @@ AC_DEFUN([OVN_CHECK_LOGDIR], [LOGDIR='${localstatedir}/log/${PACKAGE}']) AC_SUBST([LOGDIR])]) +dnl Checks for the user that should own log files. +AC_DEFUN([OVN_CHECK_LOGUSER], + [AC_ARG_WITH( + [log-user], + AS_HELP_STRING([--with-log-user=USER], + [user used for log files [[root]]]), + [LOGUSER=$withval], + [LOGUSER=root]) + AC_SUBST([LOGUSER])]) + +dnl Checks for the group that should own log files. +AC_DEFUN([OVN_CHECK_LOGGROUP], + [AC_ARG_WITH( + [log-group], + AS_HELP_STRING([--with-log-group=GROUP], + [group used for log files [[root]]]), + [LOGGROUP=$withval], + [LOGGROUP=root]) + AC_SUBST([LOGGROUP])]) + +dnl Checks for the directory in which to install tmpfiles.d configuration. +AC_DEFUN([OVN_CHECK_TMPFILESDIR], + [AC_ARG_WITH( + [tmpfilesdir], + AS_HELP_STRING([--with-tmpfilesdir=DIR], + [directory used for tmpfiles.d configuration + [[PREFIX/lib/tmpfiles.d]]]), + [TMPFILESDIR=$withval], + [TMPFILESDIR='${prefix}/lib/tmpfiles.d']) + AC_SUBST([TMPFILESDIR])]) + +dnl Checks for the directory in which to install sysusers.d configuration. +AC_DEFUN([OVN_CHECK_SYSUSERSDIR], + [AC_ARG_WITH( + [sysusersdir], + AS_HELP_STRING([--with-sysusersdir=DIR], + [directory used for sysusers.d configuration + [[PREFIX/lib/sysusers.d]]]), + [SYSUSERSDIR=$withval], + [SYSUSERSDIR='${prefix}/lib/sysusers.d']) + AS_IF([test "x$LOGGROUP" = xroot], + [SYSUSERS_GROUP_LINE=], + [SYSUSERS_GROUP_LINE="g $LOGGROUP -"]) + AS_IF([test "x$LOGUSER" = xroot], + [SYSUSERS_USER_LINE=], + [SYSUSERS_USER_LINE="u $LOGUSER -:$LOGGROUP \"OVN log user\" -"]) + AC_SUBST([SYSUSERSDIR]) + AC_SUBST([SYSUSERS_GROUP_LINE]) + AC_SUBST([SYSUSERS_USER_LINE])]) + dnl Checks for the directory in which to store the OVN database. AC_DEFUN([OVN_CHECK_DBDIR], [AC_ARG_WITH( diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in index 131b3eaab..0c90966c2 100644 --- a/rhel/ovn-fedora.spec.in +++ b/rhel/ovn-fedora.spec.in @@ -159,6 +159,8 @@ cd - --with-ovs-source=$PWD/openvswitch-%{ovsver} \ %if %{with libcapng} --enable-libcapng \ + --with-log-user=openvswitch \ + --with-log-group=openvswitch \ %else --disable-libcapng \ %endif @@ -185,6 +187,10 @@ done rm -rf $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/ +# The openvswitch user is managed by the Open vSwitch package's +# sysusers.d configuration on Fedora and RHEL. +rm -f $RPM_BUILD_ROOT%{_sysusersdir}/ovn-sysusers.conf + install -d -m 0755 $RPM_BUILD_ROOT/%{_sharedstatedir}/ovn install -d $RPM_BUILD_ROOT%{_prefix}/lib/firewalld/services/ @@ -531,6 +537,7 @@ fi %{_mandir}/man8/ovn-debug.8* %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers %config(noreplace) %{_sysconfdir}/logrotate.d/ovn +%{_tmpfilesdir}/ovn-tmpfiles.conf %{_unitdir}/[email protected] %files docker diff --git a/utilities/automake.mk b/utilities/automake.mk index b620038d0..ceeea7dbe 100644 --- a/utilities/automake.mk +++ b/utilities/automake.mk @@ -26,6 +26,8 @@ EXTRA_DIST += \ utilities/ovn-ctl \ utilities/ovn-lib.in \ utilities/ovn-ctl.8.xml \ + utilities/ovn-tmpfiles.conf.in \ + utilities/ovn-sysusers.conf.in \ utilities/ovn-docker-overlay-driver.in \ utilities/ovn-docker-underlay-driver.in \ utilities/ovn-nbctl.8.xml \ @@ -48,6 +50,8 @@ EXTRA_DIST += \ CLEANFILES += \ utilities/ovn-ctl.8 \ utilities/ovn-lib \ + utilities/ovn-tmpfiles.conf \ + utilities/ovn-sysusers.conf \ utilities/ovn-docker-overlay-driver \ utilities/ovn-docker-underlay-driver \ utilities/ovn-nbctl.8 \ @@ -66,7 +70,11 @@ CLEANFILES += \ EXTRA_DIST += utilities/ovn-sim.in noinst_SCRIPTS += utilities/ovn-sim +tmpfiles_DATA += utilities/ovn-tmpfiles.conf +sysusers_DATA += utilities/ovn-sysusers.conf utilities/ovn-lib: $(top_builddir)/config.status +utilities/ovn-tmpfiles.conf: $(top_builddir)/config.status +utilities/ovn-sysusers.conf: $(top_builddir)/config.status # ovn-nbctl bin_PROGRAMS += utilities/ovn-nbctl diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl index 3b62ca9b7..58effd5e5 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -276,9 +276,8 @@ $cluster_remote_port upgrade_db "$file" "$schema" fi - # Set the owner of the ovn_dbdir (with -R option) to OVN_USER if set. - # This is required because the ovndbs are created with root permission - # if not present when create_cluster/upgrade_db is called. + # Database files may be created as root before ovsdb-server drops + # privileges, so keep ownership aligned with OVN_USER when configured. INSTALL_USER="$(id -un)" INSTALL_GROUP="$(id -gn)" [ "$OVN_USER" != "" ] && INSTALL_USER="${OVN_USER%:*}" @@ -286,7 +285,6 @@ $cluster_remote_port chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_dbdir chown -R $INSTALL_USER:$INSTALL_GROUP $OVN_RUNDIR - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_etcdir set ovsdb-server diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in index 5a0766816..0b19149da 100644 --- a/utilities/ovn-lib.in +++ b/utilities/ovn-lib.in @@ -133,7 +133,6 @@ start_ovn_daemon () { set "$@" --detach test X"$MONITOR" = Xno || set "$@" --monitor - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_rundir start_wrapped_daemon "$wrapper" $daemon "$priority" "$@" diff --git a/utilities/ovn-sysusers.conf.in b/utilities/ovn-sysusers.conf.in new file mode 100644 index 000000000..8745d0a44 --- /dev/null +++ b/utilities/ovn-sysusers.conf.in @@ -0,0 +1,2 @@ +@SYSUSERS_GROUP_LINE@ +@SYSUSERS_USER_LINE@ \ No newline at end of file diff --git a/utilities/ovn-tmpfiles.conf.in b/utilities/ovn-tmpfiles.conf.in new file mode 100644 index 000000000..d37391f88 --- /dev/null +++ b/utilities/ovn-tmpfiles.conf.in @@ -0,0 +1,2 @@ +d @LOGDIR@ 0750 @LOGUSER@ @LOGGROUP@ - +f @LOGDIR@/ovn-controller.log 0640 @LOGUSER@ @LOGGROUP@ - \ No newline at end of file -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
