OVN startup scripts recursively chown the log directory. This overrides ownership established by systemd-tmpfiles and is unsuitable for distributions that manage log ownership through tmpfiles.
Add an opt-in tmpfiles ownership mode. When enabled, skip the recursive log-directory chown and install a tmpfiles configuration for the log directory only. Set the directory setgid bit so log files created by OVN daemons inherit the configured log group. Do not enumerate individual log files in tmpfiles. Daemons create the log files they actually use, so newly added daemons and log files receive the configured group without changes to the tmpfiles configuration. Install the sysusers configuration independently from tmpfiles. Debian uses its existing root:adm identity and disables OVN sysusers. Fedora uses the openvswitch identity supplied by the Open vSwitch package and also disables OVN sysusers by default. Its optional sysusers build condition consistently controls configure, package contents, and the post-install sysusers invocation. Keep the feature disabled by default, preserving the existing ownership behavior for source builds and deployments that do not use tmpfiles. 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. v4: - Make tmpfiles-based log ownership opt-in with the new --enable-tmpfiles-log-ownership configure option. - Keep the option disabled by default so deployments without systemd-tmpfiles retain ovn-ctl and ovn-lib's existing recursive log-directory ownership handling. - Install the tmpfiles.d and sysusers.d helpers only when the option is enabled. - Explicitly enable tmpfiles log ownership in Debian and Fedora packaging. - Configure Debian log ownership as root:adm. - Configure Fedora log ownership as openvswitch:openvswitch and apply the tmpfiles configuration in the RPM %post script. - Do not package OVN's generated sysusers.d file on Fedora/RHEL, where the openvswitch user is managed by the Open vSwitch package. v5: - Use a consistent author and Signed-off-by identity. v6: - Create only the log directory in tmpfiles; do not enumerate individual log files. - Set the directory mode to 2750 so dynamically created log files inherit the configured log group. - Decouple sysusers installation from tmpfiles installation with --enable-sysusers-log-ownership. - Disable OVN sysusers in Debian, where root:adm already exists, and install it conditionally only when staged by the build. - Add an optional ovn_sysusers RPM build condition that consistently controls configure, package contents, and %sysusers_create. - Do not add per-subpackage RPM %post tmpfiles handling; the base %post creates the shared log directory only. - Add trailing newlines to the tmpfiles and sysusers templates. Testing: - Ran utilities/checkpatch.py -1. - Ran make check TESTSUITEFLAGS="-j$(nproc)". - Built and installed the tmpfiles-enabled configuration in an Ubuntu 24.04 LXD container. - Built and installed the Fedora RPM and verified its tmpfiles configuration creates the log directory and controller log as openvswitch:openvswitch. Makefile.am | 9 ++++ configure.ac | 5 +++ debian/ovn-common.install | 1 + debian/rules | 6 ++- m4/ovn.m4 | 82 ++++++++++++++++++++++++++++++++++ rhel/ovn-fedora.spec.in | 21 +++++++++ utilities/automake.mk | 12 +++++ utilities/ovn-ctl | 9 ++-- utilities/ovn-lib.in | 5 ++- utilities/ovn-sysusers.conf.in | 2 + utilities/ovn-tmpfiles.conf.in | 2 + 11 files changed, 148 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 8a9bf1e91..e01bd2765 100644 --- a/Makefile.am +++ b/Makefile.am @@ -129,6 +129,8 @@ noinst_PROGRAMS = noinst_SCRIPTS = OVSIDL_BUILT = pkgdata_DATA = +tmpfiles_DATA = +sysusers_DATA = sbin_SCRIPTS = scripts_SCRIPTS = completion_SCRIPTS = @@ -146,6 +148,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 @@ -164,6 +168,11 @@ 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,[@]OVN_CHOWN_LOGDIR[@],$(OVN_CHOWN_LOGDIR),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 06c3cf5d4..fecb9da8d 100644 --- a/configure.ac +++ b/configure.ac @@ -88,6 +88,11 @@ OVS_CHECK_NETLINK OVS_CHECK_LINUX_NETLINK OVS_CHECK_OPENSSL OVN_CHECK_LOGDIR +OVN_CHECK_LOGUSER +OVN_CHECK_LOGGROUP +OVN_CHECK_TMPFILES_LOG_OWNERSHIP +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 6e51dffb8..a804f7c5b 100644 --- a/debian/ovn-common.install +++ b/debian/ovn-common.install @@ -9,4 +9,5 @@ usr/bin/ovn-debug usr/share/ovn/scripts/ovn-ctl usr/share/ovn/scripts/ovndb-servers.ocf usr/share/ovn/scripts/ovn-lib +usr/lib/tmpfiles.d/ovn-tmpfiles.conf usr/lib/*/libovn*.so.* diff --git a/debian/rules b/debian/rules index b25a0b48e..db1102c86 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 --enable-tmpfiles-log-ownership --disable-sysusers-log-ownership --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))) @@ -49,6 +49,10 @@ override_dh_auto_clean: override_dh_install-arch: dh_install + if test -e debian/tmp/usr/lib/sysusers.d/ovn-sysusers.conf; then \ + dh_install -povn-common --sourcedir=debian/tmp \ + usr/lib/sysusers.d/ovn-sysusers.conf; \ + fi # ovn-host cp debian/ovn-host.template debian/ovn-host/usr/share/ovn/host/default.template diff --git a/m4/ovn.m4 b/m4/ovn.m4 index 6be2bba09..e2cf54670 100644 --- a/m4/ovn.m4 +++ b/m4/ovn.m4 @@ -127,6 +127,88 @@ 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 whether tmpfiles.d manages log ownership. +AC_DEFUN([OVN_CHECK_TMPFILES_LOG_OWNERSHIP], + [AC_ARG_ENABLE( + [tmpfiles-log-ownership], + [AS_HELP_STRING([--enable-tmpfiles-log-ownership], + [manage log ownership with tmpfiles.d])], + [case "${enableval}" in + (yes) tmpfiles_log_ownership=true ;; + (no) tmpfiles_log_ownership=false ;; + (*) AC_MSG_ERROR([bad value ${enableval} for --enable-tmpfiles-log-ownership]) ;; + esac], + [tmpfiles_log_ownership=false]) + AM_CONDITIONAL([TMPFILES_LOG_OWNERSHIP], + [test x$tmpfiles_log_ownership = xtrue]) + AS_IF([test x$tmpfiles_log_ownership = xtrue], + [OVN_CHOWN_LOGDIR=no], + [OVN_CHOWN_LOGDIR=yes]) + AC_SUBST([OVN_CHOWN_LOGDIR])]) + +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']) + AC_ARG_ENABLE( + [sysusers-log-ownership], + [AS_HELP_STRING([--enable-sysusers-log-ownership], + [create log user and group with sysusers.d])], + [case "${enableval}" in + (yes) sysusers_log_ownership=true ;; + (no) sysusers_log_ownership=false ;; + (*) AC_MSG_ERROR([bad value ${enableval} for --enable-sysusers-log-ownership]) ;; + esac], + [sysusers_log_ownership=$tmpfiles_log_ownership]) + AM_CONDITIONAL([SYSUSERS_LOG_OWNERSHIP], + [test x$sysusers_log_ownership = xtrue && + { test x$LOGUSER != xroot || test x$LOGGROUP != xroot; }]) + 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 e70c581e7..822389dde 100644 --- a/rhel/ovn-fedora.spec.in +++ b/rhel/ovn-fedora.spec.in @@ -40,6 +40,11 @@ Provides: openvswitch-ovn-common = %{?epoch:%{epoch}:}%{version}-%{release} # to skip running checks, pass --without check %bcond_without check +# The Open vSwitch package normally provisions the openvswitch account. +# Pass --with ovn_sysusers only for a package that provisions a distinct OVN +# log user or group. +%bcond_with ovn_sysusers + # Nearly all of openvswitch is ASL 2.0. The lib/sflow*.[ch] files are SISSL License: ASL 2.0 and SISSL Release: 1%{?dist} @@ -162,6 +167,14 @@ cd - --disable-libcapng \ %endif --enable-ssl \ + --enable-tmpfiles-log-ownership \ + %if %{with ovn_sysusers} + --enable-sysusers-log-ownership \ + %else + --disable-sysusers-log-ownership \ + %endif + --with-log-user=openvswitch \ + --with-log-group=openvswitch \ --with-pkidir=%{_sharedstatedir}/openvswitch/pki \ --with-version-suffix=-%{release} \ PYTHON3=%{__python3} @@ -341,6 +354,10 @@ fi %post ln -sf ovn_detrace.py %{_bindir}/ovn-detrace +%if %{with ovn_sysusers} +%sysusers_create ovn-sysusers.conf +%endif +%tmpfiles_create ovn-tmpfiles.conf %if %{with libcapng} if [ $1 -eq 1 ]; then @@ -526,6 +543,10 @@ fi %{_mandir}/man8/ovn-debug.8* %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers %config(noreplace) %{_sysconfdir}/logrotate.d/ovn +%{_tmpfilesdir}/ovn-tmpfiles.conf +%if %{with ovn_sysusers} +%{_sysusersdir}/ovn-sysusers.conf +%endif %{_unitdir}/[email protected] %files docker diff --git a/utilities/automake.mk b/utilities/automake.mk index 7d3f320dc..c8d698de4 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 \ @@ -62,7 +66,15 @@ CLEANFILES += \ utilities/ovn-appctl.8 \ utilities/ovn-appctl +if TMPFILES_LOG_OWNERSHIP +tmpfiles_DATA += utilities/ovn-tmpfiles.conf +endif +if SYSUSERS_LOG_OWNERSHIP +sysusers_DATA += utilities/ovn-sysusers.conf +endif 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 788f1c14b..e0a376aa1 100755 --- a/utilities/ovn-ctl +++ b/utilities/ovn-ctl @@ -281,9 +281,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%:*}" @@ -291,7 +290,9 @@ $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 + if test "$ovn_chown_logdir" = yes; then + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir + fi chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_etcdir set ovsdb-server diff --git a/utilities/ovn-lib.in b/utilities/ovn-lib.in index 8e8eedeab..cd6556cbf 100644 --- a/utilities/ovn-lib.in +++ b/utilities/ovn-lib.in @@ -29,6 +29,7 @@ ovn_etcdir=$ovn_sysconfdir/ovn # /etc/ovn ovn_datadir=${OVN_PKGDATADIR-'@pkgdatadir@'} # /usr/share/ovn ovn_bindir=${OVN_BINDIR-'@bindir@'} # /usr/bin ovn_sbindir=${OVN_SBINDIR-'@sbindir@'} # /usr/sbin +ovn_chown_logdir='@OVN_CHOWN_LOGDIR@' # /etc/ovn or /var/lib/ovn if test X"$OVN_DBDIR" != X; then @@ -133,7 +134,9 @@ start_ovn_daemon () { set "$@" --detach test X"$MONITOR" = Xno || set "$@" --monitor - chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir + if test "$ovn_chown_logdir" = yes; then + chown -R $INSTALL_USER:$INSTALL_GROUP $ovn_logdir + fi 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..e067cadbc --- /dev/null +++ b/utilities/ovn-sysusers.conf.in @@ -0,0 +1,2 @@ +@SYSUSERS_GROUP_LINE@ +@SYSUSERS_USER_LINE@ diff --git a/utilities/ovn-tmpfiles.conf.in b/utilities/ovn-tmpfiles.conf.in new file mode 100644 index 000000000..c4d0321ce --- /dev/null +++ b/utilities/ovn-tmpfiles.conf.in @@ -0,0 +1,2 @@ +# The setgid bit makes log files created in this directory inherit @LOGGROUP@. +d @LOGDIR@ 2750 @LOGUSER@ @LOGGROUP@ - -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
