tests: Add more valgrind supressions for krb5

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>

.gitlab-ci.yml

@@ -125,7 +125,7 @@ review:
 ###############################################################################
 #                               CentOS builds                                 #
 ###############################################################################
-centos10s/openssl_3.2.x/x86_64:
+centos10s/openssl_3.5.x/x86_64:
   image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$CENTOS10_BUILD
   extends: .tests
   variables:
@@ -136,7 +136,7 @@ centos10s/openssl_3.2.x/x86_64:
       make -j$(nproc) &&
       ctest --output-on-failure
 
-centos10s/openssl_3.2.x/x86_64/fips:
+centos10s/openssl_3.5.x/x86_64/fips:
   extends: .fips
   image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$CENTOS10_BUILD
   variables:
@@ -147,7 +147,7 @@ centos10s/openssl_3.2.x/x86_64/fips:
       make -j$(nproc) &&
       OPENSSL_FORCE_FIPS_MODE=1 ctest --output-on-failure
 
-centos9s/openssl_3.x/x86_64:
+centos9s/openssl_3.5.x/x86_64:
   image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$CENTOS9_BUILD
   extends: .tests
   variables:
@@ -164,7 +164,7 @@ centos9s/mbedtls_2.x/x86_64:
   variables:
     CMAKE_ADDITIONAL_OPTIONS: "-DWITH_MBEDTLS=ON -DWITH_DEBUG_CRYPTO=ON -DWITH_BLOWFISH_CIPHER=OFF"
 
-centos9s/openssl_3.x/x86_64/fips:
+centos9s/openssl_3.5.x/x86_64/fips:
   extends: .fips
   image: $CI_REGISTRY/$BUILD_IMAGES_PROJECT:$CENTOS9_BUILD
   script:

include/libssh/crypto.h

@@ -237,9 +237,6 @@ int sshkdf_derive_key(struct ssh_crypto_struct *crypto,
                       size_t requested_len);
 
 int secure_memcmp(const void *s1, const void *s2, size_t n);
-#if defined(HAVE_LIBCRYPTO) && !defined(WITH_PKCS11_PROVIDER)
-ENGINE *pki_get_engine(void);
-#endif /* HAVE_LIBCRYPTO */
 
 void compress_cleanup(struct ssh_crypto_struct *crypto);
 

include/libssh/libcrypto.h

@@ -121,6 +121,15 @@ typedef BN_CTX* bignum_CTX;
 
 ssh_string pki_key_make_ecpoint_string(const EC_GROUP *g, const EC_POINT *p);
 int pki_key_ecgroup_name_to_nid(const char *group);
+
+#if defined(WITH_PKCS11_URI)
+#if defined(WITH_PKCS11_PROVIDER)
+int pki_load_pkcs11_provider(void);
+#else
+ENGINE *pki_get_engine(void);
+#endif
+#endif /* WITH_PKCS11_PROVIDER */
+
 #endif /* HAVE_LIBCRYPTO */
 
 #endif /* LIBCRYPTO_H_ */

src/libcrypto.c

@@ -49,8 +49,9 @@
 #include <openssl/rsa.h>
 #include <openssl/hmac.h>
 #else
-#include <openssl/param_build.h>
 #include <openssl/core_names.h>
+#include <openssl/param_build.h>
+#include <openssl/provider.h>
 #endif /* OPENSSL_VERSION_NUMBER */
 #include <openssl/rand.h>
 #if defined(WITH_PKCS11_URI) && !defined(WITH_PKCS11_PROVIDER)
@@ -96,7 +97,37 @@ void ssh_reseed(void){
 #endif
 }
 
-#if defined(WITH_PKCS11_URI) && !defined(WITH_PKCS11_PROVIDER)
+#if defined(WITH_PKCS11_URI)
+#if defined(WITH_PKCS11_PROVIDER)
+static OSSL_PROVIDER *provider = NULL;
+static bool pkcs11_provider_failed = false;
+
+int pki_load_pkcs11_provider(void)
+{
+    if (OSSL_PROVIDER_available(NULL, "pkcs11") == 1) {
+        /* the provider is already available.
+         * Loaded through a configuration file? */
+        return SSH_OK;
+    }
+
+    if (pkcs11_provider_failed) {
+        /* the loading failed previously -- do not retry */
+        return SSH_ERROR;
+    }
+
+    provider = OSSL_PROVIDER_try_load(NULL, "pkcs11", 1);
+    if (provider != NULL) {
+        return SSH_OK;
+    }
+
+    SSH_LOG(SSH_LOG_TRACE,
+            "Failed to load the pkcs11 provider: %s",
+            ERR_error_string(ERR_get_error(), NULL));
+    /* Do not attempt to load it again */
+    pkcs11_provider_failed = true;
+    return SSH_ERROR;
+}
+#else
 static ENGINE *engine = NULL;
 
 ENGINE *pki_get_engine(void)
