[LTP] [PATCH v3 0/6] DHCP tests and AppArmor/SELinux improvements

[LTP] [PATCH v3 0/6] DHCP tests and AppArmor/SELinux improvements

[Petr Vorel]

Hi,

another version of DHCP tests and AppArmor/SELinux improvements.
Changes v2->v3:
* Remove TST_SECURITY_WARN and try to detect affected test
automatically via grepping current policy with $TST_NEEDS_CMDS. There is
still printed warning on enforced AppArmor/SELinux even no policy is
found. Suggested by Alexey Kodanev.
* Warning is printed only on TFAIL. Suggested by Alexey Kodanev.
* Root check in tst_test.sh is moved to _tst_require_root()
* Add dhcpd and dhclient path fixes.

Petr Vorel (6):
  net/dhcp: Rename setup & cleanup functions
  shell: Add tst_security.sh helper
  net/dhcp: Use for allowed by AppArmor and SELinux
  net/dhcp: Check also default dhclient lease file path
  net/dhcp: Move print_dhcp_log() into dhcp library
  ver_linux: Print AppArmor and SELinux status

 testcases/lib/tst_security.sh           | 124 ++++++++++++++++++++++++
 testcases/lib/tst_test.sh               |  17 +++-
 testcases/network/dhcp/dhcp_lib.sh      |  24 ++++-
 testcases/network/dhcp/dhcpd_tests.sh   |  19 ++--
 testcases/network/dhcp/dnsmasq_tests.sh |  17 ++--
 ver_linux                               |  46 ++++++++-
 6 files changed, 219 insertions(+), 28 deletions(-)
 create mode 100644 testcases/lib/tst_security.sh

-- 
2.19.2

[LTP] [PATCH v3 1/6] net/dhcp: Rename setup & cleanup functions

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/dhcp/dhcp_lib.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/network/dhcp/dhcp_lib.sh b/testcases/network/dhcp/dhcp_lib.sh
index 29b8027de..08838a479 100755
--- a/testcases/network/dhcp/dhcp_lib.sh
+++ b/testcases/network/dhcp/dhcp_lib.sh
@@ -5,9 +5,9 @@
 #
 # Author:       Alexey Kodanev alexey.kodanev@oracle.com
 
-TST_SETUP="init"
+TST_SETUP="dhcp_lib_setup"
+TST_CLEANUP="dhcp_lib_cleanup"
 TST_TESTFUNC="test01"
-TST_CLEANUP="cleanup"
 TST_NEEDS_TMPDIR=1
 TST_NEEDS_ROOT=1
 TST_NEEDS_CMDS="cat $dhcp_name awk ip pgrep pkill dhclient"
@@ -36,7 +36,7 @@ stop_dhcp()
 	[ "$(pgrep -x $dhcp_name)" ] && return 1 || return 0
 }
 
-init()
+dhcp_lib_setup()
 {
 	if [ $TST_IPV6 ]; then
 		ip_addr="fd00:1:1:2::12/64"
@@ -69,7 +69,7 @@ init()
 		tst_brk TBROK "failed to add ip address"
 }
 
-cleanup()
+dhcp_lib_cleanup()
 {
 	[ -z "$veth_loaded" ] && return
 
-- 
2.19.2

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Petr Vorel]

It prints info about AppArmor and SELinux and allows to disable it.
This is due some false positives because improper usage or bugs
in AppArmor profiles (e.g. traceroute, dnsmasq).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: some of functions in tst_security.sh are meant to be used
also in tests (when AppArmor and SELinux has different paths).
---
 testcases/lib/tst_security.sh | 124 ++++++++++++++++++++++++++++++++++
 testcases/lib/tst_test.sh     |  17 +++--
 2 files changed, 136 insertions(+), 5 deletions(-)
 create mode 100644 testcases/lib/tst_security.sh

