On 8 Feb 2023, at 16:22, Vladislav Odintsov wrote:
> This patch adds new ovs-ctl options to pass umask configuration to allow
> OVS daemons set requested socket permissions on group. Previous
> behaviour (if using with systemd service unit) created sockets with 0750
> permissions mask (group has no write permission).
>
> Write permission for group is reasonable in usecase, where ovs-vswitchd
> or ovsdb-server runs as a non-privileged user:group (say,
> openvswitch:openvswitch) and it is needed to access unix socket from
> process running as another non-privileged user. In this case
> administrator has to add that user to openvswitch group and can connect
> to OVS sockets from a process running under that user.
>
> Two new ovs-ctl options --ovsdb-server-umask and --ovs-vswitchd-umask
> were added to manage umask values for appropriate daemons. This is
> useful for systemd users: both ovs-vswitchd and ovsdb-server systemd
> units read options from single /etc/sysconfig/openvswitch configuration
> file. So, with separate options it is possible to set umask only for
> specific daemon.
>
> OPTIONS="--ovsdb-server-umask=0002"
>
> in /etc/openvswitch/sysconfig file will set umask to 0002 value before
> starting only ovsdb-server, while
>
> OPTIONS="--ovs-vswitchd-umask=0002"
>
> will set umask to ovs-vswitchd daemon.
>
> Previous behaviour (not setting umask) is left as default.
Thanks for reworking the patch, small none technical nit below, the rest looks
good.
//Eelco
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-January/401501.html
> Signed-off-by: Vladislav Odintsov <[email protected]>
>
> ---
> v1 -> v2:
> - added item in NEWS file as Ilya's suggestion;
> - addressed Eelco's review comments;
> - moved umask call from ovs-ctl to ovs-lib;
> - added restoration of umask to effective value before the umask change;
> - previous version --ovs-umask option was split into two:
> --ovs-vswitchd-umask and --ovsdb-server-umask in order to make
> possible umask configuration for specific daemon when running with
> systemd.
>
> Signed-off-by: Vladislav Odintsov <[email protected]>
> ---
> NEWS | 7 +++++++
> utilities/ovs-ctl.in | 16 ++++++++++++----
> utilities/ovs-lib.in | 17 ++++++++++++++---
> 3 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index fe6055a27..f7df598bd 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,13 @@ Post-v3.1.0
> * OVS now collects per-interface upcall statistics that can be obtained
> via 'ovs-appctl dpctl/show -s' or the interface's statistics column
> in OVSDB. Available with upstream kernel 6.2+.
> + - ovs-ctl:
> + * Added support to set umask value when starting OVS daemons. New
> options
> + --ovsdb-server-umask=MODE and --ovs-vswitchd-umask=MODE were added for
> + that. For instance, when write access on befalf of OVS group is
> needed
> + for ovsdb-server, pass --ovsdb-umask=0002. Use --vswitchd-umask to
> set
> + umask ovs-vswitchd daemon umask. This will allow ovsdb-server or
> + ovs-vswitchd to create sockets with access mode of 0770.
>
>
> v3.1.0 - xx xxx xxxx
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index d91552588..0b2820c36 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -156,8 +156,8 @@ do_start_ovsdb () {
> [ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
> [ "$OVSDB_SERVER_OPTIONS" != "" ] && set "$@" $OVSDB_SERVER_OPTIONS
>
> - start_daemon "$OVSDB_SERVER_PRIORITY" "$OVSDB_SERVER_WRAPPER" "$@" \
> - || return 1
> + start_daemon "$OVSDB_SERVER_PRIORITY" "$OVSDB_SERVER_WRAPPER" \
> + "$OVSDB_SERVER_UMASK" "$@" || return 1
>
> # Initialize database settings.
> ovs_vsctl -- init -- set Open_vSwitch . db-version="$schemaver" \
> @@ -226,8 +226,8 @@ do_start_forwarding () {
> [ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
> [ "$OVS_VSWITCHD_OPTIONS" != "" ] &&set "$@" $OVS_VSWITCHD_OPTIONS
>
> - start_daemon "$OVS_VSWITCHD_PRIORITY" "$OVS_VSWITCHD_WRAPPER" "$@" ||
> - return 1
> + start_daemon "$OVS_VSWITCHD_PRIORITY" "$OVS_VSWITCHD_WRAPPER" \
> + "$OVS_VSWITCHD_UMASK" "$@" || return 1
> fi
> }
>
> @@ -348,6 +348,8 @@ set_defaults () {
> OVS_VSWITCHD_WRAPPER=
> OVSDB_SERVER_OPTIONS=
> OVS_VSWITCHD_OPTIONS=
> + OVSDB_SERVER_UMASK=
> + OVS_VSWITCHD_UMASK=
>
> DB_FILE=$dbdir/conf.db
> DB_SOCK=$rundir/db.sock
> @@ -421,6 +423,12 @@ Other important options for "start", "restart" and
> "force-reload-kmod":
> add given key-value pair to Open_vSwitch external-ids
> --delete-bridges delete all bridges just before starting ovs-vswitchd
> --ovs-user="user[:group]" pass the --user flag to ovs daemons
> + --ovsdb-server-umask=MODE Set umask prior to run ovsdb-server daemon.
> + This is useful to manage daemon's sockets
> permissions.
> + Default is not to change umask (inherited from
> shell).
> + --ovs-vswitchd-umask=MODE Set umask prior to run ovs-vswitchd daemon.
> + This is useful to manage daemon's sockets
> permissions.
> + Default is not to change umask (inherited from
> shell).
>
> Less important options for "start", "restart" and "force-reload-kmod":
> --daemon-cwd=DIR set working dir for OVS daemons (default:
> $DAEMON_CWD)
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 13477a6a9..004de631c 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -165,9 +165,9 @@ install_dir () {
> }
>
> start_daemon () {
> - priority=$1
> - wrapper=$2
> - shift; shift
> + priority=$1 && shift
> + wrapper=$1 && shift
> + umask=$1 && shift
> daemon=$1
> strace=""
>
> @@ -223,8 +223,19 @@ start_daemon () {
> set nice -n "$priority" "$@"
> fi
>
> + # set requested umask if any and turn previous value back
Please use a capital S for Set and end with a dot.
> + if [ -n "$umask" ]; then
> + previuos_umask_value=$(umask)
> + umask "$umask"
> + fi
> +
> action "Starting $daemon" "$@" || return 1
>
> + # if umask was set, turn umask value to previous value
Please use a capital I for If and end with a dot.
> + if [ -n "$umask" ]; then
> + umask "$previuos_umask_value"
> + fi
> +
> if test X"$strace" != X; then
> # Strace doesn't have the -D option so we attach after the fact.
> setsid $strace -o "$logdir/$daemon.strace.log" \
> --
> 2.36.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
