diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c index 8c6c4e57e0..b106e1467d 100644 --- a/modules/ssl/ssl_engine_config.c +++ b/modules/ssl/ssl_engine_config.c @@ -40,17 +40,17 @@ ** _________________________________________________________________ */ -#define SSL_MOD_CONFIG_KEY "ssl_module" -SSLModConfigRec *ssl_config_global_create(server_rec *s) +static SSLModConfigRec *ssl_config_global_create(apr_pool_t *pool, server_rec *s) { - apr_pool_t *pool = s->process->pool; SSLModConfigRec *mc; - void *vmc; - apr_pool_userdata_get(&vmc, SSL_MOD_CONFIG_KEY, pool); - if (vmc) { - return vmc; /* reused for lifetime of the server */ + if (ap_server_conf && s != ap_server_conf) { + SSLSrvConfigRec *sc = mySrvConfig(ap_server_conf); + + AP_DEBUG_ASSERT(sc->mc); + + return sc->mc; } /* @@ -68,8 +68,6 @@ SSLModConfigRec *ssl_config_global_create(server_rec *s) mc->pMutex = NULL; mc->aRandSeed = apr_array_make(pool, 4, sizeof(ssl_randseed_t)); - mc->tVHostKeys = apr_hash_make(pool); - mc->tPrivateKey = apr_hash_make(pool); #if defined(HAVE_OPENSSL_ENGINE_H) && defined(HAVE_ENGINE_INIT) mc->szCryptoDevice = NULL; #endif @@ -86,9 +84,15 @@ SSLModConfigRec *ssl_config_global_create(server_rec *s) mc->fips = UNSET; #endif - apr_pool_userdata_set(mc, SSL_MOD_CONFIG_KEY, - apr_pool_cleanup_null, - pool); + mc->retained = ap_retained_data_get(MODSSL_RETAINED_KEY); + if (!mc->retained) { + /* Allocate the retained data; the hash table is allocated out + * of the process pool. */ + mc->retained = ap_retained_data_create(MODSSL_RETAINED_KEY, + sizeof *mc->retained); + mc->retained->privkeys = apr_hash_make(s->process->pool); + mc->retained->key_ids = apr_hash_make(s->process->pool); + } return mc; } @@ -248,7 +252,7 @@ void *ssl_config_server_create(apr_pool_t *p, server_rec *s) { SSLSrvConfigRec *sc = ssl_config_server_new(p); - sc->mc = ssl_config_global_create(s); + sc->mc = ssl_config_global_create(p, s); return sc; } diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 61c2dc36a7..a59a055bab 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -226,6 +226,8 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, apr_status_t rv; apr_array_header_t *pphrases; + AP_DEBUG_ASSERT(mc); + if (SSLeay() < MODSSL_LIBRARY_VERSION) { ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(01882) "Init: this version of mod_ssl was compiled against " @@ -250,7 +252,6 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, /* * Any init round fixes the global config */ - ssl_config_global_create(base_server); /* just to avoid problems */ ssl_config_global_fix(mc); /* @@ -260,6 +261,8 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, for (s = base_server; s; s = s->next) { sc = mySrvConfig(s); + AP_DEBUG_ASSERT(sc->mc == mc); + if (sc->server) { sc->server->sc = sc; } @@ -1441,7 +1444,7 @@ static apr_status_t ssl_init_server_certs(server_rec *s, /* perhaps it's an encrypted private key, so try again */ ssl_load_encrypted_pkey(s, ptemp, i, keyfile, &pphrases); - if (!(asn1 = ssl_asn1_table_get(mc->tPrivateKey, key_id)) || + if (!(asn1 = ssl_asn1_table_get(mc->retained->privkeys, key_id)) || !(ptr = asn1->cpData) || !(pkey = d2i_AutoPrivateKey(NULL, &ptr, asn1->nData)) || (SSL_CTX_use_PrivateKey(mctx->ssl_ctx, pkey) < 1)) { diff --git a/modules/ssl/ssl_engine_pphrase.c b/modules/ssl/ssl_engine_pphrase.c index 9f6f84f70e..f1613913dd 100644 --- a/modules/ssl/ssl_engine_pphrase.c +++ b/modules/ssl/ssl_engine_pphrase.c @@ -71,38 +71,25 @@ static apr_status_t exists_and_readable(const char *fname, apr_pool_t *pool, return APR_SUCCESS; } -/* - * reuse vhost keys for asn1 tables where keys are allocated out - * of s->process->pool to prevent "leaking" each time we format - * a vhost key. since the key is stored in a table with lifetime - * of s->process->pool, the key needs to have the same lifetime. - * - * XXX: probably seems silly to use a hash table with keys and values - * being the same, but it is easier than doing a linear search - * and will make it easier to remove keys if needed in the future. - * also have the problem with apr_array_header_t that if we - * underestimate the number of vhost keys when we apr_array_make(), - * the array will get resized when we push past the initial number - * of elts. this resizing in the s->process->pool means "leaking" - * since apr_array_push() will apr_alloc arr->nalloc * 2 elts, - * leaving the original arr->elts to waste. - */ -static const char *asn1_table_vhost_key(SSLModConfigRec *mc, apr_pool_t *p, - const char *id, int i) +/* Returns the vhost-key-id which is the index into the + * mc->retained->privkeys hash table. The returned string is + * allocated from the same pool as that hash table, to ensure it has + * the correct (process) lifetime of the retained data. */ +static const char *privkey_vhost_keyid(SSLModConfigRec *mc, apr_pool_t *p, + const char *id, int i) { /* 'p' pool used here is cleared on restarts (or sooner) */ char *key = apr_psprintf(p, "%s:%d", id, i); - void *keyptr = apr_hash_get(mc->tVHostKeys, key, - APR_HASH_KEY_STRING); + const char *keyptr = apr_hash_get(mc->retained->key_ids, key, + APR_HASH_KEY_STRING); if (!keyptr) { - /* make a copy out of s->process->pool */ - keyptr = apr_pstrdup(mc->pPool, key); - apr_hash_set(mc->tVHostKeys, keyptr, - APR_HASH_KEY_STRING, keyptr); + /* Make a copy in the (process) pool used for the retained data. */ + keyptr = apr_pstrdup(apr_hash_pool_get(mc->retained->privkeys), key); + apr_hash_set(mc->retained->key_ids, keyptr, APR_HASH_KEY_STRING, keyptr); } - return (char *)keyptr; + return keyptr; } /* _________________________________________________________________ @@ -134,7 +121,7 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *s, apr_pool_t *p, int idx, { SSLModConfigRec *mc = myModConfig(s); SSLSrvConfigRec *sc = mySrvConfig(s); - const char *key_id = asn1_table_vhost_key(mc, p, sc->vhost_id, idx); + const char *key_id = privkey_vhost_keyid(mc, p, sc->vhost_id, idx); EVP_PKEY *pPrivateKey = NULL; ssl_asn1_t *asn1; int nPassPhrase = (*pphrases)->nelts; @@ -187,7 +174,7 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *s, apr_pool_t *p, int idx, * are used to give a better idea as to what failed. */ if (pkey_mtime) { - ssl_asn1_t *asn1 = ssl_asn1_table_get(mc->tPrivateKey, key_id); + ssl_asn1_t *asn1 = ssl_asn1_table_get(mc->retained->privkeys, key_id); if (asn1 && (asn1->source_mtime == pkey_mtime)) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(02575) "Reusing existing private key from %s on restart", @@ -345,7 +332,7 @@ apr_status_t ssl_load_encrypted_pkey(server_rec *s, apr_pool_t *p, int idx, /* Cache the private key in the global module configuration so it * can be used after subsequent reloads. */ - asn1 = ssl_asn1_table_set(mc->tPrivateKey, key_id, pPrivateKey); + asn1 = ssl_asn1_table_set(mc->retained->privkeys, key_id, pPrivateKey); if (ppcb_arg.nPassPhraseDialogCur != 0) { /* remember mtime of encrypted keys */ diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 0f58897831..6f48e1586b 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -553,34 +553,37 @@ typedef struct { int vhost_found; /* whether we found vhost from SNI already */ } SSLConnRec; -/* BIG FAT WARNING: SSLModConfigRec has unusual memory lifetime: it is - * allocated out of the "process" pool and only a single such - * structure is created and used for the lifetime of the process. - * (The process pool is s->process->pool and is stored in the .pPool - * field.) Most members of this structure are likewise allocated out - * of the process pool, but notably sesscache and sesscache_context - * are not. +/* Private keys are retained across reloads, since decryption + * passphrases can only be entered during startup (before detaching + * from a terminal). This structure is stored via the ap_retained_* + * API and retrieved on later module reloads. If the structure + * changes, the key name must be changed by increasing the digit at + * the end, to avoid an updated version of mod_ssl loading retained + * data with a different struct definition. * - * The structure is treated as mostly immutable after a single config - * parse has completed; the post_config hook (ssl_init_Module) flips - * the bFixed flag to true and subsequent invocations of the config - * callbacks hence do nothing. - * - * This odd lifetime strategy is used so that encrypted private keys - * can be decrypted once at startup and continue to be used across - * subsequent server reloads where the interactive password prompt is - * not possible. + * All objects used here must be allocated from the process pool + * (s->process->pool) so they also survives restarts. */ +#define MODSSL_RETAINED_KEY "mod_ssl-retained-1" + +typedef struct { + /* A hash table of vhost key-IDs used to index the privkeys hash, + * for example the string "vhost.example.com:443:0". For each + * (key, value) pair the value is the same as the key, allowing + * the keys to be retrieved on subsequent reloads rather than + * rellocated. ### This optimisation seems to be of dubious + * value. Allocating the vhost-key-ids from pconf and duping them + * when storing them in ->privkeys would be simpler. */ + apr_hash_t *key_ids; + + /* A hash table of pointers to ssl_asn1_t structures. The + * structures are used to store private keys in raw DER format + * (serialized OpenSSL PrivateKey structures). The table is + * indexed by key-IDs from the key_ids hash table. */ + apr_hash_t *privkeys; + + /* Do NOT add fields here without changing the key name, as above. */ +} modssl_retained_data_t; - * It is really an ABI nightmare waiting to happen since DSOs are - * reloaded across restarts, and nothing prevents the struct type - * changing across such reloads, yet the cached structure will be - * assumed to match regardless. - * - * This should really be fixed using a smaller structure which only - * stores that which is absolutely necessary (the private keys, maybe - * the random seed), and have that structure be strictly ABI-versioned - * for safety. - */ typedef struct { pid_t pid; apr_pool_t *pPool; @@ -589,6 +592,9 @@ typedef struct { /* OpenSSL SSL_SESS_CACHE_* flags: */ long sesscache_mode; + /* Data retained across reloads. */ + modssl_retained_data_t *retained; + /* The configured provider, and associated private data * structure. */ const ap_socache_provider_t *sesscache; @@ -596,13 +602,6 @@ typedef struct { apr_global_mutex_t *pMutex; apr_array_header_t *aRandSeed; - apr_hash_t *tVHostKeys; - - /* A hash table of pointers to ssl_asn1_t structures. The structures - * are used to store private keys in raw DER format (serialized OpenSSL - * PrivateKey structures). The table is indexed by (vhost-id, - * index), for example the string "vhost.example.com:443:0". */ - apr_hash_t *tPrivateKey; #if defined(HAVE_OPENSSL_ENGINE_H) && defined(HAVE_ENGINE_INIT) const char *szCryptoDevice; @@ -814,7 +813,6 @@ SSLSrvConfigRec *ssl_policy_lookup(apr_pool_t *pool, const char *name); extern module AP_MODULE_DECLARE_DATA ssl_module; /** configuration handling */ -SSLModConfigRec *ssl_config_global_create(server_rec *); void ssl_config_global_fix(SSLModConfigRec *); BOOL ssl_config_global_isfixed(SSLModConfigRec *); void *ssl_config_server_create(apr_pool_t *, server_rec *);