@@ -128,7 +159,8 @@ ENGINE *pki_get_engine(void)
     }
     return engine;
 }
-#endif /* defined(WITH_PKCS11_URI) && !defined(WITH_PKCS11_PROVIDER) */
+#endif /* defined(WITH_PKCS11_PROVIDER) */
+#endif /* defined(WITH_PKCS11_URI) */
 
 #ifdef HAVE_OPENSSL_EVP_KDF_CTX
 #if OPENSSL_VERSION_NUMBER < 0x30000000L
@@ -1404,6 +1436,14 @@ void ssh_crypto_finalize(void)
         engine = NULL;
     }
 #endif
+#if defined(WITH_PKCS11_URI)
+#if defined(WITH_PKCS11_PROVIDER)
+    if (provider != NULL) {
+        OSSL_PROVIDER_unload(provider);
+        provider = NULL;
+    }
+#endif /* WITH_PKCS11_PROVIDER */
+#endif /* WITH_PKCS11_URI */
 
     libcrypto_initialized = 0;
 }

src/options.c

@@ -2285,6 +2285,9 @@ ssh_bind_options_set(ssh_bind sshbind,
                               SSH_FATAL,
                               "The host key size %d is too small.",
                               ssh_key_size(key));
+                if (type != SSH_BIND_OPTIONS_IMPORT_KEY) {
+                    SSH_KEY_FREE(key);
+                }
                 return -1;
             }
             key_type = ssh_key_type(key);
@@ -2330,6 +2333,11 @@ ssh_bind_options_set(ssh_bind sshbind,
                     ssh_key_free(key);
                     return -1;
                 }
