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

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

[Petr Vorel]

Hi,

this further enhances patchset "DHCP tests and AppArmor improvements" [1]
Main change is adding tst_security.sh and using it (last 2 commits).

Thanks for review.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/list/?series=70357&state=*

Petr Vorel (6):
  net/dhcp: Rename setup & cleanup functions
  net/dhcp: Use paths allowed by AppArmor for dnsmasq
  net/dhcp: Move print_dhcp_log() into dhcp library
  ver_linux: Print AppArmor and SELinux status
  shell: Add tst_security.sh helper
  net: Add security warning flag to some net tests

 testcases/lib/tst_security.sh                 | 64 +++++++++++++++++++
 testcases/lib/tst_test.sh                     | 14 +++-
 testcases/network/dhcp/dhcp_lib.sh            | 15 +++--
 testcases/network/dhcp/dhcpd_tests.sh         |  7 +-
 testcases/network/dhcp/dnsmasq_tests.sh       | 28 +++++---
 testcases/network/mpls/mpls02.sh              |  1 +
 testcases/network/mpls/mpls_lib.sh            |  1 +
 .../network/stress/icmp/icmp-uni-basic.sh     |  1 +
 testcases/network/stress/icmp/icmp-uni-vti.sh |  1 +
 .../network/stress/interface/if-mtu-change    |  1 +
 .../network/stress/interface/if4-addr-change  |  1 +
 testcases/network/traceroute/traceroute01.sh  |  1 +
 ver_linux                                     | 46 +++++++++++--
 13 files changed, 158 insertions(+), 23 deletions(-)
 create mode 100644 testcases/lib/tst_security.sh

-- 
2.19.2

[LTP] [PATCH v2 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 v2 2/6] net/dhcp: Use paths allowed by AppArmor for dnsmasq

[Petr Vorel]

Fixes for --log-facility and --dhcp-leasefile.

Path for log file expects AppArmor commit
025c7dc6 ("dnsmasq: Add permission to open log files").

dnsmasq uses directory for /var/lib/misc/ lease file.
Fedora/RHEL/CentOS/Oracle Linux (RHEL*) use specific directory
/var/lib/dnsmasq/ [1] (since 2.41 [2]). /var/lib/misc/ also exists on
RHEL* in filesystem package (which is on every RHEL* system), but check
in case it isn't.

NOTE: AppArmor optimization isn't needed for dhcpd.

[1] https://src.fedoraproject.org/cgit/rpms/dnsmasq.git/tree/dnsmasq.spec#n67
[2] https://src.fedoraproject.org/cgit/rpms/dnsmasq.git/commit/?id=91d4b30e7b55bbb561547312e83ce4d709e505e2

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes:
removing Lease file (and directory if needed).

Kind regards,
Petr
---
 testcases/network/dhcp/dnsmasq_tests.sh | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/testcases/network/dhcp/dnsmasq_tests.sh b/testcases/network/dhcp/dnsmasq_tests.sh
index ad5885c84..24368fd33 100755
--- a/testcases/network/dhcp/dnsmasq_tests.sh
+++ b/testcases/network/dhcp/dnsmasq_tests.sh
@@ -8,10 +8,25 @@
 dhcp_name="dnsmasq"
 
 . dhcp_lib.sh
+TST_SETUP="setup"
+
+log="/var/log/tst_dnsmasq.log"
+
+lease_dir="/var/lib/misc"
+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= "
+
+setup()
+{
+	dhcp_lib_setup
+	if [ ! -d "$lease_dir" ]; then
+		mkdir -p $lease_dir
+		remove_lease_dir=1
+	fi
+}
 
 start_dhcp()
 {
@@ -33,12 +48,13 @@ start_dhcp6()
 
 cleanup_dhcp()
 {
-	rm -f tst_dnsmasq.log
+	rm -f $log $lease_file
+	[ "$remove_lease_dir" = 1 ] && rm -r $lease_dir
 }
 
 print_dhcp_log()
 {
-	cat tst_dnsmasq.log
+	cat $log
 }
 
 print_dhcp_version()
-- 
2.19.2

[LTP] [PATCH v2 3/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 08838a479..d46f86db7 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"
@@ -89,6 +91,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 ff46b825d..e8339932b 100755
--- a/testcases/network/dhcp/dhcpd_tests.sh
+++ b/testcases/network/dhcp/dhcpd_tests.sh
@@ -31,7 +31,7 @@ setup_dhcpd_conf()
 start_dhcpd()
 {
 	touch tst_hdcpd.lease
-	dhcpd -lf tst_hdcpd.lease -$TST_IPVER $iface0 > tst_dhcpd.err 2>&1
+	dhcpd -lf tst_hdcpd.lease -$TST_IPVER $iface0 > $log 2>&1
 }
 
 start_dhcp()
@@ -69,11 +69,6 @@ cleanup_dhcp()
 	[ -f dhcpd.conf ] && mv dhcpd.conf $DHCPD_CONF
 }
 
-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 24368fd33..284c3cce1 100755
--- a/testcases/network/dhcp/dnsmasq_tests.sh
+++ b/testcases/network/dhcp/dnsmasq_tests.sh
@@ -52,11 +52,6 @@ cleanup_dhcp()
 	[ "$remove_lease_dir" = 1 ] && rm -r $lease_dir
 }
 
