testsuite: hidden Segmentation fault

popovec commented 1 year ago

While viewing the test logs (cat tests/*.log), I accidentally found "Segmentation fault".

The problem can be easily reproduced (openssl version 3.0.9):

$ git clone http://github.com/opensc/libp11
$ cd libp11
$ ./bootstrap
$ ./configure
$ make check
$ cat tests/search-all-matching-tokens.softhsm.log

Here is the relevant part of log:

The private key was not found at: pkcs11:object=label-3;type=private;pin-value=1234
PKCS11_get_private_key returned NULL
Could not read private key from org.openssl.engine:pkcs11:pkcs11:object=label-3;type=private;pin-value=1234
40D7C2FA6A7F0000:error:40000065:pkcs11 engine:ERR_ENG_error:object not found:eng_back.c:932:
40D7C2FA6A7F0000:error:13000080:engine routines:ENGINE_load_private_key:failed loading private key:../crypto/engine/eng_pkey.c:79:
pkeyutl: Error initializing context
Segmentation fault
Engine "pkcs11" set.

As a first fix, I suggest fixing the test:

diff --git a/tests/search-all-matching-tokens.softhsm b/tests/search-all-matching-tokens.softhsm
index 3cd26a6..51f2e2e 100755
--- a/tests/search-all-matching-tokens.softhsm
+++ b/tests/search-all-matching-tokens.softhsm
@@ -74,7 +74,7 @@ echo "secret" > "${outdir}/in.txt"
 openssl pkeyutl -engine pkcs11 -keyform engine \
        -inkey "${PRIVATE_KEY_WITHOUT_TOKEN}" \
        -sign -out "${outdir}/signature.bin" -in "${outdir}/in.txt"
-if test $? = 0;then
+if test $? != 1;then
        echo "Did not fail when the PKCS#11 URI matched multiple tokens"
        exit 1;
 fi

It seems that there was a similar problem with freeing memory, more can be read from the comments in src/eng_front.c

https://github.com/OpenSC/libp11/blob/6c96847f1f52a5ccc76e8f8d14820cc4d6af1ecb/src/eng_front.c#L116C1-L126C1 https://github.com/OpenSC/libp11/blob/6c96847f1f52a5ccc76e8f8d14820cc4d6af1ecb/src/eng_front.c#L153C1-L166C1

I think for openssl 3.x it is also convenient to omit the ctx_finish(ctx); call. So I propose the following modification:

diff --git a/src/eng_front.c b/src/eng_front.c
index fd6940f..d1cd397 100644
--- a/src/eng_front.c
+++ b/src/eng_front.c
@@ -161,7 +161,7 @@ static int engine_finish(ENGINE *engine)
         * engine_finish() is also executed from ENGINE_finish() without
         * acquired CRYPTO_LOCK_ENGINE, and there is no way with to check
         * whether a lock is already acquired with OpenSSL < 1.1.0 API. */
-#if OPENSSL_VERSION_NUMBER >= 0x10100005L && !defined(LIBRESSL_VERSION_NUMBER)
+#if OPENSSL_VERSION_NUMBER >= 0x10100005L && !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER < 0x30000000L
        rv &= ctx_finish(ctx);
 #endif

If no one suggests a better solution, I will prepare a PR.

popovec commented 1 year ago

I have currently tested the above fix and it seems that the segfault problem exists not only when using openssl 3.X. This segfault also occurs when using openssl 1.1.1n (Debian 11, openssl 1.1.1n-0+deb11u5).

popovec commented 1 year ago

I am canceling this issue, more information in #511

OpenSC / libp11

testsuite: hidden Segmentation fault #509