| Message ID | 20230127132950.3670502-1-odivlad@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] utilities: add support to set umask in ovs-ctl | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 1/27/23 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. > > 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 <odivlad@gmail.com> > --- > utilities/ovs-ctl.in | 8 ++++++++ > 1 file changed, 8 insertions(+) Hi. Could you, please, also add a NEWS entry for this change? Thanks! Best regards, Ilya Maximets. > > 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. > + 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 > + > case $command in > start) > start_ovsdb || exit 1
Hi, sure, no problem. Should I do in a separate patch or incorporate into this one? > On 7 Feb 2023, at 15:35, Ilya Maximets <i.maximets@ovn.org> wrote: > > On 1/27/23 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. >> >> 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 <odivlad@gmail.com> >> --- >> utilities/ovs-ctl.in | 8 ++++++++ >> 1 file changed, 8 insertions(+) > > Hi. Could you, please, also add a NEWS entry for this change? > Thanks! > > Best regards, Ilya Maximets. > >> >> 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. >> + 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 >> + >> case $command in >> start) >> start_ovsdb || exit 1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Regards, Vladislav Odintsov
On 2/7/23 14:33, Vladislav Odintsov wrote: > Hi, > > sure, no problem. > Should I do in a separate patch or incorporate into this one? NEWS entry should typically go together with the change, so you can 'git blame' the NEWS file and find the feature commit. So, in the same patch. Best regards, Ilya Maximets. > >> On 7 Feb 2023, at 15:35, Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 1/27/23 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. >>> >>> 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 <odivlad@gmail.com> >>> --- >>> utilities/ovs-ctl.in | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >> >> Hi. Could you, please, also add a NEWS entry for this change? >> Thanks! >> >> Best regards, Ilya Maximets. >> >>> >>> 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. >>> + 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 >>> + >>> case $command in >>> start) >>> start_ovsdb || exit 1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > > > Regards, > Vladislav Odintsov >
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 <odivlad@gmail.com> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks for the review, Eelco! Answers are inline. > On 7 Feb 2023, at 17:02, Eelco Chaudron <echaudro@redhat.com> wrote: > > 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? I’ve checked other files: pidfile, logs, ovsdb lock files and see no difference between default behaviour (no umask call) and when umask is set to 0002, except for socket files: pid files: 0644 log files: 0640 ovsdb lock: 0600 socket files have 0750 as a default value and 0770 if umask 000X is used (X could be any value in the 0-7 range, if IIC). > >> 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 <odivlad@gmail.com> >> --- >> 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. Agree. Will address this in v2. > >> + 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. Hmm, nice point. Maybe it would be better to move setting umask to ovs-lib.in file and pass umask value to start_daemon() function? It could be placed under setting nice value., right before running process with "$@". Also, we could split --ovs-umask option into two: --ovsdb-umask, which sets umask only before ovsdb server start and --vswitchd-umask, which sets umask only before ovs-vswitchd daemon start. What do you think? > >> + >> case $command in >> start) >> start_ovsdb || exit 1 >> -- >> 2.36.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Regards, Vladislav Odintsov
On 7 Feb 2023, at 16:04, Vladislav Odintsov wrote: > Thanks for the review, Eelco! > Answers are inline. > >> On 7 Feb 2023, at 17:02, Eelco Chaudron <echaudro@redhat.com> wrote: >> >> 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? > > I’ve checked other files: pidfile, logs, ovsdb lock files and see no difference between default behaviour (no umask call) and when umask is set to 0002, except for socket files: > pid files: 0644 > log files: 0640 > ovsdb lock: 0600 > socket files have 0750 as a default value and 0770 if umask 000X is used (X could be any value in the 0-7 range, if IIC). I did not find anything quickly that could be a loose end... Maybe someone else has some ideas. >> >>> 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 <odivlad@gmail.com> >>> --- >>> 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. > > Agree. Will address this in v2. > >> >>> + 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. > > Hmm, nice point. > Maybe it would be better to move setting umask to ovs-lib.in file and pass umask value to start_daemon() function? > It could be placed under setting nice value., right before running process with "$@". Sound like a nice place to add this, also you need to restore it after running the daemon. > Also, we could split --ovs-umask option into two: > --ovsdb-umask, which sets umask only before ovsdb server start and > --vswitchd-umask, which sets umask only before ovs-vswitchd daemon start. > > What do you think? I guess a global value would still work, as you can use --no-ovs-vswitchd and --no-ovsdb-server and split the startup over two commands if you really need different umasks. But I’m fine with either way, maybe Ilya has any preference. >> >>> + >>> case $command in >>> start) >>> start_ovsdb || exit 1 >>> -- >>> 2.36.1 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org <mailto:dev@openvswitch.org> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > Regards, > Vladislav Odintsov
Hi Eelco, Ilya, I’ve submitted v2 with requested changes addressed: https://patchwork.ozlabs.org/project/openvswitch/patch/20230208152243.3686696-1-odivlad@gmail.com/Please, let me know if this revision is okay or needs any improvements. > On 7 Feb 2023, at 18:41, Eelco Chaudron <echaudro@redhat.com> wrote: > > > > On 7 Feb 2023, at 16:04, Vladislav Odintsov wrote: > >> Thanks for the review, Eelco! >> Answers are inline. >> >>> On 7 Feb 2023, at 17:02, Eelco Chaudron <echaudro@redhat.com> wrote: >>> >>> 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? >> >> I’ve checked other files: pidfile, logs, ovsdb lock files and see no difference between default behaviour (no umask call) and when umask is set to 0002, except for socket files: >> pid files: 0644 >> log files: 0640 >> ovsdb lock: 0600 >> socket files have 0750 as a default value and 0770 if umask 000X is used (X could be any value in the 0-7 range, if IIC). > > I did not find anything quickly that could be a loose end... Maybe someone else has some ideas. > >>> >>>> 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 <odivlad@gmail.com> >>>> --- >>>> 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. >> >> Agree. Will address this in v2. >> >>> >>>> + 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. >> >> Hmm, nice point. >> Maybe it would be better to move setting umask to ovs-lib.in file and pass umask value to start_daemon() function? >> It could be placed under setting nice value., right before running process with "$@". > > Sound like a nice place to add this, also you need to restore it after running the daemon. > >> Also, we could split --ovs-umask option into two: >> --ovsdb-umask, which sets umask only before ovsdb server start and >> --vswitchd-umask, which sets umask only before ovs-vswitchd daemon start. >> >> What do you think? > > I guess a global value would still work, as you can use --no-ovs-vswitchd and --no-ovsdb-server and split the startup over two commands if you really need different umasks. But I’m fine with either way, maybe Ilya has any preference. > >>> >>>> + >>>> case $command in >>>> start) >>>> start_ovsdb || exit 1 >>>> -- >>>> 2.36.1 >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> dev@openvswitch.org <mailto:dev@openvswitch.org> <mailto:dev@openvswitch.org> >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org <mailto:dev@openvswitch.org> <mailto:dev@openvswitch.org> >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> >> Regards, >> Vladislav Odintsov > > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Regards, Vladislav Odintsov
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. + 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 + case $command in start) start_ovsdb || exit 1
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. 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 <odivlad@gmail.com> --- utilities/ovs-ctl.in | 8 ++++++++ 1 file changed, 8 insertions(+)