See some comments inline below.

Cheers,

Eelco

On 27 Jan 2023, at 14:29, Vladislav Odintsov wrote:

> This patch adds a new ovs-ctl option to pass umask configuration to allow
> OVS daemons to 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 that user.

Will this affect any other files generated by vswitchd (logs?) that are now 
accessible by unprivileged users?

> Previous behaviour (not setting umask) is left as default.
>
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2023-January/401501.html
> Signed-off-by: Vladislav Odintsov <[email protected]>
> ---
>  utilities/ovs-ctl.in | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index e6e07f476..b97d568c6 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -334,6 +334,7 @@ set_defaults () {
>      SELF_CONFINEMENT=yes
>      MONITOR=yes
>      OVS_USER=
> +    OVS_UMASK=
>      OVSDB_SERVER=yes
>      OVS_VSWITCHD=yes
>      OVSDB_SERVER_PRIORITY=-10
> @@ -415,6 +416,8 @@ 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
> +  --ovs-umask=XXXX  Set umask prior to run OVS daemons.

Rather than XXXX you might want to use MODE.

> +                    This is needed to manage socket group permissions.
>
>  Less important options for "start", "restart" and "force-reload-kmod":
>    --daemon-cwd=DIR               set working dir for OVS daemons (default: 
> $DAEMON_CWD)
> @@ -542,6 +545,11 @@ do
>              ;;
>      esac
>  done
> +
> +if [ -n "$OVS_UMASK" ]; then
> +    umask "$OVS_UMASK"
> +fi

The help mentions the list of commands are important 
start/restart/force-reload-kmod commands, but you set this umask for any 
command.
Actually, the entire script is now executed with this umask, not only the 
daemons, if this is intended you might want to highlight it. Did not check if 
there is any implication for doing this.

> +
>  case $command in
>      start)
>          start_ovsdb || exit 1
> -- 
> 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

Reply via email to