From 724bd414bbf21d4ba7263aa5062246253c56170d Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Tue, 28 May 2024 15:42:03 +0000 Subject: Fix ENGINE use with OpenSSL 3.2, which appears to be broken due to a refcounting issue in mod_ssl. * modules/ssl/ssl_engine_pphrase.c (modssl_engine_cleanup): New function. (modssl_load_keypair_engine): Take pconf & ptemp arguments, don't call ENGINE_finish() immediately but register the above cleanup. (modssl_load_engine_keypair): Pass through pconf & ptemp. * modules/ssl/ssl_engine_init.c (ssl_init_server_certs): Pass through pconf and ptemp to modssl_load_engine_keypair. Github: closes #446 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1918024 13f79535-47bb-0310-9956-ffa450edef68 --- modules/ssl/ssl_engine_init.c | 2 +- modules/ssl/ssl_engine_pphrase.c | 33 +++++++++++++++++++++++++-------- modules/ssl/ssl_private.h | 3 ++- 3 files changed, 28 insertions(+), 10 deletions(-) (limited to 'modules/ssl') diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index ace87522d7..33ef4c4d7b 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -1467,7 +1467,7 @@ static apr_status_t ssl_init_server_certs(server_rec *s, if (modssl_is_engine_id(keyfile)) { apr_status_t rv; - if ((rv = modssl_load_engine_keypair(s, ptemp, vhost_id, + if ((rv = modssl_load_engine_keypair(s, p, ptemp, vhost_id, engine_certfile, keyfile, &cert, &pkey))) { return rv; diff --git a/modules/ssl/ssl_engine_pphrase.c b/modules/ssl/ssl_engine_pphrase.c index 5dd6bdda8d..4167c6f5d6 100644 --- a/modules/ssl/ssl_engine_pphrase.c +++ b/modules/ssl/ssl_engine_pphrase.c @@ -797,7 +797,17 @@ static UI_METHOD *get_passphrase_ui(apr_pool_t *p) #endif #if MODSSL_HAVE_ENGINE_API -static apr_status_t modssl_load_keypair_engine(server_rec *s, apr_pool_t *p, +static apr_status_t modssl_engine_cleanup(void *engine) +{ + ENGINE *e = engine; + + ENGINE_finish(e); + + return APR_SUCCESS; +} + +static apr_status_t modssl_load_keypair_engine(server_rec *s, apr_pool_t *pconf, + apr_pool_t *ptemp, const char *vhostid, const char *certid, const char *keyid, @@ -806,12 +816,12 @@ static apr_status_t modssl_load_keypair_engine(server_rec *s, apr_pool_t *p, { const char *c, *scheme; ENGINE *e; - UI_METHOD *ui_method = get_passphrase_ui(p); + UI_METHOD *ui_method = get_passphrase_ui(ptemp); pphrase_cb_arg_t ppcb; memset(&ppcb, 0, sizeof ppcb); ppcb.s = s; - ppcb.p = p; + ppcb.p = ptemp; ppcb.bPassPhraseDialogOnce = TRUE; ppcb.key_id = vhostid; ppcb.pkey_file = keyid; @@ -824,7 +834,7 @@ static apr_status_t modssl_load_keypair_engine(server_rec *s, apr_pool_t *p, return ssl_die(s); } - scheme = apr_pstrmemdup(p, keyid, c - keyid); + scheme = apr_pstrmemdup(ptemp, keyid, c - keyid); if (!(e = ENGINE_by_id(scheme))) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(10132) "Init: Failed to load engine for private key %s", @@ -873,7 +883,12 @@ static apr_status_t modssl_load_keypair_engine(server_rec *s, apr_pool_t *p, return ssl_die(s); } - ENGINE_finish(e); + /* Release the functional reference obtained by ENGINE_init() only + * when after the ENGINE is no longer used. */ + apr_pool_cleanup_register(pconf, e, modssl_engine_cleanup, modssl_engine_cleanup); + + /* Release the structural reference obtained by ENGINE_by_id() + * immediately. */ ENGINE_free(e); return APR_SUCCESS; @@ -974,7 +989,8 @@ static apr_status_t modssl_load_keypair_store(server_rec *s, apr_pool_t *p, } #endif -apr_status_t modssl_load_engine_keypair(server_rec *s, apr_pool_t *p, +apr_status_t modssl_load_engine_keypair(server_rec *s, + apr_pool_t *pconf, apr_pool_t *ptemp, const char *vhostid, const char *certid, const char *keyid, X509 **pubkey, EVP_PKEY **privkey) @@ -986,11 +1002,12 @@ apr_status_t modssl_load_engine_keypair(server_rec *s, apr_pool_t *p, * support was not present compile-time, or if it's built but * SSLCryptoDevice is not configured. */ if (mc->szCryptoDevice) - return modssl_load_keypair_engine(s, p, vhostid, certid, keyid, + return modssl_load_keypair_engine(s, pconf, ptemp, + vhostid, certid, keyid, pubkey, privkey); #endif #if MODSSL_HAVE_OPENSSL_STORE - return modssl_load_keypair_store(s, p, vhostid, certid, keyid, + return modssl_load_keypair_store(s, ptemp, vhostid, certid, keyid, pubkey, privkey); #else ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(10496) diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 2f8578be81..9cdf0c3754 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -1089,7 +1089,8 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *, apr_pool_t *, int, /* Load public and/or private key from the configured ENGINE. Private * key returned as *pkey. certid can be NULL, in which case *pubkey * is not altered. Errors logged on failure. */ -apr_status_t modssl_load_engine_keypair(server_rec *s, apr_pool_t *p, +apr_status_t modssl_load_engine_keypair(server_rec *s, + apr_pool_t *pconf, apr_pool_t *ptemp, const char *vhostid, const char *certid, const char *keyid, X509 **pubkey, EVP_PKEY **privkey); -- cgit v1.2.3