Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Markos Chandras]

Hello,

On 08/02/2018 08:06 PM, Timothy Redaelli wrote:
> 
> This is, hopefully, the correct git-diff:
> 
> diff --git a/utilities/ovs-lib.in 
> b/utilities/ovs-lib.in 
> index c3b76ec94..33776aac7 100644
> --- a/utilities/ovs-lib.in 
> +++ b/utilities/ovs-lib.in 
> @@ -389,7 +389,10 @@ move_ip_routes () {
> 
>  ovsdb_tool () {
>      if [ "$OVS_USER" != "" ]; then
> -        runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +        local uid=$(id -u "${OVS_USER%:*}")
> +        local gid=$(id -g "${OVS_USER%:*}")
> +        local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
> +        setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
> ovsdb-tool -vconsole:off "$@"
>      else
>          ovsdb-tool -vconsole:off "$@"
>      fi

I can also confirm that this seems to work on openSUSE as well. I will
add my Reviewed-by tag once this is properly submitted. Thank you.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Timothy Redaelli]

On Thu, Aug 2, 2018 at 4:58 PM, Timothy Redaelli 
wrote:
[...]
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index c3b76ec94..33776aac7 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -389,7 +389,10 @@ move_ip_routes () {
>
>  ovsdb_tool () {
>  if [ "$OVS_USER" != "" ]; then
> -runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +local uid=$(id -u "${OVS_USER%:*}")
> +local gid=$(id -g "${OVS_USER%:*}")
> +local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
> +setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
> ovsdb-tool -vconsole:off "$@"

^ I'm sorry, I had this long line wrapped.

>  else
>  ovsdb-tool -vconsole:off "$@"
>  fi

This is, hopefully, the correct git-diff:

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index c3b76ec94..33776aac7 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,10 @@ move_ip_routes () {

 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+local uid=$(id -u "${OVS_USER%:*}")
+local gid=$(id -g "${OVS_USER%:*}")
+local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
+setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Timothy Redaelli]

On Fri, Jul 27, 2018 at 8:16 PM, Aaron Conole  wrote:
> Markos Chandras  writes:
[...]
>
> Is it possible that the provided diff can fix most of the issue (rather
> than needing the corrective block in ovs-ctl)?  If so, I'd advocate for
> the smaller change instead.  I need to double check it on RHEL/Fedora.
>
> Flavio?  Timothy?
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 92f98ad92..8db887ef6 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -389,7 +389,7 @@ move_ip_routes () {
>
>  ovsdb_tool () {
>  if [ "$OVS_USER" != "" ]; then
> -runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
> +setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@"
>  else
>  ovsdb-tool -vconsole:off "$@"
>  fi

Hi,
I tested your solution with SUSE (Vagrant), RHEL7 and Fedora 28.

Unfortunately, as-is, it doesn't work on RHEL7 since the old setpriv
version we use on RHEL7
doesn't support an username, but it wants the userid (the numeric one).
Moreover if you don't specify --regid setpriv maintains 0 (root) as
group id and this can be bad.

I created a variant of this implementation that works on SUSE, RHEL7
and Fedora 28
and that fixes the problem, by keeping the same uid/gid/groups used by runuser.

Is it ok?

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index c3b76ec94..33776aac7 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,10 @@ move_ip_routes () {

 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+local uid=$(id -u "${OVS_USER%:*}")
+local gid=$(id -g "${OVS_USER%:*}")
+local groups=$(id -G "${OVS_USER%:*}" | tr ' ' ',')
+setpriv --reuid "$uid" --regid "$gid" --groups "$groups"
ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Ben Pfaff  writes:

> I don't understand the conclusion in this thread.  Does anyone want me
> to apply some patch?  Or should I stay tuned for further discussion?

Stay tuned for the exciting conclusion, please :)

> Thanks,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Ben Pfaff]

I don't understand the conclusion in this thread.  Does anyone want me
to apply some patch?  Or should I stay tuned for further discussion?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Markos Chandras  writes:

> Hello Aaron,
>
> On 18/07/18 16:24, Aaron Conole wrote:
>> 
>> I think there's actually a race condition here.  Most likely,
>> ovsdb-server doesn't need to be started before network.service.
>> 
>> Looking at the bug, I think we can unroll some of the dependencies that
>> each unit file has and get a cleaner systemd dependency chain, and
>> preserve the existing user downgrade.
>> 
>> I will install an OpenSUSE VM and work on it.  Strange that we don't see
>> this on the RHEL side.  I'll also try to reproduce that.
>> 
>> Thanks for pointing the issue out (and thanks to David and Franck on
>> your side).
>> 
>> -Aaron
>> 
>
> Great thank you. If you are using vagrant you can use the following
> steps for a reproducer
>
> vagrant init opensuse/openSUSE-15.0-x86_64
> vagrant up
> vagrant ssh
> (inside vagrant)
> sudo -s
> zypper -n in openvswitch
> systemctl enable openvswitch
> systemctl reboot
>
> checking the journald logs after the reboot should reveal the issue.
>
> Let me know if you need any help.

Is it possible that the provided diff can fix most of the issue (rather
than needing the corrective block in ovs-ctl)?  If so, I'd advocate for
the smaller change instead.  I need to double check it on RHEL/Fedora.

Flavio?  Timothy?

diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
index 92f98ad92..8db887ef6 100644
--- a/utilities/ovs-lib.in
+++ b/utilities/ovs-lib.in
@@ -389,7 +389,7 @@ move_ip_routes () {
 
 ovsdb_tool () {
 if [ "$OVS_USER" != "" ]; then
-runuser --user "${OVS_USER%:*}" -- ovsdb-tool -vconsole:off "$@"
+setpriv --reuid "${OVS_USER%:*}" ovsdb-tool -vconsole:off "$@"
 else
 ovsdb-tool -vconsole:off "$@"
 fi
--
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Flavio Leitner  writes:

> On Wed, Jul 18, 2018 at 11:24:43AM -0400, Aaron Conole wrote:
>> Markos Chandras  writes:
>> 
>> > When ovsdb-server is starting, it performs some DB steps such as
>> > creating and upgrading the OvS DB. When we are running as
>> > 'non-root' user, the 'runuser' tool is used to manage the privileges.
>> > However, when this happens during systemd boot, we observe the following
>> > errors in journald:
>> >
>> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add
>> > PIDs to scope's control group: No such process
>> > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
>> > openvswitch.
>> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed 
>> > state.
>> >
>> > According to the analysis performed on openSUSE bugzilla[1], it seems
>> > that ovsdb-server.service creates (via the call to runuser) a user
>> > session and therefore call pam_systemd which in its turn tries to start
>> > a systemd user instance: "user@474.service". However "user@474.service"
>> > is supposed to be started after systemd-user-sessions.service which is
>> > supposed to be started after network.target. Additionally,
>> > ovsdb-server.service uses Before=network.target hence the deadlock.
>> >
>> > We can workaround this by switching to 'root' user when we are
>> > performing this pre-startup steps and fixup the DB permissions before
>> > we start the actual ovsdb-server daemon.
>> >
>> > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
>> > Cc: Aaron Conole 
>> > Signed-off-by: Markos Chandras 
>> > ---
>> > Probably not the cleanest option so I am open to suggestions :)
>> 
>> I think there's actually a race condition here.  Most likely,
>> ovsdb-server doesn't need to be started before network.service.
>
> Unfortunately it does because network.service will ifup OVS bridges
> and ports and then we need ovsdb already running.

Good point.  Someday, I'll remember that the first time.

> fbl
>
>> Looking at the bug, I think we can unroll some of the dependencies that
>> each unit file has and get a cleaner systemd dependency chain, and
>> preserve the existing user downgrade.
>> 
>> I will install an OpenSUSE VM and work on it.  Strange that we don't see
>> this on the RHEL side.  I'll also try to reproduce that.
>> 
>> Thanks for pointing the issue out (and thanks to David and Franck on
>> your side).
>> 
>> -Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Flavio Leitner]

On Wed, Jul 18, 2018 at 11:24:43AM -0400, Aaron Conole wrote:
> Markos Chandras  writes:
> 
> > When ovsdb-server is starting, it performs some DB steps such as
> > creating and upgrading the OvS DB. When we are running as
> > 'non-root' user, the 'runuser' tool is used to manage the privileges.
> > However, when this happens during systemd boot, we observe the following
> > errors in journald:
> >
> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to 
> > scope's control group: No such process
> > Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> > openvswitch.
> > Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed 
> > state.
> >
> > According to the analysis performed on openSUSE bugzilla[1], it seems
> > that ovsdb-server.service creates (via the call to runuser) a user
> > session and therefore call pam_systemd which in its turn tries to start
> > a systemd user instance: "user@474.service". However "user@474.service"
> > is supposed to be started after systemd-user-sessions.service which is
> > supposed to be started after network.target. Additionally,
> > ovsdb-server.service uses Before=network.target hence the deadlock.
> >
> > We can workaround this by switching to 'root' user when we are
> > performing this pre-startup steps and fixup the DB permissions before
> > we start the actual ovsdb-server daemon.
> >
> > [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> > Cc: Aaron Conole 
> > Signed-off-by: Markos Chandras 
> > ---
> > Probably not the cleanest option so I am open to suggestions :)
> 
> I think there's actually a race condition here.  Most likely,
> ovsdb-server doesn't need to be started before network.service.

Unfortunately it does because network.service will ifup OVS bridges
and ports and then we need ovsdb already running.

fbl

> Looking at the bug, I think we can unroll some of the dependencies that
> each unit file has and get a cleaner systemd dependency chain, and
> preserve the existing user downgrade.
> 
> I will install an OpenSUSE VM and work on it.  Strange that we don't see
> this on the RHEL side.  I'll also try to reproduce that.
> 
> Thanks for pointing the issue out (and thanks to David and Franck on
> your side).
> 
> -Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Markos Chandras]

Hello Aaron,

On 18/07/18 16:24, Aaron Conole wrote:
> 
> I think there's actually a race condition here.  Most likely,
> ovsdb-server doesn't need to be started before network.service.
> 
> Looking at the bug, I think we can unroll some of the dependencies that
> each unit file has and get a cleaner systemd dependency chain, and
> preserve the existing user downgrade.
> 
> I will install an OpenSUSE VM and work on it.  Strange that we don't see
> this on the RHEL side.  I'll also try to reproduce that.
> 
> Thanks for pointing the issue out (and thanks to David and Franck on
> your side).
> 
> -Aaron
> 

Great thank you. If you are using vagrant you can use the following
steps for a reproducer

vagrant init opensuse/openSUSE-15.0-x86_64
vagrant up
vagrant ssh
(inside vagrant)
sudo -s
zypper -n in openvswitch
systemctl enable openvswitch
systemctl reboot

checking the journald logs after the reboot should reveal the issue.

Let me know if you need any help.

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Aaron Conole]

Markos Chandras  writes:

> When ovsdb-server is starting, it performs some DB steps such as
> creating and upgrading the OvS DB. When we are running as
> 'non-root' user, the 'runuser' tool is used to manage the privileges.
> However, when this happens during systemd boot, we observe the following
> errors in journald:
>
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to 
> scope's control group: No such process
> Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user 
> openvswitch.
> Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.
>
> According to the analysis performed on openSUSE bugzilla[1], it seems
> that ovsdb-server.service creates (via the call to runuser) a user
> session and therefore call pam_systemd which in its turn tries to start
> a systemd user instance: "user@474.service". However "user@474.service"
> is supposed to be started after systemd-user-sessions.service which is
> supposed to be started after network.target. Additionally,
> ovsdb-server.service uses Before=network.target hence the deadlock.
>
> We can workaround this by switching to 'root' user when we are
> performing this pre-startup steps and fixup the DB permissions before
> we start the actual ovsdb-server daemon.
>
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
> Cc: Aaron Conole 
> Signed-off-by: Markos Chandras 
> ---
> Probably not the cleanest option so I am open to suggestions :)

I think there's actually a race condition here.  Most likely,
ovsdb-server doesn't need to be started before network.service.

Looking at the bug, I think we can unroll some of the dependencies that
each unit file has and get a cleaner systemd dependency chain, and
preserve the existing user downgrade.

I will install an OpenSUSE VM and work on it.  Strange that we don't see
this on the RHEL side.  I'll also try to reproduce that.

Thanks for pointing the issue out (and thanks to David and Franck on
your side).

-Aaron
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] utilities: Run ovsdb-server pre-startup DB steps as root