diff --git a/testcases/lib/tst_security.sh b/testcases/lib/tst_security.sh
new file mode 100644
index 000000000..68b47347f
--- /dev/null
+++ b/testcases/lib/tst_security.sh
@@ -0,0 +1,124 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
+
+if [ -z "$TST_LIB_LOADED" ]; then
+	echo "please load tst_test.sh first" >&2
+	exit 1
+fi
+
+[ -n "$TST_SECURITY_LOADED" ] && return 0
+TST_SECURITY_LOADED=1
+
+_tst_check_security_modules()
+{
+	local cmd
+	local profiles
+
+	if tst_apparmor_enabled; then
+		tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
+		profiles=
+		for cmd in $TST_NEEDS_CMDS; do
+			tst_apparmor_used_profile $cmd && profiles="$cmd $profiles"
+		done
+		[ -z "$profiles" ] && profiles="none"
+		tst_res TINFO "loaded AppArmor profiles: $profiles"
+	fi
+
+	if tst_selinux_enabled; then
+		tst_res TINFO "SELinux enabled in enforcing mode, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
+		profiles=
+		for cmd in $TST_NEEDS_CMDS; do
+			tst_selinux_used_profile $cmd && profiles="$cmd $profiles"
+		done
+		[ -z "$profiles" ] && profiles="none"
+		tst_res TINFO "loaded SELinux profiles: $profiles"
+	fi
+}
+
+# Detect whether AppArmor profiles are loaded
+# Return 0: profiles loaded, 1: none profile loaded or AppArmor disabled
+tst_apparmor_enabled()
+{
+	local f="/sys/module/apparmor/parameters/enabled"
+	[ -f "$f" ] && [ "$(cat $f)" = "Y" ]
+}
+
+# Detect whether AppArmor profile for command is enforced
+# tst_apparmor_used_profile CMD
+# Return 0: loaded profile for CMD
+# Return 1: no profile CMD
+tst_apparmor_used_profile()
+{
+	[ $# -eq 1 ] && tst_brk TCONF "usage tst_apparmor_used_profile CMD"
+	local cmd="$1"
+	grep -q "$cmd .*(enforce)" /sys/kernel/security/apparmor/profiles 2>/dev/null
+}
+
+# Detect whether SELinux is enabled in enforcing mode
+# Return 0: enabled in enforcing mode
+# Return 1: enabled in permissive mode or disabled
+tst_selinux_enabled()
+{
+	local f="$(_tst_get_enforce)"
+
+	[ -f "$f" ] && [ "$(cat $f)" = "1" ]
+}
+
+# Detect whether SELinux profile for command is enforced
+# tst_selinux_used_profile CMD
+# Return 0: loaded profile for CMD
+# Return 1: profile for CMD not loaded or seinfo not available
+tst_selinux_used_profile()
+{
+	[ $# -eq 1 ] && tst_brk TCONF "usage tst_selinux_used_profile CMD"
+	local cmd="$1"
+
+	if ! tst_cmd_available seinfo; then
+		if [ -z "$seinfo_warn_printed" ]; then
+			tst_res "install seinfo to find used SELinux profiles"
+			export seinfo_warn_printed=1
+		fi
+		return
+	fi
+	seinfo -t 2>/dev/null | grep -q $cmd
+}
+
+# Try disable AppArmor
+# Return 0: AppArmor disabled
+# Return > 0: failed to disable AppArmor
+tst_disable_apparmor()
+{
+	_tst_require_root
+	local f="aa-teardown"
+	local action
+
+	tst_cmd_available $f && { $f; return; }
+	f="/etc/init.d/apparmor"
+	if [ -f "$f" ]; then
+		for action in teardown kill stop; do
+			$f $action >/dev/null 2>&1 && return
+		done
+	fi
+}
+
+# Try disable SELinux
+# Return 0: SELinux disabled
+# Return > 0: failed to disable SELinux
+tst_disable_selinux()
+{
+	_tst_require_root
+	local f="$(_tst_get_enforce)"
+
+	[ -f "$f" ] && cat 0 > $f
+}
+
+# Get SELinux enforce file path
+_tst_get_enforce()
+{
+	local dir="/sys/fs/selinux"
+
+	[ -d "$dir" ] || dir="/selinux"
+	local f="$dir/enforce"
+	[ -f "$f" ] && echo "$f"
+}
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index e3770d005..333061028 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -34,6 +34,7 @@ export TST_TMPDIR_RHOST=0
 export TST_LIB_LOADED=1
 
 . tst_ansi_color.sh
+. tst_security.sh
 
 # default trap function
 trap "tst_brk TBROK 'test interrupted'" INT
@@ -67,6 +68,7 @@ _tst_do_exit()
 
 	if [ $TST_FAIL -gt 0 ]; then
 		ret=$((ret|1))
+		_tst_check_security_modules
 	fi
 
 	if [ $TST_BROK -gt 0 ]; then
@@ -376,6 +378,11 @@ _tst_setup_timer()
 	_tst_setup_timer_pid=$!
 }
 
+_tst_require_root()
+{
+	[ "$(id -ru)" != 0 ] && tst_brk TCONF "Must be super/root for this test!"
+}
+
 tst_run()
 {
 	local _tst_i
@@ -386,6 +393,7 @@ tst_run()
 	if [ -n "$TST_TEST_PATH" ]; then
 		for _tst_i in $(grep TST_ "$TST_TEST_PATH" | sed 's/.*TST_//; s/[="} \t\/:`].*//'); do
 			case "$_tst_i" in
+			DISABLE_APPARMOR|DISABLE_SELINUX);;
 			SETUP|CLEANUP|TESTFUNC|ID|CNT|MIN_KVER);;
 			OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
 			NEEDS_ROOT|NEEDS_TMPDIR|TMPDIR|NEEDS_DEVICE|DEVICE);;
