Enable shellcheck and fix some issues

PATCH In review Stefan Berger 2 participants 16 messages Dec 1, 2023 – Dec 6, 2023

Subsystems (2)

Kbuild Keys-trusted

lore

Share & tools

lore

Stefan Berger

author

2023-12-01 13:31 UTC #0 lore raw

v3 cover Enable shellcheck and fix some issues

Enable shellcheck for the bash scripts in the tests directory. Fix issues on the way that are found when particular checks are enabled. In the end all shellcheck v0.9 checks are enabled and shellcheck passes without any issues.

Shellcheck can be run using the following command:

make shellcheck

Apart from style issues two real issues are detected by SC2003 and SC2295.

Regards,
Stefan

v3:
- Replace read with readarray

v2:
- Rebased on latest next-testing branch
- 1/14: Use ${var:+${var}} pattern for variables that hold multiple arguments like "--foo bar" which would become one argument if passed with "${var}"; fixed a build issue
- 4/14: remove $new_policy file in success and failure cases

Stefan Berger (14):
tests: Address issues raised by shellcheck SC2086 & enable shellcheck
tests: Address issues raised by shellcheck SC2181
tests: Address issues raised by shellcheck SC2046
tests: Address issues raised by shellcheck SC2320
tests: Address issues raised by shellcheck SC2317
tests: Address issues raised by shellcheck SC2034
tests: Address issues raised by shellcheck SC2164
tests: Address issues raised by shellcheck SC2166
tests: Address issues raised by shellcheck SC2294
tests: Address issues raised by shellcheck SC2206
tests: Address issues raised by shellcheck SC2196
tests: Address issues raised by shellcheck SC2043
tests: Address issues raised by shellcheck SC2295
tests: Address issues raised by shellcheck SC2003

 Makefile.am                     |  5 ++-
 tests/Makefile.am               | 11 ++++++-
 tests/boot_aggregate.test       | 29 +++++++----------
 tests/fsverity.test             | 22 +++++++------
 tests/functions.sh              | 36 ++++++++++----------
 tests/gen-keys.sh               |  8 +++--
 tests/ima_hash.test             |  4 +--
 tests/install-fsverity.sh       |  4 +--
 tests/install-mount-idmapped.sh |  2 +-
 tests/install-openssl3.sh       | 14 ++++----
 tests/install-swtpm.sh          |  9 ++---
 tests/install-tss.sh            |  2 +-
 tests/mmap_check.test           | 10 +++---
 tests/portable_signatures.test  | 11 +++----
 tests/sign_verify.test          | 15 ++++-----
 tests/softhsm_setup             | 58 ++++++++++++++-------------------
 16 files changed, 119 insertions(+), 121 deletions(-)

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #1 lore raw

PATCH v3 1/14 tests: Address issues raised by shellcheck SC2086 & enable shellcheck

Address issues raised by shellcheck SC2086:
"Double quote to prevent globbing and word splitting."

Add support for the make target 'shellcheck' on scripts in the test directory.

Signed-off-by Stefan Berger <[email protected]>

---
 Makefile.am               |  5 ++++-
 tests/Makefile.am         | 11 ++++++++++-
 tests/boot_aggregate.test |  4 ++--
 tests/fsverity.test       | 18 +++++++++---------
 tests/functions.sh        | 30 +++++++++++++++---------------
 tests/gen-keys.sh         |  4 ++--
 tests/install-openssl3.sh | 12 ++++++------
 tests/sign_verify.test    | 12 ++++++------
 tests/softhsm_setup       | 28 ++++++++++++++--------------
 9 files changed, 68 insertions(+), 56 deletions(-)

Makefile.am +4 -1

index 9ec5681..949c353 100644
@@ -50,4 +50,7 @@ rmman:
 doc: evmctl.1.html rmman evmctl.1
 endif
 
-.PHONY: $(tarname)
+shellcheck:
+	make -C tests shellcheck
+
+.PHONY: $(tarname) shellcheck

tests/Makefile.am +10 -1

index a28f671..6bf7eef 100644
@@ -24,6 +24,15 @@ clean-local:
 	-rm -f *.txt *.out *.sig *.sig2
 
 distclean: distclean-keys
-.PHONY: distclean-keys
+
+shellcheck:
+	shellcheck -i SC2086 \
+		functions.sh gen-keys.sh install-fsverity.sh \
+		install-mount-idmapped.sh install-openssl3.sh \
+		install-swtpm.sh install-tss.sh softhsm_setup \
+		$(check_SCRIPTS)
+
+.PHONY: distclean-keys shellcheck
 distclean-keys:
 	./gen-keys.sh clean
+

tests/boot_aggregate.test +2 -2

