fix memory leaks when using provider specific client keys

in a multi-provider setup; bump to 2.4.17rc0

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

Author: zandbelt

ChangeLog

@@ -1,3 +1,7 @@
+04/15/2025
+- fix memory leaks when using provider specific client keys in a multi-provider setup
+- bump to 2.4.17rc0
+
 04/08/2025
 - proto: pass the scope parameter as returned from the token endpoint in the OIDC_scope
   header/environment variable and make it available for `Require claim scope:` purposes

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([mod_auth_openidc],[2.4.17dev],[[email protected]])
+AC_INIT([mod_auth_openidc],[2.4.17rc0],[[email protected]])
 
 AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())
 

src/cfg/provider.c

@@ -875,6 +875,8 @@ oidc_provider_t *oidc_cfg_provider_copy(apr_pool_t *pool, const oidc_provider_t
 }
 
 void oidc_cfg_provider_destroy(oidc_provider_t *provider) {
+	if (provider == NULL)
+		return;
 	oidc_jwk_list_destroy(provider->jwks_uri.jwk_list);
 	oidc_jwk_list_destroy(provider->verify_public_keys);
 	oidc_jwk_list_destroy(provider->client_keys);

src/handle/discovery.c

@@ -367,6 +367,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
 	if (oidc_cfg_metadata_dir_get(c) == NULL) {
 		if ((oidc_provider_static_config(r, c, &provider) == TRUE) && (issuer != NULL)) {
 			if (_oidc_strcmp(oidc_cfg_provider_issuer_get(provider), issuer) != 0) {
+				oidc_cfg_provider_destroy(provider);
 				return oidc_util_html_send_error(
 				    r, "Invalid Request",
 				    apr_psprintf(
@@ -376,6 +377,7 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
 				    HTTP_INTERNAL_SERVER_ERROR);
 			}
 		}
+		oidc_cfg_provider_destroy(provider);
 		return oidc_request_authenticate_user(r, c, NULL, target_link_uri, login_hint, NULL, NULL,
 						      auth_request_params, path_scopes);
 	}
@@ -446,14 +448,19 @@ int oidc_discovery_response(request_rec *r, oidc_cfg_t *c) {
 					       oidc_cfg_provider_ssl_validate_server_get(provider), &j_jwks,
 					       &force_refresh);
 			json_decref(j_jwks);
+			oidc_cfg_provider_destroy(provider);
 			return OK;
 		} else {
 			/* now we've got a selected OP, send the user there to authenticate */
-			return oidc_request_authenticate_user(r, c, provider, target_link_uri, login_hint, NULL, NULL,
-							      auth_request_params, path_scopes);
+			int rv = oidc_request_authenticate_user(r, c, provider, target_link_uri, login_hint, NULL, NULL,
+								auth_request_params, path_scopes);
+			oidc_cfg_provider_destroy(provider);
+			return rv;
 		}
 	}
 
+	oidc_cfg_provider_destroy(provider);
+
 	/* something went wrong */
 	return oidc_util_html_send_error(r, "Invalid Request",
 					 "Could not find valid provider metadata for the selected OpenID Connect "

src/handle/info.c

@@ -106,15 +106,19 @@ int oidc_info_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *session, ap
 
 				/* get the current provider info */
 				oidc_provider_t *provider = NULL;
-				if (oidc_get_provider_from_session(r, c, session, &provider) == FALSE)
+				if (oidc_get_provider_from_session(r, c, session, &provider) == FALSE) {
+					oidc_cfg_provider_destroy(provider);
 					return HTTP_INTERNAL_SERVER_ERROR;
+				}
 
 				/* execute the actual refresh grant */
 				if (oidc_refresh_token_grant(r, c, session, provider, NULL, NULL, NULL) == FALSE) {
 					oidc_warn(r, "access_token could not be refreshed");
+					oidc_cfg_provider_destroy(provider);
 					return HTTP_INTERNAL_SERVER_ERROR;
 				}
 				needs_save = TRUE;
+				oidc_cfg_provider_destroy(provider);
 			}
 		}
 	}

src/handle/logout.c

@@ -119,6 +119,7 @@ static void oidc_logout_revoke_tokens(request_rec *r, oidc_cfg_t *c, oidc_sessio
 
 out:
 
+	oidc_cfg_provider_destroy(provider);
 	oidc_debug(r, "leave");
 }
 
@@ -233,6 +234,7 @@ int oidc_logout_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *session,
 				}
 				if (provider) {
 					oidc_logout_cleanup_by_sid(r, sid, c, provider, revoke_tokens);
+					oidc_cfg_provider_destroy(provider);
 				} else {
 					oidc_info(r, "No provider for front channel logout found");
 				}
@@ -432,6 +434,10 @@ static int oidc_logout_backchannel(request_rec *r, oidc_cfg_t *cfg) {
 		oidc_jwt_destroy(jwt);
 		jwt = NULL;
 	}