@@ -421,11 +429,10 @@ tst_run()
 		tst_brk TBROK "Number of iterations (-i) must be > 0"
 	fi
 
-	if [ "$TST_NEEDS_ROOT" = 1 ]; then
-		if [ "$(id -ru)" != 0 ]; then
-			tst_brk TCONF "Must be super/root for this test!"
-		fi
-	fi
+	[ "$TST_NEEDS_ROOT" = 1 ] && _tst_require_root
+
+	[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
+	[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
 
 	tst_test_cmds $TST_NEEDS_CMDS
 	tst_test_drivers $TST_NEEDS_DRIVERS
-- 
2.19.2

[LTP] [PATCH v3 3/6] net/dhcp: Use for allowed by AppArmor and SELinux

[Petr Vorel]

TL;DR: fix paths for --log-facility and --dhcp-leasefile.

Path for log file expects AppArmor commit
7dce58987 ("dnsmasq: Adjust pattern for log files to comply SELinux")
Path is also suitable for SELinux.

dnsmasq uses /var/lib/misc/dnsmasq.leases path for lease file [1].
Fedora/RHEL/CentOS/Oracle Linux (RHEL*) use specific directory
/var/lib/dnsmasq/ [2] (since 2.41 [3]). /var/lib/misc/ also exists on
RHEL* in filesystem package (which is on every RHEL* system).

AppArmor allows us to use different log file in the directory /var/lib/misc/.
This path is not allowed by SELinux, which until now uses RHEL* default
path /var/lib/misc/, so use it when SELinux enabled.

Used directory is created in case it does not exist (and deleted
afterwards).

[1] http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=blob;f=src/config.h;h=762c49b586bb26fb05d0eceac87d28f939693a6f;hb=HEAD#l191
[2] https://src.fedoraproject.org/cgit/rpms/dnsmasq.git/tree/dnsmasq.spec#n67
[3] https://src.fedoraproject.org/cgit/rpms/dnsmasq.git/commit/?id=91d4b30e7b55bbb561547312e83ce4d709e505e2

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/dhcp/dhcp_lib.sh      |  8 ++++++++
 testcases/network/dhcp/dhcpd_tests.sh   | 14 ++++++++++++--
 testcases/network/dhcp/dnsmasq_tests.sh | 14 ++++++++++----
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/testcases/network/dhcp/dhcp_lib.sh b/testcases/network/dhcp/dhcp_lib.sh
index 08838a479..b8611f49d 100755
--- a/testcases/network/dhcp/dhcp_lib.sh
+++ b/testcases/network/dhcp/dhcp_lib.sh
@@ -67,12 +67,20 @@ dhcp_lib_setup()
 	tst_res TINFO "add $ip_addr to $iface0"
 	ip addr add $ip_addr dev $iface0 || \
 		tst_brk TBROK "failed to add ip address"
+
+	if [ ! -d "$lease_dir" ]; then
+		mkdir -p $lease_dir
+		lease_dir_added=1
+	fi
 }
 
 dhcp_lib_cleanup()
 {
 	[ -z "$veth_loaded" ] && return
 
+	[ "$lease_dir_added" = 1 ] && rm -rf $lease_dir
+	rm -f $lease_file
+
 	stop_dhcp
 
 	pkill -f "dhclient -$TST_IPVER $iface1"
diff --git a/testcases/network/dhcp/dhcpd_tests.sh b/testcases/network/dhcp/dhcpd_tests.sh
index ff46b825d..5d695bf24 100755
--- a/testcases/network/dhcp/dhcpd_tests.sh
+++ b/testcases/network/dhcp/dhcpd_tests.sh
@@ -10,6 +10,15 @@
 dhcp_name="dhcpd"
 
 . dhcp_lib.sh
+TST_SETUP="setup_dhcp"
+lease_dir="/var/lib/misc"
+lease_file="$lease_dir/dhcpd.leases_tst"
+
+setup_dhcp()
+{
+	[ "$TST_IPV6" ] && lease="$lease_dir/dhcpd6.leases_tst"
+	dhcp_lib_setup
+}
 
 setup_dhcpd_conf()
 {
@@ -30,8 +39,8 @@ setup_dhcpd_conf()
 
 start_dhcpd()
 {
-	touch tst_hdcpd.lease
-	dhcpd -lf tst_hdcpd.lease -$TST_IPVER $iface0 > tst_dhcpd.err 2>&1
+	touch $lease_file
+	dhcpd -lf $lease_file -$TST_IPVER $iface0 > tst_dhcpd.err 2>&1
 }
 
 start_dhcp()
@@ -67,6 +76,7 @@ start_dhcp6()
 cleanup_dhcp()
 {
 	[ -f dhcpd.conf ] && mv dhcpd.conf $DHCPD_CONF
+	rm -f $lease_file
 }
 
 print_dhcp_log()
diff --git a/testcases/network/dhcp/dnsmasq_tests.sh b/testcases/network/dhcp/dnsmasq_tests.sh
index ad5885c84..8e466dafe 100755
--- a/testcases/network/dhcp/dnsmasq_tests.sh
+++ b/testcases/network/dhcp/dnsmasq_tests.sh
@@ -9,9 +9,15 @@ dhcp_name="dnsmasq"
 
 . dhcp_lib.sh
 
+log="/var/log/dnsmasq.tst.log"
+
+lease_dir="/var/lib/misc"
+tst_selinux_enabled && lease_dir="/var/lib/dnsmasq"
+lease_file="$lease_dir/dnsmasq.tst.leases"
+
 common_opt="--no-hosts --no-resolv --dhcp-authoritative \
-	--log-facility=./tst_dnsmasq.log --interface=$iface0 \
-	--dhcp-leasefile=tst_dnsmasq.lease --port=0 --conf-file= "
+	--log-facility=$log --interface=$iface0 \
+	--dhcp-leasefile=$lease_file --port=0 --conf-file= "
 
 start_dhcp()
 {
@@ -33,12 +39,12 @@ start_dhcp6()
 
 cleanup_dhcp()
 {
-	rm -f tst_dnsmasq.log
+	rm -f $log
 }
 
 print_dhcp_log()
 {
-	cat tst_dnsmasq.log
+	cat $log
 }
 
 print_dhcp_version()
-- 
2.19.2

[LTP] [PATCH v3 4/6] net/dhcp: Check also default dhclient lease file path

[Petr Vorel]

/var/lib/dhclient/dhclient{6,}.leases is used on
Fedora/RHEL/CentOS/Oracle Linux (since dhcp 4.0 in 2008)
Check also default upstream path for dhclient on Linux.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/dhcp/dhcp_lib.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/network/dhcp/dhcp_lib.sh b/testcases/network/dhcp/dhcp_lib.sh
index b8611f49d..632b1a9b1 100755
--- a/testcases/network/dhcp/dhcp_lib.sh
+++ b/testcases/network/dhcp/dhcp_lib.sh
@@ -59,6 +59,7 @@ dhcp_lib_setup()
 	stop_dhcp || tst_brk TBROK "Failed to stop dhcp server"
 
 	dhclient_lease="/var/lib/dhclient/dhclient${TST_IPV6}.leases"
+	[ -f $dhclient_lease ] || dhclient_lease="/var/lib/dhcp/dhclient${TST_IPV6}.leases"
 	if [ -f $dhclient_lease ]; then
 		tst_res TINFO "backup dhclient${TST_IPV6}.leases"
 		mv $dhclient_lease .
-- 
2.19.2

[LTP] [PATCH v3 5/6] net/dhcp: Move print_dhcp_log() into dhcp library

[Petr Vorel]

Create default path for log file, but allow test to specify it.
This is needed for AppArmor respected paths, which was already defined
in 3e76b9896 ("net/dhcp: Use paths allowed by AppArmor for dnsmasq").

Also check if file exists. This is needed for dnsmasq --log-facility
switch when AppArmor enabled, because according to dnsmasq(8) errors
whilst reading configuration will still go to syslog. And then error
report of cat of missing file is misleading.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/dhcp/dhcp_lib.sh      | 7 +++++++
 testcases/network/dhcp/dhcpd_tests.sh   | 7 +------
 testcases/network/dhcp/dnsmasq_tests.sh | 5 -----
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/testcases/network/dhcp/dhcp_lib.sh b/testcases/network/dhcp/dhcp_lib.sh
index 632b1a9b1..c92117f47 100755
--- a/testcases/network/dhcp/dhcp_lib.sh
+++ b/testcases/network/dhcp/dhcp_lib.sh
@@ -38,6 +38,8 @@ stop_dhcp()
 
 dhcp_lib_setup()
 {
+	[ -z "$log" ] && log="$PWD/$(basename $0 '.sh').log"
+
 	if [ $TST_IPV6 ]; then
 		ip_addr="fd00:1:1:2::12/64"
 		ip_addr_check="fd00:1:1:2::100/64"
@@ -98,6 +100,11 @@ dhcp_lib_cleanup()
 	[ "$veth_loaded" = "no" ] && lsmod | grep -q '^veth ' && rmmod veth
 }
 
+print_dhcp_log()
+{
+	[ -f "$log" ] && cat $log
+}
+
 test01()
 {
 	local wicked
diff --git a/testcases/network/dhcp/dhcpd_tests.sh b/testcases/network/dhcp/dhcpd_tests.sh
index 5d695bf24..23dc8a45b 100755
--- a/testcases/network/dhcp/dhcpd_tests.sh
+++ b/testcases/network/dhcp/dhcpd_tests.sh
@@ -40,7 +40,7 @@ setup_dhcpd_conf()
 start_dhcpd()
 {
 	touch $lease_file
-	dhcpd -lf $lease_file -$TST_IPVER $iface0 > tst_dhcpd.err 2>&1
+	dhcpd -lf $lease_file -$TST_IPVER $iface0 > $log 2>&1
 }
 
 start_dhcp()
@@ -79,11 +79,6 @@ cleanup_dhcp()
 	rm -f $lease_file
 }
 
-print_dhcp_log()
-{
-	cat tst_dhcpd.err
-}
-
 print_dhcp_version()
 {
 	dhcpd --version 2>&1
diff --git a/testcases/network/dhcp/dnsmasq_tests.sh b/testcases/network/dhcp/dnsmasq_tests.sh
index 8e466dafe..f80ce1e0b 100755
--- a/testcases/network/dhcp/dnsmasq_tests.sh
+++ b/testcases/network/dhcp/dnsmasq_tests.sh
@@ -42,11 +42,6 @@ cleanup_dhcp()
 	rm -f $log
 }
 
-print_dhcp_log()
-{
-	cat $log
-}
-
 print_dhcp_version()
 {
 	dnsmasq --version | head -2
-- 
2.19.2

[LTP] [PATCH v3 6/6] ver_linux: Print AppArmor and SELinux status

[Petr Vorel]

+ add some helper functions

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 ver_linux | 46 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/ver_linux b/ver_linux
index 897571703..5994c0e0f 100755
--- a/ver_linux
+++ b/ver_linux
@@ -3,8 +3,35 @@
 # typical as you use for compilation/istallation. I use
 # /bin /sbin /usr/bin /usr/sbin /usr/local/bin, but it may
 # differ on your system.
-#
+
 PATH=/sbin:/usr/sbin:/bin:/usr/bin:$PATH
+
+tst_cmd_available()
+{
+	if type command > /dev/null 2>&1; then
+		command -v $1 > /dev/null 2>&1 || return 1
+	else
+		which $1 > /dev/null 2>&1
+		if [ $? -eq 0 ]; then
+			return 0
+		else
+			return 1
+		fi
+	fi
+}
+
+tst_cmd_run()
+{
+	local cmd="$1"
+	shift
+	tst_cmd_available $cmd && eval "$cmd $@"
+}
+
+is_enabled()
+{
+	[ -f "$1" ] && [ "$(cat $1)" = "Y" -o "$(cat $1)" = "1" ]
+}
+
 echo 'If some fields are empty or look unusual you may have an old version.'
 echo 'Compare to the current minimal requirements in Documentation/Changes.'
 
@@ -101,8 +128,19 @@ free
 
 echo
 echo 'cpuinfo:'
-if which lscpu > /dev/null 2>&1; then
-	lscpu
+tst_cmd_run lscpu || cat /proc/cpuinfo
+
+echo
+if is_enabled /sys/module/apparmor/parameters/enabled; then
+	echo 'AppArmor enabled'
+	tst_cmd_run aa-status
 else
-	cat /proc/cpuinfo
+	echo 'AppArmor disabled'
+fi
+
+echo
+
+if ! tst_cmd_run sestatus; then
+	printf 'SELinux mode: '
+	tst_cmd_run getenforce || echo 'unknown'
 fi
-- 
2.19.2

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Alexey Kodanev]

Hi Petr,
On 12/18/2018 04:02 AM, Petr Vorel wrote:
> It prints info about AppArmor and SELinux and allows to disable it.
> This is due some false positives because improper usage or bugs
> in AppArmor profiles (e.g. traceroute, dnsmasq).
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: some of functions in tst_security.sh are meant to be used
> also in tests (when AppArmor and SELinux has different paths).
> ---
>  testcases/lib/tst_security.sh | 124 ++++++++++++++++++++++++++++++++++
>  testcases/lib/tst_test.sh     |  17 +++--
>  2 files changed, 136 insertions(+), 5 deletions(-)
>  create mode 100644 testcases/lib/tst_security.sh
> 
> diff --git a/testcases/lib/tst_security.sh b/testcases/lib/tst_security.sh
> new file mode 100644
> index 000000000..68b47347f
> --- /dev/null
> +++ b/testcases/lib/tst_security.sh
> @@ -0,0 +1,124 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz>
> +
> +if [ -z "$TST_LIB_LOADED" ]; then
> +	echo "please load tst_test.sh first" >&2
> +	exit 1
> +fi
> +
> +[ -n "$TST_SECURITY_LOADED" ] && return 0
> +TST_SECURITY_LOADED=1
> +
> +_tst_check_security_modules()
> +{
> +	local cmd
> +	local profiles
> +
> +	if tst_apparmor_enabled; then
> +		tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
> +		profiles=
> +		for cmd in $TST_NEEDS_CMDS; do
> +			tst_apparmor_used_profile $cmd && profiles="$cmd $profiles"
> +		done
> +		[ -z "$profiles" ] && profiles="none"
> +		tst_res TINFO "loaded AppArmor profiles: $profiles"
> +	fi
> +
> +	if tst_selinux_enabled; then
> +		tst_res TINFO "SELinux enabled in enforcing mode, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
> +		profiles=
> +		for cmd in $TST_NEEDS_CMDS; do
> +			tst_selinux_used_profile $cmd && profiles="$cmd $profiles"
> +		done
> +		[ -z "$profiles" ] && profiles="none"
> +		tst_res TINFO "loaded SELinux profiles: $profiles"
> +	fi
> +}
> +
> +# Detect whether AppArmor profiles are loaded
> +# Return 0: profiles loaded, 1: none profile loaded or AppArmor disabled
> +tst_apparmor_enabled()
> +{
> +	local f="/sys/module/apparmor/parameters/enabled"
> +	[ -f "$f" ] && [ "$(cat $f)" = "Y" ]
> +}
> +
> +# Detect whether AppArmor profile for command is enforced
> +# tst_apparmor_used_profile CMD
> +# Return 0: loaded profile for CMD
> +# Return 1: no profile CMD
> +tst_apparmor_used_profile()
> +{
> +	[ $# -eq 1 ] && tst_brk TCONF "usage tst_apparmor_used_profile CMD"
              ^
Should be "-ne", because you expect exactly one argument or change the return check to ||.

> +	local cmd="$1"
> +	grep -q "$cmd .*(enforce)" /sys/kernel/security/apparmor/profiles 2>/dev/null
> +}
> +
> +# Detect whether SELinux is enabled in enforcing mode
> +# Return 0: enabled in enforcing mode
> +# Return 1: enabled in permissive mode or disabled
> +tst_selinux_enabled()
> +{
> +	local f="$(_tst_get_enforce)"
> +
> +	[ -f "$f" ] && [ "$(cat $f)" = "1" ]
> +}
> +
> +# Detect whether SELinux profile for command is enforced
> +# tst_selinux_used_profile CMD
> +# Return 0: loaded profile for CMD
> +# Return 1: profile for CMD not loaded or seinfo not available
> +tst_selinux_used_profile()
> +{
> +	[ $# -eq 1 ] && tst_brk TCONF "usage tst_selinux_used_profile CMD"

              ^
     The same is here.


> +	local cmd="$1"
> +
> +	if ! tst_cmd_available seinfo; then
> +		if [ -z "$seinfo_warn_printed" ]; then
> +			tst_res "install seinfo to find used SELinux profiles"
                               ^
                   Please add TINFO.


> +			export seinfo_warn_printed=1
> +		fi
> +		return
                       ^
Missed "return 1" here as you're checking the return value in
_tst_check_security_modules().

> +	fi
> +	seinfo -t 2>/dev/null | grep -q $cmd
> +}
> +
> +# Try disable AppArmor
> +# Return 0: AppArmor disabled
> +# Return > 0: failed to disable AppArmor
> +tst_disable_apparmor()
> +{
> +	_tst_require_root
> +	local f="aa-teardown"
> +	local action
> +
> +	tst_cmd_available $f && { $f; return; }
> +	f="/etc/init.d/apparmor"
> +	if [ -f "$f" ]; then
> +		for action in teardown kill stop; do
> +			$f $action >/dev/null 2>&1 && return
> +		done
> +	fi
> +}
> +
> +# Try disable SELinux
> +# Return 0: SELinux disabled
> +# Return > 0: failed to disable SELinux
> +tst_disable_selinux()
> +{
> +	_tst_require_root
> +	local f="$(_tst_get_enforce)"
> +
> +	[ -f "$f" ] && cat 0 > $f
> +}
> +
> +# Get SELinux enforce file path
> +_tst_get_enforce()
> +{
> +	local dir="/sys/fs/selinux"
> +
> +	[ -d "$dir" ] || dir="/selinux"
> +	local f="$dir/enforce"
> +	[ -f "$f" ] && echo "$f"
> +}
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index e3770d005..333061028 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -34,6 +34,7 @@ export TST_TMPDIR_RHOST=0
>  export TST_LIB_LOADED=1
>  
>  . tst_ansi_color.sh
> +. tst_security.sh
>  
>  # default trap function
>  trap "tst_brk TBROK 'test interrupted'" INT
> @@ -67,6 +68,7 @@ _tst_do_exit()
>  
>  	if [ $TST_FAIL -gt 0 ]; then
>  		ret=$((ret|1))
> +		_tst_check_security_modules
>  	fi
>  
>  	if [ $TST_BROK -gt 0 ]; then
> @@ -376,6 +378,11 @@ _tst_setup_timer()
>  	_tst_setup_timer_pid=$!
>  }
>  
> +_tst_require_root()
> +{
> +	[ "$(id -ru)" != 0 ] && tst_brk TCONF "Must be super/root for this test!"
> +}
> +
>  tst_run()
>  {
>  	local _tst_i
> @@ -386,6 +393,7 @@ tst_run()
>  	if [ -n "$TST_TEST_PATH" ]; then
>  		for _tst_i in $(grep TST_ "$TST_TEST_PATH" | sed 's/.*TST_//; s/[="} \t\/:`].*//'); do
>  			case "$_tst_i" in
> +			DISABLE_APPARMOR|DISABLE_SELINUX);;
>  			SETUP|CLEANUP|TESTFUNC|ID|CNT|MIN_KVER);;
>  			OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
>  			NEEDS_ROOT|NEEDS_TMPDIR|TMPDIR|NEEDS_DEVICE|DEVICE);;
> @@ -421,11 +429,10 @@ tst_run()
>  		tst_brk TBROK "Number of iterations (-i) must be > 0"
>  	fi
>  
> -	if [ "$TST_NEEDS_ROOT" = 1 ]; then
> -		if [ "$(id -ru)" != 0 ]; then
> -			tst_brk TCONF "Must be super/root for this test!"
> -		fi
> -	fi
> +	[ "$TST_NEEDS_ROOT" = 1 ] && _tst_require_root
> +
> +	[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
> +	[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
>  
>  	tst_test_cmds $TST_NEEDS_CMDS
>  	tst_test_drivers $TST_NEEDS_DRIVERS
> 

The rest in the patch-set looks fine to me.

Thanks,
Alexey

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Petr Vorel]

Hi Alexey,

> The rest in the patch-set looks fine to me.
thank you for review and pointing out errors, I'll add your ack.
Waiting for Cyril to add his ack for this commit.

> Thanks,
> Alexey


Kind regards,
Petr

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Cyril Hrubis]

Hi!
> It prints info about AppArmor and SELinux and allows to disable it.
> This is due some false positives because improper usage or bugs
> in AppArmor profiles (e.g. traceroute, dnsmasq).

Looks good, the only piece I'm not 100% sure about is if this should be
sourced by default in tst_test.sh. Shouldn't we just include this in
network tests?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Petr Vorel]

Hi Cyril,

> > It prints info about AppArmor and SELinux and allows to disable it.
> > This is due some false positives because improper usage or bugs
> > in AppArmor profiles (e.g. traceroute, dnsmasq).

> Looks good, the only piece I'm not 100% sure about is if this should be
> sourced by default in tst_test.sh. Shouldn't we just include this in
> network tests?
Understand, most of user space related tests are network tests.

I decided to add it for all tests as AppArmor default [1] and non-default [2]
contains various non-networking tools (syslog-ng, syslogd, klogd, cron, passwd,
useradd, userdel); SELinux has some kernel [3] and system [4] related modules.

Kind regards,
Petr

[1] https://gitlab.com/apparmor/apparmor/tree/master/profiles/apparmor.d
[2] https://gitlab.com/apparmor/apparmor/tree/master/profiles/apparmor/profiles/extras
[3] https://github.com/SELinuxProject/refpolicy/tree/master/policy/modules/kernel
[4] https://github.com/SELinuxProject/refpolicy/tree/master/policy/modules/system

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Cyril Hrubis]

Hi!
> > > It prints info about AppArmor and SELinux and allows to disable it.
> > > This is due some false positives because improper usage or bugs
> > > in AppArmor profiles (e.g. traceroute, dnsmasq).
> 
> > Looks good, the only piece I'm not 100% sure about is if this should be
> > sourced by default in tst_test.sh. Shouldn't we just include this in
> > network tests?
> Understand, most of user space related tests are network tests.
> 
> I decided to add it for all tests as AppArmor default [1] and non-default [2]
> contains various non-networking tools (syslog-ng, syslogd, klogd, cron, passwd,
> useradd, userdel); SELinux has some kernel [3] and system [4] related modules.

Okay then. I checked the messages generated in a case of a test failure
and hopefully these cannot be misinterpreted as they say "may affect".
Maybe it would be a bit better if the second sentence would be "You can
try to disable it with ..."

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Petr Vorel]

