From b0ec6ee88e2efb2090c7610ca81caf830c8a7a92 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 27 Mar 2018 15:19:07 +0200 Subject: [PATCH 1/6] common: Add assert_skip() and assert_todo() --- common/test.c | 121 +++++++++++++++++++++++++++++++++++++++++++------- common/test.h | 20 +++++++++ 2 files changed, 126 insertions(+), 15 deletions(-) diff --git a/common/test.c b/common/test.c index e91770153..18cb50188 100644 --- a/common/test.c +++ b/common/test.c @@ -89,6 +89,34 @@ struct { jmp_buf jump; } gl = { NULL, NULL, 0, }; +static void +print_diagnostics (const char *filename, + int line, + const char *function, + char *output) +{ + const char *pos; + char *from; + char *next; + + for (from = output; from != NULL; ) { + next = strchr (from, '\n'); + if (next) { + next[0] = '\0'; + next += 1; + } + + printf ("# %s\n", from); + from = next; + } + + pos = strrchr (filename, '/'); + if (pos != NULL && pos[1] != '\0') + filename = pos + 1; + + printf ("# in %s() at %s:%d\n", function, filename, line); +} + void p11_test_fail (const char *filename, int line, @@ -96,10 +124,7 @@ p11_test_fail (const char *filename, const char *message, ...) { - const char *pos; char *output; - char *from; - char *next; va_list va; assert (gl.last != NULL); @@ -113,23 +138,89 @@ p11_test_fail (const char *filename, assert (0 && "vasprintf() failed"); va_end (va); - for (from = output; from != NULL; ) { - next = strchr (from, '\n'); - if (next) { - next[0] = '\0'; - next += 1; - } + print_diagnostics (filename, line, function, output); + free (output); - printf ("# %s\n", from); - from = next; + /* Let coverity know we're not supposed to return from here */ +#ifdef __COVERITY__ + abort(); +#endif + + longjmp (gl.jump, 1); +} + +void +p11_test_skip (const char *filename, + int line, + const char *function, + const char *message, + ...) +{ + char *output; + char *pos; + va_list va; + + assert (gl.last != NULL); + assert (gl.last->type == TEST); + gl.last->x.test.failed = 1; + + printf ("ok %d %s", gl.number, gl.last->x.test.name); + + va_start (va, message); + if (vasprintf (&output, message, va) < 0) + assert (0 && "vasprintf() failed"); + va_end (va); + + pos = strchr (output, '\n'); + if (pos) { + *pos = '\0'; + pos++; } + printf (" # SKIP %s\n", output); - pos = strrchr (filename, '/'); - if (pos != NULL && pos[1] != '\0') - filename = pos + 1; + if (pos) + print_diagnostics (filename, line, function, pos); + free (output); - printf ("# in %s() at %s:%d\n", function, filename, line); + /* Let coverity know we're not supposed to return from here */ +#ifdef __COVERITY__ + abort(); +#endif + + longjmp (gl.jump, 1); +} + +void +p11_test_todo (const char *filename, + int line, + const char *function, + const char *message, + ...) +{ + char *output; + char *pos; + va_list va; + + assert (gl.last != NULL); + assert (gl.last->type == TEST); + gl.last->x.test.failed = 1; + + printf ("not ok %d %s", gl.number, gl.last->x.test.name); + + va_start (va, message); + if (vasprintf (&output, message, va) < 0) + assert (0 && "vasprintf() failed"); + va_end (va); + + pos = strchr (output, '\n'); + if (pos) { + *pos = '\0'; + pos++; + } + printf (" # TODO %s\n", output); + if (pos) + print_diagnostics (filename, line, function, pos); free (output); /* Let coverity know we're not supposed to return from here */ diff --git a/common/test.h b/common/test.h index e28bb55d2..1c952b0da 100644 --- a/common/test.h +++ b/common/test.h @@ -63,6 +63,14 @@ do { const char *__s = (detail); \ p11_test_fail (__FILE__, __LINE__, __FUNCTION__, "%s%s%s", (msg), __s ? ": ": "", __s ? __s : ""); \ } while (0) +#define assert_skip(msg, detail) \ + do { const char *__s = (detail); \ + p11_test_skip (__FILE__, __LINE__, __FUNCTION__, "%s%s%s", (msg), __s ? ": ": "", __s ? __s : ""); \ + } while (0) +#define assert_todo(msg, detail) \ + do { const char *__s = (detail); \ + p11_test_todo (__FILE__, __LINE__, __FUNCTION__, "%s%s%s", (msg), __s ? ": ": "", __s ? __s : ""); \ + } while (0) #define assert_not_reached(msg) \ do { \ p11_test_fail (__FILE__, __LINE__, __FUNCTION__, "code should not be reached"); \ @@ -113,6 +121,18 @@ void p11_test_fail (const char *filename, const char *message, ...) GNUC_PRINTF(4, 5) CLANG_ANALYZER_NORETURN; +void p11_test_skip (const char *filename, + int line, + const char *function, + const char *message, + ...) GNUC_PRINTF(4, 5) CLANG_ANALYZER_NORETURN; + +void p11_test_todo (const char *filename, + int line, + const char *function, + const char *message, + ...) GNUC_PRINTF(4, 5) CLANG_ANALYZER_NORETURN; + void p11_test (void (* function) (void), const char *name, ...) GNUC_PRINTF(2, 3); From c558031eb8674d2390637f429a9b1204363b9508 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 27 Mar 2018 16:23:12 +0200 Subject: [PATCH 2/6] test: Take advantage of TAP test driver --- .gitignore | 1 + Makefile.am | 4 ++++ configure.ac | 1 + 3 files changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index db38c6810..21116e6f2 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,7 @@ Makefile Makefile.in missing stamp-h1 +tap-driver.sh temp.txt test-driver diff --git a/Makefile.am b/Makefile.am index b6d3fb8e3..2616be047 100644 --- a/Makefile.am +++ b/Makefile.am @@ -64,6 +64,10 @@ AM_TESTS_ENVIRONMENT = \ export abs_top_builddir; AM_TESTS_FD_REDIRECT = 9>&2; +LOG_DRIVER = env AM_TAP_AWK='$(AWK)' $(SHELL) \ + $(top_srcdir)/build/litter/tap-driver.sh +LOG_DRIVER_FLAGS = --comments --ignore-exit + MEMCHECK_ENV = $(TEST_RUNNER) valgrind --error-exitcode=80 --quiet LEAKCHECK_ENV = $(TEST_RUNNER) valgrind --error-exitcode=81 --quiet --leak-check=yes diff --git a/configure.ac b/configure.ac index bbb81e9f2..69d58ffb8 100644 --- a/configure.ac +++ b/configure.ac @@ -22,6 +22,7 @@ P11KIT_AGE=3 AC_CONFIG_HEADERS([config.h]) AC_CONFIG_MACRO_DIR([build/m4]) AC_CONFIG_AUX_DIR([build/litter]) +AC_REQUIRE_AUX_FILE([tap-driver.sh]) AM_INIT_AUTOMAKE([1.12 foreign subdir-objects]) AM_SANITY_CHECK AM_MAINTAINER_MODE([enable]) From 65d4feca499b3e987982baf80cb42fcedd7eb104 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 28 Mar 2018 07:50:30 +0200 Subject: [PATCH 3/6] test: Rewrite test-server.sh in TAP style --- p11-kit/test-server.sh | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/p11-kit/test-server.sh b/p11-kit/test-server.sh index f80c40975..e9455dc4d 100755 --- a/p11-kit/test-server.sh +++ b/p11-kit/test-server.sh @@ -4,7 +4,7 @@ testdir=$PWD/test-server-$$ test -d "$testdir" || mkdir "$testdir" cleanup () { - rm -rf "$testdir" + rm -rf "$testdir" } trap cleanup 0 @@ -16,24 +16,40 @@ unset P11_KIT_SERVER_PID XDG_RUNTIME_DIR="$testdir" export XDG_RUNTIME_DIR +echo 1..4 + "$abs_top_builddir"/p11-kit-server -s --provider "$abs_top_builddir"/.libs/mock-one.so pkcs11: > start.env 2> start.err -if test $? -ne 0; then - cat start.err - exit 1 +if test $? -eq 0; then + echo "ok 1 /server/start" +else + echo "not ok 1 /server/start" + sed 's/^/# /' start.err + exit 1 fi . ./start.env -test "${P11_KIT_SERVER_ADDRESS+set}" = "set" || exit 1 -test "${P11_KIT_SERVER_PID+set}" = "set" || exit 1 +if test "${P11_KIT_SERVER_ADDRESS+set}" = "set" -a "${P11_KIT_SERVER_PID+set}" = "set"; then + echo "ok 2 /server/start-env" +else + echo "not ok 2 /server/start-env" + exit 1 +fi "$abs_top_builddir"/p11-kit-server -s -k > stop.env 2> stop.err -if test $? -ne 0; then - cat stop.err - exit 1 +if test $? -eq 0; then + echo "ok 3 /server/stop" +else + echo "not ok 3 /server/stop" + sed 's/^/# /' stop.err + exit 1 fi . ./stop.env -test "${P11_KIT_SERVER_ADDRESS-unset}" = "unset" || exit 1 -test "${P11_KIT_SERVER_PID-unset}" = "unset" || exit 1 +if test "${P11_KIT_SERVER_ADDRESS-unset}" = "unset" -a "${P11_KIT_SERVER_PID-unset}" = "unset"; then + echo "ok 4 /server/stop-env" +else + echo "not ok 4 /server/stop-env" + exit 1 +fi From 497f5e52e35ffb13bf1206b015f36112f01a2d32 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 28 Mar 2018 07:49:29 +0200 Subject: [PATCH 4/6] test: Use _exit() in child process to immediately close open FDs --- p11-kit/test-proxy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p11-kit/test-proxy.c b/p11-kit/test-proxy.c index 54c6d4a47..c34415b05 100644 --- a/p11-kit/test-proxy.c +++ b/p11-kit/test-proxy.c @@ -171,7 +171,7 @@ test_initialize_child (void) rv = proxy->C_Finalize (NULL); assert_num_eq (rv, CKR_OK); - exit(0); + _exit (0); } assert (pid != -1); waitpid(pid, &st, 0); From b1533ec773da297a42648616e96b9f78255eb0fa Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 7 Mar 2018 15:40:20 +0100 Subject: [PATCH 5/6] test: Add test for error messages --- .travis.yml | 2 +- p11-kit/Makefile.am | 2 + p11-kit/test-messages.sh | 110 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100755 p11-kit/test-messages.sh diff --git a/.travis.yml b/.travis.yml index 51fab880f..d7d4cb353 100644 --- a/.travis.yml +++ b/.travis.yml @@ -51,4 +51,4 @@ after_failure: - docker exec $CONTAINER su - user sh -c "cd $BUILDDIR && cat test-suite.log" after_success: - - if test x"$COVERAGE" = xyes; then docker exec $CONTAINER pip install cpp-coveralls; docker exec -e TRAVIS_JOB_ID="$TRAVIS_JOB_ID" -e TRAVIS_BRANCH="$TRAVIS_BRANCH" $CONTAINER sh -c "cd $BUILDDIR && coveralls -b $BUILDDIR -E '(^|.*/)(frob|mock|test)-.*|(^|.*/)(virtual-fixed\.c|print-messages\.c)' --gcov-options '\-lp'"; fi + - if test x"$COVERAGE" = xyes; then docker exec $CONTAINER pip install cpp-coveralls; docker exec -e TRAVIS_JOB_ID="$TRAVIS_JOB_ID" -e TRAVIS_BRANCH="$TRAVIS_BRANCH" $CONTAINER sh -c "cd $BUILDDIR && coveralls -b $BUILDDIR -E '(^|.*/)(frob|mock|test)-.*|(^|.*/)(virtual-fixed\.c)' --gcov-options '\-lp'"; fi diff --git a/p11-kit/Makefile.am b/p11-kit/Makefile.am index f4e9069cb..e02a1eb68 100644 --- a/p11-kit/Makefile.am +++ b/p11-kit/Makefile.am @@ -293,6 +293,8 @@ check_PROGRAMS += \ print_messages_SOURCES = p11-kit/print-messages.c print_messages_LDADD = $(p11_kit_LIBS) +sh_tests += p11-kit/test-messages.sh + frob_setuid_SOURCES = p11-kit/frob-setuid.c frob_setuid_LDADD = $(p11_kit_LIBS) diff --git a/p11-kit/test-messages.sh b/p11-kit/test-messages.sh new file mode 100755 index 000000000..16c8544dd --- /dev/null +++ b/p11-kit/test-messages.sh @@ -0,0 +1,110 @@ +#!/bin/sh + +set -e + +testdir=$PWD/test-messages-$$ +test -d "$testdir" || mkdir "$testdir" + +cleanup () { + rm -rf "$testdir" +} +trap cleanup 0 + +cd "$testdir" + +cat > messages.exp < messages.out + +echo 1..1 + +: ${DIFF=diff} +if ${DIFF} messages.exp messages.out > messages.diff; then + echo "ok 1 /messages/return-code" +else + echo "not ok 1 /messages/return-code" + sed 's/^/# /' messages.diff + exit 1 +fi From 8ef701eb400713d13a4b0e17d64318f775d5f09b Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 27 Mar 2018 15:41:51 +0200 Subject: [PATCH 6/6] test: Add failing test for CKR_CRYPTOKI_ALREADY_INITIALIZED --- p11-kit/Makefile.am | 7 +- p11-kit/fixtures/system-modules/seven.module | 4 + p11-kit/mock-module-ep5.c | 80 ++++++++++++++++++++ p11-kit/test-modules.c | 25 ++++++ 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 p11-kit/fixtures/system-modules/seven.module create mode 100644 p11-kit/mock-module-ep5.c diff --git a/p11-kit/Makefile.am b/p11-kit/Makefile.am index e02a1eb68..fb08516d3 100644 --- a/p11-kit/Makefile.am +++ b/p11-kit/Makefile.am @@ -326,7 +326,8 @@ check_LTLIBRARIES += \ mock-two.la \ mock-three.la \ mock-four.la \ - mock-five.la + mock-five.la \ + mock-seven.la mock_one_la_SOURCES = p11-kit/mock-module-ep.c mock_one_la_LIBADD = libp11-test.la libp11-common.la @@ -357,6 +358,10 @@ mock_six_la_LDFLAGS = $(mock_one_la_LDFLAGS) mock_six_la_LIBADD = $(mock_one_la_LIBADD) endif +mock_seven_la_SOURCES = p11-kit/mock-module-ep5.c +mock_seven_la_LDFLAGS = $(mock_one_la_LDFLAGS) +mock_seven_la_LIBADD = $(mock_one_la_LIBADD) + EXTRA_DIST += \ p11-kit/fixtures \ p11-kit/test-mock.c \ diff --git a/p11-kit/fixtures/system-modules/seven.module b/p11-kit/fixtures/system-modules/seven.module new file mode 100644 index 000000000..933a95644 --- /dev/null +++ b/p11-kit/fixtures/system-modules/seven.module @@ -0,0 +1,4 @@ + +module: mock-seven.so +critical: yes +enable-in: test-modules diff --git a/p11-kit/mock-module-ep5.c b/p11-kit/mock-module-ep5.c new file mode 100644 index 000000000..ae8ddcc7b --- /dev/null +++ b/p11-kit/mock-module-ep5.c @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2012 Stefan Walter + * Copyright (c) 2018 Red Hat, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * * Redistributions in binary form must reproduce the + * above copyright notice, this list of conditions and + * the following disclaimer in the documentation and/or + * other materials provided with the distribution. + * * The names of contributors to this software may not be + * used to endorse or promote products derived from this + * software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + * + * Author: Stef Walter , Daiki Ueno + */ + +#include "config.h" + +#define CRYPTOKI_EXPORTS 1 +#include "pkcs11.h" + +#include "mock.h" +#include "test.h" + +static bool initialized = false; + +static CK_RV +override_initialize (CK_VOID_PTR init_args) +{ + CK_RV rv; + + if (initialized) + return CKR_CRYPTOKI_ALREADY_INITIALIZED; + rv = mock_C_Initialize (init_args); + if (rv == CKR_OK) + initialized = true; + return rv; +} + +static CK_RV +override_finalize (CK_VOID_PTR reserved) +{ + initialized = false; + return mock_C_Finalize (reserved); +} + +#ifdef OS_WIN32 +__declspec(dllexport) +#endif +CK_RV +C_GetFunctionList (CK_FUNCTION_LIST_PTR_PTR list) +{ + mock_module_init (); + mock_module.C_GetFunctionList = C_GetFunctionList; + if (list == NULL) + return CKR_ARGUMENTS_BAD; + mock_module.C_Initialize = override_initialize; + mock_module.C_Finalize = override_finalize; + *list = &mock_module; + return CKR_OK; +} diff --git a/p11-kit/test-modules.c b/p11-kit/test-modules.c index a2e1430e7..31cbcfa0b 100644 --- a/p11-kit/test-modules.c +++ b/p11-kit/test-modules.c @@ -462,6 +462,30 @@ test_config_option (void) finalize_and_free_modules (modules); } +static void +test_already_initialized (void) +{ + CK_FUNCTION_LIST_PTR_PTR modules; + CK_RV rv; + + /* This enables module seven */ + p11_kit_set_progname ("test-modules"); + + modules = initialize_and_get_modules (); + assert (lookup_module_with_name (modules, "seven") != NULL); + + rv = p11_kit_modules_initialize (modules, NULL); + if (rv != CKR_OK) { + finalize_and_free_modules (modules); + assert_todo ("not implemented", "CKR_CRYPTOKI_ALREADY_INITIALIZED handling"); + } + if (!lookup_module_with_name (modules, "seven")) { + finalize_and_free_modules (modules); + assert_todo ("not implemented", "CKR_CRYPTOKI_ALREADY_INITIALIZED handling"); + } + finalize_and_free_modules (modules); +} + int main (int argc, char *argv[]) @@ -480,6 +504,7 @@ main (int argc, p11_test (test_config_option, "/modules/test_config_option"); p11_test (test_module_trusted_only, "/modules/trusted-only"); p11_test (test_module_trust_flags, "/modules/trust-flags"); + p11_test (test_already_initialized, "/modules/already-initialized"); p11_kit_be_quiet ();