| Message ID | 20210106094335.3178261-3-patrick@puiterwijk.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima-evm-utils: Fix use of sign_hash via API | expand |
Hi Patrick, On Wed, 2021-01-06 at 10:43 +0100, Patrick Uiterwijk wrote: > This adds a test with a small program that calls the sign_hash API, to > ensure that that codepath works. > > Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org> Thank you for the regression test. Other than the couple of comments below, Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> > --- > diff --git a/tests/Makefile.am b/tests/Makefile.am > index ff928e1..74f6125 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -10,3 +10,8 @@ distclean: distclean-keys > .PHONY: distclean-keys > distclean-keys: > ./gen-keys.sh clean > + > +AUTOMAKE_OPTIONS = subdir-objects > +bin_PROGRAMS = sign_verify_apitest > +sign_verify_apitest_SOURCES = sign_verify.apitest.c ../src/utils.c > +sign_verify_apitest_LDFLAGS = -limaevm -L../src/.libs > diff --git a/tests/sign_verify.apitest.c b/tests/sign_verify.apitest.c > new file mode 100644 > index 0000000..20e2160 > --- /dev/null > +++ b/tests/sign_verify.apitest.c > @@ -0,0 +1,55 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * sign_verify.apitest: Test program for verifying sign_hash > + * > + * Copyright (C) 2020 Patrick Uiterwijk <patrick@puiterwijk.org> > + * Copyright (C) 2013,2014 Samsung Electronics > + * Copyright (C) 2011,2012,2013 Intel Corporation > + * Copyright (C) 2011 Nokia Corporation > + * Copyright (C) 2010 Cyril Hrubis <chrubis@suse.cz> > + */ Looking at the history, Dmitry Kasatkin must have been involved. From which software package is this from? > + > +#include <assert.h> > +#include <stdio.h> > +#include <string.h> > +#include <sys/types.h> > +#include <attr/xattr.h> This include fails on a couple of distros (e.g. alpine, tumbleweed, and RHEL 8.3). src/evmctl.c is using sys/xattr.h. Mimi > + > +#include "../src/imaevm.h" > +#include "../src/utils.h"
On Thu, Jan 07, 2021 at 07:25:03AM -0500, Mimi Zohar wrote: > > diff --git a/tests/sign_verify.apitest.c b/tests/sign_verify.apitest.c > > new file mode 100644 > > index 0000000..20e2160 > > --- /dev/null > > +++ b/tests/sign_verify.apitest.c > > @@ -0,0 +1,55 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * sign_verify.apitest: Test program for verifying sign_hash > > + * > > + * Copyright (C) 2020 Patrick Uiterwijk <patrick@puiterwijk.org> > > + * Copyright (C) 2013,2014 Samsung Electronics > > + * Copyright (C) 2011,2012,2013 Intel Corporation > > + * Copyright (C) 2011 Nokia Corporation > > + * Copyright (C) 2010 Cyril Hrubis <chrubis@suse.cz> > > + */ Woah, so much copyright for a simple sign_hash() call? > > Looking at the history, Dmitry Kasatkin must have been involved. From > which software package is this from?
On Thu, Jan 7, 2021 at 1:54 PM Vitaly Chikunov <vt@altlinux.org> wrote: > > On Thu, Jan 07, 2021 at 07:25:03AM -0500, Mimi Zohar wrote: > > > diff --git a/tests/sign_verify.apitest.c b/tests/sign_verify.apitest.c > > > new file mode 100644 > > > index 0000000..20e2160 > > > --- /dev/null > > > +++ b/tests/sign_verify.apitest.c > > > @@ -0,0 +1,55 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * sign_verify.apitest: Test program for verifying sign_hash > > > + * > > > + * Copyright (C) 2020 Patrick Uiterwijk <patrick@puiterwijk.org> > > > + * Copyright (C) 2013,2014 Samsung Electronics > > > + * Copyright (C) 2011,2012,2013 Intel Corporation > > > + * Copyright (C) 2011 Nokia Corporation > > > + * Copyright (C) 2010 Cyril Hrubis <chrubis@suse.cz> > > > + */ > > Woah, so much copyright for a simple sign_hash() call? I basically copied the copyright statements from evmctl.c (and put them in the reversed order as per the new syntax as Mimi did in utils.c as a fix to my previous patchset). This was because the "int main" function started out as a copy of "sign_ima" from evmctl.c. The Samsung, Intel and Nokia entries are all from Dmitry Kasatkin. Cyril's entry is there because I initially copied hex2bin from utils.c before just linking against it, I'll remove that one in a next version. > > > > > Looking at the history, Dmitry Kasatkin must have been involved. From > > which software package is this from?
diff --git a/src/evmctl.c b/src/evmctl.c index 1815f55..bb51688 100644 --- a/src/evmctl.c +++ b/src/evmctl.c @@ -165,29 +165,6 @@ struct tpm_bank_info { static char *pcrfile[MAX_PCRFILE]; static unsigned npcrfile; -static int bin2file(const char *file, const char *ext, const unsigned char *data, int len) -{ - FILE *fp; - char name[strlen(file) + (ext ? strlen(ext) : 0) + 2]; - int err; - - if (ext) - sprintf(name, "%s.%s", file, ext); - else - sprintf(name, "%s", file); - - log_info("Writing to %s\n", name); - - fp = fopen(name, "w"); - if (!fp) { - log_err("Failed to open: %s\n", name); - return -1; - } - err = fwrite(data, len, 1, fp); - fclose(fp); - return err; -} - static unsigned char *file2bin(const char *file, const char *ext, int *size) { FILE *fp; diff --git a/src/utils.c b/src/utils.c index fbb6a4b..6b99e78 100644 --- a/src/utils.c +++ b/src/utils.c @@ -112,3 +112,23 @@ int hex2bin(void *dst, const char *src, size_t count) } return 0; } + +int bin2file(const char *file, const char *ext, const unsigned char *data, int len) +{ + FILE *fp; + char name[strlen(file) + (ext ? strlen(ext) : 0) + 2]; + int err; + + if (ext) + sprintf(name, "%s.%s", file, ext); + else + sprintf(name, "%s", file); + + fp = fopen(name, "w"); + if (!fp) { + return -1; + } + err = fwrite(data, len, 1, fp); + fclose(fp); + return err; +} diff --git a/src/utils.h b/src/utils.h index 9ea179f..081997a 100644 --- a/src/utils.h +++ b/src/utils.h @@ -4,3 +4,4 @@ int get_cmd_path(const char *prog_name, char *buf, size_t buf_len); int hex_to_bin(char ch); int hex2bin(void *dst, const char *src, size_t count); +int bin2file(const char *file, const char *ext, const unsigned char *data, int len); diff --git a/tests/.gitignore b/tests/.gitignore index 9ecc984..c40735d 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -14,3 +14,5 @@ *.key *.conf +# Compiled version of apitest +sign_verify_apitest diff --git a/tests/Makefile.am b/tests/Makefile.am index ff928e1..74f6125 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -10,3 +10,8 @@ distclean: distclean-keys .PHONY: distclean-keys distclean-keys: ./gen-keys.sh clean + +AUTOMAKE_OPTIONS = subdir-objects +bin_PROGRAMS = sign_verify_apitest +sign_verify_apitest_SOURCES = sign_verify.apitest.c ../src/utils.c +sign_verify_apitest_LDFLAGS = -limaevm -L../src/.libs diff --git a/tests/sign_verify.apitest.c b/tests/sign_verify.apitest.c new file mode 100644 index 0000000..20e2160 --- /dev/null +++ b/tests/sign_verify.apitest.c @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * sign_verify.apitest: Test program for verifying sign_hash + * + * Copyright (C) 2020 Patrick Uiterwijk <patrick@puiterwijk.org> + * Copyright (C) 2013,2014 Samsung Electronics + * Copyright (C) 2011,2012,2013 Intel Corporation + * Copyright (C) 2011 Nokia Corporation + * Copyright (C) 2010 Cyril Hrubis <chrubis@suse.cz> + */ + +#include <assert.h> +#include <stdio.h> +#include <string.h> +#include <sys/types.h> +#include <attr/xattr.h> + +#include "../src/imaevm.h" +#include "../src/utils.h" + +int main(int argc, char **argv) { + unsigned char hash[MAX_DIGEST_SIZE]; + unsigned char sig[MAX_SIGNATURE_SIZE]; + int len, err; + char *file = argv[1]; + char *key = argv[2]; + char *algo = argv[3]; + char *digest = argv[4]; + + len = strlen(digest) / 2; + if (hex2bin(hash, digest, len) != 0) { + fprintf(stderr, "Error during hex2bin\n"); + return 1; + } + + len = sign_hash(algo, hash, len, key, NULL, sig + 1); + if (len <= 1) { + fprintf(stderr, "Error signing\n"); + return 1; + } + + /* add header */ + len++; + sig[0] = EVM_IMA_XATTR_DIGSIG; + + bin2file(file, "sig", sig, len); + + err = lsetxattr(file, "user.ima", sig, len, 0); + if (err < 0) { + log_err("setxattr failed: %s\n", file); + return 1; + } + + return 0; +} diff --git a/tests/sign_verify.test b/tests/sign_verify.test index 288e133..e909d01 100755 --- a/tests/sign_verify.test +++ b/tests/sign_verify.test @@ -65,14 +65,14 @@ _keyid_from_cert() { # Convert test $type into evmctl op prefix _op() { - if [ "$1" = ima ]; then + if [ "$1" = ima -o "$1" = ima_api ]; then echo ima_ fi } # Convert test $type into xattr name _xattr() { - if [ "$1" = ima ]; then + if [ "$1" = ima -o "$1" = ima_api ]; then echo user.ima else echo user.evm @@ -112,11 +112,13 @@ _evmctl_sign() { [ "$type" = ima ] && opts+=" --sigfile" # shellcheck disable=SC2086 - ADD_TEXT_FOR="$alg ($key)" ADD_DEL=$file \ + [ "$type" = ima -o "$type" = evm ] && (ADD_TEXT_FOR="$alg ($key)" ADD_DEL=$file \ _evmctl_run "$(_op "$type")sign" $opts \ - --hashalgo "$alg" --key "$key" --xattr-user "$file" || return + --hashalgo "$alg" --key "$key" --xattr-user "$file" || return) + [ "$type" = ima_api ] && ADD_TEXT_FOR="$alg ($key)" ADD_DEL=$file \ + ./sign_verify_apitest "$file" "$key" "$alg" "$(openssl dgst $OPENSSL_ENGINE -$ALG -hex -r $FILE | awk '{print $1}')" - if [ "$type" = ima ]; then + if [ "$type" = ima -o "$type" = ima_api ]; then _test_sigfile "$file" "$(_xattr "$type")" "$file.sig" "$file.sig2" fi } @@ -124,12 +126,14 @@ _evmctl_sign() { # Run and test {ima_,}sign operation check_sign() { # Arguments are passed via global vars: - # TYPE (ima or evm), + # TYPE (ima, ima_api or evm), # KEY, # ALG (hash algo), # PREFIX (signature header prefix in hex), # OPTS (additional options for evmctl), # FILE (working file to sign). + [ "$TYPE" = ima_api ] && [[ "$OPTS" =~ --rsa ]] && return "$SKIP" + local "$@" local KEY=${KEY%.*}.key local FILE=${FILE:-$ALG.txt} @@ -267,6 +271,20 @@ sign_verify() { # Multiple files and some don't verify expect_fail check_verify FILE="/dev/null $file" + setfattr -x user.ima "$FILE" + rm "$FILE.sig" + fi + + TYPE=ima_api + if expect_pass check_sign; then + + # Normal verify with proper key should pass + expect_pass check_verify + expect_pass check_verify OPTS="--sigfile" + + # Multiple files and some don't verify + expect_fail check_verify FILE="/dev/null $file" + rm "$FILE.sig" fi
This adds a test with a small program that calls the sign_hash API, to ensure that that codepath works. Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org> --- src/evmctl.c | 23 ---------------- src/utils.c | 20 ++++++++++++++ src/utils.h | 1 + tests/.gitignore | 2 ++ tests/Makefile.am | 5 ++++ tests/sign_verify.apitest.c | 55 +++++++++++++++++++++++++++++++++++++ tests/sign_verify.test | 30 ++++++++++++++++---- 7 files changed, 107 insertions(+), 29 deletions(-) create mode 100644 tests/sign_verify.apitest.c