[ovs-dev,Debian-non-root,v2,3/4] ovs-ctl: add --no-run-as-root option
diff mbox

Message ID 1444436004-25557-3-git-send-email-azhou@nicira.com
State Changes Requested
Headers show

Commit Message

Andy Zhou Oct. 10, 2015, 12:13 a.m. UTC
Add option to ovs-ctl script to specify whether to start the daemons as
root user or ovs user.  The default is 'run-as-root', which preserves
the script's current behavior.

Signed-off-by: Andy Zhou <azhou@nicira.com>
---
 utilities/ovs-ctl.in | 13 +++++++++++--
 utilities/ovs-lib.in |  9 ++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Oct. 24, 2015, 9:02 p.m. UTC | #1
On Fri, Oct 09, 2015 at 05:13:23PM -0700, Andy Zhou wrote:
> Add option to ovs-ctl script to specify whether to start the daemons as
> root user or ovs user.  The default is 'run-as-root', which preserves
> the script's current behavior.
> 
> Signed-off-by: Andy Zhou <azhou@nicira.com>

I think that the implementation is fine but I'm nervous about the option
name.  I prefer positive names over negative ones because they are
generally more meaningful to users.

Here, for example, one option would be to support --run-as=user[:group].
Then one could invoke it as --run-as=root or --run-as=ovs.  That also
gives more control to the caller since it can now specify specifically
what user it wants.

Whether the option name is changed or not, this commit should update the
ovs-ctl manpage.
Andy Zhou Oct. 26, 2015, 4:31 a.m. UTC | #2
On Sat, Oct 24, 2015 at 2:02 PM, Ben Pfaff <blp@nicira.com> wrote:
> On Fri, Oct 09, 2015 at 05:13:23PM -0700, Andy Zhou wrote:
>> Add option to ovs-ctl script to specify whether to start the daemons as
>> root user or ovs user.  The default is 'run-as-root', which preserves
>> the script's current behavior.
>>
>> Signed-off-by: Andy Zhou <azhou@nicira.com>
>
> I think that the implementation is fine but I'm nervous about the option
> name.  I prefer positive names over negative ones because they are
> generally more meaningful to users.
>
> Here, for example, one option would be to support --run-as=user[:group].
> Then one could invoke it as --run-as=root or --run-as=ovs.  That also
> gives more control to the caller since it can now specify specifically
> what user it wants.
>
> Whether the option name is changed or not, this commit should update the
> ovs-ctl manpage.
Thanks for the review and suggestion.

I will implement --run-as and fix ova-ctl manpage in the next rev.

Patch
diff mbox

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index c9d75df..191631c 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -13,8 +13,8 @@ 
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-OVS_USER=root         # Default user.
-OVS_GROUP=root        # Default group.
+OVS_USER=ovs         # Default user.
+OVS_GROUP=$OVS_USER  # Default group.
 
 case $0 in
     */*) dir0=`echo "$0" | sed 's,/[^/]*$,,'` ;;
@@ -101,6 +101,7 @@  set_system_ids () {
             else
                 log_failure_msg "missing uuidgen, could not generate system ID"
             fi
+            chown "$OVS_USER":"$OVS_GROUP" $id_file
             ;;
 
         '')
@@ -505,6 +506,7 @@  set_defaults () {
 
     DAEMON_CWD=/
     FORCE_COREFILES=yes
+    RUN_AS_ROOT=yes
     MLOCKALL=yes
     OVSDB_SERVER_PRIORITY=-10
     OVS_VSWITCHD_PRIORITY=-10
@@ -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
+  --no-run-as-root               run ovs daemons as the OVS user
   --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,12 @@  do
             ;;
     esac
 done
+
+if test X"$RUN_AS_ROOT" = Xyes; then
+     OVS_USER=root
+     OVS_GROUP=root
+fi
+
 case $command in
     start)
         start_ovsdb || exit 1
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index da52284..2045a16 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -149,10 +149,15 @@  start_daemon () {
     set "$@" --log-file="$logdir/$daemon.log"
 
     # pidfile and monitoring
-    test -d "$rundir" || install -d -m 755 -o "$OVS_USER" -g "OVS_GROUP" "$rundir"
+    test -d "$rundir" || install -d -m 770 -o "$OVS_USER" -g "$OVS_GROUP" "$rundir"
     set "$@" --pidfile="$rundir/$daemon.pid"
     set "$@" --detach --monitor
 
+    # non root user
+    if test X"$RUN_AS_ROOT" != Xyes; then
+        set "$@" --user="$OVS_USER":"$OVS_GROUP"
+    fi
+
     # wrapper
     case $wrapper in
         valgrind)
@@ -376,4 +381,6 @@  upgrade_db () {
             create_db "$DB_FILE" "$DB_SCHEMA"
         fi
     fi
+
+    chown -R "$OVS_USER":"$OVS_GROUP" `dirname $DB_FILE`
 }