+	if (provider != NULL) {
+		oidc_cfg_provider_destroy(provider);
+		provider = NULL;
+	}
 
 	oidc_http_hdr_err_out_add(r, OIDC_HTTP_HDR_CACHE_CONTROL, "no-cache, no-store");
 	oidc_http_hdr_err_out_add(r, OIDC_HTTP_HDR_PRAGMA, "no-cache");
@@ -518,5 +524,7 @@ int oidc_logout(request_rec *r, oidc_cfg_t *c, oidc_session_t *session) {
 		url = s_logout_request;
 	}
 
+	oidc_cfg_provider_destroy(provider);
+
 	return oidc_logout_request(r, c, session, url, TRUE);
 }

src/handle/refresh.c

@@ -316,6 +316,7 @@ int oidc_refresh_token_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *se
 	char *error_str = NULL;
 	char *error_description = NULL;
 	apr_byte_t needs_save = TRUE;
+	oidc_provider_t *provider = NULL;
 
 	/* get the command passed to the session management handler */
 	oidc_util_request_parameter_get(r, OIDC_REDIRECT_URI_REQUEST_REFRESH, &return_to);
@@ -354,7 +355,6 @@ int oidc_refresh_token_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *se
 	}
 
 	/* get a handle to the provider configuration */
-	oidc_provider_t *provider = NULL;
 	if (oidc_get_provider_from_session(r, c, session, &provider) == FALSE) {
 		error_code = "session_corruption";
 		goto end;
@@ -389,6 +389,8 @@ int oidc_refresh_token_request(request_rec *r, oidc_cfg_t *c, oidc_session_t *se
 	/* add the redirect location header */
 	oidc_http_hdr_out_location_set(r, return_to);
 
+	oidc_cfg_provider_destroy(provider);
+
 	return HTTP_MOVED_TEMPORARILY;
 }
 
@@ -425,16 +427,21 @@ apr_byte_t oidc_refresh_access_token_before_expiry(request_rec *r, oidc_cfg_t *c
 	if (t_expires > apr_time_now())
 		return TRUE;
 
-	if (oidc_get_provider_from_session(r, cfg, session, &provider) == FALSE)
+	if (oidc_get_provider_from_session(r, cfg, session, &provider) == FALSE) {
+		oidc_cfg_provider_destroy(provider);
 		return FALSE;
+	}
 
 	if (oidc_refresh_token_grant(r, cfg, session, provider, NULL, NULL, NULL) == FALSE) {
 		oidc_warn(r, "access_token could not be refreshed");
+		oidc_cfg_provider_destroy(provider);
 		*needs_save = FALSE;
 		return FALSE;
 	}
 
 	*needs_save = TRUE;
 
+	oidc_cfg_provider_destroy(provider);
+
 	return TRUE;
 }

src/handle/response.c

@@ -565,6 +565,7 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 			oidc_http_hdr_out_location_set(r,
 						       oidc_util_absolute_url(r, c, oidc_cfg_default_sso_url_get(c)));
 			OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_STATE_MISMATCH);
+			oidc_cfg_provider_destroy(provider);
 			return HTTP_MOVED_TEMPORARILY;
 		}
 		oidc_error(r,
@@ -577,7 +578,7 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 		}
 
 		OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_STATE_MISMATCH);
-
+		oidc_cfg_provider_destroy(provider);
 		return oidc_util_html_send_error(r, "Invalid Authorization Response",
 						 "Could not match the authorization response to an earlier request via "
 						 "the state parameter and corresponding state cookie",
@@ -587,18 +588,21 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 	/* see if the response is an error response */
 	if (apr_table_get(params, OIDC_PROTO_ERROR) != NULL) {
 		OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_PROVIDER);
+		oidc_cfg_provider_destroy(provider);
 		return oidc_response_authorization_error(r, c, proto_state, apr_table_get(params, OIDC_PROTO_ERROR),
 							 apr_table_get(params, OIDC_PROTO_ERROR_DESCRIPTION));
 	}
 
 	/* handle the code, implicit or hybrid flow */
 	if (oidc_response_flows(r, c, proto_state, provider, params, response_mode, &jwt) == FALSE) {
 		OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_PROTOCOL);
+		oidc_cfg_provider_destroy(provider);
 		return oidc_response_authorization_error(r, c, proto_state, "Error in handling response type.", NULL);
 	}
 
 	if (jwt == NULL) {
 		oidc_error(r, "no id_token was provided");
+		oidc_cfg_provider_destroy(provider);
 		return oidc_response_authorization_error(r, c, proto_state, "No id_token was provided.", NULL);
 	}
 