-print_dhcp_log()
-{
-	cat $log
-}
-
 print_dhcp_version()
 {
 	dnsmasq --version | head -2
-- 
2.19.2

[LTP] [PATCH v2 4/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 v2 5/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>
---
New in this version.
---
 testcases/lib/tst_security.sh | 64 +++++++++++++++++++++++++++++++++++
 testcases/lib/tst_test.sh     | 14 +++++++-
 2 files changed, 77 insertions(+), 1 deletion(-)
 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..a40a6d253
--- /dev/null
+++ b/testcases/lib/tst_security.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2016-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
+
+# Detect whether AppArmor profiles are loaded
+# Return 0: profiles loaded, 1: none profile loaded or AppArmor disabled
+tst_apparmor_enabled()
+{
+	local f="/sys/kernel/security/apparmor/profiles"
+	tst_test_cmds cut wc
+	[ -f "$f" ] && [ "$(wc -l $f | cut -d' ' -f1)" -gt 0 ]
+}
+
+# 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" ]
+}
+
+# Try disable AppArmor
+# Return 0: AppArmor disabled
+# Return > 0: failed to disable AppArmor
+tst_disable_apparmor()
+{
+	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()
+{
+	local f="$(_tst_get_enforce)"
+	[ -f "$f" ] && cat 0 > $f
+}
+
+_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 1a5d925f9..28d1e9c6d 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
@@ -393,6 +394,7 @@ tst_run()
 			NEEDS_DRIVERS);;
 			IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
 			RETRY_FUNC|RETRY_FN_EXP_BACKOFF);;
+			SECURITY_WARN|DISABLE_APPARMOR|DISABLE_SELINUX);;
 			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
 			esac
 		done
@@ -421,12 +423,22 @@ tst_run()
 		tst_brk TBROK "Number of iterations (-i) must be > 0"
 	fi
 
-	if [ "$TST_NEEDS_ROOT" = 1 ]; then
+	if [ "$TST_NEEDS_ROOT" = 1 ] || [ "$TST_DISABLE_APPARMOR" = 1 ] || [ "$TST_DISABLE_SELINUX" = 1 ]; then
 		if [ "$(id -ru)" != 0 ]; then
 			tst_brk TCONF "Must be super/root for this test!"
 		fi
 	fi
 