+            } else if (type == SSH_BIND_OPTIONS_IMPORT_KEY_STR) {
+                if (bind_key_loc == NULL) {
+                    ssh_key_free(key);
+                    return -1;
+                }
             } else {
                 if (bind_key_loc == NULL) {
                     return -1;

src/pki_crypto.c

@@ -46,7 +46,6 @@
 #include <openssl/param_build.h>
 #if defined(WITH_PKCS11_URI) && defined(WITH_PKCS11_PROVIDER)
 #include <openssl/store.h>
-#include <openssl/provider.h>
 #endif
 #endif /* OPENSSL_VERSION_NUMBER */
 
@@ -2856,9 +2855,6 @@ error:
 }
 
 #ifdef WITH_PKCS11_URI
-#ifdef WITH_PKCS11_PROVIDER
-static bool pkcs11_provider_failed = false;
-#endif
 
 /**
  * @internal
@@ -2924,20 +2920,11 @@ int pki_uri_import(const char *uri_name,
 
     /* The provider can be either configured in openssl.cnf or dynamically
      * loaded, assuming it does not need any special configuration */
-    if (OSSL_PROVIDER_available(NULL, "pkcs11") == 0 &&
-        !pkcs11_provider_failed) {
-        OSSL_PROVIDER *pkcs11_provider = NULL;
-
-        pkcs11_provider = OSSL_PROVIDER_try_load(NULL, "pkcs11", 1);
-        if (pkcs11_provider == NULL) {
-            SSH_LOG(SSH_LOG_TRACE,
-                    "Failed to initialize provider: %s",
-                    ERR_error_string(ERR_get_error(), NULL));
-            /* Do not attempt to load it again */
-            pkcs11_provider_failed = true;
+    rv = pki_load_pkcs11_provider();
+    if (rv != SSH_OK) {
+        SSH_LOG(SSH_LOG_TRACE, "Failed to load or initialize pkcs11 provider");
         goto fail;
     }
-    }
 
     store = OSSL_STORE_open(uri_name, NULL, NULL, NULL, NULL);
     if (store == NULL) {

tests/CMakeLists.txt

@@ -23,6 +23,11 @@ if (NOT WIN32)
         ${TORTURE_LINK_LIBRARIES}
         pthread)
 endif(NOT WIN32)
+if (WITH_GSSAPI AND GSSAPI_FOUND)
+    set(TORTURE_LINK_LIBRARIES
+        ${TORTURE_LINK_LIBRARIES}
+        crypto)
+endif (WITH_GSSAPI AND GSSAPI_FOUND)
 
 # create test library
 add_library(${TORTURE_LIBRARY}

tests/client/torture_gssapi_auth.c

@@ -272,5 +272,5 @@ torture_run_tests(void)
     rc = cmocka_run_group_tests(tests, sshd_setup, sshd_teardown);
     ssh_finalize();
 
-    pthread_exit((void *)&rc);
+    return rc;
 }

tests/fuzz/CMakeLists.txt

@@ -2,9 +2,7 @@ project(fuzzing CXX)
 
 macro(fuzzer name)
     add_executable(${name} ${name}.c)
-    target_link_libraries(${name}
-                          PRIVATE
-                              ssh::static pthread)
+    target_link_libraries(${name} PRIVATE ${TORTURE_LINK_LIBRARIES})
     if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
         set_target_properties(${name}
                               PROPERTIES

tests/fuzz/fuzzer.c

@@ -1,8 +1,14 @@
 /* Simpler gnu89 version of StandaloneFuzzTargetMain.c from LLVM */
 
+#include "config.h"
+
 #include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>
+#if defined(HAVE_LIBCRYPTO) || defined(WITH_GSSAPI)
+/* for OPENSSL_cleanup() of GSSAPI's OpenSSL context */
+#include <openssl/crypto.h>
+#endif
 
 int LLVMFuzzerTestOneInput (const unsigned char *data, size_t size);
 __attribute__((weak)) int LLVMFuzzerInitialize(int *argc, char ***argv);
@@ -35,5 +41,9 @@ main (int argc, char **argv)
 
     free (buf);
     printf ("Done!\n");
+
+#if defined(HAVE_LIBCRYPTO) || defined(WITH_GSSAPI)
+    OPENSSL_cleanup();
+#endif
     return 0;
 }

tests/pkd/CMakeLists.txt

@@ -17,10 +17,8 @@ set(pkd_hello_src
 )
 
 set(pkd_libs
-    ${CMOCKA_LIBRARY}
-    ssh::static
+    ${TORTURE_LINK_LIBRARIES}
     ${ARGP_LIBRARIES}
-    pthread
 )
 
 add_executable(pkd_hello ${pkd_hello_src})

tests/pkd/pkd_hello.c

@@ -22,6 +22,11 @@
 #include "pkd_keyutil.h"
 #include "pkd_util.h"
 
+#if defined(HAVE_LIBCRYPTO)
+/* for OPENSSL_cleanup() of OpenSSL context */
+#include <openssl/crypto.h>
+#endif
+
 #define DEFAULT_ITERATIONS 10
 static struct pkd_daemon_args pkd_dargs;
 
@@ -1019,6 +1024,9 @@ out_finalize:
     if (rc != 0) {
         fprintf(stderr, "ssh_finalize: %d\n", rc);
     }
+#if defined(HAVE_LIBCRYPTO)
+    OPENSSL_cleanup();
+#endif
 out:
     return exit_code;
 }

tests/server/test_server/CMakeLists.txt

@@ -11,7 +11,8 @@ set(server_SRCS
 add_library(testserver STATIC
             test_server.c
             default_cb.c
-            sftpserver_cb.c)
+            sftpserver_cb.c
+            testserver_common.c)
 if (WITH_COVERAGE)
     append_coverage_compiler_flags_to_target(testserver)
 endif (WITH_COVERAGE)
@@ -32,7 +33,7 @@ if (UNIX AND NOT WIN32)
     add_executable(test_server ${server_SRCS})
     target_compile_options(test_server PRIVATE ${DEFAULT_C_COMPILE_FLAGS})
     target_link_libraries(test_server
-        PRIVATE testserver ssh::ssh ${ARGP_LIBRARIES} util)
+        PRIVATE testserver ${TORTURE_LINK_LIBRARIES} ${ARGP_LIBRARIES} util)
     if (WITH_COVERAGE)
         append_coverage_compiler_flags_to_target(test_server)
     endif (WITH_COVERAGE)

tests/server/test_server/default_cb.c

@@ -21,9 +21,11 @@
  * MA 02111-1307, USA.
  */
 
+
 #include "config.h"
 #include "test_server.h"
 #include "default_cb.h"
+#include "testserver_common.h"
 
 #include <libssh/callbacks.h>
 #include <libssh/server.h>
