From 290896921d44e40a54dc5b0e4e2aa37ff528b512 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 13 Jul 2023 14:39:06 +1000 Subject: [PATCH 01/20] Clear OpenSSL errors on ECDSA_SIG_new failures (cherry picked from commit eafcd4112035ae736a1d899e2c4266d09d94ed43) --- lib/dns/opensslecdsa_link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index 3422f3beba..c83ba5c919 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -357,7 +357,7 @@ opensslecdsa_verify(dst_context_t *dctx, const isc_region_t *sig) { ecdsasig = ECDSA_SIG_new(); if (ecdsasig == NULL) { - DST_RET(ISC_R_NOMEMORY); + DST_RET(dst__openssl_toresult(ISC_R_NOMEMORY)); } r = BN_bin2bn(cp, siglen / 2, NULL); cp += siglen / 2; From fb503aa27587b9ae935e4554e03fca2359bcc6ae Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 14:00:29 +1000 Subject: [PATCH 02/20] Clear OpenSSL errors on EVP_MD_CTX_create failures (cherry picked from commit 8529be30bbbb65f8e1661466cd5c3bab2422d7a7) --- lib/dns/opensslecdsa_link.c | 2 +- lib/dns/opensslrsa_link.c | 2 +- util/gen-rsa-sha-vectors.c | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index c83ba5c919..d468492f3d 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -175,7 +175,7 @@ opensslecdsa_createctx(dst_key_t *key, dst_context_t *dctx) { evp_md_ctx = EVP_MD_CTX_create(); if (evp_md_ctx == NULL) { - DST_RET(ISC_R_NOMEMORY); + DST_RET(dst__openssl_toresult(ISC_R_NOMEMORY)); } if (dctx->key->key_alg == DST_ALG_ECDSA256) { type = EVP_sha256(); diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index f1af259959..17b8368379 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -88,7 +88,7 @@ opensslrsa_createctx(dst_key_t *key, dst_context_t *dctx) { evp_md_ctx = EVP_MD_CTX_create(); if (evp_md_ctx == NULL) { - return (ISC_R_NOMEMORY); + return (dst__openssl_toresult(ISC_R_NOMEMORY)); } switch (dctx->key->key_alg) { diff --git a/util/gen-rsa-sha-vectors.c b/util/gen-rsa-sha-vectors.c index 7f76036b84..4d4d5137a5 100644 --- a/util/gen-rsa-sha-vectors.c +++ b/util/gen-rsa-sha-vectors.c @@ -51,6 +51,7 @@ main() { unsigned int siglen = sizeof(buf); if (e == NULL || n == NULL || ctx == NULL || evp_md_ctx == NULL) { + ERR_clear_error(); return (1); } @@ -62,11 +63,13 @@ main() { EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, e) != 1 || EVP_PKEY_keygen(ctx, &pkey) != 1 || pkey == NULL) { + ERR_clear_error(); return (1); } EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_N, &n); if (n == NULL) { + ERR_clear_error(); return (1); } @@ -90,6 +93,7 @@ main() { EVP_DigestUpdate(evp_md_ctx, "test", 4) != 1 || EVP_SignFinal(evp_md_ctx, buf, &siglen, pkey) != 1) { + ERR_clear_error(); return (1); } bytes = siglen; @@ -103,6 +107,7 @@ main() { EVP_DigestUpdate(evp_md_ctx, "test", 4) != 1 || EVP_SignFinal(evp_md_ctx, buf, &siglen, pkey) != 1) { + ERR_clear_error(); return (1); } bytes = siglen; @@ -116,6 +121,7 @@ main() { EVP_DigestUpdate(evp_md_ctx, "test", 4) != 1 || EVP_SignFinal(evp_md_ctx, buf, &siglen, pkey) != 1) { + ERR_clear_error(); return (1); } bytes = siglen; From ababcd28c102633c7c4512f6b73b8e80a14016f4 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jul 2023 17:25:17 +1000 Subject: [PATCH 03/20] Clear OpenSSL errors on EVP_PKEY_get_bn_param failures (cherry picked from commit d8a9adc821e40d9a959b7ab557d3235849c6a3f0) --- lib/dns/opensslrsa_link.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index 17b8368379..20df37fbdb 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -281,6 +281,7 @@ opensslrsa_compare(const dst_key_t *key1, const dst_key_t *key2) { #else EVP_PKEY_get_bn_param(pkey1, OSSL_PKEY_PARAM_RSA_D, &d1); EVP_PKEY_get_bn_param(pkey2, OSSL_PKEY_PARAM_RSA_D, &d2); + ERR_clear_error(); #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */ if (d1 != NULL || d2 != NULL) { @@ -296,6 +297,7 @@ opensslrsa_compare(const dst_key_t *key1, const dst_key_t *key2) { EVP_PKEY_get_bn_param(pkey1, OSSL_PKEY_PARAM_RSA_FACTOR2, &q1); EVP_PKEY_get_bn_param(pkey2, OSSL_PKEY_PARAM_RSA_FACTOR1, &p2); EVP_PKEY_get_bn_param(pkey2, OSSL_PKEY_PARAM_RSA_FACTOR2, &q2); + ERR_clear_error(); #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */ if (BN_cmp(d1, d2) != 0 || BN_cmp(p1, p2) != 0 || @@ -543,6 +545,8 @@ opensslrsa_isprivate(const dst_key_t *key) { d != NULL); if (d != NULL) { BN_clear_free(d); + } else { + ERR_clear_error(); } #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */ @@ -834,6 +838,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT1, &dmp1); EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_EXPONENT2, &dmq1); EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &iqmp); + ERR_clear_error(); #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */ if (n == NULL || e == NULL) { From 386e88d0e4bd7a56a93213932f2761f87ef6eee8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 13:44:55 +1000 Subject: [PATCH 04/20] Clear OpenSSL errors on EVP_PKEY_get0_EC_KEY failures (cherry picked from commit abd8c035923a8f8c34324b707e09e85d375a8eee) --- lib/dns/opensslecdsa_link.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index d468492f3d..5f24967db1 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -439,8 +439,10 @@ opensslecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) { eckey1 = EVP_PKEY_get1_EC_KEY(pkey1); eckey2 = EVP_PKEY_get1_EC_KEY(pkey2); if (eckey1 == NULL && eckey2 == NULL) { + ERR_clear_error(); DST_RET(true); } else if (eckey1 == NULL || eckey2 == NULL) { + ERR_clear_error(); DST_RET(false); } priv1 = EC_KEY_get0_private_key(eckey1); @@ -616,6 +618,8 @@ opensslecdsa_isprivate(const dst_key_t *key) { ret = (eckey != NULL && EC_KEY_get0_private_key(eckey) != NULL); if (eckey != NULL) { EC_KEY_free(eckey); + } else { + ERR_clear_error(); } #else ret = (EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_PRIV_KEY, &priv) == From 4d37996b1ad1ae1af2615354136f451a80561bf5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 31 Aug 2023 10:57:43 +1000 Subject: [PATCH 05/20] Clear OpenSSL errors on EC_KEY_get0_private_key failures (cherry picked from commit 86b04368b015fa8890c070ef06abb524902a843f) --- lib/dns/opensslecdsa_link.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index 5f24967db1..13cbb07c68 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -455,8 +455,11 @@ opensslecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) { if (priv1 != NULL || priv2 != NULL) { if (priv1 == NULL || priv2 == NULL || BN_cmp(priv1, priv2) != 0) { + ERR_clear_error(); DST_RET(false); } + } else { + ERR_clear_error(); } ret = true; From b5b13771f2654c476f916c81299166ccc15efe94 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 13:25:47 +1000 Subject: [PATCH 06/20] Clear OpenSSL errors on EVP_PKEY_new failures (cherry picked from commit 6df53cdb8757b287023a8caa32b0d9757e4eeb0a) --- bin/tests/system/rsabigexponent/bigkey.c | 2 ++ lib/dns/opensslecdsa_link.c | 6 +++--- lib/isc/hmac.c | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/rsabigexponent/bigkey.c b/bin/tests/system/rsabigexponent/bigkey.c index c1fb2c8f15..c096e547ba 100644 --- a/bin/tests/system/rsabigexponent/bigkey.c +++ b/bin/tests/system/rsabigexponent/bigkey.c @@ -84,6 +84,7 @@ main(int argc, char **argv) { !EVP_PKEY_set1_RSA(pkey, rsa)) { fprintf(stderr, "fatal error: basic OpenSSL failure\n"); + ERR_clear_error(); exit(1); } @@ -99,6 +100,7 @@ main(int argc, char **argv) { "fatal error: RSA_generate_key_ex() fails " "at file %s line %d\n", __FILE__, __LINE__); + ERR_clear_error(); exit(1); } diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index 13cbb07c68..d994387931 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -525,7 +525,7 @@ opensslecdsa_generate(dst_key_t *key, int unused, void (*callback)(int)) { pkey = EVP_PKEY_new(); if (pkey == NULL) { - DST_RET(ISC_R_NOMEMORY); + DST_RET(dst__openssl_toresult(ISC_R_NOMEMORY)); } if (!EVP_PKEY_set1_EC_KEY(pkey, eckey)) { DST_RET(ISC_R_FAILURE); @@ -794,7 +794,7 @@ opensslecdsa_fromdns(dst_key_t *key, isc_buffer_t *data) { pkey = EVP_PKEY_new(); if (pkey == NULL) { - DST_RET(ISC_R_NOMEMORY); + DST_RET(dst__openssl_toresult(ISC_R_NOMEMORY)); } if (!EVP_PKEY_set1_EC_KEY(pkey, eckey)) { EVP_PKEY_free(pkey); @@ -1105,7 +1105,7 @@ eckey_to_pkey(EC_KEY *eckey, EVP_PKEY **pkey) { *pkey = EVP_PKEY_new(); if (*pkey == NULL) { - return (ISC_R_NOMEMORY); + return (dst__openssl_toresult(ISC_R_NOMEMORY)); } if (!EVP_PKEY_set1_EC_KEY(*pkey, eckey)) { EVP_PKEY_free(*pkey); diff --git a/lib/isc/hmac.c b/lib/isc/hmac.c index 8fce30bda9..15a217f218 100644 --- a/lib/isc/hmac.c +++ b/lib/isc/hmac.c @@ -55,6 +55,7 @@ isc_hmac_init(isc_hmac_t *hmac, const void *key, const size_t keylen, pkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_HMAC, NULL, key, keylen); if (pkey == NULL) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } From aca6f3e82d427bccd47f2b1b5e88dbb87c93a590 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 14:10:49 +1000 Subject: [PATCH 07/20] Clear OpenSSL errors on EVP failures (cherry picked from commit 4ea926934a8d08cece0469406357bbd9fd8492d8) --- lib/isc/hmac.c | 5 +++++ lib/isc/iterated_hash.c | 3 ++- lib/isc/md.c | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/isc/hmac.c b/lib/isc/hmac.c index 15a217f218..bc35befc1e 100644 --- a/lib/isc/hmac.c +++ b/lib/isc/hmac.c @@ -11,6 +11,7 @@ * information regarding copyright ownership. */ +#include #include #include @@ -61,6 +62,7 @@ isc_hmac_init(isc_hmac_t *hmac, const void *key, const size_t keylen, if (EVP_DigestSignInit(hmac, NULL, md_type, NULL, pkey) != 1) { EVP_PKEY_free(pkey); + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -74,6 +76,7 @@ isc_hmac_reset(isc_hmac_t *hmac) { REQUIRE(hmac != NULL); if (EVP_MD_CTX_reset(hmac) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -89,6 +92,7 @@ isc_hmac_update(isc_hmac_t *hmac, const unsigned char *buf, const size_t len) { } if (EVP_DigestSignUpdate(hmac, buf, len) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -105,6 +109,7 @@ isc_hmac_final(isc_hmac_t *hmac, unsigned char *digest, size_t len = *digestlen; if (EVP_DigestSignFinal(hmac, digest, &len) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } diff --git a/lib/isc/iterated_hash.c b/lib/isc/iterated_hash.c index de1f3a36a3..80d219c56b 100644 --- a/lib/isc/iterated_hash.c +++ b/lib/isc/iterated_hash.c @@ -13,6 +13,7 @@ #include +#include #include #include @@ -127,7 +128,7 @@ isc_iterated_hash(unsigned char *out, const unsigned int hashalg, fail: EVP_MD_CTX_free(ctx); EVP_MD_free(md); - + ERR_clear_error(); return (0); } diff --git a/lib/isc/md.c b/lib/isc/md.c index d094cfa0ec..53ce2c16c7 100644 --- a/lib/isc/md.c +++ b/lib/isc/md.c @@ -47,6 +47,7 @@ isc_md_init(isc_md_t *md, const isc_md_type_t *md_type) { } if (EVP_DigestInit_ex(md, md_type, NULL) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -58,6 +59,7 @@ isc_md_reset(isc_md_t *md) { REQUIRE(md != NULL); if (EVP_MD_CTX_reset(md) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -73,6 +75,7 @@ isc_md_update(isc_md_t *md, const unsigned char *buf, const size_t len) { } if (EVP_DigestUpdate(md, buf, len) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } @@ -85,6 +88,7 @@ isc_md_final(isc_md_t *md, unsigned char *digest, unsigned int *digestlen) { REQUIRE(digest != NULL); if (EVP_DigestFinal_ex(md, digest, digestlen) != 1) { + ERR_clear_error(); return (ISC_R_CRYPTOFAILURE); } From 34a0bb146cc20d92ed9c832929c579c87ecb37be Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 17:59:13 +1000 Subject: [PATCH 08/20] Clear OpenSSL errors on engine errors (cherry picked from commit 2ba62aebceadd299910927c3caa923f8644c4f9f) --- lib/dns/openssl_link.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/dns/openssl_link.c b/lib/dns/openssl_link.c index c08070717a..addf16ad98 100644 --- a/lib/dns/openssl_link.c +++ b/lib/dns/openssl_link.c @@ -104,6 +104,7 @@ cleanup_rm: ENGINE_free(e); } e = NULL; + ERR_clear_error(); #else UNUSED(engine); #endif /* if !defined(OPENSSL_NO_ENGINE) && OPENSSL_API_LEVEL < 30000 */ From 900efd613f18bd75168fc183ae97bcbe99da49a7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 13 Jul 2023 14:41:00 +1000 Subject: [PATCH 09/20] Clear OpenSSL errors on SHA failures (cherry picked from commit 247422c69fe7a865408e3ad099c37f4c5db32027) --- lib/isc/iterated_hash.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/isc/iterated_hash.c b/lib/isc/iterated_hash.c index 80d219c56b..ad623814e6 100644 --- a/lib/isc/iterated_hash.c +++ b/lib/isc/iterated_hash.c @@ -44,18 +44,22 @@ isc_iterated_hash(unsigned char *out, const unsigned int hashalg, do { if (SHA1_Init(&ctx) != 1) { + ERR_clear_error(); return (0); } if (SHA1_Update(&ctx, buf, len) != 1) { + ERR_clear_error(); return (0); } if (SHA1_Update(&ctx, salt, saltlength) != 1) { + ERR_clear_error(); return (0); } if (SHA1_Final(out, &ctx) != 1) { + ERR_clear_error(); return (0); } From 0111782f1eb46e67c40106a06531678327ae93ef Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jul 2023 17:24:34 +1000 Subject: [PATCH 10/20] Clear OpenSSL errors on context creation failures (cherry picked from commit 96db614d69b72cfb0951b5a4f14d08860274decb) --- lib/dns/openssleddsa_link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/openssleddsa_link.c b/lib/dns/openssleddsa_link.c index f89672b26e..0fddfd4517 100644 --- a/lib/dns/openssleddsa_link.c +++ b/lib/dns/openssleddsa_link.c @@ -221,7 +221,7 @@ openssleddsa_verify(dst_context_t *dctx, const isc_region_t *sig) { key->key_alg == DST_ALG_ED448); if (ctx == NULL) { - return (ISC_R_NOMEMORY); + return (dst__openssl_toresult(ISC_R_NOMEMORY)); } #if HAVE_OPENSSL_ED25519 From 894b0970e6418dac18649de6c62541f6e28121d4 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 20 Jul 2023 13:32:50 +1000 Subject: [PATCH 11/20] Clear OpenSSL errors on TSL error paths (cherry picked from commit 4f790b6c585eb9f2c81c887633d11b970e8320e0) --- lib/isc/tls.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 978f86dd99..8fd05254ac 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -961,6 +961,7 @@ isc_tlsctx_enable_peer_verification(isc_tlsctx_t *tlsctx, const bool is_server, ret = X509_VERIFY_PARAM_set1_host(param, hostname, 0); } if (ret != 1) { + ERR_clear_error(); return (ISC_R_FAILURE); } @@ -1011,6 +1012,7 @@ isc_tlsctx_load_client_ca_names(isc_tlsctx_t *ctx, const char *ca_bundle_file) { cert_names = SSL_load_client_CA_file(ca_bundle_file); if (cert_names == NULL) { + ERR_clear_error(); return (ISC_R_FAILURE); } @@ -1051,6 +1053,7 @@ isc_tls_cert_store_create(const char *ca_bundle_filename, return (ISC_R_SUCCESS); error: + ERR_clear_error(); if (store != NULL) { X509_STORE_free(store); } @@ -1531,6 +1534,7 @@ isc_tlsctx_client_session_cache_keep(isc_tlsctx_client_session_cache_t *cache, sess = SSL_get1_session(tls); if (sess == NULL) { + ERR_clear_error(); return; } else if (!ssl_session_seems_resumable(sess)) { SSL_SESSION_free(sess); From 74f9d749bfe1c3aceab784791de6ce0ce1c8cf36 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 31 Aug 2023 11:03:38 +1000 Subject: [PATCH 12/20] Clear OpenSSL error stack when exiting (cherry picked from commit eaedba6621d76506a9a6f470e4c52fe930aebc7a) --- bin/tests/system/rsabigexponent/bigkey.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/tests/system/rsabigexponent/bigkey.c b/bin/tests/system/rsabigexponent/bigkey.c index c096e547ba..19fc9bad08 100644 --- a/bin/tests/system/rsabigexponent/bigkey.c +++ b/bin/tests/system/rsabigexponent/bigkey.c @@ -67,6 +67,7 @@ EVP_PKEY *pkey; "%d\n", \ msg, isc_result_totext(result), __FILE__, \ __LINE__); \ + ERR_clear_error(); \ exit(1); \ } \ } while (0) From 788a8a7c4d386526f9c16095de83a11e7a1d381f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 30 Aug 2023 12:01:32 +1000 Subject: [PATCH 13/20] Free evp_md_ctx and pkey at exit (cherry picked from commit 936b73cb57b1df474c14b35df6a8ad5e406804c1) --- util/gen-rsa-sha-vectors.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/util/gen-rsa-sha-vectors.c b/util/gen-rsa-sha-vectors.c index 4d4d5137a5..36e5f86036 100644 --- a/util/gen-rsa-sha-vectors.c +++ b/util/gen-rsa-sha-vectors.c @@ -131,5 +131,8 @@ main() { } printf("\";\n\n"); + EVP_MD_CTX_free(evp_md_ctx); + EVP_PKEY_free(pkey); + return (0); } From 4c27f804768584ba74dba78bf2f0d93552c80f2b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 30 Aug 2023 15:31:34 +1000 Subject: [PATCH 14/20] Add missing "Design by Contract" REQUIREs (cherry picked from commit b442ae8d3e40cba1d89c3670f7df2b7084a7f3e5) --- lib/dns/opensslrsa_link.c | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index 20df37fbdb..dc7382c4c1 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -54,6 +54,7 @@ opensslrsa_createctx(dst_key_t *key, dst_context_t *dctx) { const EVP_MD *type = NULL; UNUSED(key); + REQUIRE(dctx != NULL && dctx->key != NULL); REQUIRE(dctx->key->key_alg == DST_ALG_RSASHA1 || dctx->key->key_alg == DST_ALG_NSEC3RSASHA1 || dctx->key->key_alg == DST_ALG_RSASHA256 || @@ -118,13 +119,16 @@ opensslrsa_createctx(dst_key_t *key, dst_context_t *dctx) { static void opensslrsa_destroyctx(dst_context_t *dctx) { - EVP_MD_CTX *evp_md_ctx = dctx->ctxdata.evp_md_ctx; + EVP_MD_CTX *evp_md_ctx = NULL; + REQUIRE(dctx != NULL && dctx->key != NULL); REQUIRE(dctx->key->key_alg == DST_ALG_RSASHA1 || dctx->key->key_alg == DST_ALG_NSEC3RSASHA1 || dctx->key->key_alg == DST_ALG_RSASHA256 || dctx->key->key_alg == DST_ALG_RSASHA512); + evp_md_ctx = dctx->ctxdata.evp_md_ctx; + if (evp_md_ctx != NULL) { EVP_MD_CTX_destroy(evp_md_ctx); dctx->ctxdata.evp_md_ctx = NULL; @@ -133,13 +137,16 @@ opensslrsa_destroyctx(dst_context_t *dctx) { static isc_result_t opensslrsa_adddata(dst_context_t *dctx, const isc_region_t *data) { - EVP_MD_CTX *evp_md_ctx = dctx->ctxdata.evp_md_ctx; + EVP_MD_CTX *evp_md_ctx = NULL; + REQUIRE(dctx != NULL && dctx->key != NULL); REQUIRE(dctx->key->key_alg == DST_ALG_RSASHA1 || dctx->key->key_alg == DST_ALG_NSEC3RSASHA1 || dctx->key->key_alg == DST_ALG_RSASHA256 || dctx->key->key_alg == DST_ALG_RSASHA512); + evp_md_ctx = dctx->ctxdata.evp_md_ctx; + if (!EVP_DigestUpdate(evp_md_ctx, data->base, data->length)) { return (dst__openssl_toresult3( dctx->category, "EVP_DigestUpdate", ISC_R_FAILURE)); @@ -149,17 +156,22 @@ opensslrsa_adddata(dst_context_t *dctx, const isc_region_t *data) { static isc_result_t opensslrsa_sign(dst_context_t *dctx, isc_buffer_t *sig) { - dst_key_t *key = dctx->key; + dst_key_t *key = NULL; isc_region_t r; unsigned int siglen = 0; - EVP_MD_CTX *evp_md_ctx = dctx->ctxdata.evp_md_ctx; - EVP_PKEY *pkey = key->keydata.pkey; + EVP_MD_CTX *evp_md_ctx = NULL; + EVP_PKEY *pkey = NULL; + REQUIRE(dctx != NULL && dctx->key != NULL); REQUIRE(dctx->key->key_alg == DST_ALG_RSASHA1 || dctx->key->key_alg == DST_ALG_NSEC3RSASHA1 || dctx->key->key_alg == DST_ALG_RSASHA256 || dctx->key->key_alg == DST_ALG_RSASHA512); + key = dctx->key; + evp_md_ctx = dctx->ctxdata.evp_md_ctx; + pkey = key->keydata.pkey; + isc_buffer_availableregion(sig, &r); if (r.length < (unsigned int)EVP_PKEY_size(pkey)) { @@ -178,7 +190,7 @@ opensslrsa_sign(dst_context_t *dctx, isc_buffer_t *sig) { static isc_result_t opensslrsa_verify2(dst_context_t *dctx, int maxbits, const isc_region_t *sig) { - dst_key_t *key = dctx->key; + dst_key_t *key = NULL; int status = 0; #if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 RSA *rsa; @@ -186,15 +198,20 @@ opensslrsa_verify2(dst_context_t *dctx, int maxbits, const isc_region_t *sig) { #else BIGNUM *e = NULL; #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 */ - EVP_MD_CTX *evp_md_ctx = dctx->ctxdata.evp_md_ctx; - EVP_PKEY *pkey = key->keydata.pkey; + EVP_MD_CTX *evp_md_ctx = NULL; + EVP_PKEY *pkey = NULL; int bits; + REQUIRE(dctx != NULL && dctx->key != NULL); REQUIRE(dctx->key->key_alg == DST_ALG_RSASHA1 || dctx->key->key_alg == DST_ALG_NSEC3RSASHA1 || dctx->key->key_alg == DST_ALG_RSASHA256 || dctx->key->key_alg == DST_ALG_RSASHA512); + key = dctx->key; + evp_md_ctx = dctx->ctxdata.evp_md_ctx; + pkey = key->keydata.pkey; + #if OPENSSL_VERSION_NUMBER < 0x30000000L || OPENSSL_API_LEVEL < 30000 rsa = EVP_PKEY_get1_RSA(pkey); if (rsa == NULL) { @@ -1119,18 +1136,21 @@ opensslrsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { const BIGNUM *ex = NULL; ENGINE *ep = NULL; #endif /* if !defined(OPENSSL_NO_ENGINE) && OPENSSL_API_LEVEL < 30000 */ - isc_mem_t *mctx = key->mctx; + isc_mem_t *mctx = NULL; const char *engine = NULL, *label = NULL; EVP_PKEY *pkey = NULL; BIGNUM *n = NULL, *e = NULL, *d = NULL; BIGNUM *p = NULL, *q = NULL; BIGNUM *dmp1 = NULL, *dmq1 = NULL, *iqmp = NULL; + REQUIRE(key != NULL); REQUIRE(key->key_alg == DST_ALG_RSASHA1 || key->key_alg == DST_ALG_NSEC3RSASHA1 || key->key_alg == DST_ALG_RSASHA256 || key->key_alg == DST_ALG_RSASHA512); + mctx = key->mctx; + /* read private key file */ ret = dst__privstruct_parse(key, DST_ALG_RSA, lexer, mctx, &priv); if (ret != ISC_R_SUCCESS) { From f77ffa79534d5148c4a80c59859d715a5ea2856f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 20 Jul 2023 13:10:29 +1000 Subject: [PATCH 15/20] Take ownership of pointer before freeing (cherry picked from commit 9e2288208de3d3eabac89ee5158f4b94e8ade00f) --- lib/isc/tls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 8fd05254ac..e2cd63eeb0 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -762,10 +762,12 @@ isc_tls_create(isc_tlsctx_t *ctx) { void isc_tls_free(isc_tls_t **tlsp) { + isc_tls_t *tls = NULL; REQUIRE(tlsp != NULL && *tlsp != NULL); - SSL_free(*tlsp); + tls = *tlsp; *tlsp = NULL; + SSL_free(tls); } const char * From 29a93d2889baa74b6cafa51ee17ec7596bc78013 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 11 Jul 2023 17:57:25 +1000 Subject: [PATCH 16/20] Check that buf is large enough (cherry picked from commit 299f519b09a18d9093815999c6b39664c6aaf81e) --- lib/dns/opensslecdsa_link.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/opensslecdsa_link.c b/lib/dns/opensslecdsa_link.c index d994387931..b80d637383 100644 --- a/lib/dns/opensslecdsa_link.c +++ b/lib/dns/opensslecdsa_link.c @@ -258,6 +258,8 @@ static int BN_bn2bin_fixed(const BIGNUM *bn, unsigned char *buf, int size) { int bytes = size - BN_num_bytes(bn); + INSIST(bytes >= 0); + while (bytes-- > 0) { *buf++ = 0; } From 91a6885a0180a11b532ca9cb6c7c5558b5dd663d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 20 Jul 2023 13:31:16 +1000 Subject: [PATCH 17/20] Style fix (cherry picked from commit b6e16504557dcc175b5bd86d5162a88f3c24858e) --- tests/isc/hmac_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/isc/hmac_test.c b/tests/isc/hmac_test.c index d980a5a0e9..093c6b248f 100644 --- a/tests/isc/hmac_test.c +++ b/tests/isc/hmac_test.c @@ -195,7 +195,7 @@ ISC_RUN_TEST_IMPL(isc_hmac_reset) { * so this could be only manually checked that the test will * segfault when called by hand */ - expect_assert_failure(isc_hmac_final(hmac,digest,&digestlen)); + expect_assert_failure(isc_hmac_final(hmac, digest, &digestlen)); #endif /* if 0 */ } From e10dfc2e2d3df38765e058958374d0da3b8b9525 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 28 Jun 2023 16:28:57 +1000 Subject: [PATCH 18/20] Detect uncleared libcrypto errors in rdata processing If libcrypto errors are not cleared slow memory leaks occur which are not detected at shutdown. (cherry picked from commit 14727bb4b9a62304255846d2b4cc85774f202133) --- tests/dns/Makefile.am | 12 ++++++++++-- tests/dns/rdata_test.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/dns/Makefile.am b/tests/dns/Makefile.am index d37caa5662..7062c6018e 100644 --- a/tests/dns/Makefile.am +++ b/tests/dns/Makefile.am @@ -7,8 +7,8 @@ AM_CPPFLAGS += \ $(KRB5_CFLAGS) \ -DSRCDIR=\"$(abs_srcdir)\" \ -DBUILDDIR=\"$(abs_builddir)\" \ - -I$(top_srcdir)/lib/isc \ - -I$(top_srcdir)/lib/dns + -I$(top_srcdir)/lib/dns \ + -I$(top_srcdir)/lib/isc LDADD += \ $(LIBISC_LIBS) \ @@ -106,6 +106,14 @@ rsa_test_CPPFLAGS = \ $(AM_CPPFLAGS) \ $(OPENSSL_CFLAGS) +rdata_test_CPPFLAGS = \ + $(AM_CPPFLAGS) \ + $(OPENSSL_CFLAGS) + +rdata_test_LDADD = \ + $(LDADD) \ + $(OPENSSL_LIBS) + EXTRA_sigs_test_DEPENDENCIES = testdata/master/master18.data CLEANFILES += $(EXTRA_sigs_test_DEPENDENCIES) diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c index a11bbbeed6..73e45262f5 100644 --- a/tests/dns/rdata_test.c +++ b/tests/dns/rdata_test.c @@ -23,6 +23,10 @@ #define UNIT_TESTING +#include + +#include + #include #include #include @@ -122,6 +126,23 @@ typedef struct wire_ok { #define WIRE_INVALID(FIRST, ...) WIRE_TEST(false, 0, FIRST, __VA_ARGS__) #define WIRE_SENTINEL() WIRE_TEST(false, 0) +static void +detect_uncleared_libcrypto_error(void) { + const char *file, *func, *data; + int line, flags; + long err; + bool leak = false; + while ((err = ERR_get_error_all(&file, &line, &func, &data, &flags)) != + 0L) + { + fprintf(stderr, + "# Uncleared libcrypto error: %s:%d %s %s %ld %x\n", + file, line, func, data, err, flags); + leak = true; + } + assert_false(leak); +} + /* * Call dns_rdata_fromwire() for data in 'src', which is 'srclen' octets in * size and represents RDATA of given 'type' and 'class'. Store the resulting @@ -155,6 +176,7 @@ wire_to_rdata(const unsigned char *src, size_t srclen, dns_rdataclass_t rdclass, result = dns_rdata_fromwire(rdata, rdclass, type, &source, &dctx, 0, &target); dns_decompress_invalidate(&dctx); + detect_uncleared_libcrypto_error(); return (result); } @@ -179,6 +201,7 @@ rdata_towire(dns_rdata_t *rdata, unsigned char *dst, size_t dstlen, */ dns_compress_init(&cctx, -1, mctx); result = dns_rdata_towire(rdata, &cctx, &target); + detect_uncleared_libcrypto_error(); dns_compress_invalidate(&cctx); *length = isc_buffer_usedlength(&target); @@ -270,6 +293,7 @@ check_struct_conversions(dns_rdata_t *rdata, size_t structsize, * Convert from uncompressed wire form into type-specific struct. */ result = dns_rdata_tostruct(rdata, rdata_struct, NULL); + detect_uncleared_libcrypto_error(); assert_int_equal(result, ISC_R_SUCCESS); /* @@ -402,6 +426,7 @@ check_text_ok_single(const text_ok_t *text_ok, dns_rdataclass_t rdclass, */ isc_buffer_init(&target, buf_totext, sizeof(buf_totext)); result = dns_rdata_totext(&rdata, NULL, &target); + detect_uncleared_libcrypto_error(); if (result != ISC_R_SUCCESS && debug) { size_t i; fprintf(stdout, "# dns_rdata_totext -> %s", @@ -490,6 +515,7 @@ check_text_conversions(dns_rdata_t *rdata) { */ isc_buffer_init(&target, buf_totext, sizeof(buf_totext)); result = dns_rdata_totext(rdata, NULL, &target); + detect_uncleared_libcrypto_error(); assert_int_equal(result, ISC_R_SUCCESS); /* * Ensure buf_totext is properly NUL terminated as dns_rdata_totext() @@ -543,6 +569,7 @@ check_multiline_text_conversions(dns_rdata_t *rdata) { flags = dns_master_styleflags(&dns_master_style_default); result = dns_rdata_tofmttext(rdata, dns_rootname, flags, 80 - 32, 4, "\n", &target); + detect_uncleared_libcrypto_error(); assert_int_equal(result, ISC_R_SUCCESS); /* * Ensure buf_totext is properly NUL terminated as @@ -710,6 +737,7 @@ check_compare_ok_single(const compare_ok_t *compare_ok, } answer = dns_rdata_compare(&rdata1, &rdata2); + detect_uncleared_libcrypto_error(); if (compare_ok->answer == 0 && answer != 0) { fail_msg("# line %d: dns_rdata_compare('%s', '%s'): " "expected equal, got %s", From 473538368037f3a730bb4870a3e2d782b8e4d0bf Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 28 Jun 2023 11:58:22 +1000 Subject: [PATCH 19/20] Add CHANGES note for [GL #4159] (cherry picked from commit 6c3d4d7aa24ced972f61a541c6a783c58b97ad43) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index b53ad25384..89a13719f6 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6237. [bug] Address memory leaks due to not clearing OpenSSL error + stack. [GL #4159] + 6234. [bug] Restore stale-refresh-time value after flushing the cache. [GL #4278] From 3c2704e994bacfde0063f4d6cbc9e5f3a202dd87 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 28 Jun 2023 12:01:58 +1000 Subject: [PATCH 20/20] Add release note for [GL #4159] (cherry picked from commit 6a1a73759a51e38752970892821b24e137ba5465) --- doc/notes/notes-current.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 435d928973..24f2032934 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -46,9 +46,13 @@ Bug Fixes This issue was reported independently by Eric Sesterhenn of X41 D-SEC and Cameron Whitehead. -- The value of Content-Length header in statistics channel was not bound checked - and negative or large enough value could lead to overflow and assertion failure. - :gl:`#4125` +- The value of Content-Length header in statistics channel was not + bound checked and negative or large enough value could lead to + overflow and assertion failure. :gl:`#4125` + + This issue was reported by Eric Sesterhenn of X41 D-SEC. + +- Address memory leaks due to not clearing OpenSSL error stack. :gl:`#4159` This issue was reported by Eric Sesterhenn of X41 D-SEC.