change oidc_cfg cleanup procedures

to better accomodate server rec merging

Signed-off-by: Hans Zandbelt <[email protected]>

Author: zandbelt

ChangeLog

@@ -1,6 +1,7 @@
 03/10/2023
 - shm cache: increase default maximum number of active sessions from 500 to 2000
 - shm cache: allow configuration of max 1Mb of session data for a single session
+- change oidc_cfg cleanup procedures to better accomodate server rec merging
 
 03/09/2023
 - add support for OP "signed_jwks_uri" with "OIDCProviderSignedJwksUri <uri> <jwk>"

src/config.c

@@ -797,15 +797,6 @@ static const char* oidc_set_session_max_duration(cmd_parms *cmd,
 	return OIDC_CONFIG_DIR_RV(cmd, rv);
 }
 
-static apr_status_t oidc_cleanup_keys(void *data) {
-	apr_array_header_t *keys_list = (apr_array_header_t*) data;
-	oidc_jwk_t **jwk = NULL;
-	while ((jwk = apr_array_pop(keys_list))) {
-		oidc_jwk_destroy(*jwk);
-	}
-	return APR_SUCCESS;
-}
-
 /*
  * add a public key from an X.509 file to our list of JWKs with public keys
  */
@@ -837,11 +828,8 @@ static const char* oidc_set_public_key_files(cmd_parms *cmd, void *struct_ptr,
 				kid, fname, oidc_jose_e2s(cmd->pool, err));
 	}
 
-	if (*public_keys == NULL) {
+	if (*public_keys == NULL)
 		*public_keys = apr_array_make(cmd->pool, 4, sizeof(const oidc_jwk_t*));
-		apr_pool_cleanup_register(cmd->pool, *public_keys, oidc_cleanup_keys,
-				oidc_cleanup_keys);
-	}
 
 	*(const oidc_jwk_t**) apr_array_push(*public_keys) = jwk;
 
@@ -910,12 +898,9 @@ static const char* oidc_set_private_key_files_enc(cmd_parms *cmd, void *dummy,
 				kid, fname, oidc_jose_e2s(cmd->pool, err));
 	}
 
-	if (cfg->private_keys == NULL) {
+	if (cfg->private_keys == NULL)
 		cfg->private_keys = apr_array_make(cmd->pool, 4,
 				sizeof(const oidc_jwk_t*));
-		apr_pool_cleanup_register(cmd->pool, cfg->private_keys,
-				oidc_cleanup_keys, oidc_cleanup_keys);
-	}
 
 	*(const oidc_jwk_t**) apr_array_push(cfg->private_keys) = jwk;
 
@@ -1468,11 +1453,26 @@ void oidc_cfg_provider_init(oidc_provider_t *provider) {
 	provider->auth_request_method = OIDC_DEFAULT_AUTH_REQUEST_METHOD;
 }
 
+static apr_status_t oidc_destroy_server_config(void *data) {
+	oidc_cfg *cfg = (oidc_cfg *)data;
+	// can do this even though we haven't got a deep copy
+	// since references within the oidc_jwk_t object will be set to NULL
+	if (cfg->provider.jwks_uri.jwk)
+		oidc_jwk_destroy(cfg->provider.jwks_uri.jwk);
+	oidc_jwk_list_destroy(cfg->provider.verify_public_keys);
+	oidc_jwk_list_destroy(cfg->oauth.verify_public_keys);
+	oidc_jwk_list_destroy_hash(cfg->oauth.verify_shared_keys);
+	oidc_jwk_list_destroy(cfg->public_keys);
+	oidc_jwk_list_destroy(cfg->private_keys);
+	return APR_SUCCESS;
+}
+
 /*
  * create a new server config record with defaults
  */
 void* oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
 	oidc_cfg *c = apr_pcalloc(pool, sizeof(oidc_cfg));
+	apr_pool_cleanup_register(pool, c, oidc_destroy_server_config, oidc_destroy_server_config);
 
 	c->merged = FALSE;
 
@@ -1603,6 +1603,7 @@ void* oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
  */
 void* oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {
 	oidc_cfg *c = apr_pcalloc(pool, sizeof(oidc_cfg));
+	apr_pool_cleanup_register(pool, c, oidc_destroy_server_config, oidc_destroy_server_config);
 	oidc_cfg *base = BASE;
 	oidc_cfg *add = ADD;
 
@@ -2705,20 +2706,6 @@ static apr_status_t oidc_cleanup_child(void *data) {
 				oidc_serror(sp, "cache destroy function failed");
 			}
 		}