@@ -448,9 +450,11 @@ static int exec_pty(const char *mode,
     case 0:
         close(cdata->pty_master);
         if (login_tty(cdata->pty_slave) != 0) {
+            finalize_openssl();
             exit(1);
         }
         execl("/bin/sh", "sh", mode, command, NULL);
+        finalize_openssl();
         exit(0);
     default:
         close(cdata->pty_slave);
@@ -500,6 +504,7 @@ static int exec_nopty(const char *command, struct channel_data_st *cdata)
             close(err[1]);
             /* exec the requested command. */
             execl("/bin/sh", "sh", "-c", command, NULL);
+            finalize_openssl();
             exit(0);
     }
 

tests/server/test_server/test_server.c

@@ -22,6 +22,7 @@
  */
 
 #include "test_server.h"
+#include "testserver_common.h"
 
 #include <libssh/priv.h>
 #include <libssh/libssh.h>
@@ -288,6 +289,7 @@ int run_server(struct server_state_st *state)
 
                 free_server_state(state);
                 SAFE_FREE(state);
+                finalize_openssl();
                 exit(0);
             case -1:
                 fprintf(stderr, "Failed to fork\n");
@@ -355,11 +357,8 @@ fork_run_server(struct server_state_st *state,
 
         /* The child process starts a server which will listen for connections */
         rc = run_server(state);
-        if (rc != 0) {
+        finalize_openssl();
         exit(rc);
-        }
-
-        exit(0);
     case -1:
         strerror_r(errno, err_str, 1024);
         fprintf(stderr, "Failed to fork: %s\n",

tests/server/test_server/testserver_common.c

@@ -0,0 +1,36 @@
+/*
+ * This file is part of the SSH Library
+ *
+ * Copyright (c) 2025 by Red Hat, Inc.
+ *
+ * Author: Jakub Jelen <jjelen@redhat.com>
+ *
+ * The SSH Library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at your
+ * option) any later version.
+ *
+ * The SSH Library 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 Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with the SSH Library; see the file COPYING.  If not, write to
+ * the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ */
+
+#include "testserver_common.h"
+
+#if defined(HAVE_LIBCRYPTO) || defined(WITH_GSSAPI)
+/* for OPENSSL_cleanup() of GSSAPI's OpenSSL context */
+#include <openssl/crypto.h>
+#endif
+
+void finalize_openssl(void)
+{
+#if defined(HAVE_LIBCRYPTO) || defined(WITH_GSSAPI)
+    OPENSSL_cleanup();
+#endif
+}

tests/server/test_server/testserver_common.h

@@ -0,0 +1,26 @@
+/*
+ * This file is part of the SSH Library
+ *
+ * Copyright (c) 2025 by Red Hat, Inc.
+ *
+ * Author: Jakub Jelen <jjelen@redhat.com>
+ *
+ * The SSH Library is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at your
+ * option) any later version.
+ *
+ * The SSH Library 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 Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with the SSH Library; see the file COPYING.  If not, write to
+ * the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ */
+
+#include "config.h"
+
+void finalize_openssl(void);

tests/server/torture_gssapi_server_auth.c

@@ -451,5 +451,5 @@ torture_run_tests(void)
                                 teardown_default_server);
     ssh_finalize();
 
-    pthread_exit((void *)&rc);
+    return rc;
 }

tests/server/torture_gssapi_server_auth_cb.c

@@ -475,5 +475,5 @@ torture_run_tests(void)
                                 teardown_default_server);
     ssh_finalize();
 
-    pthread_exit((void *)&rc);
+    return rc;
 }

tests/server/torture_gssapi_server_delegation.c

@@ -371,5 +371,5 @@ torture_run_tests(void)
                                 teardown_default_server);
     ssh_finalize();
 
-    pthread_exit((void *)&rc);
+    return rc;
 }

tests/server/torture_sftpserver.c

@@ -31,6 +31,10 @@
 #include <errno.h>
 #include <pwd.h>
 
+#ifdef HAVE_VALGRIND_VALGRIND_H
+#include <valgrind/valgrind.h>
+#endif
+
 #include "libssh/buffer.h"
 #include "libssh/libssh.h"
 #include "libssh/priv.h"
@@ -1199,6 +1203,20 @@ static void torture_server_sftp_payload_overrun(void **state)
     uint32_t id, bad_payload_len = 0x7ffffffc;
     int rc;
 