Hi,

I decided to print warning on all TBROK/TCONF/TFAIL/TWARN (originally it was only on TFAIL).
This is needed for dnsmasq tests, which TBROK due AppArmor profile.

And split long messages into 2 (besides obvious fixes reported by Alexey).

Kind regards,
Petr

Diff of this commit with posted version:

diff --git testcases/lib/tst_security.sh testcases/lib/tst_security.sh
index 2c8c30f42..25e085d3c 100644
--- testcases/lib/tst_security.sh
+++ testcases/lib/tst_security.sh
@@ -16,7 +16,8 @@ _tst_check_security_modules()
 	local profiles
 
 	if tst_apparmor_enabled; then
-		tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
+		tst_res TINFO "AppArmor enabled, this may affect test results"
+		tst_res TINFO "You can try to disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
 		profiles=
 		for cmd in $TST_NEEDS_CMDS; do
 			tst_apparmor_used_profile $cmd && profiles="$cmd $profiles"
@@ -26,7 +27,8 @@ _tst_check_security_modules()
 	fi
 
 	if tst_selinux_enabled; then
-		tst_res TINFO "SELinux enabled in enforcing mode, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
+		tst_res TINFO "SELinux enabled in enforcing mode, this may affect test results"
+		tst_res TINFO "You can try to disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
 		profiles=
 		for cmd in $TST_NEEDS_CMDS; do
 			tst_selinux_used_profile $cmd && profiles="$cmd $profiles"