[Markos Chandras]

When ovsdb-server is starting, it performs some DB steps such as
creating and upgrading the OvS DB. When we are running as
'non-root' user, the 'runuser' tool is used to manage the privileges.
However, when this happens during systemd boot, we observe the following
errors in journald:

Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Failed to add PIDs to 
scope's control group: No such process
Jun 21 07:32:57 virt systemd[1]: Failed to start Session c1 of user openvswitch.
Jun 21 07:32:57 virt systemd[1]: session-c1.scope: Unit entered failed state.

According to the analysis performed on openSUSE bugzilla[1], it seems
that ovsdb-server.service creates (via the call to runuser) a user
session and therefore call pam_systemd which in its turn tries to start
a systemd user instance: "user@474.service". However "user@474.service"
is supposed to be started after systemd-user-sessions.service which is
supposed to be started after network.target. Additionally,
ovsdb-server.service uses Before=network.target hence the deadlock.

We can workaround this by switching to 'root' user when we are
performing this pre-startup steps and fixup the DB permissions before
we start the actual ovsdb-server daemon.

[1]: https://bugzilla.suse.com/show_bug.cgi?id=1098630
Cc: Aaron Conole 
Signed-off-by: Markos Chandras 
---
Probably not the cleanest option so I am open to suggestions :)
---
 utilities/ovs-ctl.in | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 43c8f32b7..588f546fe 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -109,9 +109,15 @@ do_start_ovsdb () {
 if daemon_is_running ovsdb-server; then
 log_success_msg "ovsdb-server is already running"
 else
-# Create initial database or upgrade database schema.
-upgrade_db $DB_FILE $DB_SCHEMA || return 1
-
+# Create initial database or upgrade database schema. The runuser calls
+# in ovsdb_tool function will fail on system startup so we need to run
+# as root and fix permissions later on.
+[ "$OVS_USER" != "" ] && OVS_USER_OVSDB=${OVS_USER}
+OVS_USER="" upgrade_db $DB_FILE $DB_SCHEMA || return 1
+if [ ! -z "${OVS_USER_OVSDB+x}" ]; then
+OVS_USER=${OVS_USER_OVSDB}
+chown -R "$OVS_USER" $etcdir $dbdir
+fi
 # Start ovsdb-server.
 set ovsdb-server "$DB_FILE"
 for db in $EXTRA_DBS; do
-- 
2.18.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev