From: Jim Jagielski Date: Tue, 21 Jan 2014 16:44:48 +0000 (+0000) Subject: Merge r1544774, r1544812 from trunk: X-Git-Tag: 2.4.8~229 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8ee4c2a9d9a84bdef9d4e141614fedb1eee6e432;p=apache Merge r1544774, r1544812 from trunk: Address a todo listed in https://mail-archives.apache.org/mod_mbox/httpd-dev/200205.mbox/%3CPine.LNX.4.33.0205292300380.27841-100000%40mako.covalent.net%3E "init functions should return status code rather than ssl_die()" For diagnostic purposes, ssl_die() is still there, but instead of abruptly exit(1)ing, it will return APR_EGENERAL to the ssl_init_* callers in ssl_engine_init.c, and these will propagate the status back to ssl_init_Module. Followup to r1544774: do not ignore failures from ssl_server_import_{cert,key} in ssl_init_server_certs Submitted by: kbrand Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1560082 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/STATUS b/STATUS index 440cefe624..522949cae6 100644 --- a/STATUS +++ b/STATUS @@ -98,15 +98,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_ssl: address the "init functions should return status code rather - than ssl_die()" todo from 2002 (avoid orphan IPC objects after startup - or reload failures, see also the "ssl_die() and pool cleanup" thread - on httpd-dev in Nov '13) - trunk patches: https://svn.apache.org/r1544774 - https://svn.apache.org/r1544812 - 2.4.x patch: https://people.apache.org/~kbrand/mod_ssl-2.4.x-ssldie.diff - +1: kbrand, trawick, jim - * mod_auth*z scan-build warnings in new code for 2.4.8 trunk patch: https://svn.apache.org/r1558483 2.4.x patch: trunk patch works diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index a07b5737e7..b77a6b7e36 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -59,13 +59,14 @@ static void ssl_add_version_components(apr_pool_t *p, /* * Per-module initialization */ -int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, - apr_pool_t *ptemp, - server_rec *base_server) +apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, + apr_pool_t *ptemp, + server_rec *base_server) { SSLModConfigRec *mc = myModConfig(base_server); SSLSrvConfigRec *sc; server_rec *s; + apr_status_t rv; if (SSLeay() < SSL_LIBRARY_VERSION) { ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(01882) @@ -152,7 +153,9 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, * SSL external crypto device ("engine") support */ #if defined(HAVE_OPENSSL_ENGINE_H) && defined(HAVE_ENGINE_INIT) - ssl_init_Engine(base_server, p); + if ((rv = ssl_init_Engine(base_server, p)) != APR_SUCCESS) { + return rv; + } #endif ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01883) @@ -175,7 +178,7 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, else { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01885) "FIPS mode failed"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } } } @@ -191,7 +194,9 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, * anything that needs to live longer than ptemp needs to also survive * restarts, in which case they'll live inside s->process->pool. */ - ssl_pphrase_Handle(base_server, ptemp); + if ((rv = ssl_pphrase_Handle(base_server, ptemp)) != APR_SUCCESS) { + return rv; + } /* * initialize the mutex handling @@ -206,7 +211,9 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, /* * initialize session caching */ - ssl_scache_init(base_server, p); + if ((rv = ssl_scache_init(base_server, p)) != APR_SUCCESS) { + return rv; + } /* * initialize servers @@ -225,13 +232,17 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, /* * Read the server certificate and key */ - ssl_init_ConfigureServer(s, p, ptemp, sc); + if ((rv = ssl_init_ConfigureServer(s, p, ptemp, sc)) != APR_SUCCESS) { + return rv; + } } /* * Configuration consistency checks */ - ssl_init_CheckServers(base_server, ptemp); + if ((rv = ssl_init_CheckServers(base_server, ptemp)) != APR_SUCCESS) { + return rv; + } /* * Announce mod_ssl and SSL library in HTTP Server field @@ -249,7 +260,7 @@ int ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, * a hardware accellerator card for crypto operations. */ #if defined(HAVE_OPENSSL_ENGINE_H) && defined(HAVE_ENGINE_INIT) -void ssl_init_Engine(server_rec *s, apr_pool_t *p) +apr_status_t ssl_init_Engine(server_rec *s, apr_pool_t *p) { SSLModConfigRec *mc = myModConfig(s); ENGINE *e; @@ -260,7 +271,7 @@ void ssl_init_Engine(server_rec *s, apr_pool_t *p) "Init: Failed to load Crypto Device API `%s'", mc->szCryptoDevice); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } if (strEQ(mc->szCryptoDevice, "chil")) { @@ -272,7 +283,7 @@ void ssl_init_Engine(server_rec *s, apr_pool_t *p) "Init: Failed to enable Crypto Device API `%s'", mc->szCryptoDevice); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01890) "Init: loaded Crypto Device API `%s'", @@ -280,13 +291,15 @@ void ssl_init_Engine(server_rec *s, apr_pool_t *p) ENGINE_free(e); } + + return APR_SUCCESS; } #endif -static void ssl_init_server_check(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_server_check(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { /* * check for important parameters and the @@ -295,7 +308,7 @@ static void ssl_init_server_check(server_rec *s, if (!mctx->pks->cert_files[0]) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01891) "No SSL Certificate set [hint: SSLCertificateFile]"); - ssl_die(s); + return ssl_die(s); } /* @@ -311,16 +324,20 @@ static void ssl_init_server_check(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01892) "Illegal attempt to re-initialise SSL for server " "(SSLEngine On should go in the VirtualHost, not in global scope.)"); - ssl_die(s); + return ssl_die(s); } + + return APR_SUCCESS; } #ifdef HAVE_TLSEXT -static void ssl_init_ctx_tls_extensions(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_tls_extensions(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { + apr_status_t rv; + /* * Configure TLS extensions support */ @@ -337,7 +354,7 @@ static void ssl_init_ctx_tls_extensions(server_rec *s, "Unable to initialize TLS servername extension " "callback (incompatible OpenSSL version?)"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } #ifdef HAVE_OCSP_STAPLING @@ -345,7 +362,9 @@ static void ssl_init_ctx_tls_extensions(server_rec *s, * OCSP Stapling support, status_request extension */ if ((mctx->pkp == FALSE) && (mctx->stapling_enabled == TRUE)) { - modssl_init_stapling(s, p, ptemp, mctx); + if ((rv = modssl_init_stapling(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } } #endif @@ -364,7 +383,7 @@ static void ssl_init_ctx_tls_extensions(server_rec *s, "[%s seed]", mctx->srp_unknown_user_seed ? "with" : "without"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } err = SRP_VBASE_init(mctx->srp_vbase, mctx->srp_vfile); @@ -372,7 +391,7 @@ static void ssl_init_ctx_tls_extensions(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02310) "Unable to load SRP verifier file [error %d]", err); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } SSL_CTX_set_srp_username_callback(mctx->ssl_ctx, @@ -380,13 +399,14 @@ static void ssl_init_ctx_tls_extensions(server_rec *s, SSL_CTX_set_srp_cb_arg(mctx->ssl_ctx, mctx); } #endif + return APR_SUCCESS; } #endif -static void ssl_init_ctx_protocol(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_protocol(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { SSL_CTX *ctx = NULL; MODSSL_SSL_METHOD_CONST SSL_METHOD *method = NULL; @@ -400,7 +420,7 @@ static void ssl_init_ctx_protocol(server_rec *s, if (protocol == SSL_PROTOCOL_NONE) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02231) "No SSL protocols available [hint: SSLProtocol]"); - ssl_die(s); + return ssl_die(s); } cp = apr_pstrcat(p, @@ -517,6 +537,8 @@ static void ssl_init_ctx_protocol(server_rec *s, if (ap_max_mem_free != APR_ALLOCATOR_MAX_FREE_UNLIMITED) SSL_CTX_set_mode(ctx, SSL_MODE_RELEASE_BUFFERS); #endif + + return APR_SUCCESS; } static void ssl_init_ctx_session_cache(server_rec *s, @@ -548,10 +570,10 @@ static void ssl_init_ctx_callbacks(server_rec *s, SSL_CTX_set_info_callback(ctx, ssl_callback_Info); } -static void ssl_init_ctx_verify(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_verify(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { SSL_CTX *ctx = mctx->ssl_ctx; @@ -596,7 +618,7 @@ static void ssl_init_ctx_verify(server_rec *s, "Unable to configure verify locations " "for client authentication"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } if (mctx->pks && (mctx->pks->ca_name_file || mctx->pks->ca_name_path)) { @@ -611,7 +633,7 @@ static void ssl_init_ctx_verify(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01896) "Unable to determine list of acceptable " "CA certificates for client authentication"); - ssl_die(s); + return ssl_die(s); } SSL_CTX_set_client_CA_list(ctx, ca_list); @@ -631,12 +653,14 @@ static void ssl_init_ctx_verify(server_rec *s, "verification!? [Hint: SSLCACertificate*]"); } } + + return APR_SUCCESS; } -static void ssl_init_ctx_cipher_suite(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_cipher_suite(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { SSL_CTX *ctx = mctx->ssl_ctx; const char *suite; @@ -658,14 +682,16 @@ static void ssl_init_ctx_cipher_suite(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01898) "Unable to configure permitted SSL ciphers"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } + + return APR_SUCCESS; } -static void ssl_init_ctx_crl(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_crl(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { X509_STORE *store = SSL_CTX_get_cert_store(mctx->ssl_ctx); unsigned long crlflags = 0; @@ -682,9 +708,9 @@ static void ssl_init_ctx_crl(server_rec *s, "Host %s: CRL checking has been enabled, but " "neither %sCARevocationFile nor %sCARevocationPath " "is configured", mctx->sc->vhost_id, cfgp, cfgp); - ssl_die(s); + return ssl_die(s); } - return; + return APR_SUCCESS; } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01900) @@ -696,7 +722,7 @@ static void ssl_init_ctx_crl(server_rec *s, "Host %s: unable to configure X.509 CRL storage " "for certificate revocation", mctx->sc->vhost_id); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } switch (mctx->crl_check_mode) { @@ -718,12 +744,14 @@ static void ssl_init_ctx_crl(server_rec *s, "but CRL checking (%sCARevocationCheck) is not " "enabled", mctx->sc->vhost_id, cfgp); } + + return APR_SUCCESS; } -static void ssl_init_ctx_cert_chain(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx_cert_chain(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { BOOL skip_first = FALSE; int i, n; @@ -744,7 +772,7 @@ static void ssl_init_ctx_cert_chain(server_rec *s, * used only for the server certificate chain. */ if (!chain) { - return; + return APR_SUCCESS; } for (i = 0; (i < SSL_AIDX_MAX) && mctx->pks->cert_files[i]; i++) { @@ -760,45 +788,64 @@ static void ssl_init_ctx_cert_chain(server_rec *s, if (n < 0) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01903) "Failed to configure CA certificate chain!"); - ssl_die(s); + return ssl_die(s); } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01904) "Configuring server certificate chain " "(%d CA certificate%s)", n, n == 1 ? "" : "s"); + + return APR_SUCCESS; } -static void ssl_init_ctx(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ctx(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { - ssl_init_ctx_protocol(s, p, ptemp, mctx); + apr_status_t rv; + + if ((rv = ssl_init_ctx_protocol(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } ssl_init_ctx_session_cache(s, p, ptemp, mctx); ssl_init_ctx_callbacks(s, p, ptemp, mctx); - ssl_init_ctx_verify(s, p, ptemp, mctx); + if ((rv = ssl_init_ctx_verify(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } - ssl_init_ctx_cipher_suite(s, p, ptemp, mctx); + if ((rv = ssl_init_ctx_cipher_suite(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } - ssl_init_ctx_crl(s, p, ptemp, mctx); + if ((rv = ssl_init_ctx_crl(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } if (mctx->pks) { /* XXX: proxy support? */ - ssl_init_ctx_cert_chain(s, p, ptemp, mctx); + if ((rv = ssl_init_ctx_cert_chain(s, p, ptemp, mctx)) != APR_SUCCESS) { + return rv; + } #ifdef HAVE_TLSEXT - ssl_init_ctx_tls_extensions(s, p, ptemp, mctx); + if ((rv = ssl_init_ctx_tls_extensions(s, p, ptemp, mctx)) != + APR_SUCCESS) { + return rv; + } #endif } + + return APR_SUCCESS; } -static int ssl_server_import_cert(server_rec *s, - modssl_ctx_t *mctx, - const char *id, - int idx) +static apr_status_t ssl_server_import_cert(server_rec *s, + modssl_ctx_t *mctx, + const char *id, + int idx) { SSLModConfigRec *mc = myModConfig(s); ssl_asn1_t *asn1; @@ -807,7 +854,7 @@ static int ssl_server_import_cert(server_rec *s, X509 *cert; if (!(asn1 = ssl_asn1_table_get(mc->tPublicCert, id))) { - return FALSE; + return APR_NOTFOUND; } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02232) @@ -818,14 +865,14 @@ static int ssl_server_import_cert(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02233) "Unable to import %s server certificate", type); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } if (SSL_CTX_use_certificate(mctx->ssl_ctx, cert) <= 0) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02234) "Unable to configure %s server certificate", type); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } #ifdef HAVE_OCSP_STAPLING @@ -839,13 +886,13 @@ static int ssl_server_import_cert(server_rec *s, mctx->pks->certs[idx] = cert; - return TRUE; + return APR_SUCCESS; } -static int ssl_server_import_key(server_rec *s, - modssl_ctx_t *mctx, - const char *id, - int idx) +static apr_status_t ssl_server_import_key(server_rec *s, + modssl_ctx_t *mctx, + const char *id, + int idx) { SSLModConfigRec *mc = myModConfig(s); ssl_asn1_t *asn1; @@ -862,7 +909,7 @@ static int ssl_server_import_key(server_rec *s, pkey_type = (idx == SSL_AIDX_RSA) ? EVP_PKEY_RSA : EVP_PKEY_DSA; if (!(asn1 = ssl_asn1_table_get(mc->tPrivateKey, id))) { - return FALSE; + return APR_NOTFOUND; } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02236) @@ -874,14 +921,14 @@ static int ssl_server_import_key(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02237) "Unable to import %s server private key", type); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } if (SSL_CTX_use_PrivateKey(mctx->ssl_ctx, pkey) <= 0) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02238) "Unable to configure %s server private key", type); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } /* @@ -902,7 +949,7 @@ static int ssl_server_import_key(server_rec *s, mctx->pks->keys[idx] = pkey; - return TRUE; + return APR_SUCCESS; } static void ssl_check_public_cert(server_rec *s, @@ -945,10 +992,10 @@ static void ssl_check_public_cert(server_rec *s, } } -static void ssl_init_server_certs(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_server_certs(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { const char *rsa_id, *dsa_id; #ifdef HAVE_ECC @@ -959,10 +1006,10 @@ static void ssl_init_server_certs(server_rec *s, #endif const char *vhost_id = mctx->sc->vhost_id; int i; - int have_rsa, have_dsa; + apr_status_t have_rsa, have_dsa; DH *dhparams; #ifdef HAVE_ECC - int have_ecc; + apr_status_t have_ecc; #endif rsa_id = ssl_asn1_table_keyfmt(ptemp, vhost_id, SSL_AIDX_RSA); @@ -972,20 +1019,29 @@ static void ssl_init_server_certs(server_rec *s, #endif have_rsa = ssl_server_import_cert(s, mctx, rsa_id, SSL_AIDX_RSA); + if (have_rsa != APR_SUCCESS && have_rsa != APR_NOTFOUND) { + return have_rsa; + } have_dsa = ssl_server_import_cert(s, mctx, dsa_id, SSL_AIDX_DSA); + if (have_dsa != APR_SUCCESS && have_dsa != APR_NOTFOUND) { + return have_dsa; + } #ifdef HAVE_ECC have_ecc = ssl_server_import_cert(s, mctx, ecc_id, SSL_AIDX_ECC); + if (have_ecc != APR_SUCCESS && have_ecc != APR_NOTFOUND) { + return have_ecc; + } #endif - if (!(have_rsa || have_dsa + if ((have_rsa != APR_SUCCESS) && (have_dsa != APR_SUCCESS) #ifdef HAVE_ECC - || have_ecc + && (have_ecc != APR_SUCCESS) #endif -)) { +) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01910) "Oops, no " KEYTYPES " server certificate found " "for '%s:%d'?!", s->server_hostname, s->port); - ssl_die(s); + return ssl_die(s); } for (i = 0; i < SSL_AIDX_MAX; i++) { @@ -993,19 +1049,28 @@ static void ssl_init_server_certs(server_rec *s, } have_rsa = ssl_server_import_key(s, mctx, rsa_id, SSL_AIDX_RSA); + if (have_rsa != APR_SUCCESS && have_rsa != APR_NOTFOUND) { + return have_rsa; + } have_dsa = ssl_server_import_key(s, mctx, dsa_id, SSL_AIDX_DSA); + if (have_dsa != APR_SUCCESS && have_dsa != APR_NOTFOUND) { + return have_dsa; + } #ifdef HAVE_ECC have_ecc = ssl_server_import_key(s, mctx, ecc_id, SSL_AIDX_ECC); + if (have_ecc != APR_SUCCESS && have_ecc != APR_NOTFOUND) { + return have_ecc; + } #endif - if (!(have_rsa || have_dsa + if ((have_rsa != APR_SUCCESS) && (have_dsa != APR_SUCCESS) #ifdef HAVE_ECC - || have_ecc + && (have_ecc != APR_SUCCESS) #endif - )) { + ) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01911) "Oops, no " KEYTYPES " server private key found?!"); - ssl_die(s); + return ssl_die(s); } /* @@ -1041,13 +1106,15 @@ static void ssl_init_server_certs(server_rec *s, EC_KEY_new_by_curve_name(NID_X9_62_prime256v1)); } #endif + + return APR_SUCCESS; } #ifdef HAVE_TLS_SESSION_TICKETS -static void ssl_init_ticket_key(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_ticket_key(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { apr_status_t rv; apr_file_t *fp; @@ -1057,7 +1124,7 @@ static void ssl_init_ticket_key(server_rec *s, modssl_ticket_key_t *ticket_key = mctx->ticket_key; if (!ticket_key->file_path) { - return; + return APR_SUCCESS; } path = ap_server_root_relative(p, ticket_key->file_path); @@ -1069,7 +1136,7 @@ static void ssl_init_ticket_key(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02286) "Failed to open ticket key file %s: (%d) %pm", path, rv, &rv); - ssl_die(s); + return ssl_die(s); } rv = apr_file_read_full(fp, &buf[0], TLSEXT_TICKET_KEY_LEN, &len); @@ -1078,7 +1145,7 @@ static void ssl_init_ticket_key(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02287) "Failed to read %d bytes from %s: (%d) %pm", TLSEXT_TICKET_KEY_LEN, path, rv, &rv); - ssl_die(s); + return ssl_die(s); } memcpy(ticket_key->key_name, buf, 16); @@ -1091,19 +1158,21 @@ static void ssl_init_ticket_key(server_rec *s, "Unable to initialize TLS session ticket key callback " "(incompatible OpenSSL version?)"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(02288) "TLS session ticket key for %s successfully loaded from %s", (mySrvConfig(s))->vhost_id, path); + + return APR_SUCCESS; } #endif -static void ssl_init_proxy_certs(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - modssl_ctx_t *mctx) +static apr_status_t ssl_init_proxy_certs(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + modssl_ctx_t *mctx) { int n, ncerts = 0; STACK_OF(X509_INFO) *sk; @@ -1116,7 +1185,7 @@ static void ssl_init_proxy_certs(server_rec *s, ssl_callback_proxy_cert); if (!(pkp->cert_file || pkp->cert_path)) { - return; + return APR_SUCCESS; } sk = sk_X509_INFO_new_null(); @@ -1133,7 +1202,7 @@ static void ssl_init_proxy_certs(server_rec *s, sk_X509_INFO_free(sk); ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(02206) "no client certs found for SSL proxy"); - return; + return APR_SUCCESS; } /* Check that all client certs have got certificates and private @@ -1147,8 +1216,7 @@ static void ssl_init_proxy_certs(server_rec *s, ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, s, APLOGNO(02252) "incomplete client cert configured for SSL proxy " "(missing or encrypted private key?)"); - ssl_die(s); - return; + return ssl_die(s); } if (X509_check_private_key(inf->x509, inf->x_pkey->dec_pkey) != 1) { @@ -1156,8 +1224,7 @@ static void ssl_init_proxy_certs(server_rec *s, APLOGNO(02326) "proxy client certificate and " "private key do not match"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, s); - ssl_die(s); - return; + return ssl_die(s); } } @@ -1168,7 +1235,7 @@ static void ssl_init_proxy_certs(server_rec *s, if (!pkp->ca_cert_file || !store) { - return; + return APR_SUCCESS; } /* If SSLProxyMachineCertificateChainFile is configured, load all @@ -1183,7 +1250,7 @@ static void ssl_init_proxy_certs(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02208) "SSL proxy client cert initialization failed"); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } X509_STORE_load_locations(store, pkp->ca_cert_file, NULL); @@ -1241,34 +1308,51 @@ static void ssl_init_proxy_certs(server_rec *s, } X509_STORE_CTX_free(sctx); + + return APR_SUCCESS; } -static void ssl_init_proxy_ctx(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - SSLSrvConfigRec *sc) +static apr_status_t ssl_init_proxy_ctx(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + SSLSrvConfigRec *sc) { - ssl_init_ctx(s, p, ptemp, sc->proxy); + apr_status_t rv; + + if ((rv = ssl_init_ctx(s, p, ptemp, sc->proxy)) != APR_SUCCESS) { + return rv; + } - ssl_init_proxy_certs(s, p, ptemp, sc->proxy); + if ((rv = ssl_init_proxy_certs(s, p, ptemp, sc->proxy)) != APR_SUCCESS) { + return rv; + } + + return APR_SUCCESS; } -static void ssl_init_server_ctx(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - SSLSrvConfigRec *sc) +static apr_status_t ssl_init_server_ctx(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + SSLSrvConfigRec *sc) { + apr_status_t rv; #ifdef HAVE_SSL_CONF_CMD ssl_ctx_param_t *param = (ssl_ctx_param_t *)sc->server->ssl_ctx_param->elts; SSL_CONF_CTX *cctx = sc->server->ssl_ctx_config; int i; #endif - ssl_init_server_check(s, p, ptemp, sc->server); + if ((rv = ssl_init_server_check(s, p, ptemp, sc->server)) != APR_SUCCESS) { + return rv; + } - ssl_init_ctx(s, p, ptemp, sc->server); + if ((rv = ssl_init_ctx(s, p, ptemp, sc->server)) != APR_SUCCESS) { + return rv; + } - ssl_init_server_certs(s, p, ptemp, sc->server); + if ((rv = ssl_init_server_certs(s, p, ptemp, sc->server)) != APR_SUCCESS) { + return rv; + } #ifdef HAVE_SSL_CONF_CMD SSL_CONF_CTX_set_ssl_ctx(cctx, sc->server->ssl_ctx); @@ -1279,7 +1363,7 @@ static void ssl_init_server_ctx(server_rec *s, "\"SSLOpenSSLConfCmd %s %s\" failed for %s", param->name, param->value, sc->vhost_id); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } else { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02556) "\"SSLOpenSSLConfCmd %s %s\" applied to %s", @@ -1309,38 +1393,50 @@ static void ssl_init_server_ctx(server_rec *s, "SSL_CONF_CTX_finish() failed"); SSL_CONF_CTX_free(cctx); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } SSL_CONF_CTX_free(cctx); #endif #ifdef HAVE_TLS_SESSION_TICKETS - ssl_init_ticket_key(s, p, ptemp, sc->server); + if ((rv = ssl_init_ticket_key(s, p, ptemp, sc->server)) != APR_SUCCESS) { + return rv; + } #endif + + return APR_SUCCESS; } /* * Configure a particular server */ -void ssl_init_ConfigureServer(server_rec *s, - apr_pool_t *p, - apr_pool_t *ptemp, - SSLSrvConfigRec *sc) +apr_status_t ssl_init_ConfigureServer(server_rec *s, + apr_pool_t *p, + apr_pool_t *ptemp, + SSLSrvConfigRec *sc) { + apr_status_t rv; + /* Initialize the server if SSL is enabled or optional. */ if ((sc->enabled == SSL_ENABLED_TRUE) || (sc->enabled == SSL_ENABLED_OPTIONAL)) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01914) "Configuring server %s for SSL protocol", sc->vhost_id); - ssl_init_server_ctx(s, p, ptemp, sc); + if ((rv = ssl_init_server_ctx(s, p, ptemp, sc)) != APR_SUCCESS) { + return rv; + } } if (sc->proxy_enabled) { - ssl_init_proxy_ctx(s, p, ptemp, sc); + if ((rv = ssl_init_proxy_ctx(s, p, ptemp, sc)) != APR_SUCCESS) { + return rv; + } } + + return APR_SUCCESS; } -void ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p) +apr_status_t ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p) { server_rec *s, *ps; SSLSrvConfigRec *sc; @@ -1433,6 +1529,8 @@ void ssl_init_CheckServers(server_rec *base_server, apr_pool_t *p) "support (RFC 4366)"); #endif } + + return APR_SUCCESS; } static int ssl_init_FindCAList_X509NameCmp(const X509_NAME * const *a, @@ -1523,7 +1621,8 @@ STACK_OF(X509_NAME) *ssl_init_FindCAList(server_rec *s, ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02211) "Failed to open Certificate Path `%s'", ca_path); - ssl_die(s); + sk_X509_NAME_pop_free(ca_list, X509_NAME_free); + return NULL; } while ((apr_dir_read(&direntry, finfo_flags, dir)) == APR_SUCCESS) { diff --git a/modules/ssl/ssl_engine_log.c b/modules/ssl/ssl_engine_log.c index 3f6d6edc91..2c87638fa4 100644 --- a/modules/ssl/ssl_engine_log.c +++ b/modules/ssl/ssl_engine_log.c @@ -63,7 +63,7 @@ static const char *ssl_log_annotation(const char *error) return ssl_log_annotate[i].cpAnnotation; } -void ssl_die(server_rec *s) +apr_status_t ssl_die(server_rec *s) { if (s != NULL && s->is_virtual && s->error_fname != NULL) ap_log_error(APLOG_MARK, APLOG_EMERG, 0, NULL, APLOGNO(02311) @@ -75,13 +75,7 @@ void ssl_die(server_rec *s) ap_log_error(APLOG_MARK, APLOG_EMERG, 0, NULL, APLOGNO(02312) "Fatal error initialising mod_ssl, exiting."); - /* - * This is used for fatal errors and here - * it is common module practice to really - * exit from the complete program. - * XXX: The config hooks should return errors instead of calling exit(). - */ - exit(1); + return APR_EGENERAL; } /* diff --git a/modules/ssl/ssl_engine_pphrase.c b/modules/ssl/ssl_engine_pphrase.c index d3d2a3a618..342be0e9e7 100644 --- a/modules/ssl/ssl_engine_pphrase.c +++ b/modules/ssl/ssl_engine_pphrase.c @@ -135,7 +135,7 @@ static void pphrase_array_clear(apr_array_header_t *arr) * here should be split out into a separate function for improved * readability. The myCtxVarGet abomination can be thrown away with * SSLC support, vastly simplifying the code. */ -void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) +apr_status_t ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) { SSLModConfigRec *mc = myModConfig(s); SSLSrvConfigRec *sc; @@ -195,7 +195,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) "Server should be SSL-aware but has no certificate " "configured [Hint: SSLCertificateFile] (%s:%d)", pServ->defn_name, pServ->defn_line_number); - ssl_die(pServ); + return ssl_die(pServ); } /* Bitmasks for all key algorithms configured for this server; @@ -217,14 +217,14 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02201) "Init: Can't open server certificate file %s", szPath); - ssl_die(s); + return ssl_die(s); } if ((pX509Cert = SSL_read_X509(szPath, NULL, NULL)) == NULL) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(02241) "Init: Unable to read server certificate from" " file %s", szPath); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02202) "Init: Read server certificate from '%s'", @@ -240,7 +240,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) "Init: Multiple %s server certificates not " "allowed", an); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } algoCert |= at; @@ -319,7 +319,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(02243) "Init: Can't open server private key file " "%s",szPath); - ssl_die(s); + return ssl_die(s); } /* @@ -416,7 +416,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) "Init: SSLPassPhraseDialog builtin is not " "supported on Win32 (key file " "%s)", szPath); - ssl_die(s); + return ssl_die(s); } #endif /* WIN32 */ @@ -455,7 +455,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) apr_file_printf(writetty, "**Stopped\n"); } } - ssl_die(pServ); + return ssl_die(pServ); } /* If a cached private key was found, nothing more to do @@ -470,7 +470,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) "file %s [Hint: Perhaps it is in a separate file? " " See SSLCertificateKeyFile]", szPath); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } /* @@ -484,7 +484,7 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) "Init: Multiple %s server private keys not " "allowed", an); ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); - ssl_die(s); + return ssl_die(s); } algoKey |= at; @@ -570,7 +570,8 @@ void ssl_pphrase_Handle(server_rec *s, apr_pool_t *p) apr_file_close(writetty); readtty = writetty = NULL; } - return; + + return APR_SUCCESS; } static apr_status_t ssl_pipe_child_create(apr_pool_t *p, const char *progname) diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index f95f2fc6de..c09da0c098 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -778,10 +778,10 @@ const char *ssl_cmd_SSLSRPUnknownUserSeed(cmd_parms *cmd, void *dcfg, const char const char *ssl_cmd_SSLFIPS(cmd_parms *cmd, void *dcfg, int flag); /** module initialization */ -int ssl_init_Module(apr_pool_t *, apr_pool_t *, apr_pool_t *, server_rec *); -void ssl_init_Engine(server_rec *, apr_pool_t *); -void ssl_init_ConfigureServer(server_rec *, apr_pool_t *, apr_pool_t *, SSLSrvConfigRec *); -void ssl_init_CheckServers(server_rec *, apr_pool_t *); +apr_status_t ssl_init_Module(apr_pool_t *, apr_pool_t *, apr_pool_t *, server_rec *); +apr_status_t ssl_init_Engine(server_rec *, apr_pool_t *); +apr_status_t ssl_init_ConfigureServer(server_rec *, apr_pool_t *, apr_pool_t *, SSLSrvConfigRec *); +apr_status_t ssl_init_CheckServers(server_rec *, apr_pool_t *); STACK_OF(X509_NAME) *ssl_init_FindCAList(server_rec *, apr_pool_t *, const char *, const char *); void ssl_init_Child(apr_pool_t *, server_rec *); @@ -818,7 +818,7 @@ int ssl_callback_SessionTicket(SSL *, unsigned char *, unsigned char *, #endif /** Session Cache Support */ -void ssl_scache_init(server_rec *, apr_pool_t *); +apr_status_t ssl_scache_init(server_rec *, apr_pool_t *); void ssl_scache_status_register(apr_pool_t *p); void ssl_scache_kill(server_rec *); BOOL ssl_scache_store(server_rec *, UCHAR *, int, @@ -843,7 +843,7 @@ const char *ssl_cmd_SSLStaplingReturnResponderErrors(cmd_parms *, void *, int); const char *ssl_cmd_SSLStaplingFakeTryLater(cmd_parms *, void *, int); const char *ssl_cmd_SSLStaplingResponderTimeout(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLStaplingForceURL(cmd_parms *, void *, const char *); -void modssl_init_stapling(server_rec *, apr_pool_t *, apr_pool_t *, modssl_ctx_t *); +apr_status_t modssl_init_stapling(server_rec *, apr_pool_t *, apr_pool_t *, modssl_ctx_t *); void ssl_stapling_ex_init(void); int ssl_stapling_init_cert(server_rec *s, modssl_ctx_t *mctx, X509 *x); #endif @@ -877,7 +877,7 @@ void ssl_util_thread_setup(apr_pool_t *); int ssl_init_ssl_connection(conn_rec *c, request_rec *r); /** Pass Phrase Support */ -void ssl_pphrase_Handle(server_rec *, apr_pool_t *); +apr_status_t ssl_pphrase_Handle(server_rec *, apr_pool_t *); /** Diffie-Hellman Parameter Support */ DH *ssl_dh_GetParamFromFile(const char *); @@ -913,8 +913,9 @@ int ssl_stapling_mutex_reinit(server_rec *, apr_pool_t *); #define SSL_CACHE_MUTEX_TYPE "ssl-cache" #define SSL_STAPLING_MUTEX_TYPE "ssl-stapling" +apr_status_t ssl_die(server_rec *); + /** Logfile Support */ -void ssl_die(server_rec *); void ssl_log_ssl_error(const char *, int, int, server_rec *); /* ssl_log_xerror, ssl_log_cxerror and ssl_log_rxerror are wrappers for the diff --git a/modules/ssl/ssl_scache.c b/modules/ssl/ssl_scache.c index bfed6e7c1c..01f72546cd 100644 --- a/modules/ssl/ssl_scache.c +++ b/modules/ssl/ssl_scache.c @@ -37,7 +37,7 @@ ** _________________________________________________________________ */ -void ssl_scache_init(server_rec *s, apr_pool_t *p) +apr_status_t ssl_scache_init(server_rec *s, apr_pool_t *p) { SSLModConfigRec *mc = myModConfig(s); apr_status_t rv; @@ -49,7 +49,7 @@ void ssl_scache_init(server_rec *s, apr_pool_t *p) * will be immediately cleared anyway. For every subsequent * invocation, initialize the configured cache. */ if (ap_state_query(AP_SQ_MAIN_STATE) == AP_SQ_MS_CREATE_PRE_CONFIG) - return; + return APR_SUCCESS; #ifdef HAVE_OCSP_STAPLING if (mc->stapling_cache) { @@ -63,7 +63,7 @@ void ssl_scache_init(server_rec *s, apr_pool_t *p) if (rv) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01872) "Could not initialize stapling cache. Exiting."); - ssl_die(s); + return ssl_die(s); } } #endif @@ -76,7 +76,7 @@ void ssl_scache_init(server_rec *s, apr_pool_t *p) ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(01873) "Init: Session Cache is not configured " "[hint: SSLSessionCache]"); - return; + return APR_SUCCESS; } memset(&hints, 0, sizeof hints); @@ -88,8 +88,10 @@ void ssl_scache_init(server_rec *s, apr_pool_t *p) if (rv) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01874) "Could not initialize session cache. Exiting."); - ssl_die(s); + return ssl_die(s); } + + return APR_SUCCESS; } void ssl_scache_kill(server_rec *s) diff --git a/modules/ssl/ssl_util_stapling.c b/modules/ssl/ssl_util_stapling.c index 0387cf9298..7633648ce2 100644 --- a/modules/ssl/ssl_util_stapling.c +++ b/modules/ssl/ssl_util_stapling.c @@ -653,8 +653,8 @@ static int stapling_cb(SSL *ssl, void *arg) } -void modssl_init_stapling(server_rec *s, apr_pool_t *p, apr_pool_t *ptemp, - modssl_ctx_t *mctx) +apr_status_t modssl_init_stapling(server_rec *s, apr_pool_t *p, + apr_pool_t *ptemp, modssl_ctx_t *mctx) { SSL_CTX *ctx = mctx->ssl_ctx; SSLModConfigRec *mc = myModConfig(s); @@ -662,12 +662,12 @@ void modssl_init_stapling(server_rec *s, apr_pool_t *p, apr_pool_t *ptemp, if (mc->stapling_cache == NULL) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01958) "SSLStapling: no stapling cache available"); - ssl_die(s); + return ssl_die(s); } if (ssl_stapling_mutex_init(s, ptemp) == FALSE) { ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s, APLOGNO(01959) "SSLStapling: cannot initialise stapling mutex"); - ssl_die(s); + return ssl_die(s); } /* Set some default values for parameters if they are not set */ if (mctx->stapling_resptime_skew == UNSET) { @@ -690,6 +690,8 @@ void modssl_init_stapling(server_rec *s, apr_pool_t *p, apr_pool_t *ptemp, } SSL_CTX_set_tlsext_status_cb(ctx, stapling_cb); ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(01960) "OCSP stapling initialized"); + + return APR_SUCCESS; } #endif