On Thu, 2023-01-12 at 10:22 -0500, Stefan Berger wrote: > > On 1/12/23 07:24, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > > > Verify that operations on files with EVM portable signatures succeed and > > that the new kernel patch set does not break the existing kernel integrity > > expectations. Build and install mount-idmapped for ci/fedora.sh, to > > additionally test idmapped mounts. > > > > To run the tests, pass the path of the kernel private key with the > > TST_KEY_PATH environment variable. If not provided, the script searches the > > key in /lib/modules/$(uname -r)/source/certs/signing_key.pem and in the > > current directory. Root privileges are required to mount the image, > > configure IMA/EVM and set xattrs. > > > > Set UML_MODE to 1, to relaunch the script in a new environment after > > booting an UML kernel. The UML kernel must be named 'linux' and placed in > > the ima-evm-utils directory. > > > > Alternatively, set the TST_EVM_CHANGE_MODE variable to 1, to change the > > current EVM mode, if a test needs a different one. Otherwise, execute only > > the tests compatible with the current EVM mode. > > > > Also set the EVM_ALLOW_METADATA_WRITES flag in the EVM mode, before > > launching the script, to run the check_evm_revalidate() test. Execute: > > > > echo 4 > /sys/kernel/security/evm > > > > The last two environment variables above affect which tests will run the > > next time the script is executed. Without setting UML_MODE to 1, changes to > > the current EVM mode will be irreversibly done in the host. Next time, > > unless the host is rebooted, only tests compatible with the last EVM mode > > set will run. The others will be skipped. > > > > With the UML kernel, this problem does not arise as, every time the UML > > kernel is executed, it will create a clean environment with no flags set in > > the EVM mode. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > --- > > build.sh | 5 + > > ci/fedora.sh | 7 +- > > tests/Makefile.am | 2 +- > > tests/install-mount-idmapped.sh | 7 + > > tests/portable_signatures.test | 1173 +++++++++++++++++++++++++++++++ > > 5 files changed, 1192 insertions(+), 2 deletions(-) > > create mode 100755 tests/install-mount-idmapped.sh > > create mode 100755 tests/portable_signatures.test > > > > diff --git a/build.sh b/build.sh > > index 4e2f1bb7353b..0920599b2780 100755 > > --- a/build.sh > > +++ b/build.sh > > @@ -114,6 +114,11 @@ if [ $ret -eq 0 ]; then > > grep "skipped" tests/fsverity.log && \ > > grep "skipped" tests/fsverity.log | wc -l > > fi > > + if [ -f tests/portable_signatures.log ]; then > > + [ -n "$CI" ] && cat tests/portable_signatures.log || tail tests/portable_signatures.log > > + grep "skipped" tests/portable_signatures.log && \ > > + grep "skipped" tests/portable_signatures.log | wc -l > > + fi > > exit 0 > > fi > > > > diff --git a/ci/fedora.sh b/ci/fedora.sh > > index 198034a34e3c..3f75d2e1ddbd 100755 > > --- a/ci/fedora.sh > > +++ b/ci/fedora.sh > > @@ -47,7 +47,11 @@ yum -y install \ > > which \ > > zstd \ > > haveged \ > > - systemd > > + systemd \ > > + keyutils \ > > + e2fsprogs \ > > + acl \ > > + libcap > > > > yum -y install docbook5-style-xsl || true > > yum -y install swtpm || true > > @@ -59,3 +63,4 @@ fi > > yum -y install softhsm || true > > > > ./tests/install-fsverity.sh > > +./tests/install-mount-idmapped.sh > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > index 305082483f36..421fac577b55 100644 > > --- a/tests/Makefile.am > > +++ b/tests/Makefile.am > > @@ -2,7 +2,7 @@ check_SCRIPTS = > > TESTS = $(check_SCRIPTS) > > > > check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test \ > > - fsverity.test > > + fsverity.test portable_signatures.test > > > > clean-local: > > -rm -f *.txt *.out *.sig *.sig2 > > diff --git a/tests/install-mount-idmapped.sh b/tests/install-mount-idmapped.sh > > new file mode 100755 > > index 000000000000..e9768e2fbf7a > > --- /dev/null > > +++ b/tests/install-mount-idmapped.sh > > @@ -0,0 +1,7 @@ > > +#!/bin/sh > > + > > +git clone https://github.com/brauner/mount-idmapped.git > > +cd mount-idmapped > > +gcc -o mount-idmapped mount-idmapped.c > > +cd .. > > +rm -rf mount-idmapped > > Where did you just install the executable to? It looks to me like it was removed. Right, my mistake. Will fix it. > > diff --git a/tests/portable_signatures.test b/tests/portable_signatures.test > > new file mode 100755 > > index 000000000000..a6d79c929281 > > --- /dev/null > > +++ b/tests/portable_signatures.test > > @@ -0,0 +1,1173 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# Copyright (C) 2022-2023 Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > +# > > +# Check if operations on files with EVM portable signatures succeed. > > + > > +trap cleanup SIGINT SIGTERM SIGSEGV EXIT > > + > > +# Base VERBOSE on the environment variable, if set. > > +VERBOSE="${VERBOSE:-0}" > > +TST_EVM_CHANGE_MODE="${TST_EVM_CHANGE_MODE:-0}" > > +UML_MODE="${UML_MODE:-0}" > > + > > +# From security/integrity/evm/evm.h in kernel source directory. > > +let "EVM_INIT_HMAC=0x0001" > > +let "EVM_INIT_X509=0x0002" > > +let "EVM_ALLOW_METADATA_WRITES=0x0004" > > +let "EVM_SETUP_COMPLETE=0x80000000" > > + > > +cd "$(dirname "$0")" > > +export PATH=$PWD/../src:$PWD/../mount-idmapped:$PATH > > +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH > > +. ./functions.sh > > +_require evmctl > > + > > +_cleanup() {> + if [ "$loop_mounted" = "1" ]; then > > These global variables make it quite a bit tricky even though it's 'just a test case'. They > could clash with variables elsewhere. Maybe prefix them with 'g_' if you don't want to > pass them as parameters into the function, which I would think is yet more preferable. Ok. Thanks Roberto > > + popd > /dev/null > > + > > + if [ -n "$mountpoint_idmapped" ]; then > > + umount $mountpoint_idmapped > > + fi > > + > > + umount $mountpoint > > + fi > > + > > + if [ -n "$dev" ]; then > > + losetup -d $dev > > + fi > > + > > + if [ -n "$image" ]; then > > + rm -f $image > > + fi > > + > > + if [ -n "$key_path_der" ]; then > > + rm -f $key_path_der > > + fi > > + > > + if [ -n "$mountpoint" ]; then > > + rm -Rf $mountpoint > > + fi > > + > > + if [ -n "$mountpoint_idmapped" ]; then > > + rm -Rf $mountpoint_idmapped > > + fi > > +} > > + > > +cleanup() { > > + _cleanup_user_mode _cleanup > > + _report_exit_and_cleanup > > +} > > + > > +get_xattr() { > > + format="hex" > > Don't want to use 'local format=....' to avoid clashes with possibly global variables of same name? > > I would also urge to consider using shellcheck on shell script files. It helps a bit. > > For now I leave it at these comment. > > Stefan
