On Wed, Oct 7, 2015 at 6:49 PM, Ansis Atteka <aatt...@nicira.com> wrote:
> 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.
>
Is possible you have your selinux changes already applied? It worked
in my set up.
>
>> 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
Thanks, will fix.
>
>> 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.
>
The error mentioned above will cause this.
>
>>
>> ----
>> 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?
>
Good catch. I will add adduser as dependency.
>
>> +
>> + # 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?
>
This is an area I'd like to get some input. Should we add ovs to the
adm group by default and
set the ownership of those log files to ovs:adm?
>
>> + ;;
>> +
>> + 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?
Probably. I see that this is only package that creates
/var/lib/openvswitch, but I don't see any other directory
being created in addition to pki. I could be wrong since I don't
install this package often.
>> ;;
>>
>> 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)?
It is listed in openvswitch-switch.dirs, not enough?
>
>> ;;
>>
>> 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?
Yes, This is a bug. Thanks.
>> 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"
I used it in openvswitch-vtep.init since there are multiple
references. ovs:ovs is used when
it is a single change. What's your preference?
>
>
>> $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