@@ -634,6 +638,7 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 			if (_oidc_strcmp(session->remote_user, r->user) != 0) {
 				oidc_warn(r, "user set from new id_token is different from current one");
 				oidc_jwt_destroy(jwt);
+				oidc_cfg_provider_destroy(provider);
 				return oidc_response_authorization_error(r, c, proto_state, "User changed!", NULL);
 			}
 		}
@@ -647,6 +652,7 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 			apr_table_get(params, OIDC_PROTO_STATE), original_url, userinfo_jwt) == FALSE) {
 			oidc_proto_state_destroy(proto_state);
 			oidc_jwt_destroy(jwt);
+			oidc_cfg_provider_destroy(provider);
 			return HTTP_INTERNAL_SERVER_ERROR;
 		}
 
@@ -656,13 +662,15 @@ static int oidc_response_process(request_rec *r, oidc_cfg_t *c, oidc_session_t *
 		oidc_error(r, "remote user could not be set");
 		oidc_jwt_destroy(jwt);
 		OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_RESPONSE_ERROR_REMOTE_USER);
+		oidc_cfg_provider_destroy(provider);
 		return oidc_response_authorization_error(
 		    r, c, proto_state, "Remote user could not be set: contact the website administrator", NULL);
 	}
 
 	/* cleanup */
 	oidc_proto_state_destroy(proto_state);
 	oidc_jwt_destroy(jwt);
+	oidc_cfg_provider_destroy(provider);
 
 	/* check that we've actually authenticated a user; functions as error handling for oidc_get_remote_user */
 	if (r->user == NULL) {

src/handle/session_management.c

@@ -173,23 +173,27 @@ int oidc_session_management(request_rec *r, oidc_cfg_t *c, oidc_session_t *sessi
 	/* see if this is a request for the OP iframe */
 	if (_oidc_strcmp("iframe_op", cmd) == 0) {
 		if (oidc_cfg_provider_check_session_iframe_get(provider) != NULL) {
+			oidc_cfg_provider_destroy(provider);
 			return oidc_session_management_iframe_op(r, c, session,
 								 oidc_cfg_provider_check_session_iframe_get(provider));
 		}
+		oidc_cfg_provider_destroy(provider);
 		return HTTP_NOT_FOUND;
 	}
 
 	/* see if this is a request for the RP iframe */
 	if (_oidc_strcmp("iframe_rp", cmd) == 0) {
 		if ((oidc_cfg_provider_client_id_get(provider) != NULL) &&
 		    (oidc_cfg_provider_check_session_iframe_get(provider) != NULL)) {
+			oidc_cfg_provider_destroy(provider);
 			return oidc_session_management_iframe_rp(r, c, session,
 								 oidc_cfg_provider_client_id_get(provider),
 								 oidc_cfg_provider_check_session_iframe_get(provider));
 		}
 		oidc_debug(r, "iframe_rp command issued but no client (%s) and/or no check_session_iframe (%s) set",
 			   oidc_cfg_provider_client_id_get(provider),
 			   oidc_cfg_provider_check_session_iframe_get(provider));
+		oidc_cfg_provider_destroy(provider);
 		return HTTP_NOT_FOUND;
 	}
 
@@ -202,6 +206,7 @@ int oidc_session_management(request_rec *r, oidc_cfg_t *c, oidc_session_t *sessi
 		 *       those for the redirect_uri itself; do we need to store those as part of the
 		 *       session now?
 		 */
+		oidc_cfg_provider_destroy(provider);
 		return oidc_request_authenticate_user(
 		    r, c, provider, apr_psprintf(r->pool, "%s?session=iframe_rp", oidc_util_redirect_uri(r, c)), NULL,
 		    id_token_hint, "none", oidc_cfg_dir_path_auth_request_params_get(r),
@@ -211,5 +216,7 @@ int oidc_session_management(request_rec *r, oidc_cfg_t *c, oidc_session_t *sessi
 	/* handle failure in fallthrough */
 	oidc_error(r, "unknown command: %s", cmd);
 
+	oidc_cfg_provider_destroy(provider);
+
 	return HTTP_INTERNAL_SERVER_ERROR;
 }

src/handle/userinfo.c

@@ -197,6 +197,7 @@ apr_byte_t oidc_userinfo_refresh_claims(request_rec *r, oidc_cfg_t *cfg, oidc_se
 		/* get the current provider info */
 		if (oidc_get_provider_from_session(r, cfg, session, &provider) == FALSE) {
 			*needs_save = TRUE;
+			oidc_cfg_provider_destroy(provider);
 			return FALSE;
 		}
 
@@ -238,6 +239,8 @@ apr_byte_t oidc_userinfo_refresh_claims(request_rec *r, oidc_cfg_t *cfg, oidc_se
 
 	oidc_debug(r, "return: %d", rc);
 
+	oidc_cfg_provider_destroy(provider);
+
 	return rc;
 }