use deep-copy and cleanup functions for server and provider configs

fixes overriding server-level keys in vhost configs

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

Author: zandbelt

ChangeLog

@@ -1,7 +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
+- use deep-copy and cleanup functions for server and provider configs; fixes overriding server-level keys in vhost configs
 
 03/09/2023
 - add support for OP "signed_jwks_uri" with "OIDCProviderSignedJwksUri <uri> <jwk>"

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([mod_auth_openidc],[2.4.13rc5],[[email protected]])
+AC_INIT([mod_auth_openidc],[2.4.13],[[email protected]])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 

src/config.c

@@ -405,6 +405,20 @@ void oidc_jwk_list_destroy_hash(apr_hash_t *keys) {
 	}
 }
 
+apr_array_header_t* oidc_jwk_list_copy(apr_pool_t *pool,
+		apr_array_header_t *src) {
+	apr_array_header_t *dst = NULL;
+	int i = 0;
+	if (src == NULL)
+		return NULL;
+	apr_array_make(pool, src->nelts, sizeof(const oidc_jwk_t*));
+	for (i = 0; (src) && (i < src->nelts); i++) {
+		const oidc_jwk_t *jwk = ((const oidc_jwk_t**) src->elts)[i];
+		*(const oidc_jwk_t**) apr_array_push(dst) = oidc_jwk_copy(pool, jwk);
+	}
+	return src;
+}
+
 void oidc_jwk_list_destroy(apr_array_header_t *keys_list) {
 	if (keys_list == NULL)
 		return;

src/jose.c

@@ -175,6 +175,7 @@ apr_byte_t oidc_jwk_to_json(apr_pool_t *pool, const oidc_jwk_t *jwk,
 void oidc_jwk_destroy(oidc_jwk_t *jwk);
 /* destroy a list of JWKs structs */
 void oidc_jwk_list_destroy_hash(apr_hash_t *key);
+apr_array_header_t *oidc_jwk_list_copy(apr_pool_t *pool, apr_array_header_t *src);
 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,

src/jose.h

@@ -1233,12 +1233,6 @@ static void oidc_metadata_get_jwks(request_rec *r, json_t *json,
 	}
 }
 
-static apr_status_t oidc_metadata_cleanup_jwk(void *p) {
-	oidc_jwk_t *jwk = (oidc_jwk_t *)p;
-	oidc_jwk_destroy(jwk);
-	return APR_SUCCESS;
-}
-
 /*
  * parse the JSON conf metadata in to a oidc_provider_t struct
  */
@@ -1255,7 +1249,6 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg *cfg,
 	oidc_metadata_get_jwks(r, j_conf,
 			OIDC_JWK_ENC, &provider->client_encryption_keys);
 
-
 	oidc_jose_error_t err;
 	json_t *jwk = json_object_get(j_conf, "signed_jwks_uri_key");
 	if (jwk != NULL) {
@@ -1265,8 +1258,6 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg *cfg,
 					"oidc_jwk_parse_json failed for \"signed_jwks_uri_key\": %s",
 					oidc_jose_e2s(r->pool, err));
 		}
-		apr_pool_cleanup_register(r->pool, provider->jwks_uri.jwk,
-				oidc_metadata_cleanup_jwk, oidc_metadata_cleanup_jwk);
 	} else if (cfg->provider.jwks_uri.jwk != NULL) {
 		provider->jwks_uri.jwk = cfg->provider.jwks_uri.jwk;
 	}
@@ -1318,7 +1309,8 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg *cfg,
 
 	/* see if we've got a custom JWKs refresh interval */
 	oidc_metadata_get_valid_int(r, j_conf, OIDC_METADATA_JWKS_REFRESH_INTERVAL,
-			oidc_valid_jwks_refresh_interval, &provider->jwks_uri.refresh_interval,
+			oidc_valid_jwks_refresh_interval,
+			&provider->jwks_uri.refresh_interval,
 			cfg->provider.jwks_uri.refresh_interval);
 
 	/* see if we've got a custom IAT slack interval */
@@ -1527,8 +1519,7 @@ apr_byte_t oidc_metadata_get(request_rec *r, oidc_cfg *cfg, const char *issuer,
 	json_t *j_conf = NULL;
 
 	/* allocate space for a parsed-and-merged metadata struct */
-	*provider = apr_pcalloc(r->pool, sizeof(oidc_provider_t));
-	oidc_cfg_provider_init(*provider);
+	*provider = oidc_cfg_provider_create(r->pool);
 
 	/*
 	 * read and parse the provider, conf and client metadata respectively

src/metadata.c

@@ -311,8 +311,7 @@ static apr_byte_t oidc_provider_static_config(request_rec *r, oidc_cfg *c,
 		}
 	}
 
-	*provider = apr_pcalloc(r->pool, sizeof(oidc_provider_t));
-	memcpy(*provider, &c->provider, sizeof(oidc_provider_t));
+	*provider = oidc_cfg_provider_copy(r->pool, &c->provider);
 
 	if (oidc_metadata_provider_parse(r, c, j_provider, *provider) == FALSE) {
 		oidc_error(r, "could not parse metadata from url: %s",

src/mod_auth_openidc.c

@@ -790,7 +790,8 @@ int oidc_cfg_dir_refresh_access_token_before_expiry(request_rec *r);
 int oidc_cfg_dir_logout_on_error_refresh(request_rec *r);
 char *oidc_cfg_dir_state_cookie_prefix(request_rec *r);
 int oidc_cfg_delete_oldest_state_cookies(oidc_cfg *cfg);
-void oidc_cfg_provider_init(oidc_provider_t *provider);
+oidc_provider_t* oidc_cfg_provider_create(apr_pool_t *pool);
+oidc_provider_t* oidc_cfg_provider_copy(apr_pool_t *pool, const oidc_provider_t *src);
 void oidc_config_check_x_forwarded(request_rec *r, const apr_byte_t x_forwarded_headers);
 
 // oidc_util.c