[apparmor] [PATCH 2/2] tests: Fix onexec.sh races by using the transition test program
Tyler Hicks
tyhicks at canonical.com
Fri Jun 24 22:15:54 UTC 2016
The onexec.sh test has periodically exhibited unexplicable failures that
are possibly due to race conditions when onexec.sh is verifying the
/proc/PID/attr/{current,exec} values of the process under test. This
patch attempts to solve the flaky test failures by removing the need for
IPC to coordinate between the test script and the test program.
The old onexec test program is removed and the transition test program
is used instead. This allows for the test script to tell the transition
test program what its current and exec procattr labels should be via
command line options.
Since IPC is no longer needed, the signal:ALL allow rule can be dropped
from the test profile. A new allow rule is needed to grant reading of
/proc/*/attr/{current,exec} since transition must verify the contents of
these files.
Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
tests/regression/apparmor/Makefile | 1 -
tests/regression/apparmor/onexec.c | 63 ----------------
tests/regression/apparmor/onexec.sh | 147 +++++++++---------------------------
3 files changed, 35 insertions(+), 176 deletions(-)
delete mode 100644 tests/regression/apparmor/onexec.c
diff --git a/tests/regression/apparmor/Makefile b/tests/regression/apparmor/Makefile
index 8d75c69..198ca42 100644
--- a/tests/regression/apparmor/Makefile
+++ b/tests/regression/apparmor/Makefile
@@ -73,7 +73,6 @@ SRC=access.c \
at_secure.c \
introspect.c \
changeprofile.c \
- onexec.c \
changehat.c \
changehat_fork.c \
changehat_misc.c \
diff --git a/tests/regression/apparmor/onexec.c b/tests/regression/apparmor/onexec.c
deleted file mode 100644
index f9beb3a..0000000
--- a/tests/regression/apparmor/onexec.c
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * Copyright (C) 2002-2005 Novell/SUSE
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation, version 2 of the
- * License.
- */
-
-#include <stdio.h>
-#include <unistd.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <string.h>
-#include <stdlib.h>
-#include <signal.h>
-#include <linux/unistd.h>
-
-#include <sys/apparmor.h>
-#include "changehat.h"
-
-int main(int argc, char *argv[])
-{
- int rc = 0;
-
- extern char **environ;
-
- if (argc < 3){
- fprintf(stderr, "usage: %s profile executable args\n",
- argv[0]);
- return 1;
- }
-
- /* change profile if profile name != nochange */
- if (strcmp(argv[1], "nochange") != 0){
- rc = aa_change_onexec(argv[1]);
- if (rc == -1){
- fprintf(stderr, "FAIL: change_onexec %s failed - %s\n",
- argv[1], strerror(errno));
- exit(errno);
- }
- }
-
- /* stop after onexec and wait to for continue before exec so
- * caller can introspect task */
- rc = kill(getpid(), SIGSTOP);
- if (rc == -1){
- fprintf(stderr, "FAIL: signal to self failed - %s\n",
- strerror(errno));
- exit(errno);
- }
-
- (void)execve(argv[2], &argv[2], environ);
- /* exec failed, kill outselves to flag parent */
-
- rc = errno;
-
- fprintf(stderr, "FAIL: exec to '%s' failed\n", argv[2]);
-
- return rc;
-}
diff --git a/tests/regression/apparmor/onexec.sh b/tests/regression/apparmor/onexec.sh
index 9bfacd6..922ea25 100644
--- a/tests/regression/apparmor/onexec.sh
+++ b/tests/regression/apparmor/onexec.sh
@@ -18,6 +18,7 @@ bin=$pwd
. $bin/prologue.inc
+settest transition
file=$tmpdir/file
subfile=$tmpdir/file2
okperm=rw
@@ -29,63 +30,11 @@ fqsubtest="$fqsubbase//$subtest"
subtest2="$pwd//sub2"
subtest3="$pwd//sub3"
-onexec="/proc/*/attr/exec"
+exec_w="/proc/*/attr/exec:w"
+attrs_r="/proc/*/attr/{current,exec}:r"
touch $file $subfile
-check_exec()
-{
- local rc
- local actual
- local desc="$1"
- local pid="$2"
- local expected="$3"
- actual=`cat /proc/${pid}/attr/exec 2>/dev/null`
- rc=$?
-
- # /proc/${pid}/attr/exec returns invalid argument if onexec has not been called
- if [ $rc -ne 0 ] ; then
- if [ "${expected}" == "nochange" ] ; then
- return 0
- fi
- echo "ONEXEC (${desc}) - exec transition not set"
- return $rc
- fi
- if [ "${actual% (*)}" != "${expected}" ] ; then
- echo "ONEXEC (${desc}) - check exec '${actual% (*)}' != expected '${expected}'"
- return 1
- fi
-
- return 0
-}
-
-check_current()
-{
- local rc
- local actual
- local desc="$1"
- local pid="$2"
- local expected="$3"
- actual=`cat /proc/${pid}/attr/current 2>/dev/null`
- rc=$?
-
- # /proc/${pid}/attr/current return enoent if the onexec process already exited due to error
- if [ $rc -ne 0 ] ; then
- # These assume a check has already been done to see if the
- # task is still around
- echo -n "ONEXEC - check current ($1): "
- cat /proc/${pid}/attr/current
- return $rc
- fi
-
- if [ "${actual% (*)}" != "${expected}" ] ; then
- echo "ONEXEC - check current (${desc}) '${actual% (*)}' != expected '${expected}'"
- return 1
- fi
-
- return 0
-}
-
do_test()
{
local desc="$1"
@@ -94,34 +43,12 @@ do_test()
local res="$4"
shift 4
- #ignore prologue.inc error trapping that catches our subfn return values
-
- runtestbg "ONEXEC $desc ($prof -> $target_prof)" $res $target_prof "$@"
- # check that transition does not happen before exec, and that transition
- # is set
-
- # give the onexec process a chance to run
- sleep 0.05
-
- # check that task hasn't exited because change_onexec failed
- if ! [ -d "/proc/${_pid}" ] ; then
- checktestfg
- return
- fi
-
- if ! check_current "${desc}" $_pid $prof ; then
- checktestfg
- return
- fi
-
- if ! check_exec "${desc}" $_pid $target_prof ; then
- checktestfg
- return
+ desc="ONEXEC $desc ($prof -> $target_prof)"
+ if [ "$target_prof" == "nochange" ] ; then
+ runchecktest "$desc" $res -l "$prof" -- "$@"
+ else
+ runchecktest "$desc" $res -O "$target_prof" -l "$prof" -L "$target_prof" -- "$@"
fi
-
- kill -CONT $_pid
-
- checktestbg
}
@@ -146,59 +73,55 @@ do_test "override px" unconfined $bin/rw pass $bin/open $file
#------
-# NOTE: test program pauses for the driver script to catch up by sending
-# and recieving SIGSTOP/SIGCONT, so the onexec program needs access to
-# signals (this is not a script to test signal mediation)
-
# ONEXEC from CONFINED - don't change profile, open can't exec
-genprofile 'change_profile->':$bin/rw $onexec:w signal:ALL
-do_test "no px perm" $bin/onexec nochange fail $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r
+do_test "no px perm" $test nochange fail $bin/open $file
# ONEXEC from CONFINED - don't change profile, open is run unconfined
-genprofile 'change_profile->':$bin/rw $bin/open:rux $onexec:w signal:ALL
-do_test "nochange rux" $bin/onexec nochange pass $bin/open $file
+genprofile 'change_profile->':$bin/rw $bin/open:rux $exec_w $attrs_r
+do_test "nochange rux" $test nochange pass $bin/open $file
# ONEXEC from CONFINED - don't change profile, open is run confined without necessary perms
-genprofile 'change_profile->':$bin/rw $onexec:w signal:ALL -- image=$bin/open $file:rw
-do_test "nochange px - no px perm" $bin/onexec nochange fail $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r -- image=$bin/open $file:rw
+do_test "nochange px - no px perm" $test nochange fail $bin/open $file
# ONEXEC from CONFINED - don't change profile, open is run confined without necessary perms
-genprofile 'change_profile->':$bin/rw $bin/open:rpx $onexec:w signal:ALL -- image=$bin/open
-do_test "nochange px - no file perm" $bin/onexec nochange fail $bin/open $file
+genprofile 'change_profile->':$bin/rw $bin/open:rpx $exec_w $attrs_r -- image=$bin/open
+do_test "nochange px - no file perm" $test nochange fail $bin/open $file
# ONEXEC from CONFINED - target does NOT exist
-genprofile 'change_profile->':$bin/open $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
-do_test "noexist px" $bin/onexec noexist fail $bin/open $file
+genprofile 'change_profile->':$bin/open $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
+do_test "noexist px" $test noexist fail $bin/open $file
# ONEXEC from CONFINED - change to rw profile, no exec profile to override
-genprofile 'change_profile->':$bin/rw $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw
-do_test "change profile - override rix" $bin/onexec $bin/rw pass $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw
+do_test "change profile - override rix" $test $bin/rw pass $bin/open $file
-# ONEXEC from CONFINED - change to rw profile, no exec profile to override, no explicit access to /proc/*/attr/exec
-genprofile 'change_profile->':$bin/rw signal:ALL -- image=$bin/rw $bin/open:rix $file:rw
-do_test "change profile - no onexec:w" $bin/onexec $bin/rw pass $bin/open $file
+# ONEXEC from CONFINED - change to rw profile, no exec profile to override, no explicit write access to /proc/*/attr/exec
+genprofile 'change_profile->':$bin/rw $attrs_r -- image=$bin/rw $bin/open:rix $file:rw
+do_test "change profile - no exec_w" $test $bin/rw pass $bin/open $file
# ONEXEC from CONFINED - don't change profile, make sure exec profile is applied
-genprofile 'change_profile->':$bin/rw $onexec:w $bin/open:rpx signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open $file:rw
-do_test "nochange px" $bin/onexec nochange pass $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r $bin/open:rpx -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open $file:rw
+do_test "nochange px" $test nochange pass $bin/open $file
# ONEXEC from CONFINED - change to rw profile, override regular exec profile, exec profile doesn't have perms
-genprofile 'change_profile->':$bin/rw $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
-do_test "override px" $bin/onexec $bin/rw pass $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
+do_test "override px" $test $bin/rw pass $bin/open $file
# ONEXEC from - change to rw profile, override regular exec profile, exec profile has perms, rw doesn't
-genprofile 'change_profile->':$bin/rw $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix -- image=$bin/open $file:rw
-do_test "override px" $bin/onexec $bin/rw fail $bin/open $file
+genprofile 'change_profile->':$bin/rw $exec_w $attrs_r -- image=$bin/rw $bin/open:rix -- image=$bin/open $file:rw
+do_test "override px" $test $bin/rw fail $bin/open $file
# ONEXEC from COFINED - change to rw profile via glob rule, override exec profile, exec profile doesn't have perms
-genprofile 'change_profile->':/** $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
-do_test "glob override px" $bin/onexec $bin/rw pass $bin/open $file
+genprofile 'change_profile->':/** $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
+do_test "glob override px" $test $bin/rw pass $bin/open $file
# ONEXEC from COFINED - change to exec profile via glob rule, override exec profile, exec profile doesn't have perms
-genprofile 'change_profile->':/** $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
-do_test "glob override px" $bin/onexec $bin/open fail $bin/open $file
+genprofile 'change_profile->':/** $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open
+do_test "glob override px" $test $bin/open fail $bin/open $file
# ONEXEC from COFINED - change to exec profile via glob rule, override exec profile, exec profile has perms
-genprofile 'change_profile->':/** $onexec:w signal:ALL -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open $file:rw
-do_test "glob override px" $bin/onexec $bin/rw pass $bin/open $file
+genprofile 'change_profile->':/** $exec_w $attrs_r -- image=$bin/rw $bin/open:rix $file:rw -- image=$bin/open $file:rw
+do_test "glob override px" $test $bin/rw pass $bin/open $file
--
2.7.4
More information about the AppArmor
mailing list