| Message ID | 20200928035605.22701-2-tusharsu@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IMA: Add test for dm-crypt measurement | expand |
Hi Tushar, I'm very sorry about the delay. I'll finish this review in January, here just some quick thoughts (minor style nits, I'll fix it before merge). Generally LGTM, thanks for your work. Reviewed-by: Petr Vorel <pvorel@suse.cz> > New functionality is being added in IMA to measure data provided by > kernel components. Tests have to be added in LTP to validate this new > feature. The functionality in ima_keys.sh can be reused to test this new > feature if it is made generic. > Refactor check_keys_policy() and test1() implemented in ima_keys.sh to > make it generic, and move the functionality to ima_setup.sh as new > functions - check_policy_pattern() and check_ima_ascii_log_for_policy(). > Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> > --- > .../security/integrity/ima/tests/ima_keys.sh | 62 +++------------ > .../security/integrity/ima/tests/ima_setup.sh | 79 +++++++++++++++++++ > 2 files changed, 92 insertions(+), 49 deletions(-) > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > index c9eef4b68..c2120358a 100755 > --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh > @@ -6,7 +6,7 @@ > # Verify that keys are measured correctly based on policy. > -TST_NEEDS_CMDS="cmp cut grep sed xxd" > +TST_NEEDS_CMDS="cmp cut grep xxd" It still requires sed, it's just hidden in check_ima_ascii_log_for_policy Maybe just put at the top of check_ima_ascii_log_for_policy(): tst_require_cmds cut grep sed xxd And here still keep TST_NEEDS_CMDS="cmp cut grep tail xxd" This leads to duplicity in check, but it will not lead to hidden "command not found". > TST_CNT=2 > TST_NEEDS_DEVICE=1 > TST_SETUP=setup > @@ -28,64 +28,28 @@ cleanup() > tst_is_num $KEYRING_ID && keyctl clear $KEYRING_ID > } > -check_keys_policy() > -{ > - local pattern="$1" > - > - if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then > - tst_res TCONF "IMA policy must specify $pattern, $FUNC_KEYCHECK, $TEMPLATE_BUF" > - return 1 > - fi > - return 0 > -} > - > # Based on https://lkml.org/lkml/2019/12/13/564. > # (450d0fd51564 - "IMA: Call workqueue functions to measure queued keys") OK, it has been merged in v5.6-rc1. Any more relevant commits, changes since then? > test1() > { > local keycheck_lines i keyrings templates > local pattern='keyrings=[^[:space:]]+' > - local test_file="file.txt" tmp_file="file2.txt" > + local policy="keyrings" > + local tmp_file="$TST_TMPDIR/keycheck_tmp_file.txt" > + local res Will be unused, see below. > tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy" > - check_keys_policy "$pattern" > $tmp_file || return > - keycheck_lines=$(cat $tmp_file) > - keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \ > - sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') > - if [ -z "$keyrings" ]; then > - tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings" > - return > - fi > - > - templates=$(for i in $keycheck_lines; do echo "$i" | grep "template" | \ > - cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') > - > - tst_res TINFO "keyrings: '$keyrings'" > - tst_res TINFO "templates: '$templates'" > - > - grep -E "($templates).*($keyrings)" $ASCII_MEASUREMENTS | while read line > - do > - local digest expected_digest algorithm > - > - digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) > - algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) > - keyring=$(echo "$line" | cut -d' ' -f5) > + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF > $tmp_file || return > - echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file > + res="$(check_ima_ascii_log_for_policy $policy $tmp_file)" > - 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 $keyring keyring" > - return > - fi > - done > + if [ "$res" = "0" ]; then > + tst_res TPASS "specified keyrings were measured correctly" > + else > + tst_res TFAIL "failed to measure specified keyrings" > + fi Instead of: res="$(check_ima_ascii_log_for_policy $policy $tmp_file)" if [ "$res" = "0" ]; then I'd prefer to have it as: check_ima_ascii_log_for_policy $policy $tmp_file if [ $? -eq 0 ]; then > - tst_res TPASS "specified keyrings were measured correctly" > } > # Create a new keyring, import a certificate into it, and verify > @@ -97,11 +61,11 @@ test2() > local cert_file="$TST_DATAROOT/x509_ima.der" > local keyring_name="key_import_test" > local pattern="keyrings=[^[:space:]]*$keyring_name" > - local temp_file="file.txt" > + local temp_file="$TST_TMPDIR/key_import_test_file.txt" > tst_res TINFO "verify measurement of certificate imported into a keyring" > - check_keys_policy "$pattern" >/dev/null || return > + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF >/dev/null || return > KEYRING_ID=$(keyctl newring $keyring_name @s) || \ > tst_brk TBROK "unable to create a new keyring" > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > index 1f17aa707..2841d7df5 100644 > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh > @@ -54,6 +54,85 @@ compute_digest() > return 1 > } > +check_policy_pattern() > +{ > + local pattern="$1" > + local func="$2" > + local template="$3" > + > + if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then > + tst_res TCONF "IMA policy must specify $pattern, $func, $template" > + return 1 > + fi > + return 0 > +} Probably ok for now (yes, it removes the duplicity with function used in two tests, it's very policy specific). > + > +check_ima_ascii_log_for_policy() > +{ > + local test_file="$TST_TMPDIR/ascii_log_test_file.txt" > + local grep_file="$TST_TMPDIR/ascii_log_grep_file.txt" nit: Since the real description is in variable, I'd just use: local test_file="$TST_TMPDIR/test.txt" local grep_file="$TST_TMPDIR/grep.txt" > + local func_lines sources templates i src > + local input_digest_res=1 > + local policy_option="$1" > + local input_digest="$3" tst_require_cmds cut grep sed xxd > + > + func_lines=$(cat $2) > + > + sources=$(for i in $func_lines; do echo "$i" | grep "$policy_option" | \ > + sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') > + if [ -z "$sources" ]; then > + tst_res TCONF "IMA policy $policy_option is a key-value specifier, but no values specified" > + echo "1" > + return > + fi > + > + templates=$(for i in $func_lines; do echo "$i" | grep "template" | \ > + cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') > + > + tst_res TINFO "policy sources: '$sources'" > + tst_res TINFO "templates: '$templates'" > + > + grep -E "($templates).*($sources)" $ASCII_MEASUREMENTS > $grep_file > + > + while read line > + do > + local digest expected_digest algorithm > + > + digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) > + algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) > + src_line=$(echo "$line" | cut -d' ' -f5) > + > + echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file > + > + if ! expected_digest="$(compute_digest $algorithm $test_file)"; then > + tst_res TCONF "cannot compute digest for $algorithm" > + echo "1" > + return > + fi > + > + if [ "$digest" != "$expected_digest" ]; then > + tst_res TINFO "incorrect digest was found for $src_line $policy_option" > + echo "1" > + return > + fi > + > + if [ "$input_digest" ]; then > + if [ "$digest" = "$input_digest" ]; then > + input_digest_res=0 > + fi > + fi I'd prefer it as single if: if [ -n "$input_digest" -a "$digest" = "$input_digest" ]; then input_digest_res=0 fi > + > + done < $grep_file > + > + if [ "$input_digest" ]; then > + echo "$input_digest_res" > + return this return is redundant. > + else > + echo "0" > + return Also this one. > + fi And actually, instead of whole if/else block wouldn't be just this enough? echo "$input_digest_res" Isn't it the zero value set in the loop at: if [ -n "$input_digest" -a "$digest" = "$input_digest" ]; then input_digest_res=0 fi Kind regards, Petr
Hi Petr, On 2020-12-21 3:05 p.m., Petr Vorel wrote: > Hi Tushar, > > I'm very sorry about the delay. I'll finish this review in January, > here just some quick thoughts (minor style nits, I'll fix it before merge). > > Generally LGTM, thanks for your work. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Thanks for your review. My sincere apologies for missing this email and not responding in time. The device mapper measurement work is being revisited - to cover aspects like more DM targets (not just dm-crypt), better memory management, more relevant attributes from the DM targets, other corner cases etc. Therefore, even though this patch, "1/2: generalize key measurement tests", would be useful for other tests; I will have to revisit the second patch, "2/2: dm-crypt measurements", to address the DM side changes I mentioned above. I will revisit this series, esp. testing the DM target measurements part, once the kernel work I mentioned above is close to completion. I will also address your feedback on patch #1 and #2 from v2 iteration at that time. Thanks again for your review and feedback. Thanks, Tushar >> New functionality is being added in IMA to measure data provided by >> kernel components. Tests have to be added in LTP to validate this new >> feature. The functionality in ima_keys.sh can be reused to test this new >> feature if it is made generic. > >> Refactor check_keys_policy() and test1() implemented in ima_keys.sh to >> make it generic, and move the functionality to ima_setup.sh as new >> functions - check_policy_pattern() and check_ima_ascii_log_for_policy(). > >> Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> >> --- >> .../security/integrity/ima/tests/ima_keys.sh | 62 +++------------ >> .../security/integrity/ima/tests/ima_setup.sh | 79 +++++++++++++++++++ >> 2 files changed, 92 insertions(+), 49 deletions(-) > >> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> index c9eef4b68..c2120358a 100755 >> --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh >> @@ -6,7 +6,7 @@ > >> # Verify that keys are measured correctly based on policy. > >> -TST_NEEDS_CMDS="cmp cut grep sed xxd" >> +TST_NEEDS_CMDS="cmp cut grep xxd" > It still requires sed, it's just hidden in check_ima_ascii_log_for_policy > > Maybe just put at the top of check_ima_ascii_log_for_policy(): > tst_require_cmds cut grep sed xxd > > And here still keep > TST_NEEDS_CMDS="cmp cut grep tail xxd" > > This leads to duplicity in check, but it will not lead to hidden "command not > found". > >> TST_CNT=2 >> TST_NEEDS_DEVICE=1 >> TST_SETUP=setup >> @@ -28,64 +28,28 @@ cleanup() >> tst_is_num $KEYRING_ID && keyctl clear $KEYRING_ID >> } > >> -check_keys_policy() >> -{ >> - local pattern="$1" >> - >> - if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then >> - tst_res TCONF "IMA policy must specify $pattern, $FUNC_KEYCHECK, $TEMPLATE_BUF" >> - return 1 >> - fi >> - return 0 >> -} >> - >> # Based on https://lkml.org/lkml/2019/12/13/564. >> # (450d0fd51564 - "IMA: Call workqueue functions to measure queued keys") > OK, it has been merged in v5.6-rc1. Any more relevant commits, changes since > then? > >> test1() >> { >> local keycheck_lines i keyrings templates >> local pattern='keyrings=[^[:space:]]+' >> - local test_file="file.txt" tmp_file="file2.txt" >> + local policy="keyrings" >> + local tmp_file="$TST_TMPDIR/keycheck_tmp_file.txt" >> + local res > Will be unused, see below. > >> tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy" > >> - check_keys_policy "$pattern" > $tmp_file || return >> - keycheck_lines=$(cat $tmp_file) >> - keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \ >> - sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') >> - if [ -z "$keyrings" ]; then >> - tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings" >> - return >> - fi >> - >> - templates=$(for i in $keycheck_lines; do echo "$i" | grep "template" | \ >> - cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') >> - >> - tst_res TINFO "keyrings: '$keyrings'" >> - tst_res TINFO "templates: '$templates'" >> - >> - grep -E "($templates).*($keyrings)" $ASCII_MEASUREMENTS | while read line >> - do >> - local digest expected_digest algorithm >> - >> - digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) >> - algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) >> - keyring=$(echo "$line" | cut -d' ' -f5) >> + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF > $tmp_file || return > >> - echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file >> + res="$(check_ima_ascii_log_for_policy $policy $tmp_file)" > >> - 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 $keyring keyring" >> - return >> - fi >> - done >> + if [ "$res" = "0" ]; then >> + tst_res TPASS "specified keyrings were measured correctly" >> + else >> + tst_res TFAIL "failed to measure specified keyrings" >> + fi > > Instead of: > res="$(check_ima_ascii_log_for_policy $policy $tmp_file)" > if [ "$res" = "0" ]; then > > I'd prefer to have it as: > check_ima_ascii_log_for_policy $policy $tmp_file > if [ $? -eq 0 ]; then > > >> - tst_res TPASS "specified keyrings were measured correctly" >> } > >> # Create a new keyring, import a certificate into it, and verify >> @@ -97,11 +61,11 @@ test2() >> local cert_file="$TST_DATAROOT/x509_ima.der" >> local keyring_name="key_import_test" >> local pattern="keyrings=[^[:space:]]*$keyring_name" >> - local temp_file="file.txt" >> + local temp_file="$TST_TMPDIR/key_import_test_file.txt" > >> tst_res TINFO "verify measurement of certificate imported into a keyring" > >> - check_keys_policy "$pattern" >/dev/null || return >> + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF >/dev/null || return > >> KEYRING_ID=$(keyctl newring $keyring_name @s) || \ >> tst_brk TBROK "unable to create a new keyring" >> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh >> index 1f17aa707..2841d7df5 100644 >> --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh >> +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh >> @@ -54,6 +54,85 @@ compute_digest() >> return 1 >> } > >> +check_policy_pattern() >> +{ >> + local pattern="$1" >> + local func="$2" >> + local template="$3" >> + >> + if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then >> + tst_res TCONF "IMA policy must specify $pattern, $func, $template" >> + return 1 >> + fi >> + return 0 >> +} > Probably ok for now (yes, it removes the duplicity with function used in two > tests, it's very policy specific). > >> + >> +check_ima_ascii_log_for_policy() >> +{ >> + local test_file="$TST_TMPDIR/ascii_log_test_file.txt" >> + local grep_file="$TST_TMPDIR/ascii_log_grep_file.txt" > nit: Since the real description is in variable, I'd just use: > > local test_file="$TST_TMPDIR/test.txt" > local grep_file="$TST_TMPDIR/grep.txt" > >> + local func_lines sources templates i src >> + local input_digest_res=1 >> + local policy_option="$1" >> + local input_digest="$3" > > tst_require_cmds cut grep sed xxd >> + >> + func_lines=$(cat $2) >> + >> + sources=$(for i in $func_lines; do echo "$i" | grep "$policy_option" | \ >> + sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') >> + if [ -z "$sources" ]; then >> + tst_res TCONF "IMA policy $policy_option is a key-value specifier, but no values specified" >> + echo "1" >> + return >> + fi >> + >> + templates=$(for i in $func_lines; do echo "$i" | grep "template" | \ >> + cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') >> + >> + tst_res TINFO "policy sources: '$sources'" >> + tst_res TINFO "templates: '$templates'" >> + >> + grep -E "($templates).*($sources)" $ASCII_MEASUREMENTS > $grep_file >> + >> + while read line >> + do >> + local digest expected_digest algorithm >> + >> + digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) >> + algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) >> + src_line=$(echo "$line" | cut -d' ' -f5) >> + >> + echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file >> + >> + if ! expected_digest="$(compute_digest $algorithm $test_file)"; then >> + tst_res TCONF "cannot compute digest for $algorithm" >> + echo "1" >> + return >> + fi >> + >> + if [ "$digest" != "$expected_digest" ]; then >> + tst_res TINFO "incorrect digest was found for $src_line $policy_option" >> + echo "1" >> + return >> + fi >> + >> + if [ "$input_digest" ]; then >> + if [ "$digest" = "$input_digest" ]; then >> + input_digest_res=0 >> + fi >> + fi > I'd prefer it as single if: > if [ -n "$input_digest" -a "$digest" = "$input_digest" ]; then > input_digest_res=0 > fi > >> + >> + done < $grep_file >> + >> + if [ "$input_digest" ]; then >> + echo "$input_digest_res" >> + return > this return is redundant. >> + else >> + echo "0" >> + return > Also this one. > >> + fi > > And actually, instead of whole if/else block wouldn't be just this enough? > echo "$input_digest_res" > > Isn't it the zero value set in the loop at: > > if [ -n "$input_digest" -a "$digest" = "$input_digest" ]; then > input_digest_res=0 > fi > > Kind regards, > Petr >
Hi Tushar, > Thanks for your review. > My sincere apologies for missing this email and not responding in time. > The device mapper measurement work is being revisited - to cover aspects > like more DM targets (not just dm-crypt), better memory management, > more relevant attributes from the DM targets, other corner cases etc. > Therefore, even though this patch, "1/2: generalize key measurement > tests", would be useful for other tests; I will have to revisit the > second patch, "2/2: dm-crypt measurements", to address the DM side changes I > mentioned above. > I will revisit this series, esp. testing the DM target measurements > part, once the kernel work I mentioned above is close to completion. > I will also address your feedback on patch #1 and #2 from v2 iteration > at that time. > Thanks again for your review and feedback. I updated your patchset a bit and going to send it to ML. Kind regards, Petr > Thanks, > Tushar
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh index c9eef4b68..c2120358a 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh @@ -6,7 +6,7 @@ # # Verify that keys are measured correctly based on policy. -TST_NEEDS_CMDS="cmp cut grep sed xxd" +TST_NEEDS_CMDS="cmp cut grep xxd" TST_CNT=2 TST_NEEDS_DEVICE=1 TST_SETUP=setup @@ -28,64 +28,28 @@ cleanup() tst_is_num $KEYRING_ID && keyctl clear $KEYRING_ID } -check_keys_policy() -{ - local pattern="$1" - - if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then - tst_res TCONF "IMA policy must specify $pattern, $FUNC_KEYCHECK, $TEMPLATE_BUF" - return 1 - fi - return 0 -} - # Based on https://lkml.org/lkml/2019/12/13/564. # (450d0fd51564 - "IMA: Call workqueue functions to measure queued keys") test1() { local keycheck_lines i keyrings templates local pattern='keyrings=[^[:space:]]+' - local test_file="file.txt" tmp_file="file2.txt" + local policy="keyrings" + local tmp_file="$TST_TMPDIR/keycheck_tmp_file.txt" + local res tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy" - check_keys_policy "$pattern" > $tmp_file || return - keycheck_lines=$(cat $tmp_file) - keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \ - sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') - if [ -z "$keyrings" ]; then - tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings" - return - fi - - templates=$(for i in $keycheck_lines; do echo "$i" | grep "template" | \ - cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') - - tst_res TINFO "keyrings: '$keyrings'" - tst_res TINFO "templates: '$templates'" - - grep -E "($templates).*($keyrings)" $ASCII_MEASUREMENTS | while read line - do - local digest expected_digest algorithm - - digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) - algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) - keyring=$(echo "$line" | cut -d' ' -f5) + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF > $tmp_file || return - echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file + res="$(check_ima_ascii_log_for_policy $policy $tmp_file)" - 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 $keyring keyring" - return - fi - done + if [ "$res" = "0" ]; then + tst_res TPASS "specified keyrings were measured correctly" + else + tst_res TFAIL "failed to measure specified keyrings" + fi - tst_res TPASS "specified keyrings were measured correctly" } # Create a new keyring, import a certificate into it, and verify @@ -97,11 +61,11 @@ test2() local cert_file="$TST_DATAROOT/x509_ima.der" local keyring_name="key_import_test" local pattern="keyrings=[^[:space:]]*$keyring_name" - local temp_file="file.txt" + local temp_file="$TST_TMPDIR/key_import_test_file.txt" tst_res TINFO "verify measurement of certificate imported into a keyring" - check_keys_policy "$pattern" >/dev/null || return + check_policy_pattern "$pattern" $FUNC_KEYCHECK $TEMPLATE_BUF >/dev/null || return KEYRING_ID=$(keyctl newring $keyring_name @s) || \ tst_brk TBROK "unable to create a new keyring" diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh index 1f17aa707..2841d7df5 100644 --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh @@ -54,6 +54,85 @@ compute_digest() return 1 } +check_policy_pattern() +{ + local pattern="$1" + local func="$2" + local template="$3" + + if ! grep -E "$pattern" $TST_TMPDIR/policy.txt; then + tst_res TCONF "IMA policy must specify $pattern, $func, $template" + return 1 + fi + return 0 +} + +check_ima_ascii_log_for_policy() +{ + local test_file="$TST_TMPDIR/ascii_log_test_file.txt" + local grep_file="$TST_TMPDIR/ascii_log_grep_file.txt" + local func_lines sources templates i src + local input_digest_res=1 + local policy_option="$1" + local input_digest="$3" + + func_lines=$(cat $2) + + sources=$(for i in $func_lines; do echo "$i" | grep "$policy_option" | \ + sed "s/\./\\\./g" | cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') + if [ -z "$sources" ]; then + tst_res TCONF "IMA policy $policy_option is a key-value specifier, but no values specified" + echo "1" + return + fi + + templates=$(for i in $func_lines; do echo "$i" | grep "template" | \ + cut -d'=' -f2; done | sed ':a;N;$!ba;s/\n/|/g') + + tst_res TINFO "policy sources: '$sources'" + tst_res TINFO "templates: '$templates'" + + grep -E "($templates).*($sources)" $ASCII_MEASUREMENTS > $grep_file + + while read line + do + local digest expected_digest algorithm + + digest=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f2) + algorithm=$(echo "$line" | cut -d' ' -f4 | cut -d':' -f1) + src_line=$(echo "$line" | cut -d' ' -f5) + + echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file + + if ! expected_digest="$(compute_digest $algorithm $test_file)"; then + tst_res TCONF "cannot compute digest for $algorithm" + echo "1" + return + fi + + if [ "$digest" != "$expected_digest" ]; then + tst_res TINFO "incorrect digest was found for $src_line $policy_option" + echo "1" + return + fi + + if [ "$input_digest" ]; then + if [ "$digest" = "$input_digest" ]; then + input_digest_res=0 + fi + fi + + done < $grep_file + + if [ "$input_digest" ]; then + echo "$input_digest_res" + return + else + echo "0" + return + fi +} + check_policy_readable() { if [ ! -f $IMA_POLICY ]; then
New functionality is being added in IMA to measure data provided by kernel components. Tests have to be added in LTP to validate this new feature. The functionality in ima_keys.sh can be reused to test this new feature if it is made generic. Refactor check_keys_policy() and test1() implemented in ima_keys.sh to make it generic, and move the functionality to ima_setup.sh as new functions - check_policy_pattern() and check_ima_ascii_log_for_policy(). Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com> --- .../security/integrity/ima/tests/ima_keys.sh | 62 +++------------ .../security/integrity/ima/tests/ima_setup.sh | 79 +++++++++++++++++++ 2 files changed, 92 insertions(+), 49 deletions(-)