diff --git testcases/lib/tst_test.sh testcases/lib/tst_test.sh
index 333061028..e69301e54 100644
--- testcases/lib/tst_test.sh
+++ testcases/lib/tst_test.sh
@@ -68,7 +68,6 @@ _tst_do_exit()
 
 	if [ $TST_FAIL -gt 0 ]; then
 		ret=$((ret|1))
-		_tst_check_security_modules
 	fi
 
 	if [ $TST_BROK -gt 0 ]; then
@@ -83,6 +82,10 @@ _tst_do_exit()
 		ret=$((ret|32))
 	fi
 
+	if [ $TST_BROK -gt 0 -o $TST_CONF -gt 0 -o $TST_FAIL -gt 0 -o $TST_WARN -gt 0 ]; then
+		_tst_check_security_modules
+	fi
+
 	echo
 	echo "Summary:"
 	echo "passed   $TST_PASS"

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Cyril Hrubis]

Hi!
>  
> +	if [ $TST_BROK -gt 0 -o $TST_CONF -gt 0 -o $TST_FAIL -gt 0 -o $TST_WARN -gt 0 ]; then
> +		_tst_check_security_modules
> +	fi

Not sure about the TCONF here but apart of that it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 2/6] shell: Add tst_security.sh helper

[Petr Vorel]

Hi,

> > +	if [ $TST_BROK -gt 0 -o $TST_CONF -gt 0 -o $TST_FAIL -gt 0 -o $TST_WARN -gt 0 ]; then
> > +		_tst_check_security_modules
> > +	fi

> Not sure about the TCONF here but apart of that it looks good.
Whole patchset merged, without TCONF.

Kind regards,
Petr