index b0b2db4..ccc45f9 100755
@@ -129,7 +129,7 @@ check() {
 	local options=$1
 
 	echo "INFO: Calculating the boot_aggregate (PCRs 0 - 9) for multiple banks"
-	bootaggr=$(evmctl ima_boot_aggregate ${options})
+	bootaggr=$(evmctl ima_boot_aggregate "${options}")
 	if [ $? -ne 0 ]; then
 		echo "${CYAN}SKIP: evmctl ima_boot_aggregate: $bootaggr${NORM}"
 		exit "$SKIP"

@@ -197,4 +197,4 @@ if [ "$(id -u)" != 0 ] || [ ! -c "/dev/tpm0" ]; then
 	fi
 fi
 
-expect_pass check $BOOTAGGR_OPTIONS
+expect_pass check "$BOOTAGGR_OPTIONS"

tests/fsverity.test +9 -9

index e1e0c8d..2b338ca 100755
@@ -50,9 +50,9 @@ _require dd mkfs blkid e2fsck tune2fs evmctl setfattr
 trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM EXIT
 
 cleanup() {
-        if [ -e $TST_MNT ]; then
-		if [ $LOOPBACK_MOUNTED -eq 1 ]; then
-			umount $TST_MNT
+        if [ -e "$TST_MNT" ]; then
+		if [ "$LOOPBACK_MOUNTED" -eq 1 ]; then
+			umount "$TST_MNT"
 		fi
 		if [ -f "$TST_IMG" ]; then
 			rm "$TST_IMG"

@@ -139,7 +139,7 @@ create_loopback_file() {
 	fi
 
 	echo "INFO: Building an $fs_type filesystem"
-	if ! mkfs -t "$fs_type" -q "${TST_IMG}" $options; then
+	if ! mkfs -t "$fs_type" -q "${TST_IMG}" ${options:+${options}}; then
 		echo "${RED}FAILURE: Creating $fs_type filesystem${NORM}"
 		exit "$FAIL"
 	fi

@@ -202,7 +202,7 @@ create_file() {
 	local test=$1
 	local type=$2
 
-	TST_FILE=$(mktemp -p $TST_MNT -t "${type}".XXXXXX)
+	TST_FILE=$(mktemp -p "$TST_MNT" -t "${type}".XXXXXX)
 	[ "$VERBOSE" -ge 1 ] && echo "INFO: creating $TST_FILE"
 
 	# heredoc to create a script

@@ -251,13 +251,13 @@ measure-verity() {
 	digest_filename=$("$FSVERITY" digest "$TST_FILE")
 	[ "$VERBOSE" -ge 2 ] && echo "INFO: verity:$digest_filename"
 
-	grep "verity:$digest_filename" $IMA_MEASUREMENT_LIST &> /dev/null
+	grep "verity:$digest_filename" "$IMA_MEASUREMENT_LIST" &> /dev/null
 	ret=$?
 
 	# Not finding the "fsverity digest" result in the IMA measurement
 	# list is expected for non fs-verity enabled files.  The measurement
 	# list will contain zeros for the file hash.
-	if [ $ret -eq 1 ]; then
+	if [ "$ret" -eq 1 ]; then
 		error="$FAIL"
 		if [ "$verity" = "enabled" ]; then
 			echo "${RED}FAILURE: ${msg} ${NORM}"

@@ -285,7 +285,7 @@ measure-ima() {
 	create_file "$test" ima-hash
 	"$TST_FILE"
 
-	hashalg=$(grep "${TST_FILE}" $IMA_MEASUREMENT_LIST | cut -d':' -f2)
+	hashalg=$(grep "${TST_FILE}" "$IMA_MEASUREMENT_LIST" | cut -d':' -f2)
 	if [ -z "${hashalg}" ]; then
 		echo "${CYAN}SKIP: Measurement record with algorithm not found${NORM}"
 		return "$SKIP"

@@ -301,7 +301,7 @@ measure-ima() {
 	# Remove the extra space before the filename
 	digest_filename=$(${digestsum} "$TST_FILE" | sed "s/\ \ /\ /")
 	[ "$VERBOSE" -ge 2 ] && echo "$test: $digest_filename"
-	if grep "$digest_filename" $IMA_MEASUREMENT_LIST &> /dev/null; then
+	if grep "$digest_filename" "$IMA_MEASUREMENT_LIST" &> /dev/null; then
 		echo "${GREEN}SUCCESS: Measuring $TST_FILE ${NORM}"
 	else
 		error="$FAIL"

tests/functions.sh +15 ~~-15~~

index 35e925c..2105f21 100755
@@ -42,7 +42,7 @@ exit_early() {
 _require() {
   ret=
   for i; do
-    if ! type $i; then
+    if ! type "$i"; then
       echo "$i is required for test"
       ret=1
     fi

@@ -79,7 +79,7 @@ expect_pass() {
   fi
 
   if [ $TNESTED -gt 0 ]; then
-    echo $RED"expect_pass should not be run nested"$NORM
+    echo "${RED}expect_pass should not be run nested${NORM}"
     testsfail+=1
     exit "$HARDFAIL"
   fi

@@ -110,9 +110,9 @@ expect_pass_if() {
   ret=$?
 
   if [ $ret -ne 0 ] && [ $ret -ne 77 ] && [ -n "$PATCHES" ]; then
-    echo $YELLOW"Possibly missing patches:"$NORM
+    echo "${YELLOW}Possibly missing patches:${NORM}"
     for idx in $indexes; do
-      echo $YELLOW" - ${PATCHES[$((idx))]}"$NORM
+      echo "${YELLOW} - ${PATCHES[$((idx))]}${NORM}"
     done
   fi

@@ -130,7 +130,7 @@ expect_fail() {
   fi
 
   if [ $TNESTED -gt 0 ]; then
-    echo $RED"expect_fail should not be run nested"$NORM
+    echo "${RED}expect_fail should not be run nested${NORM}"
     testsfail+=1
     exit "$HARDFAIL"
   fi

@@ -166,9 +166,9 @@ expect_fail_if() {
   ret=$?
 
   if { [ $ret -eq 0 ] || [ $ret -eq 99 ]; } && [ -n "$PATCHES" ]; then
-    echo $YELLOW"Possibly missing patches:"$NORM
+    echo "${YELLOW}Possibly missing patches:${NORM}"
     for idx in $indexes; do
-      echo $YELLOW" - ${PATCHES[$((idx))]}"$NORM
+      echo "${YELLOW} - ${PATCHES[$((idx))]}${NORM}"
     done
   fi

@@ -177,12 +177,12 @@ expect_fail_if() {
 
 # return true if current test is positive
 _test_expected_to_pass() {
-  [ ! $TFAIL ]
+  [ ! "$TFAIL" ]
 }
 
 # return true if current test is negative
 _test_expected_to_fail() {
-  [ $TFAIL ]
+  [ "$TFAIL" ]
 }
 
 # Show blank line and color following text to red

@@ -201,7 +201,7 @@ color_red() {
 }
 
 color_restore() {
-  [ $COLOR_RESTORE ] && echo "$NORM"
+  [ "$COLOR_RESTORE" ] && echo "$NORM"
   COLOR_RESTORE=
 }

@@ -216,7 +216,7 @@ _evmctl_run() {
   # ADD_TEXT_FOR: append to text as 'for $ADD_TEXT_FOR'
 
   cmd="evmctl $V $EVMCTL_ENGINE $*"
-  echo $YELLOW$TMODE "$cmd"$NORM
+  echo "${YELLOW}$TMODE $cmd${NORM}"
   $cmd >"$out" 2>&1
   ret=$?

@@ -226,7 +226,7 @@ _evmctl_run() {
     echo "evmctl $op failed hard with ($ret) $text_for"
     sed 's/^/  /' "$out"
     color_restore
-    rm "$out" $ADD_DEL
+    rm "$out" "$ADD_DEL"
     ADD_DEL=
     ADD_TEXT_FOR=
     return "$HARDFAIL"

@@ -238,7 +238,7 @@ _evmctl_run() {
       sed 's/^/  /' "$out"
     fi
     color_restore
-    rm "$out" $ADD_DEL
+    rm "$out" "$ADD_DEL"
     ADD_DEL=
     ADD_TEXT_FOR=
     return "$FAIL"

@@ -371,7 +371,7 @@ _softhsm_setup() {
   msg=$(./softhsm_setup setup 2>&1)
   if [ $? -eq 0 ]; then
     echo "softhsm_setup setup succeeded: $msg"
-    PKCS11_KEYURI=$(echo $msg | sed -n 's|^keyuri: \(.*\)|\1|p')
+    PKCS11_KEYURI=$(echo "$msg" | sed -n 's|^keyuri: \(.*\)|\1|p')
 
     export EVMCTL_ENGINE="--engine pkcs11"
     export OPENSSL_ENGINE="-engine pkcs11"

@@ -402,7 +402,7 @@ _run_env() {
   if [ "$TST_ENV" = "um" ]; then
     expect_pass "$1" rootfstype=hostfs rw init="$2" quiet mem=2048M "$3"
   else
-    echo $RED"Testing environment $TST_ENV not supported"$NORM
+    echo "${RED}Testing environment $TST_ENV not supported${NORM}"
     exit "$FAIL"
   fi
 }

tests/gen-keys.sh +2 -2

index 8905cdf..0b03ba4 100755
@@ -71,9 +71,9 @@ for m in 1024 1024_skid 2048; do
     ext=
   fi
   if [ ! -e test-rsa$m.key ]; then
-    log openssl req -verbose -new -nodes -utf8 -sha256 -days 10000 -batch -x509 $ext \
+    log openssl req -verbose -new -nodes -utf8 -sha256 -days 10000 -batch -x509 "$ext" \
       -config test-ca.conf \
-      -newkey rsa:$bits \
+      -newkey "rsa:$bits" \
       -out test-rsa$m.cer -outform DER \
       -keyout test-rsa$m.key
     # for v1 signatures

tests/install-openssl3.sh +6 -6

index 911c32b..6658c23 100755
@@ -9,16 +9,16 @@ fi
 
 version=${COMPILE_SSL}
 
-wget --no-check-certificate https://github.com/openssl/openssl/archive/refs/tags/${version}.tar.gz
-tar --no-same-owner -xzf ${version}.tar.gz
-cd openssl-${version}
+wget --no-check-certificate "https://github.com/openssl/openssl/archive/refs/tags/${version}.tar.gz"
+tar --no-same-owner -xzf "${version}.tar.gz"
+cd "openssl-${version}"
 
 if [ "$VARIANT" = "i386" ]; then
 	echo "32-bit compilation"
 	FLAGS="-m32 linux-generic32"
 fi
 
-./Configure $FLAGS no-engine no-dynamic-engine --prefix=/opt/openssl3 --openssldir=/opt/openssl3
+./Configure ${FLAGS:+${FLAGS}} no-engine no-dynamic-engine --prefix=/opt/openssl3 --openssldir=/opt/openssl3
 # Uncomment for debugging
 # perl configdata.pm --dump | grep engine
 make -j$(nproc)

@@ -26,5 +26,5 @@ make -j$(nproc)
 sudo make install_sw
 
 cd ..
-rm -rf ${version}.tar.gz
-rm -rf openssl-${version}
+rm -rf "${version}.tar.gz"
+rm -rf "openssl-${version}"

tests/sign_verify.test +6 -6

index 2bc365a..5cc0393 100755
@@ -141,7 +141,7 @@ check_sign() {
   local FILE=${FILE:-$ALG.txt}
 
   # Normalize key filename if it's not a pkcs11 URI
-  if [ ${KEY:0:7} != pkcs11: ]; then
+  if [ "${KEY:0:7}" != pkcs11: ]; then
     key=${KEY%.*}.key
     key=test-${key#test-}
   else

@@ -152,8 +152,8 @@ check_sign() {
   # leave only good files for verify tests.
   _test_expected_to_fail && FILE+='~'
 
-  rm -f $FILE
-  if ! touch $FILE; then
+  rm -f "$FILE"
+  if ! touch "$FILE"; then
     color_red
     echo "Can't create test file: $FILE"
     color_restore

@@ -372,7 +372,7 @@ try_different_sigs() {
 
 ## Test v1 signatures
 # Signature v1 only supports sha1 and sha256 so any other should fail
-if [ $SIGV1 -eq 0 ]; then
+if [ "$SIGV1" -eq 0 ]; then
   __skip() { echo "IMA signature v1 tests are skipped: not supported"; return $SKIP; }
   expect_pass __skip
 else

@@ -440,8 +440,8 @@ expect_fail \
 # Test signing with key described by pkcs11 URI
 _softhsm_setup "${WORKDIR}"
 if [ -n "${PKCS11_KEYURI}" ]; then
-  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY=${PKCS11_KEYURI} ALG=sha256 PREFIX=0x030204aabbccdd0100 OPTS=--keyid=aabbccdd
-  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY=${PKCS11_KEYURI} ALG=sha1   PREFIX=0x030202aabbccdd0100 OPTS=--keyid=aabbccdd
+  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY="${PKCS11_KEYURI}" ALG=sha256 PREFIX=0x030204aabbccdd0100 OPTS=--keyid=aabbccdd
+  expect_pass check_sign FILE=pkcs11test TYPE=ima KEY="${PKCS11_KEYURI}" ALG=sha1   PREFIX=0x030202aabbccdd0100 OPTS=--keyid=aabbccdd
 else
   # to have a constant number of tests, skip these two tests
   __skip() { echo "pkcs11 test is skipped: could not setup softhsm"; return $SKIP; }

tests/softhsm_setup +14 ~~-14~~

index 35b1754..10e4013 100755
@@ -15,7 +15,7 @@ fi
 
 MAJOR=$(softhsm2-util -v | cut -d '.' -f1)
 MINOR=$(softhsm2-util -v | cut -d '.' -f2)
-if [ ${MAJOR} -lt 2 ] || [ ${MAJOR} -eq 2 -a ${MINOR} -lt 2 ]; then
+if [ "${MAJOR}" -lt 2 ] || [ "${MAJOR}" -eq 2 -a "${MINOR}" -lt 2 ]; then
 	echo "Need softhsm v2.2.0 or later"
 	exit 77
 fi

@@ -91,21 +91,21 @@ setup_softhsm() {
 				  grep -E "\.so$")"
 		fi
 		sudo mkdir -p /etc/gnutls &>/dev/null
-		sudo bash -c "echo "load=${SONAME}" > /etc/gnutls/pkcs11.conf"
+		sudo bash -c "echo 'load=${SONAME}' > /etc/gnutls/pkcs11.conf"
 		;;
 	esac
 
-	if ! [ -d $configdir ]; then
-		mkdir -p $configdir
+	if ! [ -d "$configdir" ]; then
+		mkdir -p "$configdir"
 	fi
-	mkdir -p ${tokendir}
+	mkdir -p "${tokendir}"
 
-	if [ -f $configfile ]; then
+	if [ -f "$configfile" ]; then
 		mv "$configfile" "$bakconfigfile"
 	fi
 
-	if ! [ -f $configfile ]; then
-		cat <<_EOF_ > $configfile
+	if ! [ -f "$configfile" ]; then
+		cat <<_EOF_ > "$configfile"
 directories.tokendir = ${tokendir}
 objectstore.backend = file
 log.level = DEBUG

@@ -122,8 +122,8 @@ _EOF_
 
 	if [ -z "$tokenuri" ]; then
 		msg=$(softhsm2-util \
-			--init-token --pin ${PIN} --so-pin ${SO_PIN} \
-			--free --label ${NAME} 2>&1)
+			--init-token --pin "${PIN}" --so-pin "${SO_PIN}" \
+			--free --label "${NAME}" 2>&1)
 		if [ $? -ne 0 ]; then
 			echo "Could not initialize token"
 			echo "$msg"

@@ -172,7 +172,7 @@ _EOF_
 		fi
 	fi
 
-	getkeyuri_softhsm $slot
+	getkeyuri_softhsm "$slot"
 	rc=$?
 	if [ $rc -ne 0 ]; then
 		teardown_softhsm

@@ -196,7 +196,7 @@ _getkeyuri_softhsm() {
 		echo "$msg"
 		return 6
 	fi
-	msg=$(p11tool --list-all ${tokenuri} 2>&1)
+	msg=$(p11tool --list-all "${tokenuri}" 2>&1)
 	if [ $? -ne 0 ]; then
 		echo "Could not list object under token $tokenuri"
 		echo "$msg"

@@ -260,7 +260,7 @@ main() {
 	local ret
 
 	if [ $# -lt 1 ]; then
-		usage $0
+		usage "$0"
 		echo -e "Missing command.\n\n"
 		return 1
 	fi

@@ -283,7 +283,7 @@ main() {
 		;;
 	*)
 		echo -e "Unsupported command: $1\n\n"
-		usage $0
+		usage "$0"
 		ret=1
 	esac
 	return $ret

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #2 lore raw

PATCH v3 2/14 tests: Address issues raised by shellcheck SC2181

In reply to Stefan Berger

Address issues raised by shellcheck SC2181:
"Check exit code directly with e.g. if mycmd;, not indirectly with $?."

The general replacement patterns to fix this issue are:

Old:
<cmd>
if [ $? -eq 0 ]; then ...

New:
if <cmd>; then ...

Old:
<cmd>
if [ $? -ne 0 ]; then ...

New:
if ! <cmd>; then ...

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am         |  2 +-
 tests/boot_aggregate.test | 22 ++++++++--------------
 tests/functions.sh        |  3 +--
 tests/ima_hash.test       |  4 ++--
 tests/sign_verify.test    |  3 +--
 tests/softhsm_setup       | 32 ++++++++++++--------------------
 6 files changed, 25 insertions(+), 41 deletions(-)

tests/Makefile.am +1 -1

index 6bf7eef..86796c3 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086 \
+	shellcheck -i SC2086,SC2181 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/boot_aggregate.test +8 ~~-14~~

index ccc45f9..04aef9b 100755
@@ -47,8 +47,7 @@ swtpm_start() {
 	fi
 
 	if [ -n "${swtpm}" ]; then
-		pgrep swtpm
-		if [ $? -eq 0 ]; then
+		if pgrep swtpm; then
 			echo "INFO: Software TPM (swtpm) already running"
 			return 114
 		else

@@ -60,8 +59,7 @@ swtpm_start() {
 	elif [ -n "${tpm_server}" ]; then
 		# tpm_server uses the Microsoft simulator encapsulated packet format
 		export TPM_SERVER_TYPE="mssim"
-		pgrep tpm_server
-		if [ $? -eq 0 ]; then
+		if pgrep tpm_server; then
 			echo "INFO: Software TPM (tpm_server) already running"
 			return 114
 		else

@@ -81,16 +79,13 @@ swtpm_init() {
 	fi
 
 	echo "INFO: Sending software TPM startup"
-	"${TSSDIR}/tssstartup"
-	if [ $? -ne 0 ]; then
+	if ! "${TSSDIR}/tssstartup"; then
 		echo "INFO: Retry sending software TPM startup"
 		sleep 1
-		"${TSSDIR}/tssstartup"
-	fi
-
-	if [ $? -ne 0 ]; then
-		echo "INFO: Software TPM startup failed"
-		return "$SKIP"
+		if ! "${TSSDIR}/tssstartup"; then
+			echo "INFO: Software TPM startup failed"
+			return "$SKIP"
+		fi
 	fi
 
 	echo "INFO: Walking ${BINARY_BIOS_MEASUREMENTS} initializing the software TPM"

@@ -129,8 +124,7 @@ check() {
 	local options=$1
 
 	echo "INFO: Calculating the boot_aggregate (PCRs 0 - 9) for multiple banks"
-	bootaggr=$(evmctl ima_boot_aggregate "${options}")
-	if [ $? -ne 0 ]; then
+	if ! bootaggr=$(evmctl ima_boot_aggregate "${options}"); then
 		echo "${CYAN}SKIP: evmctl ima_boot_aggregate: $bootaggr${NORM}"
 		exit "$SKIP"
 	fi

tests/functions.sh +1 -2

index 2105f21..9670b3a 100755
@@ -368,8 +368,7 @@ _softhsm_setup() {
 
   mkdir -p "${SOFTHSM_SETUP_CONFIGDIR}"
 
-  msg=$(./softhsm_setup setup 2>&1)
-  if [ $? -eq 0 ]; then
+  if msg=$(./softhsm_setup setup 2>&1); then
     echo "softhsm_setup setup succeeded: $msg"
     PKCS11_KEYURI=$(echo "$msg" | sed -n 's|^keyuri: \(.*\)|\1|p')

tests/ima_hash.test +2 -2

index e88fd59..9a8d7b6 100755
@@ -33,8 +33,8 @@ check() {
   # unless it's negative test, then pass to evmctl
   cmd="openssl dgst $OPENSSL_ENGINE -$alg $file"
   echo - "$cmd"
-  hash=$(set -o pipefail; $cmd 2>/dev/null | cut -d' ' -f2)
-  if [ $? -ne 0 ] && _test_expected_to_pass; then
+  if ! hash=$(set -o pipefail; $cmd 2>/dev/null | cut -d' ' -f2) \
+    && _test_expected_to_pass; then
     echo "${CYAN}$alg test is skipped$NORM"
     rm "$file"
     return "$SKIP"

tests/sign_verify.test +1 -2

index 5cc0393..1b6cf2a 100755
@@ -185,8 +185,7 @@ check_sign() {
 
   # Insert keyid from cert into PREFIX in-place of marker `:K:'
   if [[ $PREFIX =~ :K: ]]; then
-    keyid=$(_keyid_from_cert "$key")
-    if [ $? -ne 0 ]; then
+    if ! keyid=$(_keyid_from_cert "$key"); then
       color_red
       echo "Unable to determine keyid for $key"
       color_restore

tests/softhsm_setup +12 ~~-20~~

index 10e4013..95bf0b1 100755
@@ -30,8 +30,7 @@ UNAME_S="$(uname -s)"
 
 case "${UNAME_S}" in
 Darwin)
-	msg=$(sudo -v -n)
-	if [ $? -ne 0 ]; then
+	if ! msg=$(sudo -v -n); then
 		echo "Need password-less sudo rights on OS X to change /etc/gnutls/pkcs11.conf"
 		exit 1
 	fi

@@ -113,18 +112,16 @@ slots.removable = false
 _EOF_
 	fi
 
-	msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}" | tail -n1)
-	if [ $? -ne 0 ]; then
+	if ! msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}" | tail -n1); then
 		echo "Could not list existing tokens"
 		echo "$msg"
 	fi
 	tokenuri=$(echo "$msg" | sed -n 's/.*URL: \([[:print:]*]\)/\1/p')
 
 	if [ -z "$tokenuri" ]; then
-		msg=$(softhsm2-util \
+		if ! msg=$(softhsm2-util \
 			--init-token --pin "${PIN}" --so-pin "${SO_PIN}" \
-			--free --label "${NAME}" 2>&1)
-		if [ $? -ne 0 ]; then
+			--free --label "${NAME}" 2>&1); then
 			echo "Could not initialize token"
 			echo "$msg"
 			return 2

@@ -143,9 +140,8 @@ _EOF_
 			fi
 		fi
 
-		msg=$(p11tool --list-tokens 2>&1 | \
-			grep "token=${NAME}" | tail -n1)
-		if [ $? -ne 0 ]; then
+		if ! msg=$(p11tool --list-tokens 2>&1 | \
+			grep "token=${NAME}" | tail -n1); then
 			echo "Could not list existing tokens"
 			echo "$msg"
 		fi

@@ -156,15 +152,13 @@ _EOF_
 		fi
 
 		# more recent versions of p11tool have --generate-privkey ...
-		msg=$(GNUTLS_PIN=$PIN p11tool \
+		if ! msg=$(GNUTLS_PIN=$PIN p11tool \
 			--generate-privkey=rsa --bits 2048 --label mykey --login \
-			"${tokenuri}" 2>&1)
-		if [ $? -ne 0 ]; then
+			"${tokenuri}" 2>&1); then
 			# ... older versions have --generate-rsa
-			msg=$(GNUTLS_PIN=$PIN p11tool \
+			if ! msg=$(GNUTLS_PIN=$PIN p11tool \
 				--generate-rsa --bits 2048 --label mykey --login \
-				"${tokenuri}" 2>&1)
-			if [ $? -ne 0 ]; then
+				"${tokenuri}" 2>&1); then
 				echo "Could not create RSA key!"
 				echo "$msg"
 				return 5

@@ -184,8 +178,7 @@ _EOF_
 _getkeyuri_softhsm() {
 	local msg tokenuri keyuri
 
-	msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}")
-	if [ $? -ne 0 ]; then
+	if ! msg=$(p11tool --list-tokens 2>&1 | grep "token=${NAME}"); then
 		echo "Could not list existing tokens"
 		echo "$msg"
 		return 5

@@ -196,8 +189,7 @@ _getkeyuri_softhsm() {
 		echo "$msg"
 		return 6
 	fi
-	msg=$(p11tool --list-all "${tokenuri}" 2>&1)
-	if [ $? -ne 0 ]; then
+	if ! msg=$(p11tool --list-all "${tokenuri}" 2>&1); then
 		echo "Could not list object under token $tokenuri"
 		echo "$msg"
 		softhsm2-util --show-slots

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #3 lore raw

PATCH v3 3/14 tests: Address issues raised by shellcheck SC2046

In reply to Stefan Berger

Address issues raised by shellcheck SC2046:
"Quote this to prevent word splitting."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am         | 2 +-
 tests/install-fsverity.sh | 2 +-
 tests/install-openssl3.sh | 2 +-
 tests/install-swtpm.sh    | 9 +++++----
 tests/install-tss.sh      | 2 +-
 tests/softhsm_setup       | 2 +-
 6 files changed, 10 insertions(+), 9 deletions(-)

tests/Makefile.am +1 -1

index 86796c3..6fe18e4 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181 \
+	shellcheck -i SC2086,SC2181,SC2046 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/install-fsverity.sh +1 -1

index c760485..fa31b2b 100755
@@ -2,5 +2,5 @@
 
 git clone https://git.kernel.org/pub/scm/fs/fsverity/fsverity-utils.git
 cd fsverity-utils
-CC=gcc make -j$(nproc)
+CC=gcc make -j"$(nproc)"
 cd ..

tests/install-openssl3.sh +1 -1

index 6658c23..edd99de 100755
@@ -21,7 +21,7 @@ fi
 ./Configure ${FLAGS:+${FLAGS}} no-engine no-dynamic-engine --prefix=/opt/openssl3 --openssldir=/opt/openssl3
 # Uncomment for debugging
 # perl configdata.pm --dump | grep engine
-make -j$(nproc)
+make -j"$(nproc)"
 # only install apps and library
 sudo make install_sw

tests/install-swtpm.sh +5 -4

index ff44b52..a0e60ee 100755
@@ -1,4 +1,4 @@
-#!/bin/sh -ex
+#!/bin/bash -ex
 
 # No need to run via sudo if we already have permissions.
 # Also, some distros do not have sudo configured for root:

@@ -10,7 +10,8 @@ else
 fi
 
 git clone https://git.code.sf.net/p/ibmswtpm2/tpm2
-cd tpm2/src
-make -j$(nproc)
+pushd tpm2/src 1>/dev/null || exit 1
+make -j"$(nproc)"
 $SUDO cp tpm_server /usr/local/bin/
-cd ../..
+popd 1>/dev/null
+

tests/install-tss.sh +1 -1

index c9c179e..31ea690 100755
@@ -3,6 +3,6 @@
 set -ex
 git clone https://git.code.sf.net/p/ibmtpm20tss/tss
 cd tss
-autoreconf -i && ./configure --disable-tpm-1.2 --disable-hwtpm && make -j$(nproc) && sudo make install
+autoreconf -i && ./configure --disable-tpm-1.2 --disable-hwtpm && make -j"$(nproc)" && sudo make install
 cd ..
 rm -rf tss

tests/softhsm_setup +1 -1

index 95bf0b1..6f8a74e 100755
@@ -82,7 +82,7 @@ setup_softhsm() {
 		fi
 		sudo mv /etc/gnutls/pkcs11.conf \
 			/etc/gnutls/pkcs11.conf.bak &>/dev/null
-		if [ $(id -u) -eq 0 ]; then
+		if [ "$(id -u)" -eq 0 ]; then
 			SONAME="$(sudo -u nobody brew ls --verbose softhsm | \
 				  grep -E "\.so$")"
 		else

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #4 lore raw

PATCH v3 4/14 tests: Address issues raised by shellcheck SC2320

In reply to Stefan Berger

Address issues raised by shellcheck SC2320:
"This $? refers to echo/printf, not a previous command.
Assign to variable to avoid it being overwritten."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am              | 2 +-
 tests/mmap_check.test          | 8 +++-----
 tests/portable_signatures.test | 9 +++------
 3 files changed, 7 insertions(+), 12 deletions(-)

tests/Makefile.am +1 -1

index 6fe18e4..3713771 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046 \
+	shellcheck -i SC2086,SC2181,SC2046,SC2320 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/mmap_check.test +3 -5

index 2dd3433..e3e5c71 100755
@@ -97,14 +97,12 @@ check_load_ima_rule() {
 
 	new_policy=$(mktemp -p "$g_mountpoint")
 	echo "$1" > "$new_policy"
-	echo "$new_policy" > /sys/kernel/security/ima/policy
-	result=$?
-	rm -f "$new_policy"
-
-	if [ "$result" -ne 0 ]; then
+	if ! echo "$new_policy" > /sys/kernel/security/ima/policy; then
+		rm -f "$new_policy"
 		echo "${RED}Failed to set IMA policy${NORM}"
 		return "$HARDFAIL"
 	fi
+	rm -f "$new_policy"
 
 	return "$OK"
 }

tests/portable_signatures.test +3 -6

index 9f3339b..7ddd149 100755
@@ -80,7 +80,6 @@ METADATA_CHANGE_FOWNER_2=3002
 
 check_load_ima_rule() {
 	local rule_loaded
-	local result
 	local new_policy
 
 	rule_loaded=$(grep "$1" /sys/kernel/security/ima/policy)

@@ -88,14 +87,12 @@ check_load_ima_rule() {
 		new_policy=$(mktemp -p "$g_mountpoint")
 		echo "$1" > "$new_policy"
 		evmctl sign -o -a sha256 --imasig --key "$key_path" "$new_policy" &> /dev/null
-		echo "$new_policy" > /sys/kernel/security/ima/policy
-		result=$?
-		rm -f "$new_policy"
-
-		if [ "$result" -ne 0 ]; then
+		if ! echo "$new_policy" > /sys/kernel/security/ima/policy; then
+			rm -f "$new_policy"
 			echo "${RED}Failed to set IMA policy${NORM}"
 			return "$FAIL"
 		fi
+		rm -f "${new_policy}"
 	fi
 
 	return "$OK"

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #5 lore raw

PATCH v3 5/14 tests: Address issues raised by shellcheck SC2317

In reply to Stefan Berger

Address issues raised by shellcheck SC2317:
"Command appears to be unreachable. Check usage (or ignore if invoked
indirectly)."

Disable this check in fsverity.test since functions are called indirectly there.

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am   | 2 +-
 tests/fsverity.test | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

tests/Makefile.am +1 -1

index 3713771..e2fcb16 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046,SC2320 \
+	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/fsverity.test +2 -0

index 2b338ca..ddd6993 100755
@@ -29,6 +29,8 @@
 # since the policy rules are walked sequentially, the system's IMA
 # custom policy rules might take precedence.
 
+# shellcheck disable=SC2317
+
 cd "$(dirname "$0")" || exit 1
 PATH=../src:../fsverity-utils:/usr/sbin:$PATH
 source ./functions.sh

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #6 lore raw

PATCH v3 6/14 tests: Address issues raised by shellcheck SC2034

In reply to Stefan Berger

Address issues raised by shellcheck SC2034:
"foo appears unused. Verify it or export it."

Export PKCS11_KEYURI in a separate statement to avoid the following shellcheck issue:

SC2155 (warning): Declare and assign separately to avoid masking
return values.

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am     | 2 +-
 tests/functions.sh    | 1 +
 tests/mmap_check.test | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

tests/Makefile.am +1 -1

index e2fcb16..79e2775 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317 \
+	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/functions.sh +1 -0

index 9670b3a..4ad61ef 100755
@@ -371,6 +371,7 @@ _softhsm_setup() {
   if msg=$(./softhsm_setup setup 2>&1); then
     echo "softhsm_setup setup succeeded: $msg"
     PKCS11_KEYURI=$(echo "$msg" | sed -n 's|^keyuri: \(.*\)|\1|p')
+    export PKCS11_KEYURI
 
     export EVMCTL_ENGINE="--engine pkcs11"
     export OPENSSL_ENGINE="-engine pkcs11"

tests/mmap_check.test +2 -0

index e3e5c71..18412b5 100755
@@ -5,6 +5,8 @@
 #
 # Check the behavior of MMAP_CHECK and MMAP_CHECK_REQPROT
 
+# shellcheck disable=SC2034
+
 trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM SIGSEGV EXIT
 
 PATCHES=(

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #7 lore raw

PATCH v3 7/14 tests: Address issues raised by shellcheck SC2164

In reply to Stefan Berger

Address issues raised by shellcheck SC2164:
"Use cd ... || exit in case cd fails."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am               | 2 +-
 tests/boot_aggregate.test       | 2 +-
 tests/install-fsverity.sh       | 2 +-
 tests/install-mount-idmapped.sh | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

tests/Makefile.am +1 -1

index 79e2775..c5b2a2f 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034 \
+	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/boot_aggregate.test +1 -1

index 04aef9b..c7c2b21 100755
@@ -17,7 +17,7 @@ trap '_report_exit_and_cleanup cleanup' SIGINT SIGTERM EXIT
 # Base VERBOSE on the environment variable, if set.
 VERBOSE="${VERBOSE:-0}"
 
-cd "$(dirname "$0")"
+cd "$(dirname "$0")" || exit 1
 export PATH=../src:$PATH
 export LD_LIBRARY_PATH=$LD_LIBRARY_PATH
 . ./functions.sh

tests/install-fsverity.sh +1 -1

index fa31b2b..e2b0286 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
 git clone https://git.kernel.org/pub/scm/fs/fsverity/fsverity-utils.git
-cd fsverity-utils
+cd fsverity-utils || exit 1
 CC=gcc make -j"$(nproc)"
 cd ..

tests/install-mount-idmapped.sh +1 -1

index c954006..d8a673c 100755
@@ -1,6 +1,6 @@
 #!/bin/sh
 
 git clone https://github.com/brauner/mount-idmapped.git
-cd mount-idmapped
+cd mount-idmapped || exit 1
 gcc -o mount-idmapped mount-idmapped.c
 cd ..

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #8 lore raw

PATCH v3 8/14 tests: Address issues raised by shellcheck SC2166

In reply to Stefan Berger

Address issues raised by shellcheck SC2166:
"Prefer [ p ] && [ q ] as [ p -a q ] is not well defined."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am   | 2 +-
 tests/softhsm_setup | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tests/Makefile.am +1 -1

index c5b2a2f..df1fbff 100644
@@ -26,7 +26,7 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164 \
+	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/softhsm_setup +1 -1

index 6f8a74e..fc33596 100755
@@ -15,7 +15,7 @@ fi
 
 MAJOR=$(softhsm2-util -v | cut -d '.' -f1)
 MINOR=$(softhsm2-util -v | cut -d '.' -f2)
-if [ "${MAJOR}" -lt 2 ] || [ "${MAJOR}" -eq 2 -a "${MINOR}" -lt 2 ]; then
+if [[ "${MAJOR}" -lt 2 || ( "${MAJOR}" -eq 2  && "${MINOR}" -lt 2 ) ]]; then
 	echo "Need softhsm v2.2.0 or later"
 	exit 77
 fi

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #9 lore raw

PATCH v3 9/14 tests: Address issues raised by shellcheck SC2294

In reply to Stefan Berger

Address issues raised by shellcheck SC2294:
"eval negates the benefit of arrays. Drop eval to preserve
whitespace/symbols (or eval as string)."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am | 4 +++-
 tests/gen-keys.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

tests/Makefile.am +3 -1

index df1fbff..9092c43 100644
@@ -26,7 +26,9 @@ clean-local:
 distclean: distclean-keys
 
 shellcheck:
-	shellcheck -i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
+	shellcheck \
+		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
+		-i SC2294 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/gen-keys.sh +1 -1

index 0b03ba4..c3d5a20 100755
@@ -21,7 +21,7 @@ type openssl
 
 log() {
   echo >&2 - "$*"
-  eval "$@"
+  eval "$*"
 }
 
 if [ "$1" = clean ]; then

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #10 lore raw

PATCH v3 10/14 tests: Address issues raised by shellcheck SC2206

In reply to Stefan Berger

Address issue raised by shellcheck SC2206:
"Quote to prevent word splitting/globbing, or split robustly with
mapfile or read -a."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am         | 2 +-
 tests/boot_aggregate.test | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

tests/Makefile.am +1 -1

index 9092c43..cbf637c 100644
@@ -28,7 +28,7 @@ distclean: distclean-keys
 shellcheck:
 	shellcheck \
 		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
-		-i SC2294 \
+		-i SC2294,SC2206 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/boot_aggregate.test +1 -2

index c7c2b21..7817c2b 100755
@@ -128,8 +128,7 @@ check() {
 		echo "${CYAN}SKIP: evmctl ima_boot_aggregate: $bootaggr${NORM}"
 		exit "$SKIP"
 	fi
-
-	boot_aggr=( $bootaggr )
+	IFS=$'\n' readarray -t boot_aggr <<< "$bootaggr"
 
 	echo "INFO: Searching for the boot_aggregate in ${ASCII_RUNTIME_MEASUREMENTS}"
 	for hash in "${boot_aggr[@]}"; do

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #11 lore raw

PATCH v3 11/14 tests: Address issues raised by shellcheck SC2196

In reply to Stefan Berger

Address issues raised by shellcheck SC2196:
"egrep is non-standard and deprecated. Use grep -E instead."

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am  | 2 +-
 tests/functions.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tests/Makefile.am +1 -1

index cbf637c..3c542d7 100644
@@ -28,7 +28,7 @@ distclean: distclean-keys
 shellcheck:
 	shellcheck \
 		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
-		-i SC2294,SC2206 \
+		-i SC2294,SC2206,SC2196 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/functions.sh +1 -1

index 4ad61ef..c39b894 100755
@@ -272,7 +272,7 @@ _test_xattr() {
   local file=$1 attr=$2 prefix=$3
   local text_for=${ADD_TEXT_FOR:+ for $ADD_TEXT_FOR}
 
-  if ! getfattr -n "$attr" -e hex "$file" | egrep -qx "$attr=$prefix"; then
+  if ! getfattr -n "$attr" -e hex "$file" | grep -qx -E "$attr=$prefix"; then
     color_red_on_failure
     echo "Did not find expected hash$text_for:"
     echo "    $attr=$prefix"

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #12 lore raw

PATCH v3 12/14 tests: Address issues raised by shellcheck SC2043

In reply to Stefan Berger

Address issues raised by shellcheck SC2043:
"This loop will only ever run once for a constant value. Did you
perhaps mean to loop over dir/*, $var or $(cmd)?

Disable this check in gen-keys.sh to leave the loop alone.

Signed-off-by Stefan Berger <[email protected]>

---
 tests/Makefile.am | 2 +-
 tests/gen-keys.sh | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

tests/Makefile.am +1 -1

index 3c542d7..4ce71f4 100644
@@ -28,7 +28,7 @@ distclean: distclean-keys
 shellcheck:
 	shellcheck \
 		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
-		-i SC2294,SC2206,SC2196 \
+		-i SC2294,SC2206,SC2196,SC2043 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/gen-keys.sh +2 -0

index c3d5a20..601ded2 100755
@@ -15,6 +15,8 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 
+# shellcheck disable=SC2043
+
 cd "$(dirname "$0")" || exit 1
 PATH=../src:$PATH
 type openssl

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:31 UTC #13 lore raw

PATCH v3 13/14 tests: Address issues raised by shellcheck SC2295

In reply to Stefan Berger

Address issues raised by shellcheck SC2295:
"Expansions inside ${..} need to be quoted separately, otherwise they
will match as a pattern."

There's not variable digest_type but it's a plain string and therefore treat it as such.

Signed-off-by Stefan Berger <[email protected]>

Cc Mimi Zohar <[email protected]>

---
 tests/Makefile.am   | 2 +-
 tests/fsverity.test | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tests/Makefile.am +1 -1

index 4ce71f4..7b3d92b 100644
@@ -28,7 +28,7 @@ distclean: distclean-keys
 shellcheck:
 	shellcheck \
 		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
-		-i SC2294,SC2206,SC2196,SC2043 \
+		-i SC2294,SC2206,SC2196,SC2043,SC2295 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/fsverity.test +1 -1

index ddd6993..a6ba68b 100755
@@ -169,7 +169,7 @@ unqualified_bprm_rule() {
 	local rule_match="measure func=BPRM_CHECK"
 	local rule_dontmatch="fsuuid"
 
-	if [ -z "${rule##*$digest_type=verity*}" ]; then
+	if [ -z "${rule##*digest_type=verity*}" ]; then
 		if grep "$rule_match" $IMA_POLICY_FILE | grep -v "$rule_dontmatch"  &> /dev/null; then
 			return "$SKIP"
 		fi

Signature · 1 line

2.43.0

Stefan Berger

author

2023-12-01 13:32 UTC #14 lore raw

PATCH v3 14/14 tests: Address issues raised by shellcheck SC2003

In reply to Stefan Berger

Address issues detected by shellcheck SC2003:
expr is antiquated. Consider rewriting this using $((..)), ${} or [[ ]].

The following statement in portable_signatures.test causes the issue:

expr index "$TST_LIST" "check_evm_revalidate"

The man page for expr states:

index STRING CHARS
index in STRING where any CHARS is found, or 0

The intention is certainly not to find an index of any of the characters in "check_evm_revalidate" in $TST_LIST but to find the word "check_evm_revalidate" in $TST_LIST. Therefore, use grep -w to determine whether the word is there.

Signed-off-by Stefan Berger <[email protected]>

Cc Roberto Sassu <[email protected]>

---
 tests/Makefile.am              | 2 --
 tests/portable_signatures.test | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

tests/Makefile.am +0 -2

index 7b3d92b..a95c4d2 100644
@@ -27,8 +27,6 @@ distclean: distclean-keys
 
 shellcheck:
 	shellcheck \
-		-i SC2086,SC2181,SC2046,SC2320,SC2317,SC2034,SC2164,SC2166 \
-		-i SC2294,SC2206,SC2196,SC2043,SC2295 \
 		functions.sh gen-keys.sh install-fsverity.sh \
 		install-mount-idmapped.sh install-openssl3.sh \
 		install-swtpm.sh install-tss.sh softhsm_setup \

tests/portable_signatures.test +1 -1

index 7ddd149..10b0ad8 100755
@@ -1090,7 +1090,7 @@ if [ $((evm_value & EVM_INIT_X509)) -ne "$EVM_INIT_X509" ] && [ "$TST_EVM_CHANGE
 	echo "$EVM_INIT_X509" > /sys/kernel/security/evm 2> /dev/null
 fi
 
-if [ "$(expr index "$TST_LIST" "check_evm_revalidate")" -gt 0 ] && [ "$TST_EVM_CHANGE_MODE" -eq 1 ]; then
+if echo "$TST_LIST" | grep -q -w check_evm_revalidate && [ "$TST_EVM_CHANGE_MODE" -eq 1 ]; then
 	echo "$EVM_ALLOW_METADATA_WRITES" > /sys/kernel/security/evm 2> /dev/null
 fi

Signature · 1 line

2.43.0

Mimi Zohar

maintainer

2023-12-06 19:33 UTC #15 lore raw

In reply to Stefan Berger

Quoted 17 lines · Stefan Berger

Enable shellcheck for the bash scripts in the tests directory. Fix issues on the way that are found when particular checks are enabled. In the end all shellcheck v0.9 checks are enabled and shellcheck passes without any issues.
Shellcheck can be run using the following command:
make shellcheck
Apart from style issues two real issues are detected by SC2003 and SC2295.
Regards,
Stefan
v3:
- Replace read with readarray

Quoted 6 lines

v2:
- Rebased on latest next-testing branch
- 1/14: Use ${var:+${var}} pattern for variables that hold multiple arguments like "--foo bar" which would become one argument if passed with "${var}"; fixed a build issue
- 4/14: remove $new_policy file in success and failure cases

Thanks, Stefan. Other than modifying 1/14 as discussed, it looks good. The patch set is now queued in next-testing.

Mimi