| Message ID | 20160922005437.9955-1-joe@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Sep 22, 2016 at 3:54 AM, Joe Stringer <joe@ovn.org> wrote: > ovs-lib creates several directories directly from the script, but > doesn't make any attempt to ensure that the correct SELinux context is > applied to these directories. As a result, the created directories end > up with type var_run_t rather than openvswitch_var_run_t. > > During reboot using a tmpfs for /var/run, startup scripts will invoke > ovs-lib to create these directories with the wrong context. If SELinux > is enabled, OVS will fail to start as it cannot write to this directory. > > Fix the issue by sprinkling "restorecon" in each of the places where > directories are created. In practice, many of these should otherwise be > handled by packaging scripts but if they exist then we should ensure the > correct SELinux context is set. > > On systems where 'restorecon' is unavailable, this should be a no-op. > > VMware-BZ: #1732672 > > Signed-off-by: Joe Stringer <joe@ovn.org> Acked-by: Ansis Atteka <aatteka@ovn.org> I could give Tested-by, but only in 12 hours, if you are willing to wait. One thing that caught my attention is that "restorecon -R /" may take really long time. I guess, none of the path variables expand to / or any other directory that has bunch of files by default in it, do they? Also, as an optimization - would it make sense to call "restorecon ..." only if "test -d ..." returned false? > --- > Fortunately, the 'install' command comes with a handy '-Z' option which > should set the default SELinux context for a file when creating it. > Unfortunately, this doesn't work the way we need it to - rather than > taking the correct context for this particular file, it seems to take > some generic global default context so it doesn't fix the problem. Using > 'restorecon' seems like the most robust way to address this. > --- > utilities/ovs-lib.in | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index cbad85a36007..a6c446a9fbec 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -157,15 +157,18 @@ start_daemon () { > > # drop core files in a sensible place > test -d "$DAEMON_CWD" || install -d -m 755 -o root -g root "$DAEMON_CWD" > + restorecon -R "$DAEMON_CWD" >/dev/null 2>&1 > set "$@" --no-chdir > cd "$DAEMON_CWD" > > # log file > test -d "$logdir" || install -d -m 755 -o root -g root "$logdir" > + restorecon -R "$logdir" >/dev/null 2>&1 > set "$@" --log-file="$logdir/$daemon.log" > > # pidfile and monitoring > test -d "$rundir" || install -d -m 755 -o root -g root "$rundir" > + restorecon -R "$rundir" >/dev/null 2>&1 > set "$@" --pidfile="$rundir/$daemon.pid" > set "$@" --detach > test X"$MONITOR" = Xno || set "$@" --monitor > @@ -381,6 +384,7 @@ upgrade_db () { > if test ! -e "$DB_FILE"; then > log_warning_msg "$DB_FILE does not exist" > install -d -m 755 -o root -g root `dirname $DB_FILE` > + restorecon -R `dirname $DB_FILE` >/dev/null 2>&1 > create_db "$DB_FILE" "$DB_SCHEMA" > elif test X"`ovsdb_tool needs-conversion "$DB_FILE" "$DB_SCHEMA"`" != Xno; then > # Back up the old version. > -- > 2.9.3 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On 22 September 2016 at 12:55, Ansis Atteka <aatteka@nicira.com> wrote: > On Thu, Sep 22, 2016 at 3:54 AM, Joe Stringer <joe@ovn.org> wrote: >> ovs-lib creates several directories directly from the script, but >> doesn't make any attempt to ensure that the correct SELinux context is >> applied to these directories. As a result, the created directories end >> up with type var_run_t rather than openvswitch_var_run_t. >> >> During reboot using a tmpfs for /var/run, startup scripts will invoke >> ovs-lib to create these directories with the wrong context. If SELinux >> is enabled, OVS will fail to start as it cannot write to this directory. >> >> Fix the issue by sprinkling "restorecon" in each of the places where >> directories are created. In practice, many of these should otherwise be >> handled by packaging scripts but if they exist then we should ensure the >> correct SELinux context is set. >> >> On systems where 'restorecon' is unavailable, this should be a no-op. >> >> VMware-BZ: #1732672 >> >> Signed-off-by: Joe Stringer <joe@ovn.org> > > Acked-by: Ansis Atteka <aatteka@ovn.org> > > I could give Tested-by, but only in 12 hours, if you are willing to wait. I would appreciate that. I'd like to get this in v2.6, but I think we have a little bit of time for that. > One thing that caught my attention is that "restorecon -R /" may take > really long time. I guess, none of the path variables expand to / or > any other directory that has bunch of files by default in it, do they? > > Also, as an optimization - would it make sense to call "restorecon > ..." only if "test -d ..." returned false? I think this is reasonable. I sent a v2 to do this, and not use "-R". If this script is creating the directory, then -R is unnecessary: http://openvswitch.org/pipermail/dev/2016-September/079848.html
diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in index cbad85a36007..a6c446a9fbec 100644 --- a/utilities/ovs-lib.in +++ b/utilities/ovs-lib.in @@ -157,15 +157,18 @@ start_daemon () { # drop core files in a sensible place test -d "$DAEMON_CWD" || install -d -m 755 -o root -g root "$DAEMON_CWD" + restorecon -R "$DAEMON_CWD" >/dev/null 2>&1 set "$@" --no-chdir cd "$DAEMON_CWD" # log file test -d "$logdir" || install -d -m 755 -o root -g root "$logdir" + restorecon -R "$logdir" >/dev/null 2>&1 set "$@" --log-file="$logdir/$daemon.log" # pidfile and monitoring test -d "$rundir" || install -d -m 755 -o root -g root "$rundir" + restorecon -R "$rundir" >/dev/null 2>&1 set "$@" --pidfile="$rundir/$daemon.pid" set "$@" --detach test X"$MONITOR" = Xno || set "$@" --monitor @@ -381,6 +384,7 @@ upgrade_db () { if test ! -e "$DB_FILE"; then log_warning_msg "$DB_FILE does not exist" install -d -m 755 -o root -g root `dirname $DB_FILE` + restorecon -R `dirname $DB_FILE` >/dev/null 2>&1 create_db "$DB_FILE" "$DB_SCHEMA" elif test X"`ovsdb_tool needs-conversion "$DB_FILE" "$DB_SCHEMA"`" != Xno; then # Back up the old version.
ovs-lib creates several directories directly from the script, but doesn't make any attempt to ensure that the correct SELinux context is applied to these directories. As a result, the created directories end up with type var_run_t rather than openvswitch_var_run_t. During reboot using a tmpfs for /var/run, startup scripts will invoke ovs-lib to create these directories with the wrong context. If SELinux is enabled, OVS will fail to start as it cannot write to this directory. Fix the issue by sprinkling "restorecon" in each of the places where directories are created. In practice, many of these should otherwise be handled by packaging scripts but if they exist then we should ensure the correct SELinux context is set. On systems where 'restorecon' is unavailable, this should be a no-op. VMware-BZ: #1732672 Signed-off-by: Joe Stringer <joe@ovn.org> --- Fortunately, the 'install' command comes with a handy '-Z' option which should set the default SELinux context for a file when creating it. Unfortunately, this doesn't work the way we need it to - rather than taking the correct context for this particular file, it seems to take some generic global default context so it doesn't fix the problem. Using 'restorecon' seems like the most robust way to address this. --- utilities/ovs-lib.in | 4 ++++ 1 file changed, 4 insertions(+)