[ovs-dev,rhel,--user,v2,3/7] utilities: add --user option to ovs-ctl

Allow ovs-ctl to take --user=USER[:GROUP] option. When this option
is specified, they will be parsed and set into shell variables
$OVS_USER and $OVS_GROUP, which will be used by other shell functions,
such as start_daemon() to run OVS daemons under the specified user.

Signed-off-by: Andy Zhou <azhou@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

v1->v2: fix ovs-ctl manpage format, added more content
---
 utilities/ovs-ctl.8  |  5 +++++
 utilities/ovs-ctl.in |  6 ++++++
 utilities/ovs-lib.in | 22 ++++++++++++++++++++++
 3 files changed, 33 insertions(+)

On Fri, Nov 20, 2015 at 03:33:16AM -0800, Andy Zhou wrote:
> Allow ovs-ctl to take --user=USER[:GROUP] option. When this option
> is specified, they will be parsed and set into shell variables
> $OVS_USER and $OVS_GROUP, which will be used by other shell functions,
> such as start_daemon() to run OVS daemons under the specified user.

I understand we can do chown [USER][:GROUP]] but it would be simpler
to just have --user=USER and --group=GROUP.

Also, I am not sure about having --user=$not_root and --group=root
Maybe the default should be GROUP=USER?

Thanks,
fbl
 
> Signed-off-by: Andy Zhou <azhou@ovn.org>
> Acked-by: Ben Pfaff <blp@ovn.org>
> 
> v1->v2: fix ovs-ctl manpage format, added more content
> ---
>  utilities/ovs-ctl.8  |  5 +++++
>  utilities/ovs-ctl.in |  6 ++++++
>  utilities/ovs-lib.in | 22 ++++++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8
> index 6a9a544..7e3c870 100644
> --- a/utilities/ovs-ctl.8
> +++ b/utilities/ovs-ctl.8
> @@ -123,6 +123,11 @@ 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\fR[\fB:\fIgroup\fR]"
> +Run OVS daemons as the user specified. When this options is specified, OVS
> +daemons will run with the least privileges necessary, and switch the
> +deemon process's real and effective user and group to the ones specified.
> +.
>  .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..e128889 100755
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -535,6 +535,8 @@ set_defaults () {
>          SYSTEM_TYPE=unknown
>          SYSTEM_VERSION=unknown
>      fi
> +
> +    USER="root:root"
>  }
>  
>  usage () {
> @@ -573,6 +575,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[:GROUP]            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 +688,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 34e2041..ad223c0 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`" ]
>  }
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

On Thu, Nov 26, 2015 at 09:14:34PM -0200, Flavio Leitner wrote:
> On Fri, Nov 20, 2015 at 03:33:16AM -0800, Andy Zhou wrote:
> > Allow ovs-ctl to take --user=USER[:GROUP] option. When this option
> > is specified, they will be parsed and set into shell variables
> > $OVS_USER and $OVS_GROUP, which will be used by other shell functions,
> > such as start_daemon() to run OVS daemons under the specified user.
> 
> I understand we can do chown [USER][:GROUP]] but it would be simpler
> to just have --user=USER and --group=GROUP.
> 
> Also, I am not sure about having --user=$not_root and --group=root
> Maybe the default should be GROUP=USER?

As I read the code, that is the default.  No?

On Sun, Nov 29, 2015 at 04:17:07PM -0800, Ben Pfaff wrote:
> On Thu, Nov 26, 2015 at 09:14:34PM -0200, Flavio Leitner wrote:
> > On Fri, Nov 20, 2015 at 03:33:16AM -0800, Andy Zhou wrote:
> > > Allow ovs-ctl to take --user=USER[:GROUP] option. When this option
> > > is specified, they will be parsed and set into shell variables
> > > $OVS_USER and $OVS_GROUP, which will be used by other shell functions,
> > > such as start_daemon() to run OVS daemons under the specified user.
> > 
> > I understand we can do chown [USER][:GROUP]] but it would be simpler
> > to just have --user=USER and --group=GROUP.
> > 
> > Also, I am not sure about having --user=$not_root and --group=root
> > Maybe the default should be GROUP=USER?
> 
> As I read the code, that is the default.  No?

You're correct.

fbl