diff mbox series

[v2,3/4] IMA: Add a test to verify measurement of certificate imported into a keyring

Message ID 20200807204652.5928-4-pvorel@suse.cz
State Changes Requested
Headers show
Series IMA: verify measurement of certificate imported into a keyring | expand

Commit Message

Petr Vorel Aug. 7, 2020, 8:46 p.m. UTC 
From: Lachlan Sneff <t-josne@linux.microsoft.com>

The IMA subsystem supports measuring certificates that have been
imported into either system built-in or user-defined keyrings.
A test to verify measurement of a certificate imported
into a keyring is required.

Add an IMA measurement test that verifies that an x509 certificate
can be imported into a newly-created, user-defined keyring and measured
correctly by the IMA subsystem.

A certificate used by the test is included in the `datafiles/keys`
directory.

There can be restrictions on importing a certificate into a builtin
trusted keyring. For example, the `.ima` keyring requires that
imported certs be signed by a kernel private key in certain
kernel configurations. For this reason, this test defines
a user-defined keyring and imports a certificate into that.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
[ pvorel: Added key_import_test into keycheck.policy, reword
instructions in README.md, LTP API related fixes ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
I did various changes:
* fix error check in pipe command (grep $keyring_name
  $ASCII_MEASUREMENTS ...) $? contains exit code just of last command
  (xxd) => check for empty file.
* use more propriate error flags (TCONF => TFAIL, but some of them could
  be also TBROK)
* tst_brk => tst_res && return in test1 (to give chance test2 to be run)
* add TINFO message about test
* remove obvious comment

Kind regards,
Petr

 .../kernel/security/integrity/ima/README.md   |  12 ++--
 .../ima/datafiles/ima_keys/keycheck.policy    |   2 +-
 .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
 .../security/integrity/ima/tests/ima_keys.sh  |  68 +++++++++++++++---
 4 files changed, 69 insertions(+), 13 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der

Comments

Lakshmi Ramasubramanian Aug. 7, 2020, 9:12 p.m. UTC | #1 
On 8/7/20 1:46 PM, Petr Vorel wrote:

Hi Petr,

> From: Lachlan Sneff <t-josne@linux.microsoft.com>
> 
> The IMA subsystem supports measuring certificates that have been
> imported into either system built-in or user-defined keyrings.
> A test to verify measurement of a certificate imported
> into a keyring is required.
> 
> Add an IMA measurement test that verifies that an x509 certificate
> can be imported into a newly-created, user-defined keyring and measured
> correctly by the IMA subsystem.
> 
> A certificate used by the test is included in the `datafiles/keys`
> directory.
> 
> There can be restrictions on importing a certificate into a builtin
> trusted keyring. For example, the `.ima` keyring requires that
> imported certs be signed by a kernel private key in certain
> kernel configurations. For this reason, this test defines
> a user-defined keyring and imports a certificate into that.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
> [ pvorel: Added key_import_test into keycheck.policy, reword
> instructions in README.md, LTP API related fixes ]
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> I did various changes:
> * fix error check in pipe command (grep $keyring_name
>    $ASCII_MEASUREMENTS ...) $? contains exit code just of last command
>    (xxd) => check for empty file.
> * use more propriate error flags (TCONF => TFAIL, but some of them could
>    be also TBROK)
> * tst_brk => tst_res && return in test1 (to give chance test2 to be run)
> * add TINFO message about test
> * remove obvious comment

Thanks for updating the patches.

In testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
the following change (include x509_ima.der) needed to be made to run 
ima_keys.sh tests

INSTALL_TARGETS := *.policy x509_ima.der

Other than that the patches look good.

thanks,
  -lakshmi

> 
>   .../kernel/security/integrity/ima/README.md   |  12 ++--
>   .../ima/datafiles/ima_keys/keycheck.policy    |   2 +-
>   .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
>   .../security/integrity/ima/tests/ima_keys.sh  |  68 +++++++++++++++---
>   4 files changed, 69 insertions(+), 13 deletions(-)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
> 
> diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
> index 392e1e868..68d046678 100644
> --- a/testcases/kernel/security/integrity/ima/README.md
> +++ b/testcases/kernel/security/integrity/ima/README.md
> @@ -16,11 +16,15 @@ space, may contain equivalent measurement tcb rules, detecting them would
>   require `IMA_READ_POLICY=y` therefore ignore this option.
>   
>   ### IMA key test
> -`ima_keys.sh` requires a readable IMA policy, as well as a loaded policy
> -with `func=KEY_CHECK keyrings=...`, see example in `keycheck.policy`.
> +The measuring keys test (first test) in `ima_keys.sh` requires a readable IMA
> +policy, as well as a loaded measure policy with `func=KEY_CHECK keyrings=...`.
>   
> -As well as what's required for the IMA tests, the following are also required
> --in the kernel configuration:
> +The certificate import test (second test) require measure policy with
> +`func=KEY_CHECK keyrings=key_import_test`. Valid policy for both is in
> +`keycheck.policy`.
> +
> +As well as what's required for the IMA tests, key tests require reading the IMA
> +policy allowed in the kernel configuration:
>   ```
>   CONFIG_IMA_READ_POLICY=y
>   ```
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
> index 3f1934a3d..623162002 100644
> --- a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
> +++ b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
> @@ -1 +1 @@
> -measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf
> +measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
> new file mode 100644
> index 0000000000000000000000000000000000000000..92be058da22adffa9d6b6e51efa0c737ebbbbdcd
> GIT binary patch
> literal 650
> zcmXqLVrnyJVtl`VnTe5!NhJD#vj69`9|BBf8}FEsx@^_9$Clp>c-c6$+C196^D;7W
> zvoaV27z!HjvoVLVaPe?t<QJFZCFZ6YN*hRmgqV4R$}{p4b2Al+Gt=`j^U@WvQ!5SS
> z3}oO&a59SVLzFncG#ki?^BP(jSQr@@7#Ud_7)6Qm8W{k&hEOgIY;2s5>?=lA2Ij_I
> z27|^<rp88wchfeduxmMW^j9qUxkIugeev4q7u7yrJR_rW$*!>VOo=tim8DK0r^FsU
> zl)K`}`+CO4@4KBkoLmcj?fH`%wbDvU<d=4Z>6-SMf6Eh}{&)1rdsOoNQ-1fgBQ1t{
> zVTqGwuK95LlFE)6i{@=vlP6!2`Y}x<BF&oXU_nlDxy@C+CNp&=W=00a#jys_20XwZ
> zl@(@W{LjK<z+k`);_<VvFf*|?7|4P+d@N!tBCNWX-0#?!UAx9s`mgFmW+nI2#6kmk
> zkhC(3gn?Lt$m4W@56wQ)?QVKW<nOtpT)HJrB?Q^`z&K?FdV8b(y8m)~mOOvswu^9m
> z-o7mOwCAzaT*~`Y4wxFtmNA_89`W;j{r#iww*5OC5vgEziqD_%=ki$*`;s}`Pfwi{
> zbotZ0X`ckHOmaVLm85n@E9hN_K;~PUB>DFAzh(;(UoMln9V>g;W#-LUGS7A`nYQKY
> WBem{7L5JThS+;$vggi%(*E;~nlJ80Y
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> index 53c289054..30950904e 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> @@ -6,8 +6,8 @@
>   #
>   # Verify that keys are measured correctly based on policy.
>   
> -TST_NEEDS_CMDS="cut grep sed tr xxd"
> -TST_CNT=1
> +TST_NEEDS_CMDS="cmp cut grep sed tr xxd"
> +TST_CNT=2
>   TST_NEEDS_DEVICE=1
>   
>   . ima_setup.sh
> @@ -20,20 +20,22 @@ test1()
>   	local pattern="func=KEY_CHECK"
>   	local test_file="file.txt"
>   
> -	tst_res TINFO "verifying key measurement for keyrings and templates specified in IMA policy file"
> +	tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy"
>   
>   	require_ima_policy_content "$pattern"
>   	keycheck_lines=$(check_ima_policy_content "$pattern" "")
>   	keycheck_line=$(echo "$keycheck_lines" | grep "keyrings" | head -n1)
>   
>   	if [ -z "$keycheck_line" ]; then
> -		tst_brk TCONF "ima policy does not specify a keyrings to check"
> +		tst_res TCONF "IMA policy does not specify a keyrings to check"
> +		return
>   	fi
>   
>   	keyrings=$(echo "$keycheck_line" | tr " " "\n" | grep "keyrings" | \
>   		sed "s/\./\\\./g" | cut -d'=' -f2)
>   	if [ -z "$keyrings" ]; then
> -		tst_brk TCONF "ima policy has a keyring key-value specifier, but no specified keyrings"
> +		tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings"
> +		return
>   	fi
>   
>   	templates=$(echo "$keycheck_line" | tr " " "\n" | grep "template" | \
> @@ -49,11 +51,13 @@ test1()
>   
>   		echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file
>   
> -		expected_digest="$(compute_digest $algorithm $test_file)" || \
> -			tst_brk TCONF "cannot compute digest for $algorithm"
> +		if ! expected_digest="$(compute_digest $algorithm $test_file)"; then
> +			tst_res TCONF "cannot compute digest for $algorithm"
> +			return
> +		fi
>   
>   		if [ "$digest" != "$expected_digest" ]; then
> -			tst_res TFAIL "incorrect digest was found for the ($keyring) keyring"
> +			tst_res TFAIL "incorrect digest was found for $keyring keyring"
>   			return
>   		fi
>   	done
> @@ -61,4 +65,52 @@ test1()
>   	tst_res TPASS "specified keyrings were measured correctly"
>   }
>   
> +# Create a new keyring, import a certificate into it, and verify
> +# that the certificate is measured correctly by IMA.
> +test2()
> +{
> +	tst_require_cmds evmctl keyctl openssl
> +
> +	local cert_file="$TST_DATAROOT/x509_ima.der"
> +	local keyring_name="key_import_test"
> +	local temp_file="file.txt"
> +	local keyring_id
> +
> +	tst_res TINFO "verify measurement of certificate imported into a keyring"
> +
> +	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
> +		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
> +	fi
> +
> +	keyctl new_session > /dev/null
> +
> +	keyring_id=$(keyctl newring $keyring_name @s) || \
> +		tst_brk TBROK "unable to create a new keyring"
> +
> +	tst_is_num $keyring_id || \
> +		tst_brk TBROK "unable to parse the new keyring id"
> +
> +	evmctl import $cert_file $keyring_id > /dev/null || \
> +		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"
> +
> +	grep $keyring_name $ASCII_MEASUREMENTS | tail -n1 | cut -d' ' -f6 | \
> +		xxd -r -p > $temp_file
> +
> +	if [ ! -s $temp_file ]; then
> +		tst_res TFAIL "keyring $keyring_name not found in $ASCII_MEASUREMENTS"
> +		return
> +	fi
> +
> +	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
> +		tst_res TFAIL "logged certificate is not a valid x509 certificate"
> +		return
> +	fi
> +
> +	if cmp -s $temp_file $cert_file; then
> +		tst_res TPASS "logged certificate matches the original"
> +	else
> +		tst_res TFAIL "logged certificate does not match original"
> +	fi
> +}
> +
>   tst_run
>
Mimi Zohar Aug. 17, 2020, 3:21 a.m. UTC | #2 
Hi Petr, Lachlan,

On Fri, 2020-08-07 at 22:46 +0200, Petr Vorel wrote:
> From: Lachlan Sneff <t-josne@linux.microsoft.com>

> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> index 53c289054..30950904e 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> 
> @@ -61,4 +65,52 @@ test1()
>  	tst_res TPASS "specified keyrings were measured correctly"
>  }
> 
> +# Create a new keyring, import a certificate into it, and verify
> +# that the certificate is measured correctly by IMA.
> +test2()
> +{
> +	tst_require_cmds evmctl keyctl openssl
> +
> +	local cert_file="$TST_DATAROOT/x509_ima.der"
> +	local keyring_name="key_import_test"
> +	local temp_file="file.txt"
> +	local keyring_id
> +
> +	tst_res TINFO "verify measurement of certificate imported into a keyring"
> +
> +	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
> +		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
> +	fi
> +

If the IMA policy contains multiple KEY_CHECK measurement policy rules
it complains about "grep: Unmatched ( or \(".

Sample rules:
measure func=KEY_CHECK template=ima-buf
keyrings=.ima|.builtin_trusted_keys
measure func=KEY_CHECK template=ima-buf keyrings=key_import_test

> +	keyctl new_session > /dev/null
> +
> +	keyring_id=$(keyctl newring $keyring_name @s) || \
> +		tst_brk TBROK "unable to create a new keyring"
> +
> +	tst_is_num $keyring_id || \
> +		tst_brk TBROK "unable to parse the new keyring id"
> +
> +	evmctl import $cert_file $keyring_id > /dev/null || \
> +		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"

"cert_file" needs to be updated from 
"ltp/testcases/kernel/security/integrity/ima/tests/datafiles/x509_ima.d
er" to
"ltp/testcases/kernel/security/integrity/ima/tests/../datafiles/ima_key
s/x509_ima.der".

On failure to open the file, 
errno: No such file or directory (2)
ima_keys 2 TBROK: unable to import a certificate into key_import_test keyring
ima_keys 2 TINFO: SELinux enabled in enforcing mode, this may affect test results
ima_keys 2 TINFO: it can be disabled with TST_DISABLE_SELINUX=1 (requires super/root)
ima_keys 2 TINFO: install seinfo to find used SELinux profiles
ima_keys 2 TINFO: loaded SELinux profiles: none

Mimi

> +
> +	grep $keyring_name $ASCII_MEASUREMENTS | tail -n1 | cut -d' ' -f6 | \
> +		xxd -r -p > $temp_file
> +
> +	if [ ! -s $temp_file ]; then
> +		tst_res TFAIL "keyring $keyring_name not found in $ASCII_MEASUREMENTS"
> +		return
> +	fi
> +
> +	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
> +		tst_res TFAIL "logged certificate is not a valid x509 certificate"
> +		return
> +	fi
> +
> +	if cmp -s $temp_file $cert_file; then
> +		tst_res TPASS "logged certificate matches the original"
> +	else
> +		tst_res TFAIL "logged certificate does not match original"
> +	fi
> +}
> +
>  tst_run
Lakshmi Ramasubramanian Aug. 17, 2020, 5:13 a.m. UTC | #3 
On 8/16/20 8:21 PM, Mimi Zohar wrote:

Hi Mimi,

>> +# Create a new keyring, import a certificate into it, and verify
>> +# that the certificate is measured correctly by IMA.
>> +test2()
>> +{
>> +	tst_require_cmds evmctl keyctl openssl
>> +
>> +	local cert_file="$TST_DATAROOT/x509_ima.der"
>> +	local keyring_name="key_import_test"
>> +	local temp_file="file.txt"
>> +	local keyring_id
>> +
>> +	tst_res TINFO "verify measurement of certificate imported into a keyring"
>> +
>> +	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
>> +		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
>> +	fi
>> +
> 
> If the IMA policy contains multiple KEY_CHECK measurement policy rules
> it complains about "grep: Unmatched ( or \(".
> 
> Sample rules:
> measure func=KEY_CHECK template=ima-buf
> keyrings=.ima|.builtin_trusted_keys
> measure func=KEY_CHECK template=ima-buf keyrings=key_import_test
> 

I tried with the above policy entries, but am unable to reproduce the 
error you are seeing.

ima_keys 1 TINFO: verifying key measurement for keyrings and templates 
specified in IMA policy file
ima_keys 1 TPASS: specified keyrings were measured correctly
ima_keys 2 TPASS: logged cert matches original cert

>> +	keyctl new_session > /dev/null
>> +
>> +	keyring_id=$(keyctl newring $keyring_name @s) || \
>> +		tst_brk TBROK "unable to create a new keyring"
>> +
>> +	tst_is_num $keyring_id || \
>> +		tst_brk TBROK "unable to parse the new keyring id"
>> +
>> +	evmctl import $cert_file $keyring_id > /dev/null || \
>> +		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"
> 
> "cert_file" needs to be updated from
> "ltp/testcases/kernel/security/integrity/ima/tests/datafiles/x509_ima.d
> er" to
> "ltp/testcases/kernel/security/integrity/ima/tests/../datafiles/ima_key
> s/x509_ima.der".
> 

The problem is actually due to missing "x509_ima.der" in 
"INSTALL_TARGETS" in datafiles/keys/Makefile

Adding the following line in the Makefile fixes the problem

INSTALL_TARGETS		:= x509_ima.der

  -lakshmi

> On failure to open the file,
> errno: No such file or directory (2)
> ima_keys 2 TBROK: unable to import a certificate into key_import_test keyring
> ima_keys 2 TINFO: SELinux enabled in enforcing mode, this may affect test results
> ima_keys 2 TINFO: it can be disabled with TST_DISABLE_SELINUX=1 (requires super/root)
> ima_keys 2 TINFO: install seinfo to find used SELinux profiles
> ima_keys 2 TINFO: loaded SELinux profiles: none
> 
> Mimi
> 
>> +
>> +	grep $keyring_name $ASCII_MEASUREMENTS | tail -n1 | cut -d' ' -f6 | \
>> +		xxd -r -p > $temp_file
>> +
>> +	if [ ! -s $temp_file ]; then
>> +		tst_res TFAIL "keyring $keyring_name not found in $ASCII_MEASUREMENTS"
>> +		return
>> +	fi
>> +
>> +	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
>> +		tst_res TFAIL "logged certificate is not a valid x509 certificate"
>> +		return
>> +	fi
>> +
>> +	if cmp -s $temp_file $cert_file; then
>> +		tst_res TPASS "logged certificate matches the original"
>> +	else
>> +		tst_res TFAIL "logged certificate does not match original"
>> +	fi
>> +}
>> +
>>   tst_run
>
Petr Vorel Aug. 17, 2020, 11:09 a.m. UTC | #4 
Hi Mimi, Lakshmi,

> On Fri, 2020-08-07 at 22:46 +0200, Petr Vorel wrote:
> > From: Lachlan Sneff <t-josne@linux.microsoft.com>

> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> > index 53c289054..30950904e 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh

> > @@ -61,4 +65,52 @@ test1()
> >  	tst_res TPASS "specified keyrings were measured correctly"
> >  }

> > +# Create a new keyring, import a certificate into it, and verify
> > +# that the certificate is measured correctly by IMA.
> > +test2()
> > +{
> > +	tst_require_cmds evmctl keyctl openssl
> > +
> > +	local cert_file="$TST_DATAROOT/x509_ima.der"
> > +	local keyring_name="key_import_test"
> > +	local temp_file="file.txt"
> > +	local keyring_id
> > +
> > +	tst_res TINFO "verify measurement of certificate imported into a keyring"
> > +
> > +	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
> > +		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
> > +	fi
> > +

> If the IMA policy contains multiple KEY_CHECK measurement policy rules
> it complains about "grep: Unmatched ( or \(".

> Sample rules:
> measure func=KEY_CHECK template=ima-buf
> keyrings=.ima|.builtin_trusted_keys
> measure func=KEY_CHECK template=ima-buf keyrings=key_import_test

Good catch, reproduced, working on fix (NOTE 2nd line is obviously joined to the
first one).

Caused by later code:
grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line

> > +	keyctl new_session > /dev/null
> > +
> > +	keyring_id=$(keyctl newring $keyring_name @s) || \
> > +		tst_brk TBROK "unable to create a new keyring"
> > +
> > +	tst_is_num $keyring_id || \
> > +		tst_brk TBROK "unable to parse the new keyring id"
> > +
> > +	evmctl import $cert_file $keyring_id > /dev/null || \
> > +		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"

> "cert_file" needs to be updated from 
> "ltp/testcases/kernel/security/integrity/ima/tests/datafiles/x509_ima.d
> er" to
> "ltp/testcases/kernel/security/integrity/ima/tests/../datafiles/ima_key
> s/x509_ima.der".

As Lakshmi wrote if you apply the fix, which I included in my branch
Lachlan_Sneff/ima_keys.sh-second-test.v1.fixes [1] it'd work.

Unlike kselftest, LTP requires installing and running from installed
directory. There was fix in the past allow running uninstalled test:
435d2fd82 ("ima: Rename the folder name for policy files to datafiles"), but I
broke that in this patchset ("IMA: Refactor datafiles directory").

Maybe fixing a code in tst_test.sh
if [ -z "$LTPROOT" ]; then
	export LTPROOT="$PWD"
	export TST_DATAROOT="$LTPROOT/datafiles"
else
	export TST_DATAROOT="$LTPROOT/testcases/data/$TST_ID"
fi

To allow to redefine it, for local testing:
if [ -z "$TST_DATAROOT" ]; then
	if [ -z "$LTPROOT" ]; then
		export LTPROOT="$PWD"
		export TST_DATAROOT="$LTPROOT/datafiles"
	else
		export TST_DATAROOT="$LTPROOT/testcases/data/$TST_ID"
	fi
fi

because datafile layout does not expect subdirectory. I also hate the makefile
helper, which requires to have special directory, datafiles just require some
rethinking.

> On failure to open the file, 
> errno: No such file or directory (2)

The rest is obviously not relevant (was just a hint for problems caused with
other LSM).
> ima_keys 2 TBROK: unable to import a certificate into key_import_test keyring
> ima_keys 2 TINFO: SELinux enabled in enforcing mode, this may affect test results
> ima_keys 2 TINFO: it can be disabled with TST_DISABLE_SELINUX=1 (requires super/root)
> ima_keys 2 TINFO: install seinfo to find used SELinux profiles
> ima_keys 2 TINFO: loaded SELinux profiles: none

Kind regards,
Petr

[1] https://github.com/pevik/ltp/tree/Lachlan_Sneff/ima_keys.sh-second-test.v1.fixes
diff mbox series

Patch

diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
index 392e1e868..68d046678 100644
--- a/testcases/kernel/security/integrity/ima/README.md
+++ b/testcases/kernel/security/integrity/ima/README.md
@@ -16,11 +16,15 @@  space, may contain equivalent measurement tcb rules, detecting them would
 require `IMA_READ_POLICY=y` therefore ignore this option.
 
 ### IMA key test
-`ima_keys.sh` requires a readable IMA policy, as well as a loaded policy
-with `func=KEY_CHECK keyrings=...`, see example in `keycheck.policy`.
+The measuring keys test (first test) in `ima_keys.sh` requires a readable IMA
+policy, as well as a loaded measure policy with `func=KEY_CHECK keyrings=...`.
 
-As well as what's required for the IMA tests, the following are also required
--in the kernel configuration:
+The certificate import test (second test) require measure policy with
+`func=KEY_CHECK keyrings=key_import_test`. Valid policy for both is in
+`keycheck.policy`.
+
+As well as what's required for the IMA tests, key tests require reading the IMA
+policy allowed in the kernel configuration:
 ```
 CONFIG_IMA_READ_POLICY=y
 ```
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
index 3f1934a3d..623162002 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
@@ -1 +1 @@ 
-measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf
+measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
new file mode 100644
index 0000000000000000000000000000000000000000..92be058da22adffa9d6b6e51efa0c737ebbbbdcd
GIT binary patch
literal 650
zcmXqLVrnyJVtl`VnTe5!NhJD#vj69`9|BBf8}FEsx@^_9$Clp>c-c6$+C196^D;7W
zvoaV27z!HjvoVLVaPe?t<QJFZCFZ6YN*hRmgqV4R$}{p4b2Al+Gt=`j^U@WvQ!5SS
z3}oO&a59SVLzFncG#ki?^BP(jSQr@@7#Ud_7)6Qm8W{k&hEOgIY;2s5>?=lA2Ij_I
z27|^<rp88wchfeduxmMW^j9qUxkIugeev4q7u7yrJR_rW$*!>VOo=tim8DK0r^FsU
zl)K`}`+CO4@4KBkoLmcj?fH`%wbDvU<d=4Z>6-SMf6Eh}{&)1rdsOoNQ-1fgBQ1t{
zVTqGwuK95LlFE)6i{@=vlP6!2`Y}x<BF&oXU_nlDxy@C+CNp&=W=00a#jys_20XwZ
zl@(@W{LjK<z+k`);_<VvFf*|?7|4P+d@N!tBCNWX-0#?!UAx9s`mgFmW+nI2#6kmk
zkhC(3gn?Lt$m4W@56wQ)?QVKW<nOtpT)HJrB?Q^`z&K?FdV8b(y8m)~mOOvswu^9m
z-o7mOwCAzaT*~`Y4wxFtmNA_89`W;j{r#iww*5OC5vgEziqD_%=ki$*`;s}`Pfwi{
zbotZ0X`ckHOmaVLm85n@E9hN_K;~PUB>DFAzh(;(UoMln9V>g;W#-LUGS7A`nYQKY
WBem{7L5JThS+;$vggi%(*E;~nlJ80Y

literal 0
HcmV?d00001

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 53c289054..30950904e 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -6,8 +6,8 @@ 
 #
 # Verify that keys are measured correctly based on policy.
 
-TST_NEEDS_CMDS="cut grep sed tr xxd"
-TST_CNT=1
+TST_NEEDS_CMDS="cmp cut grep sed tr xxd"
+TST_CNT=2
 TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
@@ -20,20 +20,22 @@  test1()
 	local pattern="func=KEY_CHECK"
 	local test_file="file.txt"
 
-	tst_res TINFO "verifying key measurement for keyrings and templates specified in IMA policy file"
+	tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy"
 
 	require_ima_policy_content "$pattern"
 	keycheck_lines=$(check_ima_policy_content "$pattern" "")
 	keycheck_line=$(echo "$keycheck_lines" | grep "keyrings" | head -n1)
 
 	if [ -z "$keycheck_line" ]; then
-		tst_brk TCONF "ima policy does not specify a keyrings to check"
+		tst_res TCONF "IMA policy does not specify a keyrings to check"
+		return
 	fi
 
 	keyrings=$(echo "$keycheck_line" | tr " " "\n" | grep "keyrings" | \
 		sed "s/\./\\\./g" | cut -d'=' -f2)
 	if [ -z "$keyrings" ]; then
-		tst_brk TCONF "ima policy has a keyring key-value specifier, but no specified keyrings"
+		tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings"
+		return
 	fi
 
 	templates=$(echo "$keycheck_line" | tr " " "\n" | grep "template" | \
@@ -49,11 +51,13 @@  test1()
 
 		echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file
 
-		expected_digest="$(compute_digest $algorithm $test_file)" || \
-			tst_brk TCONF "cannot compute digest for $algorithm"
+		if ! expected_digest="$(compute_digest $algorithm $test_file)"; then
+			tst_res TCONF "cannot compute digest for $algorithm"
+			return
+		fi
 
 		if [ "$digest" != "$expected_digest" ]; then
-			tst_res TFAIL "incorrect digest was found for the ($keyring) keyring"
+			tst_res TFAIL "incorrect digest was found for $keyring keyring"
 			return
 		fi
 	done
@@ -61,4 +65,52 @@  test1()
 	tst_res TPASS "specified keyrings were measured correctly"
 }
 
+# Create a new keyring, import a certificate into it, and verify
+# that the certificate is measured correctly by IMA.
+test2()
+{
+	tst_require_cmds evmctl keyctl openssl
+
+	local cert_file="$TST_DATAROOT/x509_ima.der"
+	local keyring_name="key_import_test"
+	local temp_file="file.txt"
+	local keyring_id
+
+	tst_res TINFO "verify measurement of certificate imported into a keyring"
+
+	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
+		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
+	fi
+
+	keyctl new_session > /dev/null
+
+	keyring_id=$(keyctl newring $keyring_name @s) || \
+		tst_brk TBROK "unable to create a new keyring"
+
+	tst_is_num $keyring_id || \
+		tst_brk TBROK "unable to parse the new keyring id"
+
+	evmctl import $cert_file $keyring_id > /dev/null || \
+		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"
+
+	grep $keyring_name $ASCII_MEASUREMENTS | tail -n1 | cut -d' ' -f6 | \
+		xxd -r -p > $temp_file
+
+	if [ ! -s $temp_file ]; then
+		tst_res TFAIL "keyring $keyring_name not found in $ASCII_MEASUREMENTS"
+		return
+	fi
+
+	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
+		tst_res TFAIL "logged certificate is not a valid x509 certificate"
+		return
+	fi
+
+	if cmp -s $temp_file $cert_file; then
+		tst_res TPASS "logged certificate matches the original"
+	else
+		tst_res TFAIL "logged certificate does not match original"
+	fi
+}
+
 tst_run