On Mon, Oct 5, 2015 at 6:38 PM, Andy Zhou <az...@nicira.com> wrote:

Thanks Andy for doing this! I will have another more careful look at
this patch tomorrow, because I think I somehow managed to get into a
state where after installing debian packages /etc/openvswitch still
belonged to root.


> Changes to Debian packaging scripts to create the ovs user and group.
> Fix the permissions of ovs created files and directories so that
> they are accessible by users belong to the ovs group.

s/users belong/users that belong

> Start daemons as the ovs user.
>
> Signed-off-by: Andy Zhou <az...@nicira.com>

This patch, I believe, breaks upgrades:

Wed Oct  7 23:35:44 UTC 2015:stop
 * ovs-vswitchd is not running
 * ovsdb-server is not running
Wed Oct  7 23:35:44 UTC 2015:load-kmod
Wed Oct  7 23:35:44 UTC 2015:start --system-id=random --no-run-as-root
ovsdb-server: /var/run/openvswitch/ovsdb-server.pid.tmp: create failed
(Permission denied)

I guess this was happening because that directory still belonged to
the root user after the upgrade.


>
> ----
> This patch does not include changes to the ipsec package. Ansis has
> other plans for updating it.

Yeah, I will have to figure out how to do this from Python daemons. I
guess we have to synchronize our changes so that we don't break IPsec.

> ---
>  NEWS                                       |  3 ++-
>  debian/automake.mk                         |  1 +
>  debian/openvswitch-common.postinst         | 42 
> ++++++++++++++++++++++++++++++
>  debian/openvswitch-pki.postinst            |  2 ++
>  debian/openvswitch-switch.init             |  1 +
>  debian/openvswitch-switch.logrotate        |  2 +-
>  debian/openvswitch-switch.postinst         |  3 +++
>  debian/openvswitch-testcontroller.init     |  3 ++-
>  debian/openvswitch-testcontroller.postinst |  2 ++
>  debian/openvswitch-vtep.init               |  8 +++++-
>  10 files changed, 63 insertions(+), 4 deletions(-)
>  create mode 100755 debian/openvswitch-common.postinst
>
> diff --git a/NEWS b/NEWS
> index cdf2815..8f0e5b6 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -23,7 +23,8 @@ Post-v2.4.0
>     - Dropped support for GRE64 tunnel.
>     - Mark --syslog-target argument as deprecated.  It will be removed in
>       the next OVS release.
> -   - Added --user option to all daemons
> +   - Added --user option to all daemons.
> +   - Debain package starts daemons as the 'ovs' user.
s/Debain/Debian
>
>
>  v2.4.0 - 20 Aug 2015
> diff --git a/debian/automake.mk b/debian/automake.mk
> index c29a560..3092569 100644
> --- a/debian/automake.mk
> +++ b/debian/automake.mk
> @@ -8,6 +8,7 @@ EXTRA_DIST += \
>         debian/dkms.conf.in \
>         debian/dirs \
>         debian/openvswitch-common.dirs \
> +       debian/openvswitch-common.postinst \
>         debian/openvswitch-common.docs \
>         debian/openvswitch-common.install \
>         debian/openvswitch-common.manpages \
> diff --git a/debian/openvswitch-common.postinst 
> b/debian/openvswitch-common.postinst
> new file mode 100755
> index 0000000..c90ab5a
> --- /dev/null
> +++ b/debian/openvswitch-common.postinst
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +# postinst script for openvswitch-switch
Copy-paste error: This is openvswitch-common and not
openvswitch-switch postinst script

> +#
> +# see: dh_installdeb(1)
> +
> +set -e
> +
> +# summary of how this script can be called:
> +#        * <postinst> `configure' <most-recently-configured-version>
> +#        * <old-postinst> `abort-upgrade' <new version>
> +#        * <conflictor's-postinst> `abort-remove' `in-favour' <package>
> +#          <new-version>
> +#        * <postinst> `abort-remove'
> +#        * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
> +#          <failed-install-package> <version> `removing'
> +#          <conflicting-package> <version>
> +# for details, see http://www.debian.org/doc/debian-policy/ or
> +# the debian-policy package
> +
> +case "$1" in
> +    configure)
> +        LOGDIR=/var/log/openvswitch
> +        # Create the ovs user and group.
> +        adduser --system --group --no-create-home --quiet ovs || true
There are useradd and adduser utilities. I tried *bare minimum* Debian
8 installation and adduser was not installed by default. Should you
add adduser to dependencies in debian/control file?


> +
> +        # Fix ownership and permissions.
> +        chown -R ovs:ovs $LOGDIR
> +        chmod -R 0770 $LOGDIR
You have probably thought more about this, but now "adm" group is
dropped for OVS logs. Do you see any issue with this?


> +        ;;
> +
> +    abort-upgrade|abort-remove|abort-deconfigure)
> +        ;;
> +
> +    *)
> +        echo "postinst called with unknown argument \`$1'" >&2
> +        exit 1
> +        ;;
> +esac
> +
> +#DEBHELPER#
> +
> +exit 0
> diff --git a/debian/openvswitch-pki.postinst b/debian/openvswitch-pki.postinst
> index f4705e9..030180d 100755
> --- a/debian/openvswitch-pki.postinst
> +++ b/debian/openvswitch-pki.postinst
> @@ -31,6 +31,8 @@ case "$1" in
>          if test ! -e /var/lib/openvswitch/pki; then
>              ovs-pki init
>          fi
> +
> +        chown ovs:ovs -R /var/lib/openvswitch/pki
Shouldn't changing user recursively for /var/lib/openvswitch be a
better approach?
>          ;;
>
>      abort-upgrade|abort-remove|abort-deconfigure)
> diff --git a/debian/openvswitch-switch.init b/debian/openvswitch-switch.init
> index 8e156da..febf414 100755
> --- a/debian/openvswitch-switch.init
> +++ b/debian/openvswitch-switch.init
> @@ -64,6 +64,7 @@ start () {
>      if test X"$FORCE_COREFILES" != X; then
>         set "$@" --force-corefiles="$FORCE_COREFILES"
>      fi
> +    set "$@" --no-run-as-root
>      set "$@" $OVS_CTL_OPTS
>      "$@" || exit $?
>      if [ "$2" = "start" ] && [ "$READ_INTERFACES" != "no" ]; then
> diff --git a/debian/openvswitch-switch.logrotate 
> b/debian/openvswitch-switch.logrotate
> index a7a71bd..be929b6 100644
> --- a/debian/openvswitch-switch.logrotate
> +++ b/debian/openvswitch-switch.logrotate
> @@ -1,7 +1,7 @@
>  /var/log/openvswitch/*.log {
>      daily
>      compress
> -    create 640 root adm
> +    create 640 ovs ovs
>      delaycompress
>      missingok
>      rotate 30
> diff --git a/debian/openvswitch-switch.postinst 
> b/debian/openvswitch-switch.postinst
> index 2464572..9183bdc 100755
> --- a/debian/openvswitch-switch.postinst
> +++ b/debian/openvswitch-switch.postinst
> @@ -33,6 +33,9 @@ case "$1" in
>                  fi
>              done
>         fi
> +
> +       # fix owner and permissions for /etc/openvswitch.
> +       chown ovs:ovs -R /etc/openvswitch

can you assume that this directory unconditionally exists (I believe
all debian scripts are run with set -e and you don't want them to
terminate prematurely)?

>          ;;
>
>      abort-upgrade|abort-remove|abort-deconfigure)
> diff --git a/debian/openvswitch-testcontroller.init 
> b/debian/openvswitch-testcontroller.init
> index 67b7a99..352c95d 100755
> --- a/debian/openvswitch-testcontroller.init
> +++ b/debian/openvswitch-testcontroller.init
> @@ -109,7 +109,7 @@ start_server() {
>      fi
>
>      if [ ! -d /var/run/openvswitch ]; then
> -        install -d -m 755 -o root -g root /var/run/openvswitch
> +        install -d -m 755 -o ovs -g ovs /var/run/openvswitch
if directory exists this will not change ownership, right?
>      fi
>
>      SSL_OPTS=
> @@ -139,6 +139,7 @@ start_server() {
>          if [ -z "$DAEMONUSER" ] ; then
>              start-stop-daemon --start --pidfile $PIDFILE \
>                          --exec $DAEMON -- --detach --pidfile=$PIDFILE \
> +                        --user ovs:ovs \
it seems inconsistent that in some places you use --user ovs:ovs but
in other --user "$OVS_USER":"$OVS_GROUP"


>                          $LISTEN $DAEMON_OPTS $SSL_OPTS
>              errcode=$?
>          else
> diff --git a/debian/openvswitch-testcontroller.postinst 
> b/debian/openvswitch-testcontroller.postinst
> index 7242b4a..e8584e2 100755
> --- a/debian/openvswitch-testcontroller.postinst
> +++ b/debian/openvswitch-testcontroller.postinst
> @@ -42,6 +42,8 @@ case "$1" in
>              chmod go+r cert.pem req.pem
>              umask $oldumask
>          fi
> +
> +        chown ovs:ovs -R /etc/openvswitch-testcontroller
>          ;;
>
>      abort-upgrade|abort-remove|abort-deconfigure)
> diff --git a/debian/openvswitch-vtep.init b/debian/openvswitch-vtep.init
> index ebf4e26..6fe02a1 100644
> --- a/debian/openvswitch-vtep.init
> +++ b/debian/openvswitch-vtep.init
> @@ -10,6 +10,8 @@
>  # Description:       Initializes the Open vSwitch VTEP emulator
>  ### END INIT INFO
>
> +OVS_USER=ovs
> +OVS_GROUP=ovs
>
>  # Include defaults if available
>  default=/etc/default/openvswitch-vtep
> @@ -40,17 +42,21 @@ start () {
>          cd /etc/openvswitch && ovs-pki req ovsclient && ovs-pki self-sign 
> ovsclient
>      fi
>
> +    chown -R "$OVS_USER":"$OVS_GROUP" /etc/openvswitch
> +    chown -R "$OVS_USER":"$OVS_GROUP" /var/run/openvswitch
> +
>      ovsdb-server --pidfile --detach --log-file --remote \
>          punix:/var/run/openvswitch/db.sock \
>          --remote=db:hardware_vtep,Global,managers \
>          --private-key=/etc/openvswitch/ovsclient-privkey.pem \
>          --certificate=/etc/openvswitch/ovsclient-cert.pem \
>          --bootstrap-ca-cert=/etc/openvswitch/vswitchd.cacert \
> +        --user "$OVS_USER":"$OVS_GROUP" \
>          /etc/openvswitch/conf.db /etc/openvswitch/vtep.db
>
>      modprobe openvswitch
>
> -    ovs-vswitchd --pidfile --detach --log-file \
> +    ovs-vswitchd --pidfile --detach --log-file --user 
> "$OVS_USER":"$OVS_GROUP" \
>          unix:/var/run/openvswitch/db.sock
>  }
>
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to