From c9867893f8d37381b522d8c3f371bec487805f9e Mon Sep 17 00:00:00 2001 From: Ingo Franzki Date: Thu, 31 Mar 2022 16:55:03 +0200 Subject: [libica PATCH 1/5] OpenSSL 3.0: Cleanup OpenSSL library context during OpenSSL cleanup Usually libica's own library context is freed in the library destructor when the library is unloaded (i.e. during exit handlers). OpenSSL also performs its own cleanup in exit handlers, and it may happen that OpenSSL cleanup is performed before the library destructors are called. This may cause crashes when libica's library context has already been freed by OpenSSL cleanup, but the library destructor tries to free it a second time. This causes a double free, and very likely a crash. Register an OpenSSL cleanup handler to clean up the library context before OpenSSL performs its own cleanup. Signed-off-by: Ingo Franzki --- src/init.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/init.c b/src/init.c index 9d69bd3..03a2a80 100644 --- a/src/init.c +++ b/src/init.c @@ -65,6 +65,18 @@ void end_sigill_section(struct sigaction *oldact, sigset_t *oldset) sigprocmask(SIG_SETMASK, oldset, NULL); } +#if OPENSSL_VERSION_PREREQ(3, 0) +static void openssl_cleanup() +{ + if (openssl_provider != NULL) + OSSL_PROVIDER_unload(openssl_provider); + openssl_provider = NULL; + if (openssl_libctx != NULL) + OSSL_LIB_CTX_free(openssl_libctx); + openssl_libctx = NULL; +} +#endif + void __attribute__ ((constructor)) icainit(void) { int value; @@ -106,6 +118,17 @@ void __attribute__ ((constructor)) icainit(void) * Create a separate library context for libica's use of OpenSSL services * and explicitly load the 'default' or 'fips' provider for this context. */ + + /* + * Perform libica's context cleanup when OpenSSL cleanup is run. + * Otherwise it might happen that the library destructor is called + * after OpenSSL cleanup has already been performed, and this will + * cause crashes when trying to free our own OpenSSL library context, + * since the contexts have already been freed by OpenSSL cleanup at that + * time. + * */ + OPENSSL_atexit(openssl_cleanup); + openssl_libctx = OSSL_LIB_CTX_new(); if (openssl_libctx == NULL) { syslog(LOG_ERR, "Libica: failed to create openssl lib context\n"); @@ -148,10 +171,7 @@ void __attribute__ ((destructor)) icaexit(void) stats_munmap(SHM_CLOSE); #if OPENSSL_VERSION_PREREQ(3, 0) - if (openssl_provider != NULL) - OSSL_PROVIDER_unload(openssl_provider); - if (openssl_libctx != NULL) - OSSL_LIB_CTX_free(openssl_libctx); + openssl_cleanup(); #endif } -- 2.34.3 From 140c700f1823e9f9f2cd26d5264cc4fb0f50dfa1 Mon Sep 17 00:00:00 2001 From: Ingo Franzki Date: Tue, 5 Apr 2022 14:49:07 +0200 Subject: [libica PATCH 2/5] Revert "OpenSSL 3.0: Cleanup OpenSSL library context during OpenSSL cleanup" This reverts commit c9867893f8d37381b522d8c3f371bec487805f9e. Signed-off-by: Ingo Franzki --- src/init.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/src/init.c b/src/init.c index 03a2a80..9d69bd3 100644 --- a/src/init.c +++ b/src/init.c @@ -65,18 +65,6 @@ void end_sigill_section(struct sigaction *oldact, sigset_t *oldset) sigprocmask(SIG_SETMASK, oldset, NULL); } -#if OPENSSL_VERSION_PREREQ(3, 0) -static void openssl_cleanup() -{ - if (openssl_provider != NULL) - OSSL_PROVIDER_unload(openssl_provider); - openssl_provider = NULL; - if (openssl_libctx != NULL) - OSSL_LIB_CTX_free(openssl_libctx); - openssl_libctx = NULL; -} -#endif - void __attribute__ ((constructor)) icainit(void) { int value; @@ -118,17 +106,6 @@ void __attribute__ ((constructor)) icainit(void) * Create a separate library context for libica's use of OpenSSL services * and explicitly load the 'default' or 'fips' provider for this context. */ - - /* - * Perform libica's context cleanup when OpenSSL cleanup is run. - * Otherwise it might happen that the library destructor is called - * after OpenSSL cleanup has already been performed, and this will - * cause crashes when trying to free our own OpenSSL library context, - * since the contexts have already been freed by OpenSSL cleanup at that - * time. - * */ - OPENSSL_atexit(openssl_cleanup); - openssl_libctx = OSSL_LIB_CTX_new(); if (openssl_libctx == NULL) { syslog(LOG_ERR, "Libica: failed to create openssl lib context\n"); @@ -171,7 +148,10 @@ void __attribute__ ((destructor)) icaexit(void) stats_munmap(SHM_CLOSE); #if OPENSSL_VERSION_PREREQ(3, 0) - openssl_cleanup(); + if (openssl_provider != NULL) + OSSL_PROVIDER_unload(openssl_provider); + if (openssl_libctx != NULL) + OSSL_LIB_CTX_free(openssl_libctx); #endif } -- 2.34.3 From 7d0046c992ce927ad15943eb57fc788b147f7725 Mon Sep 17 00:00:00 2001 From: Ingo Franzki Date: Tue, 5 Apr 2022 14:54:22 +0200 Subject: [libica PATCH 3/5] OpenSSL 3.0: Do not cleanup OpenSSL library context during library destructor OpenSSL cleanup may have already run once the library destructor is called, this may result in crashes. On the other hand, we can not register an OpenSSL cleanup handler for this, because one may unload the library before OpenSSl cleanup runs, this would also cause crashes. So we can only not cleanup the library context at all, and leak it if one unloads the library. OpenSSl will anyway clean up the contexts at program termination. Signed-off-by: Ingo Franzki --- src/init.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/init.c b/src/init.c index 9d69bd3..b61e9d5 100644 --- a/src/init.c +++ b/src/init.c @@ -146,12 +146,4 @@ void __attribute__ ((destructor)) icaexit(void) rng_fini(); stats_munmap(SHM_CLOSE); - -#if OPENSSL_VERSION_PREREQ(3, 0) - if (openssl_provider != NULL) - OSSL_PROVIDER_unload(openssl_provider); - if (openssl_libctx != NULL) - OSSL_LIB_CTX_free(openssl_libctx); -#endif - } -- 2.34.3 From 82213e4c418222a7e1fc5a29c7fcf56df4b2faac Mon Sep 17 00:00:00 2001 From: Ingo Franzki Date: Wed, 6 Apr 2022 10:37:08 +0200 Subject: [libica PATCH 4/5] Add ica_cleanup function as external function Allow an application to perform cleanup of libica's internal resources. Signed-off-by: Ingo Franzki --- include/ica_api.h | 8 ++++++++ libica.map | 6 ++++++ src/icainfo.c | 5 +++++ src/init.c | 13 +++++++++++++ 4 files changed, 32 insertions(+) diff --git a/include/ica_api.h b/include/ica_api.h index 6137c4a..e6ee45b 100644 --- a/include/ica_api.h +++ b/include/ica_api.h @@ -3665,4 +3665,12 @@ ICA_EXPORT void ica_fips_powerup_tests(void); #endif /* ICA_FIPS */ +/* + * Cleanup ICA resources. Should be called before the application terminates, + * or the libica library is unloaded. + * + */ +ICA_EXPORT +void ica_cleanup(void); + #endif /* __ICA_API_H__ */ diff --git a/libica.map b/libica.map index 0d031e1..6de5533 100644 --- a/libica.map +++ b/libica.map @@ -166,3 +166,9 @@ LIBICA_3.6.0 { ica_ed448_ctx_del; local: *; } LIBICA_3.5.0; + +LIBICA_4.0.2 { + global: + ica_cleanup; + local: *; +} LIBICA_3.6.0; diff --git a/src/icainfo.c b/src/icainfo.c index 61ec2d6..dbf8312 100644 --- a/src/icainfo.c +++ b/src/icainfo.c @@ -385,6 +385,7 @@ int main(int argc, char **argv) default: fprintf(stderr, "Try '%s --help' for more" " information.\n", basename(argv[0])); + ica_cleanup(); exit(1); } } @@ -392,6 +393,7 @@ int main(int argc, char **argv) fprintf(stderr, "%s: invalid option.\n" "Try '%s --help' for more information.\n", argv[0], basename(argv[0])); + ica_cleanup(); exit(1); } @@ -400,12 +402,14 @@ int main(int argc, char **argv) if (ica_get_functionlist(NULL, &mech_len) != 0){ perror("get_functionlist: "); + ica_cleanup(); return EXIT_FAILURE; } pmech_list = malloc(sizeof(libica_func_list_element)*mech_len); if (ica_get_functionlist(pmech_list, &mech_len) != 0){ perror("get_functionlist: "); free(pmech_list); + ica_cleanup(); return EXIT_FAILURE; } @@ -470,5 +474,6 @@ int main(int argc, char **argv) printf("CPACF support (including fallbacks) is disabled in libica-cex.\n"); #endif + ica_cleanup(); return EXIT_SUCCESS; } diff --git a/src/init.c b/src/init.c index b61e9d5..796e694 100644 --- a/src/init.c +++ b/src/init.c @@ -65,6 +65,19 @@ void end_sigill_section(struct sigaction *oldact, sigset_t *oldset) sigprocmask(SIG_SETMASK, oldset, NULL); } + +void ica_cleanup(void) +{ +#if OPENSSL_VERSION_PREREQ(3, 0) + if (openssl_provider != NULL) + OSSL_PROVIDER_unload(openssl_provider); + openssl_provider = NULL; + if (openssl_libctx != NULL) + OSSL_LIB_CTX_free(openssl_libctx); + openssl_libctx = NULL; +#endif +} + void __attribute__ ((constructor)) icainit(void) { int value; -- 2.34.3 From e241c9503b1dc912ad9257a3787c56c320643a1e Mon Sep 17 00:00:00 2001 From: Ingo Franzki Date: Tue, 19 Apr 2022 09:53:51 +0200 Subject: [libica PATCH 5/5] Fix memory leak at library unload by uninstantiating global prng instance When built in non-FIPS mode, s390_prng_init() initializes a global PRNG instance in the library constructor, which must also be freed in the library destructor. Otherwise it leaks 64 bytes (direct leak) plus 240 bytes (indirect leak) when unloading the library. Signed-off-by: Ingo Franzki --- src/include/s390_prng.h | 1 + src/init.c | 2 ++ src/s390_prng.c | 6 ++++++ 3 files changed, 9 insertions(+) diff --git a/src/include/s390_prng.h b/src/include/s390_prng.h index 5219337..77ba430 100644 --- a/src/include/s390_prng.h +++ b/src/include/s390_prng.h @@ -16,5 +16,6 @@ int s390_prng_init(void); int s390_prng(unsigned char *output_data, unsigned int output_length); +void s390_prng_fini(void); #endif diff --git a/src/init.c b/src/init.c index 796e694..74fafdd 100644 --- a/src/init.c +++ b/src/init.c @@ -158,5 +158,7 @@ void __attribute__ ((destructor)) icaexit(void) { rng_fini(); + s390_prng_fini(); + stats_munmap(SHM_CLOSE); } diff --git a/src/s390_prng.c b/src/s390_prng.c index 1b057c6..b66be17 100644 --- a/src/s390_prng.c +++ b/src/s390_prng.c @@ -360,3 +360,9 @@ static int s390_prng_seed(void *srv, unsigned int count) return rc; } #endif /* ICA_FIPS */ + +void s390_prng_fini(void) +{ + if (ica_drbg_global != NULL) + ica_drbg_uninstantiate(&ica_drbg_global); +} -- 2.34.3