+	[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
+	[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
+
+	if [ "$TST_SECURITY_WARN" = 1 ]; then
+		tst_apparmor_enabled && \
+			tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
+		tst_selinux_enabled && \
+			tst_res TINFO "SELinux enabled, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
+	fi
+
 	tst_test_cmds $TST_NEEDS_CMDS
 	tst_test_drivers $TST_NEEDS_DRIVERS
 
-- 
2.19.2

[LTP] [PATCH v2 6/6] net: Add security warning flag to some net tests

[Petr Vorel]

Added to dnsmasq and traceroute tests and other network tests which use
tst_ping() and has been ported to new shell API (tst_security.sh
requirement).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in this version.
---
 testcases/network/dhcp/dnsmasq_tests.sh            | 1 +
 testcases/network/mpls/mpls02.sh                   | 1 +
 testcases/network/mpls/mpls_lib.sh                 | 1 +
 testcases/network/stress/icmp/icmp-uni-basic.sh    | 1 +
 testcases/network/stress/icmp/icmp-uni-vti.sh      | 1 +
 testcases/network/stress/interface/if-mtu-change   | 1 +
 testcases/network/stress/interface/if4-addr-change | 1 +
 testcases/network/traceroute/traceroute01.sh       | 1 +
 8 files changed, 8 insertions(+)

diff --git a/testcases/network/dhcp/dnsmasq_tests.sh b/testcases/network/dhcp/dnsmasq_tests.sh
index 284c3cce1..d5f89ae81 100755
--- a/testcases/network/dhcp/dnsmasq_tests.sh
+++ b/testcases/network/dhcp/dnsmasq_tests.sh
@@ -6,6 +6,7 @@
 # Author: Alexey Kodanev alexey.kodanev@oracle.com
 
 dhcp_name="dnsmasq"
+TST_SECURITY_WARN=1
 
 . dhcp_lib.sh
 TST_SETUP="setup"
diff --git a/testcases/network/mpls/mpls02.sh b/testcases/network/mpls/mpls02.sh
index 745a414f6..a2c93ff58 100755
--- a/testcases/network/mpls/mpls02.sh
+++ b/testcases/network/mpls/mpls02.sh
@@ -5,6 +5,7 @@
 TST_SETUP="setup"
 TST_TESTFUNC="do_test"
 TST_CLEANUP="cleanup"
+TST_SECURITY_WARN=1
 
 . mpls_lib.sh
 
diff --git a/testcases/network/mpls/mpls_lib.sh b/testcases/network/mpls/mpls_lib.sh
index 43ec16a6e..20bb24022 100755
--- a/testcases/network/mpls/mpls_lib.sh
+++ b/testcases/network/mpls/mpls_lib.sh
@@ -8,6 +8,7 @@ TST_NEEDS_ROOT=1
 TST_NEEDS_DRIVERS="mpls_router mpls_iptunnel mpls_gso"
 TST_NEEDS_CMDS="sysctl modprobe"
 TST_TEST_DATA="icmp tcp udp"
+TST_SECURITY_WARN=1
 
 . tst_net.sh
 
diff --git a/testcases/network/stress/icmp/icmp-uni-basic.sh b/testcases/network/stress/icmp/icmp-uni-basic.sh
index 5980b81e5..436f33bfd 100755
--- a/testcases/network/stress/icmp/icmp-uni-basic.sh
+++ b/testcases/network/stress/icmp/icmp-uni-basic.sh
@@ -8,6 +8,7 @@
 TST_TESTFUNC=do_test
 TST_SETUP=do_setup
 TST_CLEANUP=tst_ipsec_cleanup
+TST_SECURITY_WARN=1
 . ipsec_lib.sh
 
 do_setup()
diff --git a/testcases/network/stress/icmp/icmp-uni-vti.sh b/testcases/network/stress/icmp/icmp-uni-vti.sh
index 24eca177c..c9c39331d 100755
--- a/testcases/network/stress/icmp/icmp-uni-vti.sh
+++ b/testcases/network/stress/icmp/icmp-uni-vti.sh
@@ -7,6 +7,7 @@
 TST_TESTFUNC=do_test
 TST_SETUP=do_setup
 TST_CLEANUP=tst_ipsec_cleanup
+TST_SECURITY_WARN=1
 . ipsec_lib.sh
 
 do_setup()
diff --git a/testcases/network/stress/interface/if-mtu-change b/testcases/network/stress/interface/if-mtu-change
index 19bcffe1e..0872bca8c 100644
--- a/testcases/network/stress/interface/if-mtu-change
+++ b/testcases/network/stress/interface/if-mtu-change
@@ -21,6 +21,7 @@
 IF_CMD='ifconfig'
 TST_SETUP="do_setup"
 TST_CLEANUP="do_cleanup"
+TST_SECURITY_WARN=1
 . if-lib.sh
 
 # The interval of the mtu change [second]
diff --git a/testcases/network/stress/interface/if4-addr-change b/testcases/network/stress/interface/if4-addr-change
index 5ab67751e..8e5ab85f5 100644
--- a/testcases/network/stress/interface/if4-addr-change
+++ b/testcases/network/stress/interface/if4-addr-change
@@ -22,6 +22,7 @@ TCID=if4-addr-change
 TST_CLEANUP="do_cleanup"
 TST_TESTFUNC="test_body"
 TST_NEEDS_CMDS="ifconfig"
+TST_SECURITY_WARN=1
 . tst_net.sh
 
 CHECK_INTERVAL=${CHECK_INTERVAL:-$(($NS_TIMES / 20))}
diff --git a/testcases/network/traceroute/traceroute01.sh b/testcases/network/traceroute/traceroute01.sh
index 1c8b66f55..4ea1fd9db 100755
--- a/testcases/network/traceroute/traceroute01.sh
+++ b/testcases/network/traceroute/traceroute01.sh
@@ -18,6 +18,7 @@
 TST_TOTAL=6
 TCID="traceroute01"
 TST_CLEANUP="cleanup"
+TST_SECURITY_WARN=1
 
 TST_USE_LEGACY_API=1
 . tst_net.sh
-- 
2.19.2

[LTP] [PATCH v2 2/6] net/dhcp: Use paths allowed by AppArmor for dnsmasq

[Petr Vorel]

Hi,

> Fixes for --log-facility and --dhcp-leasefile.
...

> +++ b/testcases/network/dhcp/dnsmasq_tests.sh
> @@ -8,10 +8,25 @@
>  dhcp_name="dnsmasq"

>  . dhcp_lib.sh
> +TST_SETUP="setup"
> +
> +log="/var/log/tst_dnsmasq.log"
This is going to be
log="/var/log/dnsmasq.tst.log"
As I updated it in upstream co match SELinux path [1].

Lease file isn't handled by SELinux (for AppArmor we use path, which is reserved
for lxc-net). maybe it'd make sense use /var/lib/dnsmasq/ directory, when
SELinux enabled (checked with tst_selinux_enabled() from 5th commit).

Kind regards,
Petr

[1] https://gitlab.com/apparmor/apparmor/commit/3ef8df6ac05057e46720b2eba099bad3416f763b

[LTP] [PATCH v2 5/6] shell: Add tst_security.sh helper

[Alexey Kodanev]

Hi Petr,
On 07.12.2018 15:35, 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>
> ---
> 
...
>  
> -	if [ "$TST_NEEDS_ROOT" = 1 ]; then
> +	if [ "$TST_NEEDS_ROOT" = 1 ] || [ "$TST_DISABLE_APPARMOR" = 1 ] || [ "$TST_DISABLE_SELINUX" = 1 ]; then
>  		if [ "$(id -ru)" != 0 ]; then
>  			tst_brk TCONF "Must be super/root for this test!"
>  		fi
>  	fi
>  
> +	[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
> +	[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
> +
> +	if [ "$TST_SECURITY_WARN" = 1 ]; then
> +		tst_apparmor_enabled && \
> +			tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
> +		tst_selinux_enabled && \
> +			tst_res TINFO "SELinux enabled, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
> +	fi
> +
>  	tst_test_cmds $TST_NEEDS_CMDS

Since it may not affect test results, I don't think we should complicate
the tests with TST_SECURITY_WARN parameter. The usage is not obvious either.
Whether the test used dnsmasq or ping, for example, can be obtained from
$TST_NEEDS_CMDS.

And it's better to write these messages only if the test fails, suggesting
to change configuration.

[LTP] [PATCH v2 5/6] shell: Add tst_security.sh helper

[Petr Vorel]

Hi Alexey,

thanks for your review.

> On 07.12.2018 15:35, 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>
> > ---

> ...

> > -	if [ "$TST_NEEDS_ROOT" = 1 ]; then
> > +	if [ "$TST_NEEDS_ROOT" = 1 ] || [ "$TST_DISABLE_APPARMOR" = 1 ] || [ "$TST_DISABLE_SELINUX" = 1 ]; then
> >  		if [ "$(id -ru)" != 0 ]; then
> >  			tst_brk TCONF "Must be super/root for this test!"
> >  		fi
> >  	fi

> > +	[ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
> > +	[ "$TST_DISABLE_SELINUX" = 1 ] && tst_disable_selinux
> > +
> > +	if [ "$TST_SECURITY_WARN" = 1 ]; then
> > +		tst_apparmor_enabled && \
> > +			tst_res TINFO "AppArmor enabled, this may affect test results. Disable it with TST_DISABLE_APPARMOR=1 (requires super/root)"
> > +		tst_selinux_enabled && \
> > +			tst_res TINFO "SELinux enabled, this may affect test results. Disable it with TST_DISABLE_SELINUX=1 (requires super/root)"
> > +	fi
> > +
> >  	tst_test_cmds $TST_NEEDS_CMDS

> Since it may not affect test results, I don't think we should complicate
> the tests with TST_SECURITY_WARN parameter. The usage is not obvious either.
> Whether the test used dnsmasq or ping, for example, can be obtained from
> $TST_NEEDS_CMDS.
I didn't want to add this warning for any test failure when AppArmor/SELinux
enabled. Sometimes it's not obvious, that problem was caused by
AppArmor/SELinux, but this is relevant only for user space tests (networking and
some command tests).
For AppArmor it's easy to list policies (for newer kernels) and by parsing $TST_NEEDS_CMDS
detect detect the relevance of warning. I'll have a look if this is the same for
SELinux. Then TST_SECURITY_WARN flag wouldn't be needed.

> And it's better to write these messages only if the test fails, suggesting
> to change configuration.
This make sense, I'll change it.


Kind regards,
Petr