[ovs-dev] [Debian-non-root 4/4] Debian: start daemons as ovs(non-root) user
Ansis Atteka
aatteka at nicira.com
Thu Oct 8 23:01:50 UTC 2015
On Wed, Oct 7, 2015 at 8:20 PM, Andy Zhou <azhou at nicira.com> wrote:
> On Wed, Oct 7, 2015 at 6:49 PM, Ansis Atteka <aatteka at nicira.com> wrote:
>> On Mon, Oct 5, 2015 at 6:38 PM, Andy Zhou <azhou at 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.
This is Debian so SElinux (and my RHEL patch) does not come into
picture here. Will spend a little more time to understand what exactly
happened, but to give a hint I tried to install packages *one by one*
(opposed to a single dpkg -i command) so perhaps this might have to do
something with the order if it wasn't my setup itself.
>>
>>> 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 <azhou at 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.
The error I mentioned above was for /etc/openvswitch. This is for
/var/run/openvswitch/. And they happened independently.
>>
>>>
>>> ----
>>> 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?
This is the best I could find:
http://serverfault.com/questions/485473/what-is-the-canonical-use-for-the-sys-and-adm-groups
Basically after your patch I can't see
/var/log/openvswitch/ovs-vswitchd.log anymore with my regular Ubuntu
user. However, on Ubuntu there is already precedent with
speech-dispatcher that has the same behavior:
aatteka at aatteka-MacBookPro:/var/log$ cat speech-dispatcher/
cat: speech-dispatcher/: Permission denied
>>
>>> + ;;
>>> +
>>> + 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.
I am a bit afraid that we have plenty of OVS packages creating
sub-directories in other package directories (either from debian.dirs
file, postinst script, init.d script or the daemon itself). I really
don't have a good alternative, but this is something, I am afraid,
could get easily out of hand and become hard to maintain, if we will
be calling mkdir and chown from various places.
>>> ;;
>>>
>>> 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?
I think yesterday offline we discussed something among the lines where
OVS should be able to handle gracefully corner case scenario where
unknowing user deletes /etc/openvswitch directory. I think we got into
this discussion because we tried to understand rationale on why OVS
init.d script creates /etc/openvswitch directory if it is not present
already.
aatteka at aatteka-PowerEdge-T110:~/Git/ovs$ sudo rm -rf /etc/openvswitch/
aatteka at aatteka-PowerEdge-T110:~/Git/ovs$ sudo dpkg-reconfigure
openvswitch-switch
chown: cannot access ‘/etc/openvswitch’: No such file or directory
Overall, I think we should have a clear stance on what OVS should do
in such corner cases - either attempt to recover system or tell user
that if he deleted /etc/openvswitch directory then he should figure
out on his own how to recover setup. I haven't looked in Debian
Maintainerš book if it has any recommendations about this subject.
>>
>>> ;;
>>>
>>> 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
By they way DAEMONUSER and OVS_USER seem to replicate the same
use-cases. Do we really need both?
>>> 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?
each time you hard code "ovs" (or "ovs:ovs") user you are repeating
yourself. The other place in this postinst script, I believe, is
"install" command:
aatteka at aatteka-PowerEdge-T110:~/Git/ovs$ cat
debian/openvswitch-testcontroller.init | grep ovs
DAEMON=/usr/bin/ovs-testcontroller # Introduce the server's location here
NAME=ovs-testcontroller # Introduce the short server's name here
DESC=ovs-testcontroller # Introduce a short description here
install -d -m 755 -o ovs -g ovs /var/run/openvswitch
--user ovs:ovs \
Here are my preferences:
1. define "ovs" user and group in one place and somehow pass it to all
postinst scripts. So that if one day you would need to change user
from "ovs" to something else a single line change would be sufficient.
2. define "ovs" user and group at most once in each postinst script.
So that if one day you would need to change user from "ovs" to
something else a single line change in each postinst script would be
sufficient.
3. define "ovs" user and group inline or sometimes in postinst script.
>
>>
>>
>>> $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 at openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list