* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes
@ 2018-01-11 20:28 Petr Vorel
2018-01-11 20:28 ` [LTP] [RFC PATCH 1/2] security/ima: " Petr Vorel
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Petr Vorel @ 2018-01-11 20:28 UTC (permalink / raw)
To: ltp
Hi,
I rewrote IMA tests to use new API + add small fixes.
I haven't tested ima_tpm.sh as I have no TPM :-(.
Comments are welcomed.
Kind regards,
Petr
Petr Vorel (2):
security/ima: Rewrite tests into new API + fixes
security/ima: Run measurements after policy
runtest/ima | 8 +-
.../integrity/ima/tests/ima_measurements.sh | 182 +++++++-----------
.../security/integrity/ima/tests/ima_policy.sh | 145 +++++++-------
.../security/integrity/ima/tests/ima_setup.sh | 109 +++++------
.../kernel/security/integrity/ima/tests/ima_tpm.sh | 129 ++++++-------
.../security/integrity/ima/tests/ima_violations.sh | 214 ++++++++++-----------
6 files changed, 337 insertions(+), 450 deletions(-)
mode change 100755 => 100644 testcases/kernel/security/integrity/ima/tests/ima_setup.sh
--
2.15.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [LTP] [RFC PATCH 1/2] security/ima: Rewrite tests into new API + fixes 2018-01-11 20:28 [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel @ 2018-01-11 20:28 ` Petr Vorel 2018-01-26 13:09 ` Cyril Hrubis 2018-01-11 20:28 ` [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy Petr Vorel ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Petr Vorel @ 2018-01-11 20:28 UTC (permalink / raw) To: ltp * simplify code, remove duplicity * ima_measurements.sh: - add support for sha256sum - check for i_version only for ext[2-4] filesystems (other filesystems don't report it) - chown only UID (GID of nobody is different on some OS, so it's better not to set it as it's not necessary for the test) * ima_policy.sh: - break tests instead of print TINFO when kernel is not configured to enable multiple writes to the IMA policy (IMA_WRITE_POLICY) - add warning when policy has been updated that reboot is needed * ima_violations.sh: - change check to measure occurrence of messages in log (previous way to grep tail of the log was buggy) + add sleep when SUT uses /var/log/messages to prevent failure during * remove duplicate whitespace from runtest file Signed-off-by: Petr Vorel <pvorel@suse.cz> --- runtest/ima | 8 +- .../integrity/ima/tests/ima_measurements.sh | 182 +++++++----------- .../security/integrity/ima/tests/ima_policy.sh | 145 +++++++------- .../security/integrity/ima/tests/ima_setup.sh | 109 +++++------ .../kernel/security/integrity/ima/tests/ima_tpm.sh | 129 ++++++------- .../security/integrity/ima/tests/ima_violations.sh | 214 ++++++++++----------- 6 files changed, 337 insertions(+), 450 deletions(-) mode change 100755 => 100644 testcases/kernel/security/integrity/ima/tests/ima_setup.sh diff --git a/runtest/ima b/runtest/ima index 251458af4..20d2e0810 100644 --- a/runtest/ima +++ b/runtest/ima @@ -1,5 +1,5 @@ #DESCRIPTION:Integrity Measurement Architecture (IMA) -ima01 ima_measurements.sh -ima02 ima_policy.sh -ima03 ima_tpm.sh -ima04 ima_violations.sh +ima01 ima_measurements.sh +ima02 ima_policy.sh +ima03 ima_tpm.sh +ima04 ima_violations.sh diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh index a3c357c8b..3993a575d 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh @@ -1,139 +1,93 @@ #!/bin/sh - -################################################################################ -## ## -## Copyright (C) 2009 IBM Corporation ## -## ## -## This program is free software; you can redistribute it and#or modify ## -## it under the terms of the GNU General Public License as published by ## -## the Free Software Foundation; either version 2 of the License, or ## -## (at your option) any later version. ## -## ## -## This program is distributed in the hope that it will be useful, but ## -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## -## for more details. ## -## ## -## You should have received a copy of the GNU General Public License ## -## along with this program; if not, write to the Free Software Foundation, ## -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## -## ## -################################################################################ +# Copyright (c) 2009 IBM Corporation +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# File : ima_measurements.sh +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. # -# Description: This file verifies measurements are added to the measurement -# list based on policy. +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com # -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com -################################################################################ -export TST_TOTAL=3 -export TCID="ima_measurements" +# Verify that measurements are added to the measurement list based on policy. + +TST_TESTFUNC="test" +TST_CNT=3 +. ima_setup.sh + +TEST_FILE="test.txt" +HASH_COMMAND="sha1sum" +POLICY="$IMA_DIR/policy" init() { - tst_check_cmds sha1sum - - # verify using default policy - if [ ! -f "$IMA_DIR/policy" ]; then - tst_resm TINFO "not using default policy" - fi + grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ + HASH_COMMAND="sha256sum" + tst_res TINFO "detected IMA algoritm: ${HASH_COMMAND%sum}" + tst_check_cmds $HASH_COMMAND + [ -f "$POLICY" ] || tst_res TINFO "not using default policy" } -# Function: test01 -# Description - Verify reading a file causes a new measurement to -# be added to the IMA measurement list. -test01() +ima_check() { - # Create file test.txt - cat > test.txt <<-EOF - $(date) - this is a test file - EOF - if [ $? -ne 0 ]; then - tst_brkm TBROK "Unable to create test file" - fi - - # Calculating the sha1sum of test.txt should add - # the measurement to the measurement list. - # (Assumes SHA1 IMA measurements.) - hash=$(sha1sum "test.txt" | sed 's/ -//') - - # Check if the file is measured - # (i.e. contained in the ascii measurement list.) - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements - sleep 1 - $(grep $hash measurements > /dev/null) - if [ $? -ne 0 ]; then - tst_resm TFAIL "TPM ascii measurement list does not contain sha1sum" - else - tst_resm TPASS "TPM ascii measurement list contains sha1sum" - fi + EXPECT_PASS grep -q $($HASH_COMMAND $TEST_FILE) $ASCII_MEASUREMENTS } -# Function: test02 -# Description - Verify modifying, then reading, a file causes a new -# measurement to be added to the IMA measurement list. -test02() +test1() { - # Modify test.txt - echo $(date) - file modified >> test.txt + tst_res TINFO "verify adding record to the IMA measurement list" + ROD echo "$(date) this is a test file" \> $TEST_FILE + ima_check +} - # Calculating the sha1sum of test.txt should add - # the new measurement to the measurement list - hash=$(sha1sum test.txt | sed 's/ -//') +test2() +{ + local device - # Check if the new measurement exists - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements - $(grep $hash measurements > /dev/null) + tst_res TINFO "verify updating record in the IMA measurement list" - if [ $? -ne 0 ]; then - tst_resm TFAIL "Modified file not measured" - tst_resm TINFO "iversion not supported; or not mounted with iversion" + device="$(df . | sed -e 1d | cut -f1 -d ' ')" + if grep -q $device /proc/mounts; then + if grep -q "${device}.*ext[2-4]" /proc/mounts; then + grep -q "${device}.*ext[2-4].*i_version" /proc/mounts || \ + tst_res TINFO "device '$device' is not mounted with iversion" + fi else - tst_resm TPASS "Modified file measured" + tst_res TWARN "could not find mount info for device '$device'" fi + + ROD echo "$(date) modified file" \> $TEST_FILE + ima_check } -# Function: test03 -# Description - Verify files are measured based on policy -# (Default policy does not measure user files.) -test03() +test3() { - # create file user-test.txt - mkdir -m 0700 user - chown nobody.nobody user - cd user - hash=0 - - # As user nobody, create and cat the new file - # (The LTP tests assumes existence of 'nobody'.) - sudo -n -u nobody sh -c "echo $(date) - create test.txt > ./test.txt; - cat ./test.txt > /dev/null" - - # Calculating the hash will add the measurement to the measurement - # list, so only calc the hash value after getting the measurement - # list. - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements - hash=$(sha1sum test.txt | sed 's/ -//') - cd - >/dev/null - - # Check if the file is measured - grep $hash measurements > /dev/null - if [ $? -ne 0 ]; then - tst_resm TPASS "user file test.txt not measured" - else - tst_resm TFAIL "user file test.txt measured" - fi -} + local dir="user" + local user="nobody" -. ima_setup.sh + tst_res TINFO "verify measuring user files" -setup -TST_CLEANUP=cleanup + id $user >/dev/null 2>/dev/null || tst_brk TCONF "missing system user $user (wrong installation)" + tst_check_cmds sudo -init -test01 -test02 -test03 + mkdir -m 0700 $dir + chown $user $dir + cd $dir + + sudo -n -u $user sh -c "echo $(date) user file > $TEST_FILE; + cat $TEST_FILE > /dev/null" -tst_exit + ima_check + cd .. +} + +init +tst_run diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh index ad5900975..162d323a1 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh @@ -1,127 +1,114 @@ #!/bin/sh -################################################################################ -## ## -## Copyright (C) 2009 IBM Corporation ## -## ## -## This program is free software; you can redistribute it and#or modify ## -## it under the terms of the GNU General Public License as published by ## -## the Free Software Foundation; either version 2 of the License, or ## -## (at your option) any later version. ## -## ## -## This program is distributed in the hope that it will be useful, but ## -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## -## for more details. ## -## ## -## You should have received a copy of the GNU General Public License ## -## along with this program; if not, write to the Free Software ## -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## -## ## -################################################################################ +# Copyright (c) 2009 IBM Corporation +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> # -# File : ima_policy.sh +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. # -# Description: This file tests replacing the default integrity measurement -# policy. +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com -################################################################################ -export TST_TOTAL=3 -export TCID="ima_policy" +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com +# +# Test replacing the default integrity measurement policy. + +TST_TESTFUNC="test" +TST_CNT=3 +. ima_setup.sh init() { - # verify using default policy - IMA_POLICY=$IMA_DIR/policy - if [ ! -f $IMA_POLICY ]; then - tst_resm TINFO "default policy already replaced" - fi + IMA_POLICY="$IMA_DIR/policy" + [ -f $IMA_POLICY ] || \ + tst_brk TCONF "IMA policy already loaded and kernel not configured to enable multiple writes it" - VALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy - if [ ! -f $VALID_POLICY ]; then - tst_resm TINFO "missing $VALID_POLICY" - fi + VALID_POLICY="$LTPROOT/testcases/data/ima_policy/measure.policy" + [ -f $VALID_POLICY ] || tst_brk TCONF "missing $VALID_POLICY" - INVALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy-invalid - if [ ! -f $INVALID_POLICY ]; then - tst_resm TINFO "missing $INVALID_POLICY" - fi + INVALID_POLICY="$LTPROOT/testcases/data/ima_policy/measure.policy-invalid" + [ -f $INVALID_POLICY ] || tst_brk TCONF "missing $INVALID_POLICY" } load_policy() { + local ret + exec 2>/dev/null 4>$IMA_POLICY - if [ $? -ne 0 ]; then - exit 1 - fi + [ $? -eq 0 ] || exit 1 cat $1 | - while read line ; do - { - if [ "${line#\#}" = "${line}" ] ; then - echo $line >&4 2> /dev/null + while read line; do + if [ "${line#\#}" = "${line}" ]; then + echo "$line" >&4 2> /dev/null if [ $? -ne 0 ]; then exec 4>&- return 1 fi fi - } done -} + ret=$? + [ $ret -eq 0 ] && \ + tst_res TINFO "IMA policy updated, please reboot after testing to restore settings" -# Function: test01 -# Description - Verify invalid policy doesn't replace default policy. -test01() + return $ret +} + +test1() { + tst_res TINFO "verify that invalid policy doesn't replace default policy" + + local p1 + load_policy $INVALID_POLICY & p1=$! wait "$p1" if [ $? -ne 0 ]; then - tst_resm TPASS "didn't load invalid policy" + tst_res TPASS "didn't load invalid policy" else - tst_resm TFAIL "loaded invalid policy" + tst_res TFAIL "loaded invalid policy" fi } -# Function: test02 -# Description - Verify policy file is opened sequentially, not concurrently -# and install new policy -test02() +test2() { + tst_res TINFO "verify that policy file is opened sequentially and installs new policy" + + local p1 p2 rc1 rc2 + load_policy $VALID_POLICY & p1=$! # forked process 1 load_policy $VALID_POLICY & p2=$! # forked process 2 - wait "$p1"; RC1=$? - wait "$p2"; RC2=$? - if [ $RC1 -eq 0 ] && [ $RC2 -eq 0 ]; then - tst_resm TFAIL "measurement policy opened concurrently" - elif [ $RC1 -eq 0 ] || [ $RC2 -eq 0 ]; then - tst_resm TPASS "replaced default measurement policy" + wait "$p1"; rc1=$? + wait "$p2"; rc2=$? + if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then + tst_res TFAIL "measurement policy opened concurrently" + elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then + tst_res TPASS "replaced default measurement policy" else - tst_resm TFAIL "problems opening measurement policy" + tst_res TFAIL "problems opening measurement policy" fi } -# Function: test03 -# Description - Verify can't load another measurement policy. -test03() +test3() { + tst_res TINFO "verify that valid policy isn't replaced" + + local p1 + load_policy $INVALID_POLICY & p1=$! wait "$p1" if [ $? -ne 0 ]; then - tst_resm TPASS "didn't replace valid policy" + tst_res TPASS "didn't replace valid policy" else - tst_resm TFAIL "replaced valid policy" + tst_res TFAIL "replaced valid policy" fi } -. ima_setup.sh - -setup -TST_CLEANUP=cleanup - init -test01 -test02 -test03 - -tst_exit +tst_run diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh old mode 100755 new mode 100644 index 0ff38d23b..7e19e3959 --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh @@ -1,86 +1,67 @@ #!/bin/sh -################################################################################ -## ## -## Copyright (C) 2009 IBM Corporation ## -## ## -## This program is free software; you can redistribute it and#or modify ## -## it under the terms of the GNU General Public License as published by ## -## the Free Software Foundation; either version 2 of the License, or ## -## (at your option) any later version. ## -## ## -## This program is distributed in the hope that it will be useful, but ## -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## -## for more details. ## -## ## -## You should have received a copy of the GNU General Public License ## -## along with this program; if not, write to the Free Software Foundation, ## -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## -## ## -################################################################################ +# Copyright (c) 2009 IBM Corporation +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> # -# File : ima_setup.sh +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. # -# Description: setup/cleanup routines for the integrity tests. +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com -################################################################################ -. test.sh -mount_sysfs() -{ - SYSFS=$(mount 2>/dev/null | awk '$5 == "sysfs" { print $3 }') - if [ "x$SYSFS" = x ] ; then +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com - SYSFS=/sys +TST_CLEANUP="cleanup" +TST_NEEDS_TMPDIR=1 +TST_NEEDS_ROOT=1 +. tst_test.sh - test -d $SYSFS || mkdir -p $SYSFS 2>/dev/null - if [ $? -ne 0 ] ; then - tst_brkm TBROK "Failed to mkdir $SYSFS" - fi - if ! mount -t sysfs sysfs $SYSFS 2>/dev/null ; then - tst_brkm TBROK "Failed to mount $SYSFS" - fi +export TCID="${TCID:-$(basename $0 | cut -d. -f1)}" - fi -} +UMOUNT= -mount_securityfs() +mount_helper() { - SECURITYFS=$(mount 2>/dev/null | awk '$5 == "securityfs" { print $3 }') - if [ "x$SECURITYFS" = x ] ; then - - SECURITYFS="$SYSFS/kernel/security" + local type="$1" + local default_dir="$2" + local dir - test -d $SECURITYFS || mkdir -p $SECURITYFS 2>/dev/null - if [ $? -ne 0 ] ; then - tst_brkm TBROK "Failed to mkdir $SECURITYFS" - fi - if ! mount -t securityfs securityfs $SECURITYFS 2>/dev/null ; then - tst_brkm TBROK "Failed to mount $SECURITYFS" - fi + dir="$(grep ^$type /proc/mounts | cut -d ' ' -f2 | head -1)" + [ -n "$dir" ] && { echo "$dir"; return; } + if ! mkdir -p $default_dir; then + tst_brk TBROK "Failed to create $default_dir" + fi + if ! mount -t $type $type $default_dir; then + tst_brk TBROK "Failed to mount $type" fi + UMOUNT="$default_dir $UMOUNT" + echo $default_dir } setup() { - tst_require_root + SYSFS="$(mount_helper sysfs /sys)" + SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)" - tst_tmpdir - - mount_sysfs - - # mount securityfs if it is not already mounted - mount_securityfs - - # IMA must be configured in the kernel - IMA_DIR=$SECURITYFS/ima - if [ ! -d "$IMA_DIR" ]; then - tst_brkm TCONF "IMA not enabled in kernel" - fi + IMA_DIR="$SECURITYFS/ima" + [ -d "$IMA_DIR" ] || tst_brk TCONF "IMA not enabled in kernel" + ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements" + BINARY_MEASUREMENTS="$IMA_DIR/binary_runtime_measurements" } cleanup() { - tst_rmdir + local dir + for dir in $UMOUNT; do + umount $dir + done } + +setup diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh index 333bf5f8a..a3d1739cd 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh @@ -1,70 +1,61 @@ #!/bin/sh - -################################################################################ -## ## -## Copyright (C) 2009 IBM Corporation ## -## ## -## This program is free software; you can redistribute it and#or modify ## -## it under the terms of the GNU General Public License as published by ## -## the Free Software Foundation; either version 2 of the License, or ## -## (at your option) any later version. ## -## ## -## This program is distributed in the hope that it will be useful, but ## -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## -## for more details. ## -## ## -## You should have received a copy of the GNU General Public License ## -## along with this program; if not, write to the Free Software ## -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## -## ## -################################################################################ +# Copyright (c) 2009 IBM Corporation +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. # -# File : ima_tpm.sh +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# Description: This file verifies the boot and PCR aggregates +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. # -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com # -# Return - zero on success -# - non zero on failure. return value from commands ($RC) -################################################################################ -export TST_TOTAL=3 -export TCID="ima_tpm" +# Verify the boot and PCR aggregates. + +TST_TESTFUNC="test" +TST_CNT=3 +. ima_setup.sh init() { tst_check_cmds ima_boot_aggregate ima_measure } -# Function: test01 -# Description - Verify boot aggregate value is correct -test01() +test1() { - zero="0000000000000000000000000000000000000000" + tst_res TINFO "verify boot aggregate" + + local zero="0000000000000000000000000000000000000000" + local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements" + local ima_measurements="$ASCII_MEASUREMENTS" + local ima_aggr line # IMA boot aggregate - ima_measurements=$SECURITYFS/ima/ascii_runtime_measurements read line < $ima_measurements ima_aggr=$(expr substr "${line}" 49 40) - # verify TPM is available and enabled. - tpm_bios=$SECURITYFS/tpm0/binary_bios_measurements if [ ! -f "$tpm_bios" ]; then - tst_brkm TCONF "TPM not builtin kernel, or TPM not enabled" + tst_brk TCONF "TPM not builtin kernel, or TPM not enabled" if [ "${ima_aggr}" = "${zero}" ]; then - tst_resm TPASS "bios boot aggregate is 0." + tst_res TPASS "bios boot aggregate is 0" else - tst_resm TFAIL "bios boot aggregate is not 0." + tst_res TFAIL "bios boot aggregate is not 0" fi else boot_aggregate=$(ima_boot_aggregate $tpm_bios) boot_aggr=$(expr substr $boot_aggregate 16 40) if [ "x${ima_aggr}" = "x${boot_aggr}" ]; then - tst_resm TPASS "bios aggregate matches IMA boot aggregate." + tst_res TPASS "bios aggregate matches IMA boot aggregate" else - tst_resm TFAIL "bios aggregate does not match IMA boot aggregate." + tst_res TFAIL "bios aggregate does not match IMA boot aggregate" fi fi } @@ -74,64 +65,54 @@ test01() # the PCR values from /sys/devices. validate_pcr() { - ima_measurements=$SECURITYFS/ima/binary_runtime_measurements - aggregate_pcr=$(ima_measure $ima_measurements --validate) - dev_pcrs=$1 - RC=0 + tst_res TINFO "verify PCR (Process Control Register)" - while read line ; do + local ima_measurements="$BINARY_MEASUREMENTS" + local aggregate_pcr="$(ima_measure $ima_measurements --validate)" + local dev_pcrs="$1" + local ret=0 + + while read line; do pcr=$(expr substr "${line}" 1 6) if [ "${pcr}" = "PCR-10" ]; then aggr=$(expr substr "${aggregate_pcr}" 26 59) pcr=$(expr substr "${line}" 9 59) - [ "${pcr}" = "${aggr}" ] || RC=$? + [ "${pcr}" = "${aggr}" ] || ret=$? fi done < $dev_pcrs - return $RC + return $ret } -# Function: test02 -# Description - Verify ima calculated aggregate PCR values matches -# actual PCR value. -test02() +test2() { + tst_res TINFO "verify PCR values" - # Would be nice to know where the PCRs are located. Is this safe? - PCRS_PATH=$(find /$SYSFS/devices/ | grep pcrs) + # Would be nice to know where the PCRs are located. Is this safe? + local pcrs_path="$(find $SYSFS/devices/ | grep pcrs)" if [ $? -eq 0 ]; then - validate_pcr $PCRS_PATH + validate_pcr $pcrs_path if [ $? -eq 0 ]; then - tst_resm TPASS "aggregate PCR value matches real PCR value." + tst_res TPASS "aggregate PCR value matches real PCR value" else - tst_resm TFAIL "aggregate PCR value does not match real PCR value." + tst_res TFAIL "aggregate PCR value does not match real PCR value" fi else - tst_resm TFAIL "TPM not enabled, no PCR value to validate" + tst_res TFAIL "TPM not enabled, no PCR value to validate" fi } -# Function: test03 -# Description - Verify template hash value for IMA entry is correct. -test03() +test3() { + tst_res TINFO "verify template hash value" - ima_measurements=$SECURITYFS/ima/binary_runtime_measurements - aggregate_pcr=$(ima_measure $ima_measurements --verify --validate) > /dev/null + local ima_measurements="$BINARY_MEASUREMENTS" + ima_measure $ima_measurements --verify --validate if [ $? -eq 0 ]; then - tst_resm TPASS "verified IMA template hash values." + tst_res TPASS "verified IMA template hash values" else - tst_resm TFAIL "error verifing IMA template hash values." + tst_res TFAIL "error verifing IMA template hash values" fi } -. ima_setup.sh - -setup -TST_CLEANUP=cleanup - init -test01 -test02 -test03 - -tst_exit +tst_run diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh index 1b86b5f1a..80a01a546 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh @@ -1,44 +1,45 @@ #!/bin/sh -################################################################################ -## ## -## Copyright (C) 2009 IBM Corporation ## -## ## -## This program is free software; you can redistribute it and#or modify ## -## it under the terms of the GNU General Public License as published by ## -## the Free Software Foundation; either version 2 of the License, or ## -## (at your option) any later version. ## -## ## -## This program is distributed in the hope that it will be useful, but ## -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## -## for more details. ## -## ## -## You should have received a copy of the GNU General Public License ## -## along with this program; if not, write to the Free Software ## -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## -## ## -################################################################################ +# Copyright (c) 2009 IBM Corporation +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> # -# File : ima_violations.sh +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. # -# Description: This file tests ToMToU and open_writer violations invalidate -# the PCR and are logged. +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. # -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. # -# Return - zero on success -# - non zero on failure. return value from commands ($RC) -################################################################################ +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com +# +# Test whether ToMToU and open_writer violations invalidatethe PCR and are logged. -export TST_TOTAL=3 -export TCID="ima_violations" +TST_TESTFUNC="test" +TST_CNT=3 +. ima_setup.sh -open_file_read() +FILE="test.txt" +IMA_VIOLATIONS="$SECURITYFS/ima/violations" + +init() { - exec 3< $1 - if [ $? -ne 0 ]; then - exit 1 + LOG="/var/log/messages" + SLEEP="500ms" + if service auditd status > /dev/null 2>&1; then + LOG="/var/log/audit/audit.log" + tst_res TINFO "requires integrity auditd patch" fi + tst_res TINFO "using log $LOG" +} + +open_file_read() +{ + exec 3< $FILE || exit 1 } close_file_read() @@ -48,11 +49,8 @@ close_file_read() open_file_write() { - exec 4> $1 - if [ $? -ne 0 ]; then - exit 1 - echo 'testing, testing, ' >&4 - fi + exec 4> $FILE || exit 1 + echo 'test writing' >&4 } close_file_write() @@ -60,103 +58,89 @@ close_file_write() exec 4>&- } -init() +get_count() { - service auditd status > /dev/null 2>&1 - if [ $? -ne 0 ]; then - log=/var/log/messages - else - log=/var/log/audit/audit.log - tst_resm TINFO "requires integrity auditd patch" - fi - - ima_violations=$SECURITYFS/ima/violations + local search="$1" + echo $(grep -c "${search}.*${FILE}" $LOG) } -# Function: test01 -# Description - Verify open writers violation -test01() +validate() { - read num_violations < $ima_violations - - TMPFN=test.txt - open_file_write $TMPFN - open_file_read $TMPFN - close_file_read - close_file_write - read num_violations_new < $ima_violations - num=$(($(expr $num_violations_new - $num_violations))) - if [ $num -gt 0 ]; then - tail $log | grep test.txt | grep -q 'open_writers' - if [ $? -eq 0 ]; then - tst_resm TPASS "open_writers violation added(test.txt)" + local num_violations="$1" + local count="$2" + local search="$3" + local count2="$(get_count $search)" + local num_violations_new + + [ -n "$SLEEP" ] && tst_sleep $SLEEP + + read num_violations_new < $IMA_VIOLATIONS + if [ $(($num_violations_new - $num_violations)) -gt 0 ]; then + if [ $count2 -gt $count ]; then + tst_res TPASS "$search violation added" else - tst_resm TFAIL "(message ratelimiting?)" + tst_res TFAIL "$search not found in $LOG" fi else - tst_resm TFAIL "open_writers violation not added(test.txt)" + tst_res TFAIL "$search violation not added" fi } -# Function: test02 -# Description - Verify ToMToU violation -test02() +test1() { - read num_violations < $ima_violations + tst_res TINFO "verify open writers violation" - TMPFN=test.txt - open_file_read $TMPFN - open_file_write $TMPFN - close_file_write + local search="open_writers" + local count num_violations + + read num_violations < $IMA_VIOLATIONS + count="$(get_count $search)" + + open_file_write + open_file_read close_file_read - read num_violations_new < $ima_violations - num=$(($(expr $num_violations_new - $num_violations))) - if [ $num -gt 0 ]; then - tail $log | grep test.txt | grep -q 'ToMToU' - if [ $? -eq 0 ]; then - tst_resm TPASS "ToMToU violation added(test.txt)" - else - tst_resm TFAIL "(message ratelimiting?)" - fi - else - tst_resm TFAIL "ToMToU violation not added(test.txt)" - fi + close_file_write + + validate $num_violations $count $search } -# Function: test03 -# Description - verify open_writers using mmapped files -test03() +test2() { - read num_violations < $ima_violations - - TMPFN=test.txtb - echo 'testing testing ' > $TMPFN - ima_mmap $TMPFN & p1=$! - sleep 1 # got to wait for ima_mmap to mmap the file - open_file_read $TMPFN - read num_violations_new < $ima_violations - num=$(($(expr $num_violations_new - $num_violations))) - if [ $num -gt 0 ]; then - tail $log | grep test.txtb | grep -q 'open_writers' - if [ $? -eq 0 ]; then - tst_resm TPASS "mmapped open_writers violation added(test.txtb)" - else - tst_resm TFAIL "(message ratelimiting?)" - fi - else - tst_resm TFAIL "mmapped open_writers violation not added(test.txtb)" - fi + tst_res TINFO "verify ToMToU violation" + + local search="ToMToU" + local count num_violations + + read num_violations < $IMA_VIOLATIONS + count="$(get_count $search)" + + open_file_read + open_file_write + close_file_write close_file_read + + validate $num_violations $count $search } -. ima_setup.sh +test3() +{ + tst_res TINFO "verify open_writers using mmapped files" -setup -TST_CLEANUP=cleanup + local search="open_writers" + local count num_violations -init -test01 -test02 -test03 + read num_violations < $IMA_VIOLATIONS + count="$(get_count $search)" + + echo 'testing testing ' > $FILE + ima_mmap $FILE & + sleep 1 -tst_exit + open_file_read + close_file_read + + validate $num_violations $count $search +} + +init +tst_run -- 2.15.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 1/2] security/ima: Rewrite tests into new API + fixes 2018-01-11 20:28 ` [LTP] [RFC PATCH 1/2] security/ima: " Petr Vorel @ 2018-01-26 13:09 ` Cyril Hrubis 0 siblings, 0 replies; 19+ messages in thread From: Cyril Hrubis @ 2018-01-26 13:09 UTC (permalink / raw) To: ltp Hi! > +# Verify that measurements are added to the measurement list based on policy. > + > +TST_TESTFUNC="test" > +TST_CNT=3 > +. ima_setup.sh > + > +TEST_FILE="test.txt" > +HASH_COMMAND="sha1sum" > +POLICY="$IMA_DIR/policy" > > init() > { > - tst_check_cmds sha1sum > - > - # verify using default policy > - if [ ! -f "$IMA_DIR/policy" ]; then > - tst_resm TINFO "not using default policy" > - fi > + grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ > + HASH_COMMAND="sha256sum" Grepping /boot/config-$foo is really broken, isn't there some sysfs or ioctl interface where we can figure out this info? > + tst_res TINFO "detected IMA algoritm: ${HASH_COMMAND%sum}" > + tst_check_cmds $HASH_COMMAND > + [ -f "$POLICY" ] || tst_res TINFO "not using default policy" > } > > -# Function: test01 > -# Description - Verify reading a file causes a new measurement to > -# be added to the IMA measurement list. > -test01() > +ima_check() > { > - # Create file test.txt > - cat > test.txt <<-EOF > - $(date) - this is a test file > - EOF > - if [ $? -ne 0 ]; then > - tst_brkm TBROK "Unable to create test file" > - fi > - > - # Calculating the sha1sum of test.txt should add > - # the measurement to the measurement list. > - # (Assumes SHA1 IMA measurements.) > - hash=$(sha1sum "test.txt" | sed 's/ -//') > - > - # Check if the file is measured > - # (i.e. contained in the ascii measurement list.) > - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements > - sleep 1 > - $(grep $hash measurements > /dev/null) > - if [ $? -ne 0 ]; then > - tst_resm TFAIL "TPM ascii measurement list does not contain sha1sum" > - else > - tst_resm TPASS "TPM ascii measurement list contains sha1sum" > - fi > + EXPECT_PASS grep -q $($HASH_COMMAND $TEST_FILE) $ASCII_MEASUREMENTS > } > > -# Function: test02 > -# Description - Verify modifying, then reading, a file causes a new > -# measurement to be added to the IMA measurement list. > -test02() > +test1() > { > - # Modify test.txt > - echo $(date) - file modified >> test.txt > + tst_res TINFO "verify adding record to the IMA measurement list" > + ROD echo "$(date) this is a test file" \> $TEST_FILE > + ima_check > +} > > - # Calculating the sha1sum of test.txt should add > - # the new measurement to the measurement list > - hash=$(sha1sum test.txt | sed 's/ -//') > +test2() > +{ > + local device > > - # Check if the new measurement exists > - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements > - $(grep $hash measurements > /dev/null) > + tst_res TINFO "verify updating record in the IMA measurement list" > > - if [ $? -ne 0 ]; then > - tst_resm TFAIL "Modified file not measured" > - tst_resm TINFO "iversion not supported; or not mounted with iversion" > + device="$(df . | sed -e 1d | cut -f1 -d ' ')" > + if grep -q $device /proc/mounts; then > + if grep -q "${device}.*ext[2-4]" /proc/mounts; then > + grep -q "${device}.*ext[2-4].*i_version" /proc/mounts || \ > + tst_res TINFO "device '$device' is not mounted with iversion" > + fi > else > - tst_resm TPASS "Modified file measured" > + tst_res TWARN "could not find mount info for device '$device'" > fi > + > + ROD echo "$(date) modified file" \> $TEST_FILE > + ima_check > } > > -# Function: test03 > -# Description - Verify files are measured based on policy > -# (Default policy does not measure user files.) > -test03() > +test3() > { > - # create file user-test.txt > - mkdir -m 0700 user > - chown nobody.nobody user > - cd user > - hash=0 > - > - # As user nobody, create and cat the new file > - # (The LTP tests assumes existence of 'nobody'.) > - sudo -n -u nobody sh -c "echo $(date) - create test.txt > ./test.txt; > - cat ./test.txt > /dev/null" > - > - # Calculating the hash will add the measurement to the measurement > - # list, so only calc the hash value after getting the measurement > - # list. > - cat /sys/kernel/security/ima/ascii_runtime_measurements > measurements > - hash=$(sha1sum test.txt | sed 's/ -//') > - cd - >/dev/null > - > - # Check if the file is measured > - grep $hash measurements > /dev/null > - if [ $? -ne 0 ]; then > - tst_resm TPASS "user file test.txt not measured" > - else > - tst_resm TFAIL "user file test.txt measured" > - fi > -} > + local dir="user" > + local user="nobody" > > -. ima_setup.sh > + tst_res TINFO "verify measuring user files" > > -setup > -TST_CLEANUP=cleanup > + id $user >/dev/null 2>/dev/null || tst_brk TCONF "missing system user $user (wrong installation)" > + tst_check_cmds sudo > > -init > -test01 > -test02 > -test03 > + mkdir -m 0700 $dir > + chown $user $dir > + cd $dir > + > + sudo -n -u $user sh -c "echo $(date) user file > $TEST_FILE; > + cat $TEST_FILE > /dev/null" > > -tst_exit > + ima_check > + cd .. > +} > + > +init ^ Any reason we don't pass this as TST_SETUP ? > +tst_run > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh > index ad5900975..162d323a1 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh > @@ -1,127 +1,114 @@ > #!/bin/sh > -################################################################################ > -## ## > -## Copyright (C) 2009 IBM Corporation ## > -## ## > -## This program is free software; you can redistribute it and#or modify ## > -## it under the terms of the GNU General Public License as published by ## > -## the Free Software Foundation; either version 2 of the License, or ## > -## (at your option) any later version. ## > -## ## > -## This program is distributed in the hope that it will be useful, but ## > -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## > -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## > -## for more details. ## > -## ## > -## You should have received a copy of the GNU General Public License ## > -## along with this program; if not, write to the Free Software ## > -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## > -## ## > -################################################################################ > +# Copyright (c) 2009 IBM Corporation > +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> > # > -# File : ima_policy.sh > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > # > -# Description: This file tests replacing the default integrity measurement > -# policy. > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > # > -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > -################################################################################ > -export TST_TOTAL=3 > -export TCID="ima_policy" > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > +# > +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > +# > +# Test replacing the default integrity measurement policy. > + > +TST_TESTFUNC="test" > +TST_CNT=3 > +. ima_setup.sh > > init() > { > - # verify using default policy > - IMA_POLICY=$IMA_DIR/policy > - if [ ! -f $IMA_POLICY ]; then > - tst_resm TINFO "default policy already replaced" > - fi > + IMA_POLICY="$IMA_DIR/policy" > + [ -f $IMA_POLICY ] || \ > + tst_brk TCONF "IMA policy already loaded and kernel not configured to enable multiple writes it" > > - VALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy > - if [ ! -f $VALID_POLICY ]; then > - tst_resm TINFO "missing $VALID_POLICY" > - fi > + VALID_POLICY="$LTPROOT/testcases/data/ima_policy/measure.policy" ^ $TST_DATAROOT > + [ -f $VALID_POLICY ] || tst_brk TCONF "missing $VALID_POLICY" > > - INVALID_POLICY=$LTPROOT/testcases/data/ima_policy/measure.policy-invalid > - if [ ! -f $INVALID_POLICY ]; then > - tst_resm TINFO "missing $INVALID_POLICY" > - fi > + INVALID_POLICY="$LTPROOT/testcases/data/ima_policy/measure.policy-invalid" > + [ -f $INVALID_POLICY ] || tst_brk TCONF "missing $INVALID_POLICY" > } > > load_policy() > { > + local ret > + > exec 2>/dev/null 4>$IMA_POLICY > - if [ $? -ne 0 ]; then > - exit 1 > - fi > + [ $? -eq 0 ] || exit 1 > > cat $1 | > - while read line ; do > - { > - if [ "${line#\#}" = "${line}" ] ; then > - echo $line >&4 2> /dev/null > + while read line; do > + if [ "${line#\#}" = "${line}" ]; then > + echo "$line" >&4 2> /dev/null > if [ $? -ne 0 ]; then > exec 4>&- > return 1 > fi > fi > - } > done > -} > + ret=$? > > + [ $ret -eq 0 ] && \ > + tst_res TINFO "IMA policy updated, please reboot after testing to restore settings" > > -# Function: test01 > -# Description - Verify invalid policy doesn't replace default policy. > -test01() > + return $ret > +} > + > +test1() > { > + tst_res TINFO "verify that invalid policy doesn't replace default policy" > + > + local p1 > + > load_policy $INVALID_POLICY & p1=$! > wait "$p1" > if [ $? -ne 0 ]; then > - tst_resm TPASS "didn't load invalid policy" > + tst_res TPASS "didn't load invalid policy" > else > - tst_resm TFAIL "loaded invalid policy" > + tst_res TFAIL "loaded invalid policy" > fi > } > > -# Function: test02 > -# Description - Verify policy file is opened sequentially, not concurrently > -# and install new policy > -test02() > +test2() > { > + tst_res TINFO "verify that policy file is opened sequentially and installs new policy" > + > + local p1 p2 rc1 rc2 > + > load_policy $VALID_POLICY & p1=$! # forked process 1 > load_policy $VALID_POLICY & p2=$! # forked process 2 > - wait "$p1"; RC1=$? > - wait "$p2"; RC2=$? > - if [ $RC1 -eq 0 ] && [ $RC2 -eq 0 ]; then > - tst_resm TFAIL "measurement policy opened concurrently" > - elif [ $RC1 -eq 0 ] || [ $RC2 -eq 0 ]; then > - tst_resm TPASS "replaced default measurement policy" > + wait "$p1"; rc1=$? > + wait "$p2"; rc2=$? > + if [ $rc1 -eq 0 ] && [ $rc2 -eq 0 ]; then > + tst_res TFAIL "measurement policy opened concurrently" > + elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then > + tst_res TPASS "replaced default measurement policy" > else > - tst_resm TFAIL "problems opening measurement policy" > + tst_res TFAIL "problems opening measurement policy" > fi > } > > -# Function: test03 > -# Description - Verify can't load another measurement policy. > -test03() > +test3() > { > + tst_res TINFO "verify that valid policy isn't replaced" > + > + local p1 > + > load_policy $INVALID_POLICY & p1=$! > wait "$p1" > if [ $? -ne 0 ]; then > - tst_resm TPASS "didn't replace valid policy" > + tst_res TPASS "didn't replace valid policy" > else > - tst_resm TFAIL "replaced valid policy" > + tst_res TFAIL "replaced valid policy" > fi > } > > -. ima_setup.sh > - > -setup > -TST_CLEANUP=cleanup > - > init > -test01 > -test02 > -test03 > - > -tst_exit > +tst_run > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > old mode 100755 > new mode 100644 > index 0ff38d23b..7e19e3959 > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > @@ -1,86 +1,67 @@ > #!/bin/sh > -################################################################################ > -## ## > -## Copyright (C) 2009 IBM Corporation ## > -## ## > -## This program is free software; you can redistribute it and#or modify ## > -## it under the terms of the GNU General Public License as published by ## > -## the Free Software Foundation; either version 2 of the License, or ## > -## (at your option) any later version. ## > -## ## > -## This program is distributed in the hope that it will be useful, but ## > -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## > -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## > -## for more details. ## > -## ## > -## You should have received a copy of the GNU General Public License ## > -## along with this program; if not, write to the Free Software Foundation, ## > -## Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## > -## ## > -################################################################################ > +# Copyright (c) 2009 IBM Corporation > +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> > # > -# File : ima_setup.sh > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > # > -# Description: setup/cleanup routines for the integrity tests. > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > # > -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > -################################################################################ > -. test.sh > -mount_sysfs() > -{ > - SYSFS=$(mount 2>/dev/null | awk '$5 == "sysfs" { print $3 }') > - if [ "x$SYSFS" = x ] ; then > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > +# > +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > > - SYSFS=/sys > +TST_CLEANUP="cleanup" > +TST_NEEDS_TMPDIR=1 > +TST_NEEDS_ROOT=1 > +. tst_test.sh > > - test -d $SYSFS || mkdir -p $SYSFS 2>/dev/null > - if [ $? -ne 0 ] ; then > - tst_brkm TBROK "Failed to mkdir $SYSFS" > - fi > - if ! mount -t sysfs sysfs $SYSFS 2>/dev/null ; then > - tst_brkm TBROK "Failed to mount $SYSFS" > - fi > +export TCID="${TCID:-$(basename $0 | cut -d. -f1)}" > > - fi > -} > +UMOUNT= > > -mount_securityfs() > +mount_helper() > { > - SECURITYFS=$(mount 2>/dev/null | awk '$5 == "securityfs" { print $3 }') > - if [ "x$SECURITYFS" = x ] ; then > - > - SECURITYFS="$SYSFS/kernel/security" > + local type="$1" > + local default_dir="$2" > + local dir > > - test -d $SECURITYFS || mkdir -p $SECURITYFS 2>/dev/null > - if [ $? -ne 0 ] ; then > - tst_brkm TBROK "Failed to mkdir $SECURITYFS" > - fi > - if ! mount -t securityfs securityfs $SECURITYFS 2>/dev/null ; then > - tst_brkm TBROK "Failed to mount $SECURITYFS" > - fi > + dir="$(grep ^$type /proc/mounts | cut -d ' ' -f2 | head -1)" > + [ -n "$dir" ] && { echo "$dir"; return; } > > + if ! mkdir -p $default_dir; then > + tst_brk TBROK "Failed to create $default_dir" > + fi > + if ! mount -t $type $type $default_dir; then > + tst_brk TBROK "Failed to mount $type" > fi > + UMOUNT="$default_dir $UMOUNT" > + echo $default_dir > } > > setup() > { > - tst_require_root > + SYSFS="$(mount_helper sysfs /sys)" Do we really still need to mount /sys as far as I can tell it's mounted automatically for more than 10 years now. > + SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)" > > - tst_tmpdir > - > - mount_sysfs > - > - # mount securityfs if it is not already mounted > - mount_securityfs > - > - # IMA must be configured in the kernel > - IMA_DIR=$SECURITYFS/ima > - if [ ! -d "$IMA_DIR" ]; then > - tst_brkm TCONF "IMA not enabled in kernel" > - fi > + IMA_DIR="$SECURITYFS/ima" > + [ -d "$IMA_DIR" ] || tst_brk TCONF "IMA not enabled in kernel" > + ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements" > + BINARY_MEASUREMENTS="$IMA_DIR/binary_runtime_measurements" > } > > cleanup() > { > - tst_rmdir > + local dir > + for dir in $UMOUNT; do > + umount $dir > + done > } > + > +setup > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh > index 333bf5f8a..a3d1739cd 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh > @@ -1,70 +1,61 @@ > #!/bin/sh > - > -################################################################################ > -## ## > -## Copyright (C) 2009 IBM Corporation ## > -## ## > -## This program is free software; you can redistribute it and#or modify ## > -## it under the terms of the GNU General Public License as published by ## > -## the Free Software Foundation; either version 2 of the License, or ## > -## (at your option) any later version. ## > -## ## > -## This program is distributed in the hope that it will be useful, but ## > -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## > -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## > -## for more details. ## > -## ## > -## You should have received a copy of the GNU General Public License ## > -## along with this program; if not, write to the Free Software ## > -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## > -## ## > -################################################################################ > +# Copyright (c) 2009 IBM Corporation > +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > # > -# File : ima_tpm.sh > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > # > -# Description: This file verifies the boot and PCR aggregates > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > # > -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > # > -# Return - zero on success > -# - non zero on failure. return value from commands ($RC) > -################################################################################ > -export TST_TOTAL=3 > -export TCID="ima_tpm" > +# Verify the boot and PCR aggregates. > + > +TST_TESTFUNC="test" > +TST_CNT=3 > +. ima_setup.sh > > init() > { > tst_check_cmds ima_boot_aggregate ima_measure > } > > -# Function: test01 > -# Description - Verify boot aggregate value is correct > -test01() > +test1() > { > - zero="0000000000000000000000000000000000000000" > + tst_res TINFO "verify boot aggregate" > + > + local zero="0000000000000000000000000000000000000000" > + local tpm_bios="$SECURITYFS/tpm0/binary_bios_measurements" > + local ima_measurements="$ASCII_MEASUREMENTS" > + local ima_aggr line > > # IMA boot aggregate > - ima_measurements=$SECURITYFS/ima/ascii_runtime_measurements > read line < $ima_measurements > ima_aggr=$(expr substr "${line}" 49 40) > > - # verify TPM is available and enabled. > - tpm_bios=$SECURITYFS/tpm0/binary_bios_measurements > if [ ! -f "$tpm_bios" ]; then > - tst_brkm TCONF "TPM not builtin kernel, or TPM not enabled" > + tst_brk TCONF "TPM not builtin kernel, or TPM not enabled" > > if [ "${ima_aggr}" = "${zero}" ]; then > - tst_resm TPASS "bios boot aggregate is 0." > + tst_res TPASS "bios boot aggregate is 0" > else > - tst_resm TFAIL "bios boot aggregate is not 0." > + tst_res TFAIL "bios boot aggregate is not 0" > fi > else > boot_aggregate=$(ima_boot_aggregate $tpm_bios) > boot_aggr=$(expr substr $boot_aggregate 16 40) > if [ "x${ima_aggr}" = "x${boot_aggr}" ]; then > - tst_resm TPASS "bios aggregate matches IMA boot aggregate." > + tst_res TPASS "bios aggregate matches IMA boot aggregate" > else > - tst_resm TFAIL "bios aggregate does not match IMA boot aggregate." > + tst_res TFAIL "bios aggregate does not match IMA boot aggregate" > fi > fi > } > @@ -74,64 +65,54 @@ test01() > # the PCR values from /sys/devices. > validate_pcr() > { > - ima_measurements=$SECURITYFS/ima/binary_runtime_measurements > - aggregate_pcr=$(ima_measure $ima_measurements --validate) > - dev_pcrs=$1 > - RC=0 > + tst_res TINFO "verify PCR (Process Control Register)" > > - while read line ; do > + local ima_measurements="$BINARY_MEASUREMENTS" > + local aggregate_pcr="$(ima_measure $ima_measurements --validate)" > + local dev_pcrs="$1" > + local ret=0 > + > + while read line; do > pcr=$(expr substr "${line}" 1 6) > if [ "${pcr}" = "PCR-10" ]; then > aggr=$(expr substr "${aggregate_pcr}" 26 59) > pcr=$(expr substr "${line}" 9 59) > - [ "${pcr}" = "${aggr}" ] || RC=$? > + [ "${pcr}" = "${aggr}" ] || ret=$? > fi > done < $dev_pcrs > - return $RC > + return $ret > } > > -# Function: test02 > -# Description - Verify ima calculated aggregate PCR values matches > -# actual PCR value. > -test02() > +test2() > { > + tst_res TINFO "verify PCR values" > > - # Would be nice to know where the PCRs are located. Is this safe? > - PCRS_PATH=$(find /$SYSFS/devices/ | grep pcrs) > + # Would be nice to know where the PCRs are located. Is this safe? > + local pcrs_path="$(find $SYSFS/devices/ | grep pcrs)" > if [ $? -eq 0 ]; then > - validate_pcr $PCRS_PATH > + validate_pcr $pcrs_path > if [ $? -eq 0 ]; then > - tst_resm TPASS "aggregate PCR value matches real PCR value." > + tst_res TPASS "aggregate PCR value matches real PCR value" > else > - tst_resm TFAIL "aggregate PCR value does not match real PCR value." > + tst_res TFAIL "aggregate PCR value does not match real PCR value" > fi > else > - tst_resm TFAIL "TPM not enabled, no PCR value to validate" > + tst_res TFAIL "TPM not enabled, no PCR value to validate" > fi > } > > -# Function: test03 > -# Description - Verify template hash value for IMA entry is correct. > -test03() > +test3() > { > + tst_res TINFO "verify template hash value" > > - ima_measurements=$SECURITYFS/ima/binary_runtime_measurements > - aggregate_pcr=$(ima_measure $ima_measurements --verify --validate) > /dev/null > + local ima_measurements="$BINARY_MEASUREMENTS" > + ima_measure $ima_measurements --verify --validate > if [ $? -eq 0 ]; then > - tst_resm TPASS "verified IMA template hash values." > + tst_res TPASS "verified IMA template hash values" > else > - tst_resm TFAIL "error verifing IMA template hash values." > + tst_res TFAIL "error verifing IMA template hash values" > fi > } > > -. ima_setup.sh > - > -setup > -TST_CLEANUP=cleanup > - > init Here as well. > -test01 > -test02 > -test03 > - > -tst_exit > +tst_run > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh > index 1b86b5f1a..80a01a546 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh > @@ -1,44 +1,45 @@ > #!/bin/sh > -################################################################################ > -## ## > -## Copyright (C) 2009 IBM Corporation ## > -## ## > -## This program is free software; you can redistribute it and#or modify ## > -## it under the terms of the GNU General Public License as published by ## > -## the Free Software Foundation; either version 2 of the License, or ## > -## (at your option) any later version. ## > -## ## > -## This program is distributed in the hope that it will be useful, but ## > -## WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY ## > -## or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License ## > -## for more details. ## > -## ## > -## You should have received a copy of the GNU General Public License ## > -## along with this program; if not, write to the Free Software ## > -## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA ## > -## ## > -################################################################################ > +# Copyright (c) 2009 IBM Corporation > +# Copyright (c) 2018 Petr Vorel <pvorel@suse.cz> > # > -# File : ima_violations.sh > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > # > -# Description: This file tests ToMToU and open_writer violations invalidate > -# the PCR and are logged. > +# This program is distributed in the hope that it would be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > # > -# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > # > -# Return - zero on success > -# - non zero on failure. return value from commands ($RC) > -################################################################################ > +# Author: Mimi Zohar, zohar@ibm.vnet.ibm.com > +# > +# Test whether ToMToU and open_writer violations invalidatethe PCR and are logged. > > -export TST_TOTAL=3 > -export TCID="ima_violations" > +TST_TESTFUNC="test" > +TST_CNT=3 > +. ima_setup.sh > > -open_file_read() > +FILE="test.txt" > +IMA_VIOLATIONS="$SECURITYFS/ima/violations" > + > +init() > { > - exec 3< $1 > - if [ $? -ne 0 ]; then > - exit 1 > + LOG="/var/log/messages" > + SLEEP="500ms" > + if service auditd status > /dev/null 2>&1; then Here we depend on service being installed, which unfortunately is not the case for all currently supported distributions. Have a look at testcases/lib/daemonlib.sh and status_daemon() function there. > + LOG="/var/log/audit/audit.log" > + tst_res TINFO "requires integrity auditd patch" > fi > + tst_res TINFO "using log $LOG" > +} > + > +open_file_read() > +{ > + exec 3< $FILE || exit 1 > } > > close_file_read() > @@ -48,11 +49,8 @@ close_file_read() > > open_file_write() > { > - exec 4> $1 > - if [ $? -ne 0 ]; then > - exit 1 > - echo 'testing, testing, ' >&4 > - fi > + exec 4> $FILE || exit 1 > + echo 'test writing' >&4 > } > > close_file_write() > @@ -60,103 +58,89 @@ close_file_write() > exec 4>&- > } > > -init() > +get_count() > { > - service auditd status > /dev/null 2>&1 > - if [ $? -ne 0 ]; then > - log=/var/log/messages > - else > - log=/var/log/audit/audit.log > - tst_resm TINFO "requires integrity auditd patch" > - fi > - > - ima_violations=$SECURITYFS/ima/violations > + local search="$1" > + echo $(grep -c "${search}.*${FILE}" $LOG) > } > > -# Function: test01 > -# Description - Verify open writers violation > -test01() > +validate() > { > - read num_violations < $ima_violations > - > - TMPFN=test.txt > - open_file_write $TMPFN > - open_file_read $TMPFN > - close_file_read > - close_file_write > - read num_violations_new < $ima_violations > - num=$(($(expr $num_violations_new - $num_violations))) > - if [ $num -gt 0 ]; then > - tail $log | grep test.txt | grep -q 'open_writers' > - if [ $? -eq 0 ]; then > - tst_resm TPASS "open_writers violation added(test.txt)" > + local num_violations="$1" > + local count="$2" > + local search="$3" > + local count2="$(get_count $search)" > + local num_violations_new > + > + [ -n "$SLEEP" ] && tst_sleep $SLEEP > + > + read num_violations_new < $IMA_VIOLATIONS > + if [ $(($num_violations_new - $num_violations)) -gt 0 ]; then > + if [ $count2 -gt $count ]; then > + tst_res TPASS "$search violation added" > else > - tst_resm TFAIL "(message ratelimiting?)" > + tst_res TFAIL "$search not found in $LOG" > fi > else > - tst_resm TFAIL "open_writers violation not added(test.txt)" > + tst_res TFAIL "$search violation not added" > fi > } > > -# Function: test02 > -# Description - Verify ToMToU violation > -test02() > +test1() > { > - read num_violations < $ima_violations > + tst_res TINFO "verify open writers violation" > > - TMPFN=test.txt > - open_file_read $TMPFN > - open_file_write $TMPFN > - close_file_write > + local search="open_writers" > + local count num_violations > + > + read num_violations < $IMA_VIOLATIONS > + count="$(get_count $search)" > + > + open_file_write > + open_file_read > close_file_read > - read num_violations_new < $ima_violations > - num=$(($(expr $num_violations_new - $num_violations))) > - if [ $num -gt 0 ]; then > - tail $log | grep test.txt | grep -q 'ToMToU' > - if [ $? -eq 0 ]; then > - tst_resm TPASS "ToMToU violation added(test.txt)" > - else > - tst_resm TFAIL "(message ratelimiting?)" > - fi > - else > - tst_resm TFAIL "ToMToU violation not added(test.txt)" > - fi > + close_file_write > + > + validate $num_violations $count $search > } > > -# Function: test03 > -# Description - verify open_writers using mmapped files > -test03() > +test2() > { > - read num_violations < $ima_violations > - > - TMPFN=test.txtb > - echo 'testing testing ' > $TMPFN > - ima_mmap $TMPFN & p1=$! > - sleep 1 # got to wait for ima_mmap to mmap the file > - open_file_read $TMPFN > - read num_violations_new < $ima_violations > - num=$(($(expr $num_violations_new - $num_violations))) > - if [ $num -gt 0 ]; then > - tail $log | grep test.txtb | grep -q 'open_writers' > - if [ $? -eq 0 ]; then > - tst_resm TPASS "mmapped open_writers violation added(test.txtb)" > - else > - tst_resm TFAIL "(message ratelimiting?)" > - fi > - else > - tst_resm TFAIL "mmapped open_writers violation not added(test.txtb)" > - fi > + tst_res TINFO "verify ToMToU violation" > + > + local search="ToMToU" > + local count num_violations > + > + read num_violations < $IMA_VIOLATIONS > + count="$(get_count $search)" > + > + open_file_read > + open_file_write > + close_file_write > close_file_read > + > + validate $num_violations $count $search > } > > -. ima_setup.sh > +test3() > +{ > + tst_res TINFO "verify open_writers using mmapped files" > > -setup > -TST_CLEANUP=cleanup > + local search="open_writers" > + local count num_violations > > -init > -test01 > -test02 > -test03 > + read num_violations < $IMA_VIOLATIONS > + count="$(get_count $search)" > + > + echo 'testing testing ' > $FILE > + ima_mmap $FILE & > + sleep 1 What do we sleep here for? > + open_file_read > + close_file_read > + > + validate $num_violations $count $search > +} > + > +init > +tst_run > -- > 2.15.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy 2018-01-11 20:28 [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel 2018-01-11 20:28 ` [LTP] [RFC PATCH 1/2] security/ima: " Petr Vorel @ 2018-01-11 20:28 ` Petr Vorel 2018-01-26 13:11 ` Cyril Hrubis 2018-01-24 17:12 ` [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel 2018-01-24 17:36 ` Mimi Zohar 3 siblings, 1 reply; 19+ messages in thread From: Petr Vorel @ 2018-01-11 20:28 UTC (permalink / raw) To: ltp This fixes failing policy tests when no IMA is configured on SUT. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- runtest/ima | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtest/ima b/runtest/ima index 20d2e0810..3462d12b1 100644 --- a/runtest/ima +++ b/runtest/ima @@ -1,5 +1,5 @@ #DESCRIPTION:Integrity Measurement Architecture (IMA) -ima01 ima_measurements.sh -ima02 ima_policy.sh +ima01 ima_policy.sh +ima02 ima_measurements.sh ima03 ima_tpm.sh ima04 ima_violations.sh -- 2.15.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy 2018-01-11 20:28 ` [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy Petr Vorel @ 2018-01-26 13:11 ` Cyril Hrubis 2018-01-26 18:03 ` Petr Vorel 0 siblings, 1 reply; 19+ messages in thread From: Cyril Hrubis @ 2018-01-26 13:11 UTC (permalink / raw) To: ltp Hi! > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > runtest/ima | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/runtest/ima b/runtest/ima > index 20d2e0810..3462d12b1 100644 > --- a/runtest/ima > +++ b/runtest/ima > @@ -1,5 +1,5 @@ > #DESCRIPTION:Integrity Measurement Architecture (IMA) > -ima01 ima_measurements.sh > -ima02 ima_policy.sh > +ima01 ima_policy.sh > +ima02 ima_measurements.sh > ima03 ima_tpm.sh > ima04 ima_violations.sh Uh, depending on order of testcases in runtest file is broken anyways, what is the real problem here? Also I suppose that we may as well rename the test ids (e.g. ima01) to match the shell script name, since I find it more descriptive. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy 2018-01-26 13:11 ` Cyril Hrubis @ 2018-01-26 18:03 ` Petr Vorel 2018-01-28 0:57 ` Mimi Zohar 0 siblings, 1 reply; 19+ messages in thread From: Petr Vorel @ 2018-01-26 18:03 UTC (permalink / raw) To: ltp Hi, > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > runtest/ima | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/runtest/ima b/runtest/ima > > index 20d2e0810..3462d12b1 100644 > > --- a/runtest/ima > > +++ b/runtest/ima > > @@ -1,5 +1,5 @@ > > #DESCRIPTION:Integrity Measurement Architecture (IMA) > > -ima01 ima_measurements.sh > > -ima02 ima_policy.sh > > +ima01 ima_policy.sh > > +ima02 ima_measurements.sh > > ima03 ima_tpm.sh > > ima04 ima_violations.sh > Uh, depending on order of testcases in runtest file is broken anyways, > what is the real problem here? If system is configured with no policy, ima_measurements.sh fails. ima_policy.sh loads some policy (if none loaded) / adds to policy (if policy already loaded and it's allowed by kernel). So, the first case prevents failing ima_measurements.sh. One problem with IMA testing I see is that IMHO it's not possible to revert policy. That's why I added warnings that reboot is required. I know that this is against LTP principle. Mimi, Dmitry, am I right? > Also I suppose that we may as well rename the test ids (e.g. ima01) to > match the shell script name, since I find it more descriptive. Sure! Kind regards, Petr ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy 2018-01-26 18:03 ` Petr Vorel @ 2018-01-28 0:57 ` Mimi Zohar 0 siblings, 0 replies; 19+ messages in thread From: Mimi Zohar @ 2018-01-28 0:57 UTC (permalink / raw) To: ltp On Fri, 2018-01-26 at 19:03 +0100, Petr Vorel wrote: > Hi, > > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > > --- > > > runtest/ima | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/runtest/ima b/runtest/ima > > > index 20d2e0810..3462d12b1 100644 > > > --- a/runtest/ima > > > +++ b/runtest/ima > > > @@ -1,5 +1,5 @@ > > > #DESCRIPTION:Integrity Measurement Architecture (IMA) > > > -ima01 ima_measurements.sh > > > -ima02 ima_policy.sh > > > +ima01 ima_policy.sh > > > +ima02 ima_measurements.sh > > > ima03 ima_tpm.sh > > > ima04 ima_violations.sh > > > Uh, depending on order of testcases in runtest file is broken anyways, > > what is the real problem here? > If system is configured with no policy, ima_measurements.sh fails. ima_policy.sh loads > some policy (if none loaded) / adds to policy (if policy already loaded and it's allowed > by kernel). So, the first case prevents failing ima_measurements.sh. > One problem with IMA testing I see is that IMHO it's not possible to revert policy. > That's why I added warnings that reboot is required. I know that this is against LTP > principle. > Mimi, Dmitry, am I right? The current ordering of the tests assume that the system was booted with the builtin "ima_tcb" policy enabled on the boot command line. Assuming that the kernel doesn't require policies to be signed, changing the order of the tests is fine. Or simply test whether the system was booted with either "ima_tcb" or "ima_policy=tcb" boot command line options. Mimi > > Also I suppose that we may as well rename the test ids (e.g. ima01) to > > match the shell script name, since I find it more descriptive. > Sure! > > > Kind regards, > Petr > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-11 20:28 [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel 2018-01-11 20:28 ` [LTP] [RFC PATCH 1/2] security/ima: " Petr Vorel 2018-01-11 20:28 ` [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy Petr Vorel @ 2018-01-24 17:12 ` Petr Vorel 2018-01-24 17:36 ` Mimi Zohar 3 siblings, 0 replies; 19+ messages in thread From: Petr Vorel @ 2018-01-24 17:12 UTC (permalink / raw) To: ltp Hi Cyril, > I rewrote IMA tests to use new API + add small fixes. > I haven't tested ima_tpm.sh as I have no TPM :-(. > Comments are welcomed. > Kind regards, > Petr > Petr Vorel (2): > security/ima: Rewrite tests into new API + fixes > security/ima: Run measurements after policy As nobody from IMA devs cares, I suggest merging it. But general review from LTP point of view would be nice. Kind regards, Petr ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-11 20:28 [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel ` (2 preceding siblings ...) 2018-01-24 17:12 ` [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel @ 2018-01-24 17:36 ` Mimi Zohar 2018-01-25 20:30 ` Petr Vorel 3 siblings, 1 reply; 19+ messages in thread From: Mimi Zohar @ 2018-01-24 17:36 UTC (permalink / raw) To: ltp Hi Petr, [Cc'ing Roberto] On Thu, 2018-01-11 at 21:28 +0100, Petr Vorel wrote: > Hi, > > I rewrote IMA tests to use new API + add small fixes. > I haven't tested ima_tpm.sh as I have no TPM :-(. > > Comments are welcomed. The LTP tests are quite dated, and need some major rework. I really appreciate your addressing some of the issues. Below are some additional ones. Tests "ima02 ima_measurement.sh" and "ima04 ima_violations.sh" assume files are created on a filesystem in policy. The "measure.policy" excludes tmpfs, yet TMPDIR defaults to a tmpfs filesystem. There are a couple of ways of resolving this problem (eg. removing tmpfs from the "measure.policy", use a RAM block device instead of tmpfs, etc). Since the builtin "ima_policy=tcb" also excludes tmpfs, not using a tmpfs filesystem would be preferable. Originally IMA allowed a builtin policy to be replaced with a custom policy, by simply cat'ing a file into the securityfs IMA policy file. Currently, if new rules can be added to the custom policy (Kconfig IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, if the builtin "secure-boot" policy is defined on the boot command line, the custom policy must be signed. Test "ima01 ima_policy.sh" should first detect if the policy must be signed, before running the tests. ima_boot_aggregate.c defines the BIOS MAX_EVENT_SIZE BIOS size as 500, but I'm currently seeing BIOS events larger than 4k. Since these tests were first written, Roberto's IMA templates and Dmitry's support for larger digests were upstreamed. With the new template format, the file hash is prefixed with the hash algorithm. Before comparing the calculated boot aggregate with the value in the IMA measurement list, the hash algorithm needs to be removed. For the new template format measurement lists, walking the measurement list, re-calculating the PCRs and comparing them with the HW or vTPM PCRs fail. The ima-evm-utils package has a working version. Invoke "evmctl" with the "ima_mesaurement" option. thanks, Mimi ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-24 17:36 ` Mimi Zohar @ 2018-01-25 20:30 ` Petr Vorel 2018-01-25 20:40 ` Petr Vorel ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Petr Vorel @ 2018-01-25 20:30 UTC (permalink / raw) To: ltp Hi Mimi, > Hi Petr, > [Cc'ing Roberto] > On Thu, 2018-01-11 at 21:28 +0100, Petr Vorel wrote: > > Hi, > > I rewrote IMA tests to use new API + add small fixes. > > I haven't tested ima_tpm.sh as I have no TPM :-(. > > Comments are welcomed. > The LTP tests are quite dated, and need some major rework. I really > appreciate your addressing some of the issues. Below are some > additional ones. Thanks a lot for your comments. As you didn't report any regression in my patch-set, I'm for merging it as it's an improvement. But I see there is more work to be done. Your comments or patches are always welcomed. Is there more recent version of testcases/kernel/security/integrity/ima/README [1] ? > Tests "ima02 ima_measurement.sh" and "ima04 ima_violations.sh" assume > files are created on a filesystem in policy. The "measure.policy" > excludes tmpfs, yet TMPDIR defaults to a tmpfs filesystem. There are > a couple of ways of resolving this problem (eg. removing tmpfs from > the "measure.policy", use a RAM block device instead of tmpfs, etc). > Since the builtin "ima_policy=tcb" also excludes tmpfs, not using a > tmpfs filesystem would be preferable. OK, I'll try to implement test using RAM block device. > Originally IMA allowed a builtin policy to be replaced with a custom > policy, by simply cat'ing a file into the securityfs IMA policy file. > Currently, if new rules can be added to the custom policy (Kconfig > IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, > if the builtin "secure-boot" policy is defined on the boot command > line, the custom policy must be signed. Test "ima01 ima_policy.sh" > should first detect if the policy must be signed, before running the > tests. Right, I'll check it. Is there other way how to detect it than reading /boot/config-$(uname -r) or /proc/config.gz ? I'm asking because IMA might be using on embedded devices (guessing from [2], [3]), which might not have either of them. > ima_boot_aggregate.c defines the BIOS MAX_EVENT_SIZE BIOS size as 500, > but I'm currently seeing BIOS events larger than 4k. So, what is the recommended size? Any reference to it? > Since these tests were first written, Roberto's IMA templates and > Dmitry's support for larger digests were upstreamed. With the new > template format, the file hash is prefixed with the hash algorithm. > Before comparing the calculated boot aggregate with the value in the > IMA measurement list, the hash algorithm needs to be removed. Do you mean entries in /sys/kernel/security/ima/ascii_runtime_measurements ? system with config CONFIG_IMA_DEFAULT_HASH_SHA256=y 10 4814642f7955ad7a9c7b47785d002374b34902fd ima-ng sha256:f20cec9d158c4c453899f97595c40257c2518a40a310a550a1cd26a63e7fff7a /usr/lib64/libsha1detectcoll.so.1.0.0 system with config CONFIG_IMA_DEFAULT_HASH_SHA1=y 10 2990cfe74ff309268e4fb928102574c28f9bb876 ima-ng sha1:71b543ad6af36b0976d0e3f71fed4ce0954eda0c /var/log/messages As it's done with grep it shouldn't be needed: grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ HASH_COMMAND="sha256sum" I kept sha1sum as the default command for checking and I'm detecting with CONFIG_IMA_DEFAULT_HASH_SHA256 whether to use sha256: This is not enough, I'll add checks for CONFIG_IMA_DEFAULT_HASH_SHA512 and CONFIG_IMA_DEFAULT_HASH_WP512. > > For the new template format measurement lists, walking the measurement > list, re-calculating the PCRs and comparing them with the HW or vTPM > PCRs fail. The ima-evm-utils package has a working version. Invoke > "evmctl" with the "ima_measurement" option. So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your repository on sf.net [4]? Fortunately this package is on all major distros [5] (except Debian, but Ubuntu package is installable on Debian), so we don't need to include your repository as submodule. > thanks, > Mimi Kind regards, Petr [1] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/security/integrity/ima/README [2] http://kernsec.org/files/lss2015/ima-applications-slides.pdf [3] https://archive.fosdem.org/2014/schedule/event/integrity_protection_solutions_for_embedded_systems/attachments/slides/414/export/events/attachments/integrity_protection_solutions_for_embedded_systems/slides/414/Integrity_Protection_For_Embedded_Systems_FOSDEM_2014.pdf [4] https://git.code.sf.net/p/linux-ima/ima-evm-utils [5] https://pkgs.org/download/ima-evm-utils ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-25 20:30 ` Petr Vorel @ 2018-01-25 20:40 ` Petr Vorel 2018-01-25 22:29 ` Mimi Zohar 2018-01-26 13:16 ` Cyril Hrubis 2 siblings, 0 replies; 19+ messages in thread From: Petr Vorel @ 2018-01-25 20:40 UTC (permalink / raw) To: ltp Hi Mimi, > > Since these tests were first written, Roberto's IMA templates and > > Dmitry's support for larger digests were upstreamed. With the new > > template format, the file hash is prefixed with the hash algorithm. > > Before comparing the calculated boot aggregate with the value in the > > IMA measurement list, the hash algorithm needs to be removed. > Do you mean entries in /sys/kernel/security/ima/ascii_runtime_measurements ? > system with config CONFIG_IMA_DEFAULT_HASH_SHA256=y > 10 4814642f7955ad7a9c7b47785d002374b34902fd ima-ng sha256:f20cec9d158c4c453899f97595c40257c2518a40a310a550a1cd26a63e7fff7a /usr/lib64/libsha1detectcoll.so.1.0.0 > system with config CONFIG_IMA_DEFAULT_HASH_SHA1=y > 10 2990cfe74ff309268e4fb928102574c28f9bb876 ima-ng sha1:71b543ad6af36b0976d0e3f71fed4ce0954eda0c /var/log/messages > As it's done with grep it shouldn't be needed: > grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ > HASH_COMMAND="sha256sum" Here is the part where I grep. ASCII_MEASUREMENTS="$IMA_DIR/ascii_runtime_measurements" # from ima_setup.sh ima_check() { EXPECT_PASS grep -q $($HASH_COMMAND $TEST_FILE) $ASCII_MEASUREMENTS } Or is it your note for other test. BTW as I don't have any TPM hw, it would be great if anyone with it could test the code. Kind regards, Petr ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-25 20:30 ` Petr Vorel 2018-01-25 20:40 ` Petr Vorel @ 2018-01-25 22:29 ` Mimi Zohar 2018-01-26 17:51 ` Petr Vorel 2018-01-26 13:16 ` Cyril Hrubis 2 siblings, 1 reply; 19+ messages in thread From: Mimi Zohar @ 2018-01-25 22:29 UTC (permalink / raw) To: ltp On Thu, 2018-01-25 at 21:30 +0100, Petr Vorel wrote: > Hi Mimi, > > > Hi Petr, > > > [Cc'ing Roberto] > > > On Thu, 2018-01-11 at 21:28 +0100, Petr Vorel wrote: > > > Hi, > > > > I rewrote IMA tests to use new API + add small fixes. > > > I haven't tested ima_tpm.sh as I have no TPM :-(. > > > > Comments are welcomed. > > > The LTP tests are quite dated, and need some major rework. I really > > appreciate your addressing some of the issues. Below are some > > additional ones. > Thanks a lot for your comments. As you didn't report any regression in my patch-set, I'm > for merging it as it's an improvement. But I see there is more work to be done. Sounds good. > Your comments or patches are always welcomed. Thank you. > Is there more recent version of testcases/kernel/security/integrity/ima/README [1] ? Ok, I'll take a look. > > Tests "ima02 ima_measurement.sh" and "ima04 ima_violations.sh" assume > > files are created on a filesystem in policy. The "measure.policy" > > excludes tmpfs, yet TMPDIR defaults to a tmpfs filesystem. There are > > a couple of ways of resolving this problem (eg. removing tmpfs from > > the "measure.policy", use a RAM block device instead of tmpfs, etc). > > Since the builtin "ima_policy=tcb" also excludes tmpfs, not using a > > tmpfs filesystem would be preferable. > OK, I'll try to implement test using RAM block device. It would be nice to be able to define policies that limit testing to a specific filesystem/device. Without being able to limit IMA-appraisal testing to specific devices, things might stop working rather quickly. > > > Originally IMA allowed a builtin policy to be replaced with a custom > > policy, by simply cat'ing a file into the securityfs IMA policy file. > > Currently, if new rules can be added to the custom policy (Kconfig > > IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, > > if the builtin "secure-boot" policy is defined on the boot command > > line, the custom policy must be signed. Test "ima01 ima_policy.sh" > > should first detect if the policy must be signed, before running the > > tests. > Right, I'll check it. Is there other way how to detect it than reading > /boot/config-$(uname -r) or /proc/config.gz ? I'm asking because IMA might be using on > embedded devices (guessing from [2], [3]), which might not have either of them. Until the IMA-measurement list supports TPM 2.0 hash agility, the template name defines the old ("ima") vs. new formats. The builtin new template formats are "ima-ng" and "ima-sig", but custom formats can be defined (eg. ima_template_fmt="d-ng|n-ng|sig") on the boot command line. "d-ng|n-ng|sig" is the definition for ima-sig. > > ima_boot_aggregate.c defines the BIOS MAX_EVENT_SIZE BIOS size as 500, > > but I'm currently seeing BIOS events larger than 4k. > So, what is the recommended size? > Any reference to it? According to Kenneth Goldman, the maximum BIOS event entry for TPM 1.2 is undefined. For TPM 2.0 this was corrected. The spec says, "For software parsing the event log, the parser can choose an arbitrary maximum size, but this specification recommends a maximum value for the TCG_PCR_EVENT2.eventSize field of 1MB." I think the BIOS event log is currently only exported via sysfs for TPM 1.2. > > Since these tests were first written, Roberto's IMA templates and > > Dmitry's support for larger digests were upstreamed. With the new > > template format, the file hash is prefixed with the hash algorithm. > > Before comparing the calculated boot aggregate with the value in the > > IMA measurement list, the hash algorithm needs to be removed. > Do you mean entries in /sys/kernel/security/ima/ascii_runtime_measurements ? > system with config CONFIG_IMA_DEFAULT_HASH_SHA256=y > 10 4814642f7955ad7a9c7b47785d002374b34902fd ima-ng sha256:f20cec9d158c4c453899f97595c40257c2518a40a310a550a1cd26a63e7fff7a /usr/lib64/libsha1detectcoll.so.1.0.0 > system with config CONFIG_IMA_DEFAULT_HASH_SHA1=y > 10 2990cfe74ff309268e4fb928102574c28f9bb876 ima-ng sha1:71b543ad6af36b0976d0e3f71fed4ce0954eda0c /var/log/messages Exactly. The following code snippet strips off the hash algorithm, before doing the hash comparison. I'm sure there are better ways of doing it. +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh @@ -39,7 +39,7 @@ test1() # IMA boot aggregate read line < $ima_measurements - ima_aggr=$(expr substr "${line}" 49 40) + ima_aggr=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) > As it's done with grep it shouldn't be needed: > grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ > HASH_COMMAND="sha256sum" > > I kept sha1sum as the default command for checking and I'm detecting with > CONFIG_IMA_DEFAULT_HASH_SHA256 whether to use sha256: > This is not enough, I'll add checks for CONFIG_IMA_DEFAULT_HASH_SHA512 and > CONFIG_IMA_DEFAULT_HASH_WP512. The list of supported hash algorithms is defined in include/uapi/linux/hash_info.h. The first entry, the boot aggregate, is always sha1. The rest of the measurements will be the same hash algorithm, either as Kconfig defined or as specified on the boot command line "ima_hash=" option, per boot. With the support for carrying the IMA-measurement list across kexec, the measurement list might contain entries with different hash algorithms. > > > > For the new template format measurement lists, walking the measurement > > list, re-calculating the PCRs and comparing them with the HW or vTPM > > PCRs fail. The ima-evm-utils package has a working version. Invoke > > "evmctl" with the "ima_measurement" option. > So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your > repository on sf.net [4]? Fortunately this package is on all major distros [5] (except > Debian, but Ubuntu package is installable on Debian), so we don't need to include your > repository as submodule. Fantastic! So we'll concentrate on extending ima-evm-utils. thanks, Mimi ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-25 22:29 ` Mimi Zohar @ 2018-01-26 17:51 ` Petr Vorel 2018-01-28 0:47 ` Mimi Zohar 2018-01-31 15:01 ` Nayna Jain 0 siblings, 2 replies; 19+ messages in thread From: Petr Vorel @ 2018-01-26 17:51 UTC (permalink / raw) To: ltp Hi Mimi, > > Thanks a lot for your comments. As you didn't report any regression in my patch-set, I'm > > for merging it as it's an improvement. But I see there is more work to be done. > Sounds good. Well, there will be v2, as Cyril Hrubis asked for some more fixes. > > Is there more recent version of testcases/kernel/security/integrity/ima/README [1] ? > Ok, I'll take a look. Thanks a lot. You're definitely better person to review it :-). > > > Tests "ima02 ima_measurement.sh" and "ima04 ima_violations.sh" assume > > > files are created on a filesystem in policy. The "measure.policy" > > > excludes tmpfs, yet TMPDIR defaults to a tmpfs filesystem. There are > > > a couple of ways of resolving this problem (eg. removing tmpfs from > > > the "measure.policy", use a RAM block device instead of tmpfs, etc). > > > Since the builtin "ima_policy=tcb" also excludes tmpfs, not using a > > > tmpfs filesystem would be preferable. > > OK, I'll try to implement test using RAM block device. > It would be nice to be able to define policies that limit testing to a > specific filesystem/device. Without being able to limit IMA-appraisal > testing to specific devices, things might stop working rather quickly. Not sure how to define it, I need to study the specification. Or can you be more specific? BTW I suppose that kernel code supports both TPM 2.0 and the old 1.2. > > > Originally IMA allowed a builtin policy to be replaced with a custom > > > policy, by simply cat'ing a file into the securityfs IMA policy file. > > > Currently, if new rules can be added to the custom policy (Kconfig > > > IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, > > > if the builtin "secure-boot" policy is defined on the boot command > > > line, the custom policy must be signed. Test "ima01 ima_policy.sh" > > > should first detect if the policy must be signed, before running the > > > tests. > > Right, I'll check it. Is there other way how to detect it than reading > > /boot/config-$(uname -r) or /proc/config.gz ? I'm asking because IMA might be using on > > embedded devices (guessing from [2], [3]), which might not have either of them. This is important. As Cyril agreed with me grepping /boot/config-$(uname -r) or /proc/config.gz isn't good solution. I don't see any ioctl interface and security/integrity/ima/ima_fs.c which handles IMA sysfs doesn't have this functionality. Is it deliberate (security reason), that it's not exported to users? > Until the IMA-measurement list supports TPM 2.0 hash agility, the > template name defines the old ("ima") vs. new formats. The builtin > new template formats are "ima-ng" and "ima-sig", but custom formats > can be defined (eg. ima_template_fmt="d-ng|n-ng|sig") on the boot > command line. "d-ng|n-ng|sig" is the definition for ima-sig. OK, we cannot rely on the default format. I'll fix it :-). > > > ima_boot_aggregate.c defines the BIOS MAX_EVENT_SIZE BIOS size as 500, > > > but I'm currently seeing BIOS events larger than 4k. > > So, what is the recommended size? > > Any reference to it? > According to Kenneth Goldman, the maximum BIOS event entry for TPM 1.2 > is undefined. For TPM 2.0 this was corrected. The spec says, "For > software parsing the event log, the parser can choose an arbitrary > maximum size, but this specification recommends a maximum value for > the TCG_PCR_EVENT2.eventSize field of 1MB." Great, thanks for info. Just for a record links to the specification [1]. > I think the BIOS event log is currently only exported via sysfs for > TPM 1.2. This commit 4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event log") adds security [1]. And indeed structs tcg_pcr_event and tcg_pcr_event2 defined in [2] on pages 15 and 16 were added in this commit and used in TPM 2.0 related functions calc_tpm2_event_size(), tpm2_bios_measurements_start() and tpm2_bios_measurements_next(). > > > Since these tests were first written, Roberto's IMA templates and > > > Dmitry's support for larger digests were upstreamed. With the new > > > template format, the file hash is prefixed with the hash algorithm. > > > Before comparing the calculated boot aggregate with the value in the > > > IMA measurement list, the hash algorithm needs to be removed. > > Do you mean entries in /sys/kernel/security/ima/ascii_runtime_measurements ? > > system with config CONFIG_IMA_DEFAULT_HASH_SHA256=y > > 10 4814642f7955ad7a9c7b47785d002374b34902fd ima-ng sha256:f20cec9d158c4c453899f97595c40257c2518a40a310a550a1cd26a63e7fff7a /usr/lib64/libsha1detectcoll.so.1.0.0 > > system with config CONFIG_IMA_DEFAULT_HASH_SHA1=y > > 10 2990cfe74ff309268e4fb928102574c28f9bb876 ima-ng sha1:71b543ad6af36b0976d0e3f71fed4ce0954eda0c /var/log/messages > Exactly. The following code snippet strips off the hash algorithm, > before doing the hash comparison. I'm sure there are better ways of > doing it. > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh > @@ -39,7 +39,7 @@ test1() > # IMA boot aggregate > read line < $ima_measurements > - ima_aggr=$(expr substr "${line}" 49 40) > + ima_aggr=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) > > As it's done with grep it shouldn't be needed: > > grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ > > HASH_COMMAND="sha256sum" > > I kept sha1sum as the default command for checking and I'm detecting with > > CONFIG_IMA_DEFAULT_HASH_SHA256 whether to use sha256: > > This is not enough, I'll add checks for CONFIG_IMA_DEFAULT_HASH_SHA512 and > > CONFIG_IMA_DEFAULT_HASH_WP512. > The list of supported hash algorithms is defined in > include/uapi/linux/hash_info.h. I see. And constants I take into account are values for ima template. > The first entry, the boot aggregate, is always sha1. The rest of the > measurements will be the same hash algorithm, either as Kconfig > defined or as specified on the boot command line "ima_hash=" option, > per boot. > With the support for carrying the IMA-measurement list across kexec, > the measurement list might contain entries with different hash > algorithms. OK, I'll detect correct hash algorithm and check it. > > > > > > For the new template format measurement lists, walking the measurement > > > list, re-calculating the PCRs and comparing them with the HW or vTPM > > > PCRs fail. The ima-evm-utils package has a working version. Invoke > > > "evmctl" with the "ima_measurement" option. > > So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your > > repository on sf.net [4]? Fortunately this package is on all major distros [5] (except > > Debian, but Ubuntu package is installable on Debian), so we don't need to include your > > repository as submodule. > Fantastic! So we'll concentrate on extending ima-evm-utils. There might be problem in old distros, where users will have to compile it themselves. It'd be more comfortable for users to have all the source codes in LTP tree, so if they report problems to us we might copy your source to LTP git or use it as submodule. But I'll try to avoid it. > thanks, Thanks a lot for your help! > Mimi Kind regards, Petr [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.15-rc9&id=4d23cc323cdbee1cbcd8a7f059fff9ef2b0c473d [2] https://www.trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev9-150513_Public-Review.pdf ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-26 17:51 ` Petr Vorel @ 2018-01-28 0:47 ` Mimi Zohar 2018-01-29 19:58 ` Mimi Zohar 2018-01-31 15:01 ` Nayna Jain 1 sibling, 1 reply; 19+ messages in thread From: Mimi Zohar @ 2018-01-28 0:47 UTC (permalink / raw) To: ltp On Fri, 2018-01-26 at 18:51 +0100, Petr Vorel wrote: > > It would be nice to be able to define policies that limit testing to a > > specific filesystem/device. Without being able to limit IMA-appraisal > > testing to specific devices, things might stop working rather quickly. > Not sure how to define it, I need to study the specification. Or can > you be more specific? These tests are for the IMA-measurement aspect only, not IMA- appraisal. Adding measurements to the measurement list won't cause the system to stop working, unless keys are sealed to a particular TPM PCR value. Nobody is or should be sealing keys to PCR-10, since the ordering of the measurements is non deterministic. As we add IMA-appraisal tests requiring files to be signed, things will fail if either the public key isn't on the IMA keyring or the file isn't properly signed. For this reason, limiting file IMA- appraisal tests to a particular filesystem simplifies testing. > BTW I suppose that kernel code supports both TPM 2.0 and the old 1.2. Yes, Jarkko added TPM 2.0 support, including IMA support. > > > > Originally IMA allowed a builtin policy to be replaced with a custom > > > > policy, by simply cat'ing a file into the securityfs IMA policy file. > > > > Currently, if new rules can be added to the custom policy (Kconfig > > > > IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, > > > > if the builtin "secure-boot" policy is defined on the boot command > > > > line, the custom policy must be signed. Test "ima01 ima_policy.sh" > > > > should first detect if the policy must be signed, before running the > > > > tests. > > > > Right, I'll check it. Is there other way how to detect it than reading > > > /boot/config-$(uname -r) or /proc/config.gz ? I'm asking because IMA might be using on > > > embedded devices (guessing from [2], [3]), which might not have either of them. > This is important. As Cyril agreed with me grepping /boot/config-$(uname -r) or > /proc/config.gz isn't good solution. I don't see any ioctl interface and > security/integrity/ima/ima_fs.c which handles IMA sysfs doesn't have this functionality. > Is it deliberate (security reason), that it's not exported to users? This isn't really an IMA issue, but a TPM one. The TPM device driver would need to export this information. Mimi ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-28 0:47 ` Mimi Zohar @ 2018-01-29 19:58 ` Mimi Zohar 0 siblings, 0 replies; 19+ messages in thread From: Mimi Zohar @ 2018-01-29 19:58 UTC (permalink / raw) To: ltp On Sat, 2018-01-27 at 19:47 -0500, Mimi Zohar wrote: > On Fri, 2018-01-26 at 18:51 +0100, Petr Vorel wrote: > > > > > Originally IMA allowed a builtin policy to be replaced with a custom > > > > > policy, by simply cat'ing a file into the securityfs IMA policy file. > > > > > Currently, if new rules can be added to the custom policy (Kconfig > > > > > IMA_WRITE_POLICY enabled), the policy file must be signed. Similarly, > > > > > if the builtin "secure-boot" policy is defined on the boot command > > > > > line, the custom policy must be signed. Test "ima01 ima_policy.sh" > > > > > should first detect if the policy must be signed, before running the > > > > > tests. > > > > > > Right, I'll check it. Is there other way how to detect it than reading > > > > /boot/config-$(uname -r) or /proc/config.gz ? I'm asking because IMA might be using on > > > > embedded devices (guessing from [2], [3]), which might not have either of them. > > This is important. As Cyril agreed with me grepping /boot/config-$(uname -r) or > > /proc/config.gz isn't good solution. I don't see any ioctl interface and > > security/integrity/ima/ima_fs.c which handles IMA sysfs doesn't have this functionality. > > Is it deliberate (security reason), that it's not exported to users? > > This isn't really an IMA issue, but a TPM one. The TPM device driver > would need to export this information. Sorry, that was an answer to the wrong question. In ima_tpm.sh, there's the question: # Would be nice to know where the PCRs are located. Is this safe?". Commit 313d21e "tpm: device class for tpm" moved the TPM sysfs location from /sys/class/misc/tpmX/device/ to /sys/class/tpm/tpmX/device/. The pcrs are To answer your question, if after writing the custom policy, the policy file disappears, then you obviously can't extend the policy. If the policy file doesn't disappear, you might not be able to extend the policy, just view it. Sorry, there's no method of knowing apriori whether the policy can be extended. Mimi ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-26 17:51 ` Petr Vorel 2018-01-28 0:47 ` Mimi Zohar @ 2018-01-31 15:01 ` Nayna Jain 1 sibling, 0 replies; 19+ messages in thread From: Nayna Jain @ 2018-01-31 15:01 UTC (permalink / raw) To: ltp On 01/26/2018 11:21 PM, Petr Vorel wrote: > >> Until the IMA-measurement list supports TPM 2.0 hash agility, the >> template name defines the old ("ima") vs. new formats. The builtin >> new template formats are "ima-ng" and "ima-sig", but custom formats >> can be defined (eg. ima_template_fmt="d-ng|n-ng|sig") on the boot >> command line. "d-ng|n-ng|sig" is the definition for ima-sig. > OK, we cannot rely on the default format. I'll fix it :-). > >>>> ima_boot_aggregate.c defines the BIOS MAX_EVENT_SIZE BIOS size as 500, >>>> but I'm currently seeing BIOS events larger than 4k. >>> So, what is the recommended size? >>> Any reference to it? >> According to Kenneth Goldman, the maximum BIOS event entry for TPM 1.2 >> is undefined. For TPM 2.0 this was corrected. The spec says, "For >> software parsing the event log, the parser can choose an arbitrary >> maximum size, but this specification recommends a maximum value for >> the TCG_PCR_EVENT2.eventSize field of 1MB." > Great, thanks for info. Just for a record links to the specification [1]. > >> I think the BIOS event log is currently only exported via sysfs for >> TPM 1.2. > This commit 4d23cc323cdb ("tpm: add securityfs support for TPM 2.0 firmware event log") > adds security [1]. And indeed structs tcg_pcr_event and tcg_pcr_event2 defined > in [2] on pages 15 and 16 were added in this commit and used in TPM 2.0 related > functions calc_tpm2_event_size(), tpm2_bios_measurements_start() and > tpm2_bios_measurements_next(). Unlike TPM 1.2, TPM 2.0 support for firmware event log exports only binary_bios_measurements to securityfs. The binary file can be parsed to generate the ascii format. The initial commit added support to retrieve the TPM 2.0 event log for device tree based platforms. Similar support for EFI is being upstreamed in this open window for Linux 4.16(https://lkml.org/lkml/2018/1/8/199) Thanks & Regards, - Nayna > >>>> Since these tests were first written, Roberto's IMA templates and >>>> Dmitry's support for larger digests were upstreamed. With the new >>>> template format, the file hash is prefixed with the hash algorithm. >>>> Before comparing the calculated boot aggregate with the value in the >>>> IMA measurement list, the hash algorithm needs to be removed. >>> Do you mean entries in /sys/kernel/security/ima/ascii_runtime_measurements ? >>> system with config CONFIG_IMA_DEFAULT_HASH_SHA256=y >>> 10 4814642f7955ad7a9c7b47785d002374b34902fd ima-ng sha256:f20cec9d158c4c453899f97595c40257c2518a40a310a550a1cd26a63e7fff7a /usr/lib64/libsha1detectcoll.so.1.0.0 >>> system with config CONFIG_IMA_DEFAULT_HASH_SHA1=y >>> 10 2990cfe74ff309268e4fb928102574c28f9bb876 ima-ng sha1:71b543ad6af36b0976d0e3f71fed4ce0954eda0c /var/log/messages >> Exactly. The following code snippet strips off the hash algorithm, >> before doing the hash comparison. I'm sure there are better ways of >> doing it. >> +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh >> @@ -39,7 +39,7 @@ test1() >> # IMA boot aggregate >> read line < $ima_measurements >> - ima_aggr=$(expr substr "${line}" 49 40) >> + ima_aggr=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) > >>> As it's done with grep it shouldn't be needed: >>> grep -q '^CONFIG_IMA_DEFAULT_HASH_SHA256=y' /boot/config-$(uname -r) && \ >>> HASH_COMMAND="sha256sum" >>> I kept sha1sum as the default command for checking and I'm detecting with >>> CONFIG_IMA_DEFAULT_HASH_SHA256 whether to use sha256: >>> This is not enough, I'll add checks for CONFIG_IMA_DEFAULT_HASH_SHA512 and >>> CONFIG_IMA_DEFAULT_HASH_WP512. >> The list of supported hash algorithms is defined in >> include/uapi/linux/hash_info.h. > I see. And constants I take into account are values for ima template. > >> The first entry, the boot aggregate, is always sha1. The rest of the >> measurements will be the same hash algorithm, either as Kconfig >> defined or as specified on the boot command line "ima_hash=" option, >> per boot. >> With the support for carrying the IMA-measurement list across kexec, >> the measurement list might contain entries with different hash >> algorithms. > OK, I'll detect correct hash algorithm and check it. > >>>> >>>> For the new template format measurement lists, walking the measurement >>>> list, re-calculating the PCRs and comparing them with the HW or vTPM >>>> PCRs fail. The ima-evm-utils package has a working version. Invoke >>>> "evmctl" with the "ima_measurement" option. >>> So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your >>> repository on sf.net [4]? Fortunately this package is on all major distros [5] (except >>> Debian, but Ubuntu package is installable on Debian), so we don't need to include your >>> repository as submodule. >> Fantastic! So we'll concentrate on extending ima-evm-utils. > There might be problem in old distros, where users will have to compile it themselves. > It'd be more comfortable for users to have all the source codes in LTP tree, so if they > report problems to us we might copy your source to LTP git or use it as submodule. But > I'll try to avoid it. > >> thanks, > Thanks a lot for your help! > >> Mimi > > Kind regards, > Petr > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.15-rc9&id=4d23cc323cdbee1cbcd8a7f059fff9ef2b0c473d > [2] https://www.trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev9-150513_Public-Review.pdf > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-25 20:30 ` Petr Vorel 2018-01-25 20:40 ` Petr Vorel 2018-01-25 22:29 ` Mimi Zohar @ 2018-01-26 13:16 ` Cyril Hrubis 2018-01-26 18:11 ` Petr Vorel 2018-02-06 13:19 ` Mimi Zohar 2 siblings, 2 replies; 19+ messages in thread From: Cyril Hrubis @ 2018-01-26 13:16 UTC (permalink / raw) To: ltp Hi! > > For the new template format measurement lists, walking the measurement > > list, re-calculating the PCRs and comparing them with the HW or vTPM > > PCRs fail. ??The ima-evm-utils package has a working version. ??Invoke > > "evmctl" with the "ima_measurement" option. > So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your > repository on sf.net [4]? Fortunately this package is on all major distros [5] (except > Debian, but Ubuntu package is installable on Debian), so we don't need to include your > repository as submodule. Well if the package is included in major distributions we may as just state the dependency in the README and TCONF the test if it's not installed. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-26 13:16 ` Cyril Hrubis @ 2018-01-26 18:11 ` Petr Vorel 2018-02-06 13:19 ` Mimi Zohar 1 sibling, 0 replies; 19+ messages in thread From: Petr Vorel @ 2018-01-26 18:11 UTC (permalink / raw) To: ltp Hi, > > > For the new template format measurement lists, walking the measurement > > > list, re-calculating the PCRs and comparing them with the HW or vTPM > > > PCRs fail. ??The ima-evm-utils package has a working version. ??Invoke > > > "evmctl" with the "ima_measurement" option. > > So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your > > repository on sf.net [4]? Fortunately this package is on all major distros [5] (except > > Debian, but Ubuntu package is installable on Debian), so we don't need to include your > > repository as submodule. > Well if the package is included in major distributions we may as just > state the dependency in the README and TCONF the test if it's not > installed. Agree. BTW it would be nice if tst_check_cmds() allowed to print custom message. Something like: tst_check_cmds -m "install ima-evm-utils package or compile it from sources" "from https://git.code.sf.net/p/linux-ima/ima-evm-utils" evmctl Maybe this idea is corner case, I'll just test it without help of tst_check_cmds. Kind regards, Petr ^ permalink raw reply [flat|nested] 19+ messages in thread
* [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes 2018-01-26 13:16 ` Cyril Hrubis 2018-01-26 18:11 ` Petr Vorel @ 2018-02-06 13:19 ` Mimi Zohar 1 sibling, 0 replies; 19+ messages in thread From: Mimi Zohar @ 2018-02-06 13:19 UTC (permalink / raw) To: ltp On Fri, 2018-01-26 at 14:16 +0100, Cyril Hrubis wrote: > Hi! > > > For the new template format measurement lists, walking the measurement > > > list, re-calculating the PCRs and comparing them with the HW or vTPM > > > PCRs fail. ??The ima-evm-utils package has a working version. ??Invoke > > > "evmctl" with the "ima_measurement" option. > > So you mean that src/ima_measure.c is broken and should be replaced by evmctl from your > > repository on sf.net [4]? Fortunately this package is on all major distros [5] (except > > Debian, but Ubuntu package is installable on Debian), so we don't need to include your > > repository as submodule. > > Well if the package is included in major distributions we may as just > state the dependency in the README and TCONF the test if it's not > installed. I've cleaned up "evmctl ima_measurement" a bit, so that there are different levels of output. The default is to just return errors. Verbose (-v) returns the keys used in the verification, the calculated PCR and the HW PCR. Verbose+ (-v -v) includes the measurement list as well. example: $ sudo src/evmctl ima_measurement -k "/etc/keys/ima/distro-cert-6e6c1046.der, /etc/keys/ima/app-cert-c4e2426e.der, /etc/keys/ima/local-cert-14c2d147.der" -v /sys/kernel/security/ima/binary_runtime_measurements key 1: 6e6c1046 /etc/keys/ima/distro-cert-6e6c1046.der key 2: c4e2426e /etc/keys/ima/app-cert-c4e2426e.der key 3: 14c2d147 /etc/keys/ima/local-cert-14c2d147.der PCRAgg 10: a19dfba0ac6eef26cb342470374b0808aea80a12 HW PCR-10: a19dfba0ac6eef26cb342470374b0808aea80a12 The patches for this version are in the next branch. Mimi ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-02-06 13:19 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-11 20:28 [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel 2018-01-11 20:28 ` [LTP] [RFC PATCH 1/2] security/ima: " Petr Vorel 2018-01-26 13:09 ` Cyril Hrubis 2018-01-11 20:28 ` [LTP] [RFC PATCH 2/2] security/ima: Run measurements after policy Petr Vorel 2018-01-26 13:11 ` Cyril Hrubis 2018-01-26 18:03 ` Petr Vorel 2018-01-28 0:57 ` Mimi Zohar 2018-01-24 17:12 ` [LTP] [RFC PATCH 0/2] IMA: Rewrite tests into new API + fixes Petr Vorel 2018-01-24 17:36 ` Mimi Zohar 2018-01-25 20:30 ` Petr Vorel 2018-01-25 20:40 ` Petr Vorel 2018-01-25 22:29 ` Mimi Zohar 2018-01-26 17:51 ` Petr Vorel 2018-01-28 0:47 ` Mimi Zohar 2018-01-29 19:58 ` Mimi Zohar 2018-01-31 15:01 ` Nayna Jain 2018-01-26 13:16 ` Cyril Hrubis 2018-01-26 18:11 ` Petr Vorel 2018-02-06 13:19 ` Mimi Zohar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox