| Message ID | 1447966722-18204-4-git-send-email-azhou@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Nov 19, 2015 at 12:58:39PM -0800, Andy Zhou wrote: > Allow ovs-ctl to take --user=USER option. When this option is specified > 'USER' will be parsed in the format of 'user:group" and be set into > shell variables $OVS_USER and $OVS_GROUP, which will be used > by shell functions, such as start_daemon() to run OVS daemons under > the specified user. > > Signed-off-by: Andy Zhou <azhou@ovn.org> > --- > utilities/ovs-ctl.8 | 4 ++++ > utilities/ovs-ctl.in | 4 ++++ > utilities/ovs-lib.in | 22 ++++++++++++++++++++++ > 3 files changed, 30 insertions(+) > > diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8 > index 6a9a544..50e3118 100644 > --- a/utilities/ovs-ctl.8 > +++ b/utilities/ovs-ctl.8 > @@ -123,6 +123,10 @@ another string is specified \fBovs\-ctl\fR uses it literally. > The following options should be specified if the defaults are not > suitable: > . > +.IP "\fB\-\-user=\fIuser[:group]\fR" I think the right formatting would be: .IP "\fB\-\-user=\fIuser\fR[\fB:\fIgroup\fR]" i.e. : in bold, [] in roman. > +Run OVS daemons as the user specified. When this options is specified, OVS > +daemons will run with the least privileges necessary. The above should probably be more specific (mention the group at least?). I think that this should say "--user=USER[:GROUP]": > + --user=USER run ovs daemons as the root user of ovs user (default: $OVS_USER:$OVS_GROUP) > --ovsdb-server-priority=NICE set ovsdb-server's niceness (default: $OVSDB_SERVER_PRIORITY) > --ovs-vswitchd-priority=NICE set ovs-vswitchd's niceness (default: $OVS_VSWITCHD_PRIORITY) > +set_ovs_user_group() { > + value=$1 # user spec (e.g. ovs:ovs) > + > + case $value in > + [a-z]*:*) > + OVS_USER=`expr X"$value" : 'X\(.*\):.*'` > + OVS_GROUP=`expr X"$value" : 'X[^:]*:\(.*\)'` > + if test X"$OVS_GROUP" = X; then > + OVS_GROUP=$OVS_USER > + fi > + ;; > + [a-z]*) > + OVS_USER=`expr X"$value" : 'X\(.*\)'` > + OVS_GROUP=$OVS_USER I think that the above two cases can be combined. The code for the first case handles the second just fine, doesn't it? > + ;; > + *) > + OVS_USER=root > + OVS_GROUP=root > + ;; > + esac > +} None of this is a big deal, so: Acked-by: Ben Pfaff <blp@ovn.org> Thanks, Ben.
On Tue, Nov 24, 2015 at 3:25 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Nov 19, 2015 at 12:58:39PM -0800, Andy Zhou wrote: > > Allow ovs-ctl to take --user=USER option. When this option is specified > > 'USER' will be parsed in the format of 'user:group" and be set into > > shell variables $OVS_USER and $OVS_GROUP, which will be used > > by shell functions, such as start_daemon() to run OVS daemons under > > the specified user. > > > > Signed-off-by: Andy Zhou <azhou@ovn.org> > > --- > > utilities/ovs-ctl.8 | 4 ++++ > > utilities/ovs-ctl.in | 4 ++++ > > utilities/ovs-lib.in | 22 ++++++++++++++++++++++ > > 3 files changed, 30 insertions(+) > > > > diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8 > > index 6a9a544..50e3118 100644 > > --- a/utilities/ovs-ctl.8 > > +++ b/utilities/ovs-ctl.8 > > @@ -123,6 +123,10 @@ another string is specified \fBovs\-ctl\fR uses it > literally. > > The following options should be specified if the defaults are not > > suitable: > > . > > +.IP "\fB\-\-user=\fIuser[:group]\fR" > > I think the right formatting would be: > .IP "\fB\-\-user=\fIuser\fR[\fB:\fIgroup\fR]" > i.e. : in bold, [] in roman. > O.K. will fix > > > +Run OVS daemons as the user specified. When this options is specified, > OVS > > +daemons will run with the least privileges necessary. > > The above should probably be more specific (mention the group at > least?). > I changed it as follows: daemons will run with the least privileges necessary. +daemons will run with the least privileges necessary, and switch the +deemon process's real and effective user and group to the ones specified. > > I think that this should say "--user=USER[:GROUP]": > > + --user=USER run ovs daemons as the root user of > ovs user (default: $OVS_USER:$OVS_GROUP) > > --ovsdb-server-priority=NICE set ovsdb-server's niceness (default: > $OVSDB_SERVER_PRIORITY) > > --ovs-vswitchd-priority=NICE set ovs-vswitchd's niceness (default: > $OVS_VSWITCHD_PRIORITY) > > > +set_ovs_user_group() { > > + value=$1 # user spec (e.g. ovs:ovs) > > + > > + case $value in > > + [a-z]*:*) > > + OVS_USER=`expr X"$value" : 'X\(.*\):.*'` > > + OVS_GROUP=`expr X"$value" : 'X[^:]*:\(.*\)'` > > + if test X"$OVS_GROUP" = X; then > > + OVS_GROUP=$OVS_USER > > + fi > > + ;; > > + [a-z]*) > > + OVS_USER=`expr X"$value" : 'X\(.*\)'` > > + OVS_GROUP=$OVS_USER > > I think that the above two cases can be combined. The code for the > first case handles the second just fine, doesn't it? > ':' seems to be significant. 'ovs' without the ':' seems to fall into the 3rd case. Did I miss something? > > > + ;; > > + *) > > + OVS_USER=root > > + OVS_GROUP=root > > + ;; > > + esac > > +} > > None of this is a big deal, so: > Acked-by: Ben Pfaff <blp@ovn.org> > > Thanks for the review.
diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8 index 6a9a544..50e3118 100644 --- a/utilities/ovs-ctl.8 +++ b/utilities/ovs-ctl.8 @@ -123,6 +123,10 @@ another string is specified \fBovs\-ctl\fR uses it literally. The following options should be specified if the defaults are not suitable: . +.IP "\fB\-\-user=\fIuser[:group]\fR" +Run OVS daemons as the user specified. When this options is specified, OVS +daemons will run with the least privileges necessary. +. .IP "\fB\-\-system\-type=\fItype\fR" .IQ "\fB\-\-system\-version=\fIversion\fR" Sets the value to store in the \fBsystem-type\fR and diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index c9d75df..4162add 100755 --- a/utilities/ovs-ctl.in +++ b/utilities/ovs-ctl.in @@ -573,6 +573,7 @@ Less important options for "start", "restart" and "force-reload-kmod": --daemon-cwd=DIR set working dir for OVS daemons (default: $DAEMON_CWD) --no-force-corefiles do not force on core dumps for OVS daemons --no-mlockall do not lock all of ovs-vswitchd into memory + --user=USER run ovs daemons as the root user of ovs user (default: $OVS_USER:$OVS_GROUP) --ovsdb-server-priority=NICE set ovsdb-server's niceness (default: $OVSDB_SERVER_PRIORITY) --ovs-vswitchd-priority=NICE set ovs-vswitchd's niceness (default: $OVS_VSWITCHD_PRIORITY) @@ -685,6 +686,9 @@ do ;; esac done + +set_ovs_user_group $USER + case $command in start) start_ovsdb || exit 1 diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index 3fbc2f5..38b9905 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -128,6 +128,28 @@ pid_exists () { test -d /proc/"$1" } +set_ovs_user_group() { + value=$1 # user spec (e.g. ovs:ovs) + + case $value in + [a-z]*:*) + OVS_USER=`expr X"$value" : 'X\(.*\):.*'` + OVS_GROUP=`expr X"$value" : 'X[^:]*:\(.*\)'` + if test X"$OVS_GROUP" = X; then + OVS_GROUP=$OVS_USER + fi + ;; + [a-z]*) + OVS_USER=`expr X"$value" : 'X\(.*\)'` + OVS_GROUP=$OVS_USER + ;; + *) + OVS_USER=root + OVS_GROUP=root + ;; + esac +} + pid_comm_check () { [ "$1" = "`cat /proc/$2/comm`" ] }
Allow ovs-ctl to take --user=USER option. When this option is specified 'USER' will be parsed in the format of 'user:group" and be set into shell variables $OVS_USER and $OVS_GROUP, which will be used by shell functions, such as start_daemon() to run OVS daemons under the specified user. Signed-off-by: Andy Zhou <azhou@ovn.org> --- utilities/ovs-ctl.8 | 4 ++++ utilities/ovs-ctl.in | 4 ++++ utilities/ovs-lib.in | 22 ++++++++++++++++++++++ 3 files changed, 30 insertions(+)