[ovs-dev] [Debian-non-root 4/4] Debian: start daemons as ovs(non-root) user

Andy Zhou azhou at nicira.com
Sat Oct 10 00:24:03 UTC 2015


On Thu, Oct 8, 2015 at 4:01 PM, Ansis Atteka <aatteka at nicira.com> wrote:
> 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.

I think I found the bug; it should be fixed in v2.
>
>>>
>>>>
>>>> ----
>>>> 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:
Before my changes /var/log/openvswitch was set to allow read access by
"others'. The v2 just
posted keeps this setting.
>
> 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.
I think SElinux may post a stricter restriction on what script can do.
I think we should
revisit this taking SElinux into consideration. v2 changes does not
fully address those
issues yet.
>
>>>
>>>>          ;;
>>>>
>>>>      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.
I have adopted #2 for V2.  I can't figure out a clean way to implement #1.

Overall, thanks for testing out the changes and feedbacks. They are
very helpful.



More information about the dev mailing list