From f21d237374cf2ada5da109ea11465f5b306c4722 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ayd=C4=B1n=20Mercan?= Date: Mon, 1 Dec 2025 13:49:46 +0300 Subject: [PATCH] move openssl error reporting to isc/ossl_wrap While being the best place at the time, the tlserr2result doesn't belong inside TLS code since it is generic to OpenSSL and mostly used in the dst interface. The newly created ossl_wrap interface is the idea place for flushing the OpenSSL thread error queue. --- lib/dns/dst_openssl.h | 22 +++++---- lib/isc/crypto/ossl1_1.c | 13 +++--- lib/isc/crypto/ossl3.c | 19 ++++---- lib/isc/include/isc/ossl_wrap.h | 16 +++++++ lib/isc/include/isc/tls.h | 8 ---- lib/isc/ossl_wrap/ossl_common.c | 82 ++++++++++++++++++++++++++++++++- lib/isc/tls.c | 77 ------------------------------- 7 files changed, 126 insertions(+), 111 deletions(-) diff --git a/lib/dns/dst_openssl.h b/lib/dns/dst_openssl.h index 8d57211af2..55d42645c4 100644 --- a/lib/dns/dst_openssl.h +++ b/lib/dns/dst_openssl.h @@ -21,18 +21,20 @@ #include #include +#include #include -#include -#define dst__openssl_toresult(fallback) \ - isc__tlserr2result(ISC_LOGCATEGORY_INVALID, ISC_LOGMODULE_INVALID, \ - NULL, fallback, __FILE__, __LINE__) -#define dst__openssl_toresult2(funcname, fallback) \ - isc__tlserr2result(DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_CRYPTO, \ - funcname, fallback, __FILE__, __LINE__) -#define dst__openssl_toresult3(category, funcname, fallback) \ - isc__tlserr2result(category, DNS_LOGMODULE_CRYPTO, funcname, fallback, \ - __FILE__, __LINE__) +#define dst__openssl_toresult(fallback) \ + isc__ossl_wrap_logged_toresult(ISC_LOGCATEGORY_INVALID, \ + ISC_LOGMODULE_INVALID, NULL, fallback, \ + __FILE__, __LINE__) +#define dst__openssl_toresult2(funcname, fallback) \ + isc__ossl_wrap_logged_toresult(DNS_LOGCATEGORY_GENERAL, \ + DNS_LOGMODULE_CRYPTO, funcname, \ + fallback, __FILE__, __LINE__) +#define dst__openssl_toresult3(category, funcname, fallback) \ + isc__ossl_wrap_logged_toresult(category, DNS_LOGMODULE_CRYPTO, \ + funcname, fallback, __FILE__, __LINE__) isc_result_t dst__openssl_fromlabel(int key_base_id, const char *label, const char *pin, diff --git a/lib/isc/crypto/ossl1_1.c b/lib/isc/crypto/ossl1_1.c index e47e294ddd..6ecbac47bc 100644 --- a/lib/isc/crypto/ossl1_1.c +++ b/lib/isc/crypto/ossl1_1.c @@ -26,8 +26,8 @@ #include #include #include +#include #include -#include #include #include "crypto_p.h" @@ -322,9 +322,9 @@ isc_crypto_fips_enable(void) { } if (FIPS_mode_set(1) == 0) { - return isc_tlserr2result(ISC_LOGCATEGORY_GENERAL, - ISC_LOGMODULE_CRYPTO, "FIPS_mode_set", - ISC_R_CRYPTOFAILURE); + return isc_ossl_wrap_logged_toresult( + ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, + "FIPS_mode_set", ISC_R_CRYPTOFAILURE); } register_algorithms(); @@ -389,8 +389,9 @@ isc__crypto_initialize(void) { /* Protect ourselves against unseeded PRNG */ if (RAND_status() != 1) { - isc_tlserr2result(ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, - "RAND_status", ISC_R_CRYPTOFAILURE); + isc_ossl_wrap_logged_toresult( + ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, + "RAND_status", ISC_R_CRYPTOFAILURE); FATAL_ERROR("OpenSSL pseudorandom number generator " "cannot be initialized (see the `PRNG not " "seeded' message in the OpenSSL FAQ)"); diff --git a/lib/isc/crypto/ossl3.c b/lib/isc/crypto/ossl3.c index 9b6e19bb27..dc3388a35a 100644 --- a/lib/isc/crypto/ossl3.c +++ b/lib/isc/crypto/ossl3.c @@ -29,9 +29,9 @@ #include #include #include +#include #include #include -#include #include #include "crypto_p.h" @@ -399,7 +399,7 @@ isc_crypto_fips_enable(void) { INSIST(fips == NULL); fips = OSSL_PROVIDER_load(NULL, "fips"); if (fips == NULL) { - return isc_tlserr2result( + return isc_ossl_wrap_logged_toresult( ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, "OSSL_PROVIDER_load", ISC_R_CRYPTOFAILURE); } @@ -408,16 +408,16 @@ isc_crypto_fips_enable(void) { base = OSSL_PROVIDER_load(NULL, "base"); if (base == NULL) { OSSL_PROVIDER_unload(fips); - return isc_tlserr2result( + return isc_ossl_wrap_logged_toresult( ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, "OSS_PROVIDER_load", ISC_R_CRYPTOFAILURE); } if (EVP_default_properties_enable_fips(NULL, 1) == 0) { - return isc_tlserr2result(ISC_LOGCATEGORY_GENERAL, - ISC_LOGMODULE_CRYPTO, - "EVP_default_properties_enable_fips", - ISC_R_CRYPTOFAILURE); + return isc_ossl_wrap_logged_toresult( + ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, + "EVP_default_properties_enable_fips", + ISC_R_CRYPTOFAILURE); } unregister_algorithms(); @@ -466,8 +466,9 @@ isc__crypto_initialize(void) { /* Protect ourselves against unseeded PRNG */ if (RAND_status() != 1) { - isc_tlserr2result(ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, - "RAND_status", ISC_R_CRYPTOFAILURE); + isc_ossl_wrap_logged_toresult( + ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_CRYPTO, + "RAND_status", ISC_R_CRYPTOFAILURE); FATAL_ERROR("OpenSSL pseudorandom number generator " "cannot be initialized (see the `PRNG not " "seeded' message in the OpenSSL FAQ)"); diff --git a/lib/isc/include/isc/ossl_wrap.h b/lib/isc/include/isc/ossl_wrap.h index acfe13e762..4b9b330f04 100644 --- a/lib/isc/include/isc/ossl_wrap.h +++ b/lib/isc/include/isc/ossl_wrap.h @@ -12,3 +12,19 @@ */ #pragma once + +#include +#include + +#define isc_ossl_wrap_logged_toresult(category, module, funcname, fallback) \ + isc__ossl_wrap_logged_toresult(category, module, funcname, fallback, \ + __FILE__, __LINE__) + +isc_result_t +isc_ossl_wrap_toresult(isc_result_t fallback); + +isc_result_t +isc__ossl_wrap_logged_toresult(isc_logcategory_t category, + isc_logmodule_t module, const char *funcname, + isc_result_t fallback, const char *file, + int line); diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index 50755ac2b1..1e10a7d649 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -615,11 +615,3 @@ isc_tls_valid_sni_hostname(const char *hostname); * string. Returns 'true' if the hostname is likely a domain name, and * 'false' if it represents an IP address. */ - -#define isc_tlserr2result(category, module, funcname, fallback) \ - isc__tlserr2result(category, module, funcname, fallback, __FILE__, \ - __LINE__) -isc_result_t -isc__tlserr2result(isc_logcategory_t category, isc_logmodule_t module, - const char *funcname, isc_result_t fallback, - const char *file, int line); diff --git a/lib/isc/ossl_wrap/ossl_common.c b/lib/isc/ossl_wrap/ossl_common.c index 844583345b..cf09d93a18 100644 --- a/lib/isc/ossl_wrap/ossl_common.c +++ b/lib/isc/ossl_wrap/ossl_common.c @@ -11,7 +11,87 @@ * information regarding copyright ownership. */ +#include + #include #include -EMPTY_TRANSLATION_UNIT; +#include "../openssl_shim.h" + +isc_result_t +isc_ossl_wrap_toresult(isc_result_t fallback) { + isc_result_t result = fallback; + unsigned long err = ERR_peek_error(); +#ifdef ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED + int lib = ERR_GET_LIB(err); +#endif /* ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED */ + int reason = ERR_GET_REASON(err); + + switch (reason) { + /* + * ERR_* errors are globally unique; others + * are unique per sublibrary + */ + case ERR_R_MALLOC_FAILURE: + result = ISC_R_NOMEMORY; + break; + default: +#ifdef ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED + if (lib == ERR_R_ECDSA_LIB && + reason == ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) + { + result = ISC_R_NOENTROPY; + break; + } +#endif /* ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED */ + break; + } + + return result; +} + +isc_result_t +isc__ossl_wrap_logged_toresult(isc_logcategory_t category, + isc_logmodule_t module, const char *funcname, + isc_result_t fallback, const char *file, + int line) { + isc_result_t result = isc_ossl_wrap_toresult(fallback); + + /* + * This is an exception - normally, we don't allow this, but the + * compatibility shims in dst_openssl.h needs a call that just + * translates the error code and don't do any logging. + */ + if (category == ISC_LOGCATEGORY_INVALID) { + goto done; + } + + isc_log_write(category, module, ISC_LOG_WARNING, + "%s (%s:%d) failed (%s)", funcname, file, line, + isc_result_totext(result)); + + if (result == ISC_R_NOMEMORY) { + goto done; + } + + for (;;) { + const char *func, *data; + int flags; + unsigned long err = ERR_get_error_all(&file, &line, &func, + &data, &flags); + if (err == 0U) { + break; + } + + char buf[256]; + ERR_error_string_n(err, buf, sizeof(buf)); + + isc_log_write(category, module, ISC_LOG_INFO, "%s:%s:%d:%s", + buf, file, line, + ((flags & ERR_TXT_STRING) != 0) ? data : ""); + } + +done: + ERR_clear_error(); + return result; +} diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 7477a1b9de..c4a3fd5ee9 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -1550,80 +1550,3 @@ isc_tls_valid_sni_hostname(const char *hostname) { return true; } - -static isc_result_t -isc__tls_toresult(isc_result_t fallback) { - isc_result_t result = fallback; - unsigned long err = ERR_peek_error(); -#if defined(ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) - int lib = ERR_GET_LIB(err); -#endif /* if defined(ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) */ - int reason = ERR_GET_REASON(err); - - switch (reason) { - /* - * ERR_* errors are globally unique; others - * are unique per sublibrary - */ - case ERR_R_MALLOC_FAILURE: - result = ISC_R_NOMEMORY; - break; - default: -#if defined(ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) - if (lib == ERR_R_ECDSA_LIB && - reason == ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) - { - result = ISC_R_NOENTROPY; - break; - } -#endif /* if defined(ECDSA_R_RANDOM_NUMBER_GENERATION_FAILED) */ - break; - } - - return result; -} - -isc_result_t -isc__tlserr2result(isc_logcategory_t category, isc_logmodule_t module, - const char *funcname, isc_result_t fallback, - const char *file, int line) { - isc_result_t result = isc__tls_toresult(fallback); - - /* - * This is an exception - normally, we don't allow this, but the - * compatibility shims in dst_openssl.h needs a call that just - * translates the error code and don't do any logging. - */ - if (category == ISC_LOGCATEGORY_INVALID) { - goto done; - } - - isc_log_write(category, module, ISC_LOG_WARNING, - "%s (%s:%d) failed (%s)", funcname, file, line, - isc_result_totext(result)); - - if (result == ISC_R_NOMEMORY) { - goto done; - } - - for (;;) { - const char *func, *data; - int flags; - unsigned long err = ERR_get_error_all(&file, &line, &func, - &data, &flags); - if (err == 0U) { - break; - } - - char buf[256]; - ERR_error_string_n(err, buf, sizeof(buf)); - - isc_log_write(category, module, ISC_LOG_INFO, "%s:%s:%d:%s", - buf, file, line, - ((flags & ERR_TXT_STRING) != 0) ? data : ""); - } - -done: - ERR_clear_error(); - return result; -}