+#ifdef HAVE_VALGRIND_VALGRIND_H
+    if (RUNNING_ON_VALGRIND) {
+        /* This malformed message does not crash the server, but keeps waiting
+         * for more data as announced in the payloiad length so the opened FD on
+         * the server side is leaking when the server terminates.
+         * Given that the custom sftp server could store anything into the
+         * handles, it should take care of cleaning up the outstanding handles,
+         * but this is something to solve in the future. Now just skipping the
+         * test.
+         */
+        skip();
+    }
+#endif /* HAVE_VALGRIND_VALGRIND_H */
+
     assert_non_null(tss);
 
     s = tss->state;

tests/torture.c

@@ -53,6 +53,11 @@
 #include <valgrind/valgrind.h>
 #endif
 
+#ifdef WITH_GSSAPI
+/* for OPENSSL_cleanup() of GSSAPI's OpenSSL context */
+#include <openssl/crypto.h>
+#endif
+
 #define TORTURE_SSHD_SRV_IPV4 "127.0.0.10"
 #define TORTURE_SSHD_SRV1_IPV4 "127.0.0.11"
 /* socket wrapper IPv6 prefix  fd00::5357:5fxx */
@@ -1976,10 +1981,31 @@ __attribute__((weak)) int torture_run_tests(void)
 }
 #endif /* defined(HAVE_WEAK_ATTRIBUTE) && defined(TORTURE_SHARED) */
 
+/**
+ * Finalize the torture context. No-op except for OpenSSL or GSSAPI
+ *
+ * When OpenSSL is built without the at-exit handlers, it won't call the
+ * OPENSSL_cleanup() from destructor or at-exit handler, which means we need to
+ * do it manually in the tests.
+ *
+ * It is never a good idea to call this function from the library context as we
+ * can not be sure the libssh is really the last one using the OpenSSL.
+ *
+ * This needs to be called at the end of the main function or any time before
+ * any forked process (servers) exits.
+ */
+void torture_finalize(void)
+{
+#if defined(HAVE_LIBCRYPTO) || defined(WITH_GSSAPI)
+    OPENSSL_cleanup();
+#endif
+}
+
 int main(int argc, char **argv)
 {
     struct argument_s arguments;
     char *env = getenv("LIBSSH_VERBOSITY");
+    int rv;
 
     arguments.verbose = 0;
     arguments.pattern = NULL;
@@ -1997,7 +2023,11 @@ int main(int argc, char **argv)
     cmocka_set_test_filter(pattern);
 #endif
 
-    return torture_run_tests();
+    rv = torture_run_tests();
+
+    torture_finalize();
+
+    return rv;
 }
 
 /**

tests/torture.h

@@ -194,4 +194,6 @@ void torture_unsetenv(char const *variable);
 int torture_setup_ssh_agent(struct torture_state *s, const char *add_key);
 int torture_cleanup_ssh_agent(void);
 
+void torture_finalize(void);
+
 #endif /* _TORTURE_H */

tests/valgrind.supp

@@ -248,23 +248,9 @@
    Reachable memory from getaddrinfo
    Memcheck:Leak
    match-leak-kinds: reachable
-   fun:malloc
-   fun:UnknownInlinedFun
-   fun:_dl_new_object
-   fun:_dl_map_object_from_fd
-   fun:_dl_map_object
-   fun:dl_open_worker_begin
-   fun:_dl_catch_exception
-   fun:dl_open_worker
-   fun:_dl_catch_exception
-   fun:_dl_open
-   fun:do_dlopen
-   fun:_dl_catch_exception
-   fun:_dl_catch_error
-   fun:dlerror_run
-   fun:__libc_dlopen_mode
-   fun:module_load
+   ...
    fun:__nss_module_get_function
+   ...
    fun:getaddrinfo
    ...
    fun:krb5_sname_to_principal
@@ -285,7 +271,6 @@
    fun:torture_run_tests
    fun:main
 }
-
 ## libkrb5
 # krb5_mcc_generate_new allocates a hashtab on a static global variable
 # It doesn't get freed.
@@ -298,3 +283,30 @@
    ...
    fun:krb5_mcc_generate_new*
 }
+{
+   Error string from acquire creds in krb5
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:krb5_gss_save_error_string
+   fun:UnknownInlinedFun
+   fun:acquire_cred_context.isra.0
+   fun:acquire_cred_from.isra.0
+   fun:gss_add_cred_from
+   fun:gss_acquire_cred_from
+   ...
+   fun:gss_acquire_cred
+}
+{
+   error string from gss init sec context
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:krb5_gss_save_error_string
+   fun:UnknownInlinedFun
+   fun:krb5_gss_init_sec_context_ext
+   fun:krb5_gss_init_sec_context
+   fun:gss_init_sec_context
+}