-
-		// can do this even though we haven't got a deep copy
-		// since references within the oidc_jwk_t object will be set to NULL
-		if (cfg->provider.jwks_uri.jwk)
-			oidc_jwk_destroy(cfg->provider.jwks_uri.jwk);
-		oidc_jwk_list_destroy(sp->process->pool,
-				cfg->provider.verify_public_keys);
-		oidc_jwk_list_destroy(sp->process->pool,
-				cfg->oauth.verify_public_keys);
-		oidc_jwk_list_destroy_hash(sp->process->pool,
-				cfg->oauth.verify_shared_keys);
-		oidc_jwk_list_destroy(sp->process->pool, cfg->public_keys);
-		oidc_jwk_list_destroy(sp->process->pool, cfg->private_keys);
-
 		sp = sp->next;
 	}
 

src/jose.c

@@ -394,18 +394,18 @@ void oidc_jwk_destroy(oidc_jwk_t *jwk) {
 /*
  * destroy a list of JWKs structs
  */
-void oidc_jwk_list_destroy_hash(apr_pool_t *pool, apr_hash_t *keys) {
+void oidc_jwk_list_destroy_hash(apr_hash_t *keys) {
 	apr_hash_index_t *hi = NULL;
 	if (keys == NULL)
 		return;
-	for (hi = apr_hash_first(pool, keys); hi; hi = apr_hash_next(hi)) {
+	for (hi = apr_hash_first(NULL, keys); hi; hi = apr_hash_next(hi)) {
 		oidc_jwk_t *jwk = NULL;
 		apr_hash_this(hi, NULL, NULL, (void**) &jwk);
 		oidc_jwk_destroy(jwk);
 	}
 }
 
-void oidc_jwk_list_destroy(apr_pool_t *pool, apr_array_header_t *keys_list) {
+void oidc_jwk_list_destroy(apr_array_header_t *keys_list) {
 	if (keys_list == NULL)
 		return;
 	oidc_jwk_t **jwk = NULL;

src/jose.h

@@ -174,8 +174,8 @@ apr_byte_t oidc_jwk_to_json(apr_pool_t *pool, const oidc_jwk_t *jwk,
 /* destroy resources allocated for a JWK struct */
 void oidc_jwk_destroy(oidc_jwk_t *jwk);
 /* destroy a list of JWKs structs */
-void oidc_jwk_list_destroy_hash(apr_pool_t *pool, apr_hash_t *key);
-void oidc_jwk_list_destroy(apr_pool_t *pool, apr_array_header_t *keys_list);
+void oidc_jwk_list_destroy_hash(apr_hash_t *key);
+void oidc_jwk_list_destroy(apr_array_header_t *keys_list);
 /* create an "oct" symmetric JWK */
 oidc_jwk_t* oidc_jwk_create_symmetric_key(apr_pool_t *pool, const char *kid,
 		const unsigned char *key, unsigned int key_len, apr_byte_t set_kid,

src/proto.c

@@ -1568,7 +1568,7 @@ apr_byte_t oidc_proto_jwt_verify(request_rec *r, oidc_cfg *cfg, oidc_jwt_t *jwt,
 		/* get the key from the JWKs that corresponds with the key specified in the header */
 		if (oidc_proto_get_keys_from_jwks_uri(r, cfg, jwt, jwks_uri,
 				ssl_validate_server, dynamic_keys, &force_refresh) == FALSE) {
-			oidc_jwk_list_destroy_hash(r->pool, dynamic_keys);
+			oidc_jwk_list_destroy_hash(dynamic_keys);
 			return FALSE;
 		}
 	}
@@ -1580,15 +1580,15 @@ apr_byte_t oidc_proto_jwt_verify(request_rec *r, oidc_cfg *cfg, oidc_jwt_t *jwt,
 			&err) == FALSE) {
 		oidc_error(r, "JWT signature verification failed: %s",
 				oidc_jose_e2s(r->pool, err));
-		oidc_jwk_list_destroy_hash(r->pool, dynamic_keys);
+		oidc_jwk_list_destroy_hash(dynamic_keys);
 		return FALSE;
 	}
 
 	oidc_debug(r,
 			"JWT signature verification with algorithm \"%s\" was successful",
 			jwt->header.alg);
 
-	oidc_jwk_list_destroy_hash(r->pool, dynamic_keys);
+	oidc_jwk_list_destroy_hash(dynamic_keys);
 	return TRUE;
 }