* [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl
@ 2020-03-27 4:25 Vitaly Chikunov
2020-03-27 4:25 ` [PATCH v8 1/2] " Vitaly Chikunov
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Vitaly Chikunov @ 2020-03-27 4:25 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
This series adds simple evmctl tests for hash, sign, and verify operations.
Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
Changelog since v7:
- Absence of key file cause test skip instead of fail.
- Mono patch is split into two by Mimi Zohar request.
Changelog since v6:
- ima_hash.test: Compare generated hashes with known values.
I found that on power8 qemu with default -cpu calculates hashes wrongly.
For exmaple: `openssl dgst -sha224 -hex /dev/null' results in
83b2514951547ee00c7062fa3eb42079ff19f280ec81eedbdb0e3997 instead of
d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f.
- Make `_keyid' use temporary files to not rely on presence of `/des/stdout'.
- evm sign: add `--generation 0' option.
I found that on overlay fs (with ext4 over 9p) FS_IOC_GETVERSION does
not work.
- gen-keys.sh: allows `force' argument to regenerate all test keys.
Changelog since v5:
- Fix tests if gost-engine is not present. Thx to Mimi Zohar for testing on
Xenial.
Changelog since v4:
- Fix bugs found by Mimi Zohar:
- Fix typos in variable names
- Fix `-out -' in asn1parse for openssl 1.0.x
Changelog since v3:
- Apply changes based on some suggestions from Petr Vorel, such as
- Add .gitignore
- Do not color output if stdout is not tty.
- Fix blkid error with --uuid option.
- Remove or change some comments
- Replace ENGINE with EVMCTL_ENGINE and OPENSSL_ENGINE.
- All tests pass over next branch.
Changelog since v2:
- ima_sign.test and ima_verify.test merged into sign_verify.test
which is also able to test evm signatures.
- Apply Mimi Zohar suggestions regarding commenting, variable renaming,
code rearranging, etc. with intent to simplify the code and review.
Changelog since v1:
- Apply suggestions by Petr Vorel:
- Rename function names and variables to be more understandable.
- Rename tests/functions to tests/functions.sh.
- Define exit codes (77, 99, ...) as variables.
- Added more comments and remove single letter variables (for Mimi Zohar).
- Move getfattr check into function.
- Move evmctl run and check into single function.
- Add sign/verify tests for v1 signatures.
- Minor improvements.
- Since I still edit all 5 files I did not split the patch into multiple
commits to separate the files, otherwise editing will become too
complicated, as I ought to continuously rebase and edit different
commits. This was really non-productive suggestion.
Vitaly Chikunov (2):
ima-evm-utils: Add some tests for evmctl
ima-evm-utils: Add sign/verify tests for evmctl
.gitignore | 2 +-
Makefile.am | 2 +-
configure.ac | 1 +
tests/.gitignore | 16 +++
tests/Makefile.am | 12 ++
tests/functions.sh | 272 ++++++++++++++++++++++++++++++++++++
tests/gen-keys.sh | 97 +++++++++++++
tests/ima_hash.test | 80 +++++++++++
tests/sign_verify.test | 364 +++++++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 844 insertions(+), 2 deletions(-)
create mode 100644 tests/.gitignore
create mode 100644 tests/Makefile.am
create mode 100755 tests/functions.sh
create mode 100755 tests/gen-keys.sh
create mode 100755 tests/ima_hash.test
create mode 100755 tests/sign_verify.test
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl 2020-03-27 4:25 [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl Vitaly Chikunov @ 2020-03-27 4:25 ` Vitaly Chikunov [not found] ` <f9b36972-df5d-db9a-d840-52e9ff76d271@linux.microsoft.com> 2020-03-31 14:25 ` Mimi Zohar 2020-03-27 4:25 ` [PATCH v8 2/2] ima-evm-utils: Add sign/verify " Vitaly Chikunov [not found] ` <d39b433e-4504-d0a4-116f-dd33ca238f7f@linux.microsoft.com> 2 siblings, 2 replies; 15+ messages in thread From: Vitaly Chikunov @ 2020-03-27 4:25 UTC (permalink / raw) To: Mimi Zohar, Dmitry Kasatkin, linux-integrity Run `make check' to execute the tests. This commit only adds ima_hash test. Signed-off-by: Vitaly Chikunov <vt@altlinux.org> --- .gitignore | 2 +- Makefile.am | 2 +- configure.ac | 1 + tests/.gitignore | 16 ++++ tests/Makefile.am | 7 ++ tests/functions.sh | 272 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/ima_hash.test | 80 ++++++++++++++++ 7 files changed, 378 insertions(+), 2 deletions(-) create mode 100644 tests/.gitignore create mode 100644 tests/Makefile.am create mode 100755 tests/functions.sh create mode 100755 tests/ima_hash.test diff --git a/.gitignore b/.gitignore index cb82166..c579199 100644 --- a/.gitignore +++ b/.gitignore @@ -21,7 +21,7 @@ missing compile libtool ltmain.sh - +test-driver # Compiled executables *.o diff --git a/Makefile.am b/Makefile.am index dba408d..45c6f82 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = src +SUBDIRS = src tests dist_man_MANS = evmctl.1 doc_DATA = examples/ima-genkey-self.sh examples/ima-genkey.sh examples/ima-gen-local-ca.sh diff --git a/configure.ac b/configure.ac index 099c3b1..f246182 100644 --- a/configure.ac +++ b/configure.ac @@ -72,6 +72,7 @@ EVMCTL_MANPAGE_DOCBOOK_XSL AC_CONFIG_FILES([Makefile src/Makefile + tests/Makefile packaging/ima-evm-utils.spec ]) AC_OUTPUT diff --git a/tests/.gitignore b/tests/.gitignore new file mode 100644 index 0000000..9ecc984 --- /dev/null +++ b/tests/.gitignore @@ -0,0 +1,16 @@ +# Generated by test driver +*.log +*.trs + +# Generated by tests +*.txt +*.out +*.sig +*.sig2 + +# Generated certs and keys (by gen-keys.sh) +*.cer +*.pub +*.key +*.conf + diff --git a/tests/Makefile.am b/tests/Makefile.am new file mode 100644 index 0000000..e37b958 --- /dev/null +++ b/tests/Makefile.am @@ -0,0 +1,7 @@ +check_SCRIPTS = +TESTS = $(check_SCRIPTS) + +check_SCRIPTS += ima_hash.test + +clean-local: + -rm -f *.txt *.out *.sig *.sig2 diff --git a/tests/functions.sh b/tests/functions.sh new file mode 100755 index 0000000..16c8d89 --- /dev/null +++ b/tests/functions.sh @@ -0,0 +1,272 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# ima-evm-utils tests bash functions +# +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> +# +# 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, 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. + +# Tests accounting +declare -i testspass=0 testsfail=0 testsskip=0 + +# Exit codes (compatible with automake) +declare -r OK=0 +declare -r FAIL=1 +declare -r HARDFAIL=99 # hard failure no matter testing mode +declare -r SKIP=77 + +# You can set env VERBOSE=1 to see more output from evmctl +V=vvvv +V=${V:0:$VERBOSE} +V=${V:+-$V} + +# Exit if env FAILEARLY is defined. +# Used in expect_{pass,fail}. +exit_early() { + if [ $FAILEARLY ]; then + exit $1 + fi +} + +# Require particular executables to be present +_require() { + ret= + for i; do + if ! type $i; then + echo "$i is required for test" + ret=1 + fi + done + [ $ret ] && exit $HARDFAIL +} + +# Only allow color output on tty +if [ -t 1 ]; then + RED=$'\e[1;31m' + GREEN=$'\e[1;32m' + YELLOW=$'\e[1;33m' + BLUE=$'\e[1;34m' + CYAN=$'\e[1;36m' + NORM=$'\e[m' +fi + +# Test mode determined by TFAIL variable: +# undefined: to success testing +# defined: failure testing +TFAIL= +TMODE=+ # mode character to prepend running command in log +declare -i TNESTED=0 # just for sanity checking + +# Run positive test (one that should pass) and account its result +expect_pass() { + local ret + + if [ $TNESTED -gt 0 ]; then + echo $RED"expect_pass should not be run nested"$NORM + testsfail+=1 + exit $HARDFAIL + fi + TFAIL= + TMODE=+ + TNESTED+=1 + [[ "$VERBOSE" -gt 1 ]] && echo "____ START positive test: $@" + "$@" + ret=$? + [[ "$VERBOSE" -gt 1 ]] && echo "^^^^ STOP ($ret) positive test: $@" + TNESTED+=-1 + case $ret in + 0) testspass+=1 ;; + 77) testsskip+=1 ;; + 99) testsfail+=1; exit_early 1 ;; + *) testsfail+=1; exit_early 2 ;; + esac + return $ret +} + +# Eval negative test (one that should fail) and account its result +expect_fail() { + local ret + + if [ $TNESTED -gt 0 ]; then + echo $RED"expect_fail should not be run nested"$NORM + testsfail+=1 + exit $HARDFAIL + fi + + TFAIL=yes + TMODE=- + TNESTED+=1 + [[ "$VERBOSE" -gt 1 ]] && echo "____ START negative test: $@" + "$@" + ret=$? + [[ "$VERBOSE" -gt 1 ]] && echo "^^^^ STOP ($ret) negative test: $@" + TNESTED+=-1 + case $ret in + 0) testsfail+=1; exit_early 3 ;; + 77) testsskip+=1 ;; + 99) testsfail+=1; exit_early 4 ;; + *) testspass+=1 ;; + esac + # Restore defaults (as in positive tests) + # for tests to run without wrappers + TFAIL= + TMODE=+ + return $ret +} + +# return true if current test is positive +_is_expect_pass() { + [ ! $TFAIL ] +} + +# return true if current test is negative +_is_expect_fail() { + [ $TFAIL ] +} + +# Show blank line and color following text to red +# if it's real error (ie we are in expect_pass mode). +red_if_failure() { + if _is_expect_pass; then + echo $@ $RED + COLOR_RESTORE=1 + fi +} + +# For hard errors +red_always() { + echo $@ $RED + COLOR_RESTORE=1 +} + +color_restore() { + [ $COLOR_RESTORE ] && echo $@ $NORM + COLOR_RESTORE= +} + +ADD_DEL= +ADD_TEXT_FOR= +# _evmctl_run should be run as `_evmctl_run ... || return' +_evmctl_run() { + local op=$1 out=$1-$$.out + local text_for=${FOR:+for $ADD_TEXT_FOR} + # Additional parameters: + # ADD_DEL: additional files to rm on failure + # ADD_TEXT_FOR: append to text as 'for $ADD_TEXT_FOR' + + cmd="evmctl $V $EVMCTL_ENGINE $@" + echo $YELLOW$TMODE $cmd$NORM + $cmd >$out 2>&1 + ret=$? + + # Shell special and signal exit codes (except 255) + if [ $ret -ge 126 -a $ret -lt 255 ]; then + red_always + echo "evmctl $op failed hard with ($ret) $text_for" + sed 's/^/ /' $out + color_restore + rm $out $ADD_DEL + ADD_DEL= + ADD_TEXT_FOR= + return $HARDFAIL + elif [ $ret -gt 0 ]; then + red_if_failure + echo "evmctl $op failed" ${TFAIL:+properly} "with ($ret) $text_for" + # Show evmctl output only in verbose mode or if real failure. + if _is_expect_pass || [ "$VERBOSE" ]; then + sed 's/^/ /' $out + fi + color_restore + rm $out $ADD_DEL + ADD_DEL= + ADD_TEXT_FOR= + return $FAIL + elif _is_expect_fail; then + red_always + echo "evmctl $op wrongly succeeded $text_for" + sed 's/^/ /' $out + color_restore + else + [ "$VERBOSE" ] && sed 's/^/ /' $out + fi + rm $out + ADD_DEL= + ADD_TEXT_FOR= + return $OK +} + +# Extract xattr $attr from $file into $out file skipping $pref'ix +_extract_xattr() { + local file=$1 attr=$2 out=$3 pref=$4 + + getfattr -n $attr -e hex $file \ + | grep "^$attr=" \ + | sed "s/^$attr=$pref//" \ + | xxd -r -p > $out +} + +# Test if xattr $attr in $file matches $pref'ix +# Show error and fail otherwise. +_test_xattr() { + local file=$1 attr=$2 pref=$3 + local text_for=${ADD_TEXT_FOR:+ for $ADD_TEXT_FOR} + + if ! getfattr -n $attr -e hex $file | egrep -qx "$attr=$pref"; then + red_if_failure + echo "Did not find expected hash$text_for:" + echo " $attr=$pref" + echo "" + echo "Actual output below:" + getfattr -n $attr -e hex $file | sed 's/^/ /' + color_restore + rm $file + ADD_TEXT_FOR= + return $FAIL + fi + ADD_TEXT_FOR= +} + +# Try to enable gost-engine if needed. +_enable_gost_engine() { + # Do not enable if it's already working (enabled by user) + if ! openssl md_gost12_256 /dev/null >/dev/null 2>&1 \ + && openssl engine gost >/dev/null 2>&1; then + EVMCTL_ENGINE="--engine gost" + OPENSSL_ENGINE="-engine gost" + fi +} + +# Show test stats and exit into automake test system +# with proper exit code (same as ours). +_report_exit() { + if [ $testsfail -gt 0 ]; then + echo "=================================" + echo " Run with FAILEARLY=1 $0 $@" + echo " To stop after first failure" + echo "=================================" + fi + [ $testspass -gt 0 ] && echo -n $GREEN || echo -n $NORM + echo -n "PASS: $testspass" + [ $testsskip -gt 0 ] && echo -n $YELLOW || echo -n $NORM + echo -n " SKIP: $testsskip" + [ $testsfail -gt 0 ] && echo -n $RED || echo -n $NORM + echo " FAIL: $testsfail" + echo $NORM + if [ $testsfail -gt 0 ]; then + exit $FAIL + elif [ $testspass -gt 0 ]; then + exit $OK + else + exit $SKIP + fi +} + diff --git a/tests/ima_hash.test b/tests/ima_hash.test new file mode 100755 index 0000000..30aec19 --- /dev/null +++ b/tests/ima_hash.test @@ -0,0 +1,80 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# evmctl ima_hash tests +# +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> +# +# 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, 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. + +cd $(dirname $0) +PATH=../src:$PATH +source ./functions.sh +_require evmctl openssl getfattr + +trap _report_exit EXIT +set -f # disable globbing + +check() { + local alg=$1 pref=$2 chash=$3 hash + local file=$alg-hash.txt + + rm -f $file + touch $file + # Generate hash with openssl, if it's failed skip test, + # unless it's negative test, then pass to evmctl + cmd="openssl dgst $OPENSSL_ENGINE -$alg $file" + echo - $cmd + hash=$(set -o pipefail; $cmd 2>/dev/null | cut -d' ' -f2) + if [ $? -ne 0 ] && _is_expect_pass; then + echo $CYAN"$alg test is skipped"$NORM + rm $file + return $SKIP + fi + if [ "$chash" ] && [ "$chash" != "$hash" ]; then + red_always + echo "Invalid hash for $alg from openssl" + echo "Expected: $chash" + echo "Returned: $hash" + color_restore + rm $file + return $HARDFAIL + fi + + ADD_TEXT_FOR=$alg ADD_DEL=$file \ + _evmctl_run ima_hash --hashalgo $alg --xattr-user $file || return + ADD_TEXT_FOR=$alg \ + _test_xattr $file user.ima $pref$hash || return + rm $file + return $OK +} + +# check args: algo hdr-prefix canonic-hash +expect_pass check md4 0x01 31d6cfe0d16ae931b73c59d7e0c089c0 +expect_pass check md5 0x01 d41d8cd98f00b204e9800998ecf8427e +expect_pass check sha1 0x01 da39a3ee5e6b4b0d3255bfef95601890afd80709 +expect_fail check SHA1 0x01 # uppercase +expect_fail check sha512-224 0x01 # valid for pkcs1 +expect_fail check sha512-256 0x01 # valid for pkcs1 +expect_fail check unknown 0x01 # nonexistent +expect_pass check sha224 0x0407 d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f +expect_pass check sha256 0x0404 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 +expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114be07434c0cc7bf63f6e1da274edebfe76f65fbd51ad2f14898b95b +expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e +expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31 +expect_fail check sm3 0x01 +expect_fail check sm3-256 0x01 +_enable_gost_engine +expect_pass check md_gost12_256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb +expect_pass check streebog256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb +expect_pass check md_gost12_512 0x0413 8e945da209aa869f0455928529bcae4679e9873ab707b55315f56ceb98bef0a7362f715528356ee83cda5f2aac4c6ad2ba3a715c1bcd81cb8e9f90bf4c1c1a8a +expect_pass check streebog512 0x0413 8e945da209aa869f0455928529bcae4679e9873ab707b55315f56ceb98bef0a7362f715528356ee83cda5f2aac4c6ad2ba3a715c1bcd81cb8e9f90bf4c1c1a8a + -- 2.11.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <f9b36972-df5d-db9a-d840-52e9ff76d271@linux.microsoft.com>]
* Re: [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl [not found] ` <f9b36972-df5d-db9a-d840-52e9ff76d271@linux.microsoft.com> @ 2020-03-30 16:29 ` Lakshmi Ramasubramanian 2020-03-30 17:11 ` Vitaly Chikunov 0 siblings, 1 reply; 15+ messages in thread From: Lakshmi Ramasubramanian @ 2020-03-30 16:29 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, linux-integrity On 3/30/20 9:16 AM, Lakshmi Ramasubramanian wrote: > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: > >> Run `make check' to execute the tests. > > autogen.sh followed by configure need to be run. Could you please add > that in the patch description? > >> diff --git a/tests/functions.sh b/tests/functions.sh >> new file mode 100755 >> index 0000000..16c8d89 >> --- /dev/null >> +++ b/tests/functions.sh >> @@ -0,0 +1,272 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-2.0 > > In some source files in the kernel repo I have seen the following: > > // SPDX-License-Identifier: GPL-2.0+ > > Not sure if it is applicable here. > >> +check() { >> + local alg=$1 pref=$2 chash=$3 hash >> + local file=$alg-hash.txt >> + >> + rm -f $file >> + touch $file >> + # Generate hash with openssl, if it's failed skip test, > > "if it failed skip test" > >> + # unless it's negative test, then pass to evmctl >> + cmd="openssl dgst $OPENSSL_ENGINE -$alg $file" >> +} >> + > >> +# check args: algo hdr-prefix canonic-hash >> +expect_pass check md4 0x01 31d6cfe0d16ae931b73c59d7e0c089c0 >> +expect_pass check md5 0x01 d41d8cd98f00b204e9800998ecf8427e > Is MD4 and MD5 tests still needed? > > > -lakshmi + Mimi and linux-integrity ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl 2020-03-30 16:29 ` Lakshmi Ramasubramanian @ 2020-03-30 17:11 ` Vitaly Chikunov 0 siblings, 0 replies; 15+ messages in thread From: Vitaly Chikunov @ 2020-03-30 17:11 UTC (permalink / raw) To: Lakshmi Ramasubramanian; +Cc: Mimi Zohar, linux-integrity Lakshmi, On Mon, Mar 30, 2020 at 09:29:29AM -0700, Lakshmi Ramasubramanian wrote: > On 3/30/20 9:16 AM, Lakshmi Ramasubramanian wrote: > > > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: > > > > > Run `make check' to execute the tests. > > > > autogen.sh followed by configure need to be run. Could you please add > > that in the patch description? Why this patch should have this and not any other patch/commit? > > > diff --git a/tests/functions.sh b/tests/functions.sh > > > new file mode 100755 > > > index 0000000..16c8d89 > > > --- /dev/null > > > +++ b/tests/functions.sh > > > @@ -0,0 +1,272 @@ > > > +#!/bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > > > In some source files in the kernel repo I have seen the following: > > > > // SPDX-License-Identifier: GPL-2.0+ > > > > Not sure if it is applicable here. I'm sure it's applicable. > > > > > +check() { > > > + local alg=$1 pref=$2 chash=$3 hash > > > + local file=$alg-hash.txt > > > + > > > + rm -f $file > > > + touch $file > > > + # Generate hash with openssl, if it's failed skip test, > > > > "if it failed skip test" > > > > > + # unless it's negative test, then pass to evmctl > > > + cmd="openssl dgst $OPENSSL_ENGINE -$alg $file" > > > +} > > > + > > > > > +# check args: algo hdr-prefix canonic-hash > > > +expect_pass check md4 0x01 31d6cfe0d16ae931b73c59d7e0c089c0 > > > +expect_pass check md5 0x01 d41d8cd98f00b204e9800998ecf8427e > > Is MD4 and MD5 tests still needed? If it's supported why it shouldn't be tested. > > > > > > -lakshmi > > + Mimi and linux-integrity ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl 2020-03-27 4:25 ` [PATCH v8 1/2] " Vitaly Chikunov [not found] ` <f9b36972-df5d-db9a-d840-52e9ff76d271@linux.microsoft.com> @ 2020-03-31 14:25 ` Mimi Zohar 2020-03-31 15:14 ` Vitaly Chikunov 1 sibling, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2020-03-31 14:25 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity Hi Vitaly, On Fri, 2020-03-27 at 07:25 +0300, Vitaly Chikunov wrote: > Run `make check' to execute the tests. > This commit only adds ima_hash test. > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> Thank you for breaking up the patch based on tests. "ima_hash.test" may be executed by running "make check", but obviously also by invoking it directly. A couple of comments/questions inline below. > diff --git a/tests/functions.sh b/tests/functions.sh > new file mode 100755 > index 0000000..16c8d89 > --- /dev/null > +++ b/tests/functions.sh > @@ -0,0 +1,272 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# > +# ima-evm-utils tests bash functions > +# > +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> > +# > +# 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, 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. > + > +# Tests accounting > +declare -i testspass=0 testsfail=0 testsskip=0 > + > +# Exit codes (compatible with automake) > +declare -r OK=0 > +declare -r FAIL=1 > +declare -r HARDFAIL=99 # hard failure no matter testing mode > +declare -r SKIP=77 > + > +# You can set env VERBOSE=1 to see more output from evmctl > +V=vvvv > +V=${V:0:$VERBOSE} > +V=${V:+-$V} > + > +# Exit if env FAILEARLY is defined. > +# Used in expect_{pass,fail}. > +exit_early() { > + if [ $FAILEARLY ]; then Throughout variables are not double quoted, though the tests themselves turn off globbing (set -f). shellcheck doesn't detect that globbing has been turned off. It complains about each variable usage that is not double quoted. > + exit $1 > + fi > +} > + > +# Require particular executables to be present > +_require() { > + ret= > + for i; do > + if ! type $i; then > + echo "$i is required for test" > + ret=1 > + fi > + done > + [ $ret ] && exit $HARDFAIL > +} > + > +# Only allow color output on tty > +if [ -t 1 ]; then > + RED=$'\e[1;31m' > + GREEN=$'\e[1;32m' > + YELLOW=$'\e[1;33m' > + BLUE=$'\e[1;34m' > + CYAN=$'\e[1;36m' > + NORM=$'\e[m' > +fi > + > +# Test mode determined by TFAIL variable: > +# undefined: to success testing > +# defined: failure testing > +TFAIL= > +TMODE=+ # mode character to prepend running command in log > +declare -i TNESTED=0 # just for sanity checking > + > +# Run positive test (one that should pass) and account its result > +expect_pass() { > + local ret > + > + if [ $TNESTED -gt 0 ]; then > + echo $RED"expect_pass should not be run nested"$NORM > + testsfail+=1 > + exit $HARDFAIL > + fi > + TFAIL= > + TMODE=+ > + TNESTED+=1 > + [[ "$VERBOSE" -gt 1 ]] && echo "____ START positive test: $@" Unless VERBOSE is set as an environment variable, it isn't defined. This isn't an issue when using [[ ... ]], but should it be initialized: VERBOSE="${VERBOSE:-0}"? > + "$@" > + ret=$? > + [[ "$VERBOSE" -gt 1 ]] && echo "^^^^ STOP ($ret) positive test: $@" > + TNESTED+=-1 > + case $ret in > + 0) testspass+=1 ;; > + 77) testsskip+=1 ;; > + 99) testsfail+=1; exit_early 1 ;; > + *) testsfail+=1; exit_early 2 ;; > + esac > + return $ret > +} > + > +# Eval negative test (one that should fail) and account its result > +expect_fail() { > + local ret > + > + if [ $TNESTED -gt 0 ]; then > + echo $RED"expect_fail should not be run nested"$NORM > + testsfail+=1 > + exit $HARDFAIL > + fi > + > + TFAIL=yes > + TMODE=- > + TNESTED+=1 > + [[ "$VERBOSE" -gt 1 ]] && echo "____ START negative test: $@" > + "$@" > + ret=$? > + [[ "$VERBOSE" -gt 1 ]] && echo "^^^^ STOP ($ret) negative test: $@" > + TNESTED+=-1 > + case $ret in > + 0) testsfail+=1; exit_early 3 ;; > + 77) testsskip+=1 ;; > + 99) testsfail+=1; exit_early 4 ;; > + *) testspass+=1 ;; > + esac > + # Restore defaults (as in positive tests) > + # for tests to run without wrappers > + TFAIL= > + TMODE=+ > + return $ret > +} > + > +# return true if current test is positive > +_is_expect_pass() { > + [ ! $TFAIL ] > +} > + > +# return true if current test is negative > +_is_expect_fail() { > + [ $TFAIL ] > +} > + > +# Show blank line and color following text to red > +# if it's real error (ie we are in expect_pass mode). > +red_if_failure() { > + if _is_expect_pass; then > + echo $@ $RED > + COLOR_RESTORE=1 > + fi > +} > + > +# For hard errors > +red_always() { > + echo $@ $RED A few functions - "red_always", "red_if_failure", "color_restore" - use "$@", but none of the function callers pass any parameters. Is there a reason for the "$@" or just something left over from debugging? > + COLOR_RESTORE=1 > +} > + > +color_restore() { > + [ $COLOR_RESTORE ] && echo $@ $NORM > + COLOR_RESTORE= > +} > + > +ADD_DEL= > +ADD_TEXT_FOR= > +# _evmctl_run should be run as `_evmctl_run ... || return' > +_evmctl_run() { > + local op=$1 out=$1-$$.out > + local text_for=${FOR:+for $ADD_TEXT_FOR} > + # Additional parameters: > + # ADD_DEL: additional files to rm on failure > + # ADD_TEXT_FOR: append to text as 'for $ADD_TEXT_FOR' > + > + cmd="evmctl $V $EVMCTL_ENGINE $@" > + echo $YELLOW$TMODE $cmd$NORM > + $cmd >$out 2>&1 > + ret=$? > + > + # Shell special and signal exit codes (except 255) > + if [ $ret -ge 126 -a $ret -lt 255 ]; then > + red_always > + echo "evmctl $op failed hard with ($ret) $text_for" > + sed 's/^/ /' $out > + color_restore > + rm $out $ADD_DEL > + ADD_DEL= > + ADD_TEXT_FOR= > + return $HARDFAIL > + elif [ $ret -gt 0 ]; then > + red_if_failure > + echo "evmctl $op failed" ${TFAIL:+properly} "with ($ret) $text_for" > + # Show evmctl output only in verbose mode or if real failure. > + if _is_expect_pass || [ "$VERBOSE" ]; then > + sed 's/^/ /' $out > + fi > + color_restore > + rm $out $ADD_DEL > + ADD_DEL= > + ADD_TEXT_FOR= > + return $FAIL > + elif _is_expect_fail; then > + red_always > + echo "evmctl $op wrongly succeeded $text_for" > + sed 's/^/ /' $out > + color_restore > + else > + [ "$VERBOSE" ] && sed 's/^/ /' $out > + fi > + rm $out > + ADD_DEL= > + ADD_TEXT_FOR= > + return $OK > +} > + > +# Extract xattr $attr from $file into $out file skipping $pref'ix > +_extract_xattr() { > + local file=$1 attr=$2 out=$3 pref=$4 > + > + getfattr -n $attr -e hex $file \ > + | grep "^$attr=" \ > + | sed "s/^$attr=$pref//" \ > + | xxd -r -p > $out > +} > + > +# Test if xattr $attr in $file matches $pref'ix > +# Show error and fail otherwise. > +_test_xattr() { > + local file=$1 attr=$2 pref=$3 > + local text_for=${ADD_TEXT_FOR:+ for $ADD_TEXT_FOR} > + > + if ! getfattr -n $attr -e hex $file | egrep -qx "$attr=$pref"; then > + red_if_failure > + echo "Did not find expected hash$text_for:" > + echo " $attr=$pref" > + echo "" > + echo "Actual output below:" > + getfattr -n $attr -e hex $file | sed 's/^/ /' > + color_restore > + rm $file > + ADD_TEXT_FOR= > + return $FAIL > + fi > + ADD_TEXT_FOR= > +} > + > +# Try to enable gost-engine if needed. > +_enable_gost_engine() { > + # Do not enable if it's already working (enabled by user) > + if ! openssl md_gost12_256 /dev/null >/dev/null 2>&1 \ Interesting /dev/null usage here as a filename. > + && openssl engine gost >/dev/null 2>&1; then > + EVMCTL_ENGINE="--engine gost" > + OPENSSL_ENGINE="-engine gost" > + fi > +} > + > +# Show test stats and exit into automake test system > +# with proper exit code (same as ours). > +_report_exit() { > + if [ $testsfail -gt 0 ]; then > + echo "=================================" > + echo " Run with FAILEARLY=1 $0 $@" > + echo " To stop after first failure" > + echo "=================================" > + fi > + [ $testspass -gt 0 ] && echo -n $GREEN || echo -n $NORM > + echo -n "PASS: $testspass" > + [ $testsskip -gt 0 ] && echo -n $YELLOW || echo -n $NORM > + echo -n " SKIP: $testsskip" > + [ $testsfail -gt 0 ] && echo -n $RED || echo -n $NORM > + echo " FAIL: $testsfail" > + echo $NORM > + if [ $testsfail -gt 0 ]; then > + exit $FAIL > + elif [ $testspass -gt 0 ]; then > + exit $OK > + else > + exit $SKIP > + fi > +} > + > diff --git a/tests/ima_hash.test b/tests/ima_hash.test > new file mode 100755 > index 0000000..30aec19 > --- /dev/null > +++ b/tests/ima_hash.test > @@ -0,0 +1,80 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# > +# evmctl ima_hash tests > +# > +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> > +# > +# 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, 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. > + > +cd $(dirname $0) > +PATH=../src:$PATH > +source ./functions.sh > +_require evmctl openssl getfattr > + > +trap _report_exit EXIT > +set -f # disable globbing > + > +check() { > + local alg=$1 pref=$2 chash=$3 hash > + local file=$alg-hash.txt > + > + rm -f $file > + touch $file > + # Generate hash with openssl, if it's failed skip test, > + # unless it's negative test, then pass to evmctl > + cmd="openssl dgst $OPENSSL_ENGINE -$alg $file" > + echo - $cmd > + hash=$(set -o pipefail; $cmd 2>/dev/null | cut -d' ' -f2) > + if [ $? -ne 0 ] && _is_expect_pass; then > + echo $CYAN"$alg test is skipped"$NORM > + rm $file > + return $SKIP > + fi > + if [ "$chash" ] && [ "$chash" != "$hash" ]; then > + red_always Only when "ima_hash.test" is invoked directly, the output is colored red. Really confusing. > + echo "Invalid hash for $alg from openssl" > + echo "Expected: $chash" > + echo "Returned: $hash" > + color_restore > + rm $file > + return $HARDFAIL > + fi > + > + ADD_TEXT_FOR=$alg ADD_DEL=$file \ > + _evmctl_run ima_hash --hashalgo $alg --xattr-user $file || return > + ADD_TEXT_FOR=$alg \ > + _test_xattr $file user.ima $pref$hash || return > + rm $file > + return $OK > +} > + > +# check args: algo hdr-prefix canonic-hash > +expect_pass check md4 0x01 31d6cfe0d16ae931b73c59d7e0c089c0 > +expect_pass check md5 0x01 d41d8cd98f00b204e9800998ecf8427e > +expect_pass check sha1 0x01 da39a3ee5e6b4b0d3255bfef95601890afd80709 > +expect_fail check SHA1 0x01 # uppercase > +expect_fail check sha512-224 0x01 # valid for pkcs1 > +expect_fail check sha512-256 0x01 # valid for pkcs1 > +expect_fail check unknown 0x01 # nonexistent > +expect_pass check sha224 0x0407 d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f > +expect_pass check sha256 0x0404 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 > +expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114be07434c0cc7bf63f6e1da274edebfe76f65fbd51ad2f14898b95b > +expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e > +expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31 > +expect_fail check sm3 0x01 > +expect_fail check sm3-256 0x01 > +_enable_gost_engine > +expect_pass check md_gost12_256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb > +expect_pass check streebog256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb > +expect_pass check md_gost12_512 0x0413 8e945da209aa869f0455928529bcae4679e9873ab707b55315f56ceb98bef0a7362f715528356ee83cda5f2aac4c6ad2ba3a715c1bcd81cb8e9f90bf4c1c1a8a > +expect_pass check streebog512 0x0413 8e945da209aa869f0455928529bcae4679e9873ab707b55315f56ceb98bef0a7362f715528356ee83cda5f2aac4c6ad2ba3a715c1bcd81cb8e9f90bf4c1c1a8a > + Nice! The code is very concisely written. Reviewing this patch would be a lot easier, if it was broken up into smaller pieces. For example, and this is only an example, the initial patch could define the base ima_hash.test, a subsequent patch could add coloring for the base ima_hash.test, another patch could introduce "make check" and add its coloring. There's all sorts of ways to break up this patch to simplify review. Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl 2020-03-31 14:25 ` Mimi Zohar @ 2020-03-31 15:14 ` Vitaly Chikunov 2020-03-31 16:04 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Vitaly Chikunov @ 2020-03-31 15:14 UTC (permalink / raw) To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity Mimi, On Tue, Mar 31, 2020 at 10:25:24AM -0400, Mimi Zohar wrote: > > +# For hard errors > > +red_always() { > > + echo $@ $RED > > A few functions - "red_always", "red_if_failure", "color_restore" - > use "$@", but none of the function callers pass any parameters. Is > there a reason for the "$@" or just something left over from > debugging? It was to pass `-n` I think, but it is never needed in the end. > > + if [ "$chash" ] && [ "$chash" != "$hash" ]; then > > + red_always > > Only when "ima_hash.test" is invoked directly, the output is colored > red. Really confusing. Non-TTY output is never colored to not clutter log files. But logic is to color the errors in red. So it thought as 'always red', _when_ there is colored output (TTY). And it's "always" in contract to "red if failure" - which make text red only when the test is expected to pass (thus, this is real error condition), when the test is expected to fail there is no point to color it red, because it's not real error (to not confuse user). Because it is unconditional (in that sense) is it named "red always". I can rename it to something like `color_red'. And rename `red_if_failure' to `color_red_on_failure'. > Nice! The code is very concisely written. > > Reviewing this patch would be a lot easier, if it was broken up into > smaller pieces. For example, and this is only an example, the initial > patch could define the base ima_hash.test, a subsequent patch could > add coloring for the base ima_hash.test, another patch could introduce > "make check" and add its coloring. There's all sorts of ways to break > up this patch to simplify review. This would make following commits to change code which is already committed in previous commits. This would make editing code extra hard. Especially, when tests was reworked a lot. Also, I don't think splitting coloring into separate patch improves review. Instead, we can just remember the rule that (real) errors are going to be printed in red. For example, if we prefix every error message with word "ERROR:" - why it would be easier to review if we split adding this prefix to every message in a separate commit? Red color, similarly to uppercase "ERROR", just improves visibility of errors. (Which is useful, because there is really a lot of tests). Thanks, > > Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 1/2] ima-evm-utils: Add some tests for evmctl 2020-03-31 15:14 ` Vitaly Chikunov @ 2020-03-31 16:04 ` Mimi Zohar 0 siblings, 0 replies; 15+ messages in thread From: Mimi Zohar @ 2020-03-31 16:04 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity On Tue, 2020-03-31 at 18:14 +0300, Vitaly Chikunov wrote: > Mimi, > > On Tue, Mar 31, 2020 at 10:25:24AM -0400, Mimi Zohar wrote: > > > +# For hard errors > > > +red_always() { > > > + echo $@ $RED > > > > A few functions - "red_always", "red_if_failure", "color_restore" - > > use "$@", but none of the function callers pass any parameters. Is > > there a reason for the "$@" or just something left over from > > debugging? > > It was to pass `-n` I think, but it is never needed in the end. > > > > + if [ "$chash" ] && [ "$chash" != "$hash" ]; then > > > + red_always > > > > Only when "ima_hash.test" is invoked directly, the output is colored > > red. Really confusing. > > Non-TTY output is never colored to not clutter log files. > But logic is to color the errors in red. > > So it thought as 'always red', _when_ there is colored output (TTY). > > And it's "always" in contract to "red if failure" - which make text > red only when the test is expected to pass (thus, this is real error > condition), when the test is expected to fail there is no point to > color it red, because it's not real error (to not confuse user). > > Because it is unconditional (in that sense) is it named "red always". > > I can rename it to something like `color_red'. And rename > `red_if_failure' to `color_red_on_failure'. Sure, and add a short comment - "Non-TTY output is never colored". > > > Nice! The code is very concisely written. > > > > Reviewing this patch would be a lot easier, if it was broken up into > > smaller pieces. For example, and this is only an example, the initial > > patch could define the base ima_hash.test, a subsequent patch could > > add coloring for the base ima_hash.test, another patch could introduce > > "make check" and add its coloring. There's all sorts of ways to break > > up this patch to simplify review. > > This would make following commits to change code which is already > committed in previous commits. Nothing is committed yet. I pushed it out in order for others to review and comment, not only your patches, but mine as well. > This would make editing code extra hard. > Especially, when tests was reworked a lot. > > Also, I don't think splitting coloring into separate patch improves > review. Instead, we can just remember the rule that (real) errors are > going to be printed in red. > > For example, if we prefix every error message with word "ERROR:" - why > it would be easier to review if we split adding this prefix to every > message in a separate commit? > > Red color, similarly to uppercase "ERROR", just improves visibility of > errors. (Which is useful, because there is really a lot of tests). Trust me, I understand breaking up patches to simplify review is a lot of work. It's one of the hardest things to teach newbies and explain to them why it is necessary and why a clean history is worthwhile, but you know how to do it. These were just suggestions, just as separating the two tests was just a suggestion. BTW, in case it got lost along the way, I really do appreciate your help. Thanks! Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl 2020-03-27 4:25 [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl Vitaly Chikunov 2020-03-27 4:25 ` [PATCH v8 1/2] " Vitaly Chikunov @ 2020-03-27 4:25 ` Vitaly Chikunov [not found] ` <98cfccc0-2191-6072-aebe-296e6e150e0c@linux.microsoft.com> 2020-04-01 18:00 ` Mimi Zohar [not found] ` <d39b433e-4504-d0a4-116f-dd33ca238f7f@linux.microsoft.com> 2 siblings, 2 replies; 15+ messages in thread From: Vitaly Chikunov @ 2020-03-27 4:25 UTC (permalink / raw) To: Mimi Zohar, Dmitry Kasatkin, linux-integrity This commit adds (evm) sign, (evm) verify, ima_sign, and ima_verify tests for different algos. Signed-off-by: Vitaly Chikunov <vt@altlinux.org> --- tests/Makefile.am | 7 +- tests/gen-keys.sh | 97 +++++++++++++ tests/sign_verify.test | 364 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 467 insertions(+), 1 deletion(-) create mode 100755 tests/gen-keys.sh create mode 100755 tests/sign_verify.test diff --git a/tests/Makefile.am b/tests/Makefile.am index e37b958..029f2ff 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,7 +1,12 @@ check_SCRIPTS = TESTS = $(check_SCRIPTS) -check_SCRIPTS += ima_hash.test +check_SCRIPTS += ima_hash.test sign_verify.test clean-local: -rm -f *.txt *.out *.sig *.sig2 + +distclean: distclean-keys +.PHONY: distclean-keys +distclean-keys: + ./gen-keys.sh clean diff --git a/tests/gen-keys.sh b/tests/gen-keys.sh new file mode 100755 index 0000000..53eb32f --- /dev/null +++ b/tests/gen-keys.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Generate keys for the tests +# +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> +# +# 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, 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. + +cd $(dirname $0) +PATH=../src:$PATH +type openssl + +log() { + echo - "$*" + eval "$@" +} + +if [ "$1" = clean ]; then + rm -f test-ca.conf +elif [ "$1" = force ] || [ ! -e test-ca.conf ]; then +cat > test-ca.conf <<- EOF + [ req ] + distinguished_name = req_distinguished_name + prompt = no + string_mask = utf8only + x509_extensions = v3_ca + + [ req_distinguished_name ] + O = IMA-CA + CN = IMA/EVM certificate signing key + emailAddress = ca@ima-ca + + [ v3_ca ] + basicConstraints=CA:TRUE + subjectKeyIdentifier=hash + authorityKeyIdentifier=keyid:always,issuer +EOF +fi + +# RSA +# Second key will be used for wrong key tests. +for m in 1024 2048; do + if [ "$1" = clean ] || [ "$1" = force ]; then + rm -f test-rsa$m.cer test-rsa$m.key test-rsa$m.pub + fi + if [ "$1" = clean ]; then + continue + fi + if [ ! -e test-rsa$m.key ]; then + log openssl req -verbose -new -nodes -utf8 -sha1 -days 10000 -batch -x509 \ + -config test-ca.conf \ + -newkey rsa:$m \ + -out test-rsa$m.cer -outform DER \ + -keyout test-rsa$m.key + # for v1 signatures + log openssl pkey -in test-rsa$m.key -out test-rsa$m.pub -pubout + fi +done + +# EC-RDSA +for m in \ + gost2012_256:A \ + gost2012_256:B \ + gost2012_256:C \ + gost2012_512:A \ + gost2012_512:B; do + IFS=':' read -r algo param <<< "$m" + if [ "$1" = clean ] || [ "$1" = force ]; then + rm -f test-$algo-$param.key test-$algo-$param.cer test-$algo-$param.pub + fi + if [ "$1" = clean ]; then + continue + fi + [ -e test-$algo-$param.key ] && continue + log openssl req -nodes -x509 -utf8 -days 10000 -batch \ + -config test-ca.conf \ + -newkey $algo \ + -pkeyopt paramset:$param \ + -out test-$algo-$param.cer -outform DER \ + -keyout test-$algo-$param.key + if [ -s test-$algo-$param.key ]; then + log openssl pkey -in test-$algo-$param.key -out test-$algo-$param.pub -pubout + fi +done + +# This script leaves test-ca.conf, *.cer, *.pub, *.key files for sing/verify tests. +# They are never deleted except by `make distclean'. + diff --git a/tests/sign_verify.test b/tests/sign_verify.test new file mode 100755 index 0000000..190b23a --- /dev/null +++ b/tests/sign_verify.test @@ -0,0 +1,364 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# evmctl {,ima_}{sign,verify} tests +# +# Copyright (C) 2019 Vitaly Chikunov <vt@altlinux.org> +# +# 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, 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. + +cd $(dirname $0) +PATH=../src:$PATH +source ./functions.sh +_require evmctl openssl xxd getfattr +./gen-keys.sh >/dev/null 2>&1 + +trap _report_exit EXIT +set -f # disable globbing + +# Determine keyid from a cert +_keyid() { + local cer=${1%.*}.cer cmd + local tmp + + cer=test-${cer#test-} + cmd="openssl x509 $OPENSSL_ENGINE \ + -in $cer -inform DER -pubkey -noout" + id=$($cmd 2>/dev/null \ + | openssl asn1parse \ + | grep BIT.STRING \ + | cut -d: -f1) + if [ -z "$id" ]; then + echo - $cmd >&2 + echo "Cannot asn1parse $cer to determine keyid" >&2 + exit 1 + fi + tmp=$(mktemp) + openssl x509 $OPENSSL_ENGINE \ + -in $cer -inform DER -pubkey -noout 2>/dev/null \ + | openssl asn1parse -strparse $id -out $tmp -noout + cat $tmp \ + | openssl dgst -c -sha1 \ + | cut -d' ' -f2 \ + | grep -o ":..:..:..:..$" \ + | tr -d : + rm -f $tmp +} + +# Convert test $type into evmctl op prefix +_op() { + if [ $1 = ima ]; then + echo ima_ + fi +} + +# Convert test $type into xattr name +_xattr() { + if [ $1 = ima ]; then + echo user.ima + else + echo user.evm + fi +} + +# Check that detached signature matches xattr signature +_test_sigfile() { + local file=$1 attr=$2 file_sig=$3 file_sig2=$4 + + if [ ! -e $file_sig ]; then + red_always + echo "evmctl ima_sign: no detached signature $file_sig" + color_restore + rm $file + return $FAIL + fi + + _extract_xattr $file $attr $file_sig2 + if ! cmp -bl $file_sig $file_sig2; then + red_always + echo "evmctl ima_sign: xattr signature on $file differ from detached $file_sig" + color_restore + rm $file $file_sig $file_sig2 + return $FAIL + fi + + rm $file_sig $file_sig2 +} + +# Run single sign command +_evmctl_sign() { + local type=$1 key=$2 alg=$3 file=$4 opts=$5 + + # Can check --sigfile for ima_sign + [ $type = ima ] && opts+=" --sigfile" + + ADD_TEXT_FOR="$alg ($key)" ADD_DEL=$file \ + _evmctl_run $(_op $type)sign $opts \ + --hashalgo $alg --key $key --xattr-user $file || return + + if [ $type = ima ]; then + _test_sigfile $file $(_xattr $type) $file.sig $file.sig2 + fi +} + +# Run and test {ima_,}sign operation +check_sign() { + # Arguments are passed via global vars: + # TYPE (ima or evm), + # KEY, + # ALG (hash algo), + # PREF (signature header prefix), + # OPTS (additional options for evmctl), + # FILE (working file to sign). + local "$@" + local KEY=${KEY%.*}.key + local FILE=${FILE:-$ALG.txt} + + # Normalize key filename + KEY=test-${KEY#test-} + + # Append suffix to files for negative tests, because we may + # leave only good files for verify tests. + _is_expect_fail && FILE+='~' + + rm -f $FILE + if ! touch $FILE; then + red_always + echo "Can't create test file: $FILE" + color_restore + return $HARDFAIL + fi + + if _is_expect_pass; then + # Can openssl work with this digest? + cmd="openssl dgst $OPENSSL_ENGINE -$ALG $FILE" + echo - $cmd + if ! $cmd >/dev/null; then + echo $CYAN"$ALG ($KEY) test is skipped (openssl is unable to digest)"$NORM + return $SKIP + fi + + if [ ! -e $KEY ]; then + echo $CYAN"$ALG ($KEY) test is skipped (key file not found)"$NORM + return $SKIP + fi + + # Can openssl sign with this digest and key? + cmd="openssl dgst $OPENSSL_ENGINE -$ALG -sign $KEY -hex $FILE" + echo - $cmd + if ! $cmd >/dev/null; then + echo $CYAN"$ALG ($KEY) test is skipped (openssl is unable to sign)"$NORM + return $SKIP + fi + fi + + # Fix keyid in the prefix. + if [[ $PREF =~ K ]]; then + keyid=$(_keyid $KEY) + if [ $? -ne 0 ]; then + red_always + echo "Unable to determine keyid for $KEY" + color_restore + return $HARDFAIL + fi + [[ "$VERBOSE" -gt 2 ]] && echo " Expected keyid: $keyid" + PREF=$(echo $PREF | sed "s/K/$keyid/") + fi + + # Perform signing by evmctl + _evmctl_sign $TYPE $KEY $ALG $FILE "$OPTS" || return + + # First simple pattern match the signature. + ADD_TEXT_FOR=$ALG \ + _test_xattr $FILE $(_xattr $TYPE) $PREF.* || return + + # This is all we can do for v1 signatures. + [[ "$OPTS" =~ --rsa ]] && return $OK + + # This is all we can do for evm. + [[ $TYPE =~ evm ]] && return $OK + + # Extract signature to a file + _extract_xattr $FILE $(_xattr $TYPE) $FILE.sig2 $PREF + + # Verify extracted signature with openssl + cmd="openssl dgst $OPENSSL_ENGINE -$ALG -verify ${KEY%.*}.pub \ + -signature $FILE.sig2 $FILE" + echo - $cmd + if ! $cmd; then + red_if_failure + echo "Signature v2 verification with openssl is failed." + color_restore + rm $FILE.sig2 + return $FAIL + fi + + rm $FILE.sig2 + return $OK +} + +# Test verify operation +check_verify() { + # Arguments are passed via global vars: + # TYPE (ima or evm), + # KEY, + # ALG (hash algo), + # OPTS (additional options for evmctl), + # FILE (filename to verify). + local "$@" + + if ! openssl dgst $OPENSSL_ENGINE -$ALG /dev/null >/dev/null 2>&1; then + echo $CYAN"$ALG ($KEY) test is skipped (openssl does not support $ALG)"$NORM + return $SKIP + fi + + ADD_TEXT_FOR="$FILE ($KEY)" \ + _evmctl_run $(_op $TYPE)verify --key $KEY --xattr-user $OPTS $FILE +} + +# Test runners + +# Perform sign and verify ima and evm testing +sign_verify() { + local key=$1 alg=$2 pref="$3" opts="$4" + local file=$alg.txt + + # Set defaults: + # Public key is different for v1 and v2 (where x509 cert is used). + if [[ $opts =~ --rsa ]]; then + KEY=test-$key.pub + else + KEY=test-$key.cer + fi + ALG=$alg + PREF=$pref + OPTS=$opts + FILE=$file + + TYPE=ima + if expect_pass check_sign; then + + # Normal verify with proper key should pass + expect_pass check_verify + + # Multiple files and some don't verify + expect_fail check_verify FILE=/dev/null,$file + fi + + TYPE=evm + # Avoid running blkid for evm tests which may require root + # No generation on overlayfs: + # ioctl(3, FS_IOC_GETVERSION, 0x7ffd8e0bd628) = -1 ENOTTY (Inappropriate ioctl for device) + OPTS="$opts --uuid --generation 0" + if expect_pass check_sign; then + + # Normal verify with proper key + expect_pass check_verify + + # Verify with wrong key + expect_fail check_verify KEY=rsa2048 + fi + + # Note: Leaving TYPE=evm and file is evm signed +} + +# Test --keys +try_different_keys() { + # This run after sign_verify which leaves + # TYPE=evm and file is evm signed + + # v2 signing can work with multiple keys in --key option + if [[ ! $OPTS =~ --rsa ]]; then + + # Have correct key in the key list + expect_pass check_verify KEY=test-rsa2048.cer,$KEY + expect_pass check_verify KEY=/dev/null,$KEY, + fi + + # Try key that is not used for signing + expect_fail check_verify KEY=rsa2048 + + # Try completely wrong key files + expect_fail check_verify KEY=/dev/null + expect_fail check_verify KEY=/dev/zero +} + +try_different_sigs() { + # TYPE=evm and file is evm signed + + # Test --imasig + if expect_pass check_sign OPTS="$OPTS --imasig"; then + + # Verify both evm and ima sigs + expect_pass check_verify + expect_pass check_verify TYPE=ima + fi + + # Test --imahash + if expect_pass check_sign OPTS="$OPTS --imahash"; then + + expect_pass check_verify + + # IMA hash is not verifiable by ima_verify + expect_fail check_verify TYPE=ima + fi + + # Test --portable + expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05 + # Cannot be verified + + # Test -i (immutable) + expect_pass check_sign OPTS="$OPTS -i" PREF=0x0303 + # Cannot be verified +} + +# Single test args: type key hash signature-prefix "evmctl-options" +# sign_verify args: key hash signature-prefix "evmctl-options" +# Only single test can be prefixed with expect_{fail,pass} +# `sign_verify' can not be prefixed with expect_{fail,pass} because +# it runs multiple tests inside. See more tests there. +# signature-prefix can contain `K' which will be resolved to keyid (v2 only) + +## Test v1 signatures +# Signature v1 only supports sha1 and sha256 so any other should fail +expect_fail \ + check_sign TYPE=ima KEY=rsa1024 ALG=md5 PREF=0x0301 OPTS=--rsa + +sign_verify rsa1024 sha1 0x0301 --rsa +sign_verify rsa1024 sha256 0x0301 --rsa + try_different_keys + try_different_sigs + +## Test v2 signatures with RSA PKCS#1 +# List of allowed hashes much greater but not all are supported. +sign_verify rsa1024 md5 0x030201K0080 +sign_verify rsa1024 sha1 0x030202K0080 +sign_verify rsa1024 sha224 0x030207K0080 +sign_verify rsa1024 sha256 0x030204K0080 + try_different_keys + try_different_sigs +sign_verify rsa1024 sha384 0x030205K0080 +sign_verify rsa1024 sha512 0x030206K0080 +sign_verify rsa1024 rmd160 0x030203K0080 + +# Test v2 signatures with EC-RDSA +_enable_gost_engine +sign_verify gost2012_256-A md_gost12_256 0x030212K0040 +sign_verify gost2012_256-B md_gost12_256 0x030212K0040 +sign_verify gost2012_256-C md_gost12_256 0x030212K0040 +sign_verify gost2012_512-A md_gost12_512 0x030213K0080 +sign_verify gost2012_512-B md_gost12_512 0x030213K0080 +# Test if signing with wrong key length does not work. +expect_fail \ + check_sign TYPE=ima KEY=gost2012_512-B ALG=md_gost12_256 PREF=0x0302 OPTS= +expect_fail \ + check_sign TYPE=ima KEY=gost2012_256-B ALG=md_gost12_512 PREF=0x0302 OPTS= + -- 2.11.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <98cfccc0-2191-6072-aebe-296e6e150e0c@linux.microsoft.com>]
* Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl [not found] ` <98cfccc0-2191-6072-aebe-296e6e150e0c@linux.microsoft.com> @ 2020-03-30 16:29 ` Lakshmi Ramasubramanian 2020-03-30 17:16 ` Vitaly Chikunov 0 siblings, 1 reply; 15+ messages in thread From: Lakshmi Ramasubramanian @ 2020-03-30 16:29 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, linux-integrity On 3/30/20 9:26 AM, Lakshmi Ramasubramanian wrote: > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: > >> --- /dev/null >> +++ b/tests/gen-keys.sh >> @@ -0,0 +1,97 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-2.0 > Should this be "# SPDX-License-Identifier: GPL-2.0+" > >> +# This script leaves test-ca.conf, *.cer, *.pub, *.key files for >> sing/verify tests. >> +# They are never deleted except by `make distclean'. >> + >> diff --git a/tests/sign_verify.test b/tests/sign_verify.test >> new file mode 100755 >> index 0000000..190b23a >> --- /dev/null >> +++ b/tests/sign_verify.test >> @@ -0,0 +1,364 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-2.0 > Same comment as above. > >> +# Check that detached signature matches xattr signature >> +_test_sigfile() { >> + local file=$1 attr=$2 file_sig=$3 file_sig2=$4 >> + >> + if [ ! -e $file_sig ]; then >> + red_always >> + echo "evmctl ima_sign: no detached signature $file_sig" >> + color_restore >> + rm $file > > Should the file be not deleted in case of failure, so that one can check > the file manually? > >> + return $FAIL >> + fi >> + >> + _extract_xattr $file $attr $file_sig2 >> + if ! cmp -bl $file_sig $file_sig2; then >> + red_always >> + echo "evmctl ima_sign: xattr signature on $file differ from >> detached $file_sig" >> + color_restore >> + rm $file $file_sig $file_sig2 > Same as above - should the files be not deleted in case of failure? > > thanks, > -lakshmi + Mimi and linux-integrity ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl 2020-03-30 16:29 ` Lakshmi Ramasubramanian @ 2020-03-30 17:16 ` Vitaly Chikunov 0 siblings, 0 replies; 15+ messages in thread From: Vitaly Chikunov @ 2020-03-30 17:16 UTC (permalink / raw) To: Lakshmi Ramasubramanian; +Cc: Mimi Zohar, linux-integrity Lakshmi, On Mon, Mar 30, 2020 at 09:29:54AM -0700, Lakshmi Ramasubramanian wrote: > On 3/30/20 9:26 AM, Lakshmi Ramasubramanian wrote: > > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: > > > > > --- /dev/null > > > +++ b/tests/gen-keys.sh > > > @@ -0,0 +1,97 @@ > > > +#!/bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > Should this be "# SPDX-License-Identifier: GPL-2.0+" > > > > > +# This script leaves test-ca.conf, *.cer, *.pub, *.key files for > > > sing/verify tests. > > > +# They are never deleted except by `make distclean'. > > > + > > > diff --git a/tests/sign_verify.test b/tests/sign_verify.test > > > new file mode 100755 > > > index 0000000..190b23a > > > --- /dev/null > > > +++ b/tests/sign_verify.test > > > @@ -0,0 +1,364 @@ > > > +#!/bin/bash > > > +# SPDX-License-Identifier: GPL-2.0 > > Same comment as above. > > > > > +# Check that detached signature matches xattr signature > > > +_test_sigfile() { > > > + local file=$1 attr=$2 file_sig=$3 file_sig2=$4 > > > + > > > + if [ ! -e $file_sig ]; then > > > + red_always > > > + echo "evmctl ima_sign: no detached signature $file_sig" > > > + color_restore > > > + rm $file > > > > Should the file be not deleted in case of failure, so that one can check > > the file manually? > > > > > + return $FAIL > > > + fi > > > + > > > + _extract_xattr $file $attr $file_sig2 > > > + if ! cmp -bl $file_sig $file_sig2; then > > > + red_always > > > + echo "evmctl ima_sign: xattr signature on $file differ from > > > detached $file_sig" > > > + color_restore > > > + rm $file $file_sig $file_sig2 > > Same as above - should the files be not deleted in case of failure? These files will be overwritten by subsequent tests anyway, so there is not point to keep them. Thanks, > > > > thanks, > > -lakshmi > > + Mimi and linux-integrity ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl 2020-03-27 4:25 ` [PATCH v8 2/2] ima-evm-utils: Add sign/verify " Vitaly Chikunov [not found] ` <98cfccc0-2191-6072-aebe-296e6e150e0c@linux.microsoft.com> @ 2020-04-01 18:00 ` Mimi Zohar 2020-04-02 7:19 ` Vitaly Chikunov 1 sibling, 1 reply; 15+ messages in thread From: Mimi Zohar @ 2020-04-01 18:00 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Fri, 2020-03-27 at 07:25 +0300, Vitaly Chikunov wrote: <snip> > +# Perform sign and verify ima and evm testing > +sign_verify() { > + local key=$1 alg=$2 pref="$3" opts="$4" > + local file=$alg.txt > + > + # Set defaults: > + # Public key is different for v1 and v2 (where x509 cert is used). > + if [[ $opts =~ --rsa ]]; then > + KEY=test-$key.pub > + else > + KEY=test-$key.cer > + fi > + ALG=$alg > + PREF=$pref > + OPTS=$opts > + FILE=$file > + > + TYPE=ima > + if expect_pass check_sign; then > + > + # Normal verify with proper key should pass > + expect_pass check_verify > + > + # Multiple files and some don't verify > + expect_fail check_verify FILE=/dev/null,$file The comment and the code don't seem to be in sync. This seems to be a single file, for example, named "/dev/null,sha1.txt", which properly fails. ____ START positive test: check_verify + evmctl -vvv ima_verify --key test-rsa1024.pub --xattr-user --rsa sha1.txt hash-v1: da39a3ee5e6b4b0d3255bfef95601890afd80709 sighash: e34807780b93cc17bdac89573df40ee4a3e984f1 sha1.txt: verification is OK ^^^^ STOP (0) positive test: check_verify ____ START negative test: check_verify FILE=/dev/null,sha1.txt - evmctl -vvv ima_verify --key test-rsa1024.pub --xattr-user --rsa /dev/null,sha1.txt evmctl ima_verify failed properly with (1) getxattr failed: /dev/null,sha1.txt errno: No such file or directory (2) ^^^^ STOP (1) negative test: check_verify FILE=/dev/null,sha1.txt > + fi > + > + TYPE=evm > + # Avoid running blkid for evm tests which may require root > + # No generation on overlayfs: > + # ioctl(3, FS_IOC_GETVERSION, 0x7ffd8e0bd628) = -1 ENOTTY (Inappropriate ioctl for device) > + OPTS="$opts --uuid --generation 0" > + if expect_pass check_sign; then > + > + # Normal verify with proper key > + expect_pass check_verify > + > + # Verify with wrong key > + expect_fail check_verify KEY=rsa2048 > + fi > + > + # Note: Leaving TYPE=evm and file is evm signed > +} > + > +# Test --keys > +try_different_keys() { > + # This run after sign_verify which leaves > + # TYPE=evm and file is evm signed > + > + # v2 signing can work with multiple keys in --key option > + if [[ ! $OPTS =~ --rsa ]]; then > + > + # Have correct key in the key list > + expect_pass check_verify KEY=test-rsa2048.cer,$KEY > + expect_pass check_verify KEY=/dev/null,$KEY, First test has multiple keys in the key list. The key list with "/dev/null" obviously fails to add the first key, so it lands up being a single key on the list. > + fi > + > + # Try key that is not used for signing > + expect_fail check_verify KEY=rsa2048 > + > + # Try completely wrong key files > + expect_fail check_verify KEY=/dev/null > + expect_fail check_verify KEY=/dev/zero > +} > + > +try_different_sigs() { > + # TYPE=evm and file is evm signed > + > + # Test --imasig > + if expect_pass check_sign OPTS="$OPTS --imasig"; then > + > + # Verify both evm and ima sigs > + expect_pass check_verify > + expect_pass check_verify TYPE=ima > + fi > + > + # Test --imahash > + if expect_pass check_sign OPTS="$OPTS --imahash"; then > + > + expect_pass check_verify > + > + # IMA hash is not verifiable by ima_verify > + expect_fail check_verify TYPE=ima > + fi > + > + # Test --portable > + expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05 > + # Cannot be verified True, evmctl does not support verifying portable signatures, but it should be possible not only locally, but remotely to verify a portable signature. That's the whole point of having portable EVM signatures. The comment is a bit misleading and could say something to that effect - "todo: add support for evmctl portable signature verification". Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl 2020-04-01 18:00 ` Mimi Zohar @ 2020-04-02 7:19 ` Vitaly Chikunov 2020-04-02 11:14 ` Mimi Zohar 0 siblings, 1 reply; 15+ messages in thread From: Vitaly Chikunov @ 2020-04-02 7:19 UTC (permalink / raw) To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity Mimi, On Wed, Apr 01, 2020 at 02:00:55PM -0400, Mimi Zohar wrote: > On Fri, 2020-03-27 at 07:25 +0300, Vitaly Chikunov wrote: > > <snip> > > + # Multiple files and some don't verify > > + expect_fail check_verify FILE=/dev/null,$file > > The comment and the code don't seem to be in sync. This seems to be a > single file, for example, named "/dev/null,sha1.txt", which properly > fails. Looks like my mistake. There was code that parse multiple files separated by comma. And it seems that there I thought this logic should be applicable here. Of course this should be space separated file list. > > +# Test --keys > > +try_different_keys() { > > + # This run after sign_verify which leaves > > + # TYPE=evm and file is evm signed > > + > > + # v2 signing can work with multiple keys in --key option > > + if [[ ! $OPTS =~ --rsa ]]; then > > + > > + # Have correct key in the key list > > + expect_pass check_verify KEY=test-rsa2048.cer,$KEY > > + expect_pass check_verify KEY=/dev/null,$KEY, > > First test has multiple keys in the key list. The key list with > "/dev/null" obviously fails to add the first key, so it lands up being > a single key on the list. All tests do obvious things. So I don't see a problem in this test. (There comma separated list is correct.) > > + > > + # Test --portable > > + expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05 > > + # Cannot be verified > > True, evmctl does not support verifying portable signatures, but it > should be possible not only locally, but remotely to verify a portable > signature. That's the whole point of having portable EVM signatures. > The comment is a bit misleading and could say something to that > effect - "todo: add support for evmctl portable signature > verification". Well, tests are not right place to note todos for other code. This todo would look like we need to add test case to the test, like test is missing something. While now it says that it impossible to test. I will change text to something like "Cannot be verified for now, until that support is added to evmctl". Thanks, > > Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 2/2] ima-evm-utils: Add sign/verify tests for evmctl 2020-04-02 7:19 ` Vitaly Chikunov @ 2020-04-02 11:14 ` Mimi Zohar 0 siblings, 0 replies; 15+ messages in thread From: Mimi Zohar @ 2020-04-02 11:14 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity On Thu, 2020-04-02 at 10:19 +0300, Vitaly Chikunov wrote: > > > + > > > + # Test --portable > > > + expect_pass check_sign OPTS="$OPTS --portable" PREF=0x05 > > > + # Cannot be verified > > > > True, evmctl does not support verifying portable signatures, but it > > should be possible not only locally, but remotely to verify a portable > > signature. That's the whole point of having portable EVM signatures. > > The comment is a bit misleading and could say something to that > > effect - "todo: add support for evmctl portable signature > > verification". > > Well, tests are not right place to note todos for other code. > This todo would look like we need to add test case to the test, like > test is missing something. While now it says that it impossible to test. > > I will change text to something like "Cannot be verified for now, until > that support is added to evmctl". Thanks. While you're updating the "sign_verify.test", could you expand this comment a bit? Referencing an example would help. # Fix keyid in the prefix. if [[ $PREF =~ K ]]; then keyid=$(_keyid $KEY) thanks, Mimi ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <d39b433e-4504-d0a4-116f-dd33ca238f7f@linux.microsoft.com>]
* Re: [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl [not found] ` <d39b433e-4504-d0a4-116f-dd33ca238f7f@linux.microsoft.com> @ 2020-03-30 16:28 ` Lakshmi Ramasubramanian 2020-03-30 17:47 ` James Bottomley 0 siblings, 1 reply; 15+ messages in thread From: Lakshmi Ramasubramanian @ 2020-03-30 16:28 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, linux-integrity On 3/30/20 9:16 AM, Lakshmi Ramasubramanian wrote: > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: >> This series adds simple evmctl tests for hash, sign, and verify >> operations. > > Could you please add the steps to initialize, configure, and run the > tests in cover letter? > > thanks, > -lakshmi Sorry - forgot to reply-all. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl 2020-03-30 16:28 ` [PATCH v8 0/2] ima-evm-utils: Add some " Lakshmi Ramasubramanian @ 2020-03-30 17:47 ` James Bottomley 0 siblings, 0 replies; 15+ messages in thread From: James Bottomley @ 2020-03-30 17:47 UTC (permalink / raw) To: Lakshmi Ramasubramanian, Vitaly Chikunov, Mimi Zohar, linux-integrity On Mon, 2020-03-30 at 09:28 -0700, Lakshmi Ramasubramanian wrote: > On 3/30/20 9:16 AM, Lakshmi Ramasubramanian wrote: > > > On 3/26/20 9:25 PM, Vitaly Chikunov wrote: > > > This series adds simple evmctl tests for hash, sign, and verify > > > operations. > > > > Could you please add the steps to initialize, configure, and run > > the tests in cover letter? > > > > thanks, > > -lakshmi > > Sorry - forgot to reply-all. It looks like it's the standard autoreconf -fiv; ./configure; make; make check? That's basically the universal autoconf way of running tests ... I'm not really sure documenting how to do this is a necessary requirement for the series. James ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-04-02 11:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 4:25 [PATCH v8 0/2] ima-evm-utils: Add some tests for evmctl Vitaly Chikunov
2020-03-27 4:25 ` [PATCH v8 1/2] " Vitaly Chikunov
[not found] ` <f9b36972-df5d-db9a-d840-52e9ff76d271@linux.microsoft.com>
2020-03-30 16:29 ` Lakshmi Ramasubramanian
2020-03-30 17:11 ` Vitaly Chikunov
2020-03-31 14:25 ` Mimi Zohar
2020-03-31 15:14 ` Vitaly Chikunov
2020-03-31 16:04 ` Mimi Zohar
2020-03-27 4:25 ` [PATCH v8 2/2] ima-evm-utils: Add sign/verify " Vitaly Chikunov
[not found] ` <98cfccc0-2191-6072-aebe-296e6e150e0c@linux.microsoft.com>
2020-03-30 16:29 ` Lakshmi Ramasubramanian
2020-03-30 17:16 ` Vitaly Chikunov
2020-04-01 18:00 ` Mimi Zohar
2020-04-02 7:19 ` Vitaly Chikunov
2020-04-02 11:14 ` Mimi Zohar
[not found] ` <d39b433e-4504-d0a4-116f-dd33ca238f7f@linux.microsoft.com>
2020-03-30 16:28 ` [PATCH v8 0/2] ima-evm-utils: Add some " Lakshmi Ramasubramanian
2020-03-30 17:47 ` James Bottomley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).