refactoring: move state functions to state.c/state.h and rename

Signed-off-by: Hans Zandbelt <hans.zandbelt@openidc.com>

ChangeLog

@@ -1,3 +1,6 @@
+09/21/2024
+- refactor state and userinfo
+
 09/11/2024
 - change warnings about not passing unknown claim types into debug messages; see #1263; thanks @nclarkau
 - bump to 2.4.16.4rc1

Makefile.am

@@ -4,6 +4,7 @@ noinst_LTLIBRARIES = libauth_openidc.la
 
 libauth_openidc_la_SOURCES = \
 	src/mod_auth_openidc.c \
+	src/state.c \
 	src/cfg/cfg.c \
 	src/cfg/cache.c \
 	src/cfg/provider.c \
@@ -92,6 +93,7 @@ noinst_HEADERS = \
 	src/cfg/dir.h \
 	src/cfg/parse.h \
 	src/mod_auth_openidc.h \
+	src/state.h \
 	src/handle/handle.h \
 	src/proto/proto.h \
 	src/cache/cache.h \

src/handle/request.c

@@ -44,6 +44,7 @@
 #include "metrics.h"
 #include "mod_auth_openidc.h"
 #include "proto/proto.h"
+#include "state.h"
 #include "util.h"
 
 static int oidc_request_check_cookie_domain(request_rec *r, oidc_cfg_t *c, oidc_proto_state_t *proto_state,
@@ -120,7 +121,7 @@ static int oidc_request_authorization_set_cookie(request_rec *r, oidc_cfg_t *c,
 	 * try to avoid the number of state cookies exceeding a max
 	 */
 	int number_of_cookies =
-	    oidc_clean_expired_state_cookies(r, c, NULL, oidc_cfg_delete_oldest_state_cookies_get(c));
+	    oidc_state_cookies_clean_expired(r, c, NULL, oidc_cfg_delete_oldest_state_cookies_get(c));
 	int max_number_of_cookies = oidc_cfg_max_number_of_state_cookies_get(c);
 	if ((max_number_of_cookies > 0) && (number_of_cookies >= max_number_of_cookies)) {
 		oidc_warn(r,
@@ -131,7 +132,7 @@ static int oidc_request_authorization_set_cookie(request_rec *r, oidc_cfg_t *c,
 	}
 
 	/* assemble the cookie name for the state cookie */
-	const char *cookieName = oidc_get_state_cookie_name(r, state);
+	const char *cookieName = oidc_state_cookie_name(r, state);
 
 	/* set it as a cookie */
 	oidc_http_set_cookie(r, cookieName, cookieValue, -1, OIDC_COOKIE_SAMESITE_LAX(c, r));
@@ -221,7 +222,7 @@ int oidc_request_authenticate_user(request_rec *r, oidc_cfg_t *c, oidc_provider_
 		oidc_proto_state_set_pkce_state(proto_state, pkce_state);
 
 	/* get a hash value that fingerprints the browser concatenated with the random input */
-	const char *state = oidc_get_browser_state_hash(r, c, nonce);
+	const char *state = oidc_state_browser_fingerprint(r, c, nonce);
 
 	/*
 	 * create state that restores the context when the authorization response comes in

src/handle/response.c

@@ -45,6 +45,7 @@
 #include "metrics.h"
 #include "mod_auth_openidc.h"
 #include "proto/proto.h"
+#include "state.h"
 #include "util.h"
 
 /*
@@ -338,10 +339,10 @@ static apr_byte_t oidc_response_proto_state_restore(request_rec *r, oidc_cfg_t *
 
 	oidc_debug(r, "enter");
 
-	const char *cookieName = oidc_get_state_cookie_name(r, state);
+	const char *cookieName = oidc_state_cookie_name(r, state);
 
 	/* clean expired state cookies to avoid pollution */
-	oidc_clean_expired_state_cookies(r, c, cookieName, FALSE);
+	oidc_state_cookies_clean_expired(r, c, cookieName, FALSE);
 
 	/* get the state cookie value first */
 	char *cookieValue = oidc_http_get_cookie(r, cookieName);
@@ -360,7 +361,7 @@ static apr_byte_t oidc_response_proto_state_restore(request_rec *r, oidc_cfg_t *
 	const char *nonce = oidc_proto_state_get_nonce(*proto_state);
 
 	/* calculate the hash of the browser fingerprint concatenated with the nonce */
-	char *calc = oidc_get_browser_state_hash(r, c, nonce);
+	char *calc = oidc_state_browser_fingerprint(r, c, nonce);
 	/* compare the calculated hash with the value provided in the authorization response */
 	if (_oidc_strcmp(calc, state) != 0) {
 		oidc_error(

src/mod_auth_openidc.c

@@ -69,7 +69,6 @@
 #endif
 
 #include <apr_portable.h>
-#include <apr_sha1.h>
 
 /*
  * clean any suspicious headers in the HTTP request sent by the user agent
@@ -199,67 +198,6 @@ void oidc_strip_cookies(request_rec *r) {
 	}
 }
 
-#define OIDC_SHA1_LEN 20
-
-/*
- * calculates a hash value based on request fingerprint plus a provided nonce string.
- */
-char *oidc_get_browser_state_hash(request_rec *r, oidc_cfg_t *c, const char *nonce) {
-
-	oidc_debug(r, "enter");
-
-	/* helper to hold to header values */
-	const char *value = NULL;
-	/* the hash context */
-	apr_sha1_ctx_t sha1;
-
-	/* Initialize the hash context */
-	apr_sha1_init(&sha1);
-
-	if (oidc_cfg_state_input_headers_get(c) & OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR) {
-		/* get the X-FORWARDED-FOR header value  */
-		value = oidc_http_hdr_in_x_forwarded_for_get(r);
-		/* if we have a value for this header, concat it to the hash input */
-		if (value != NULL)
-			apr_sha1_update(&sha1, value, _oidc_strlen(value));
-	}
-
-	if (oidc_cfg_state_input_headers_get(c) & OIDC_STATE_INPUT_HEADERS_USER_AGENT) {
-		/* get the USER-AGENT header value  */
-		value = oidc_http_hdr_in_user_agent_get(r);
-		/* if we have a value for this header, concat it to the hash input */
-		if (value != NULL)
-			apr_sha1_update(&sha1, value, _oidc_strlen(value));
-	}
-
-	/* get the remote client IP address or host name */
-	/*
-	 int remotehost_is_ip;
-	 value = ap_get_remote_host(r->connection, r->per_dir_config,
-	 REMOTE_NOLOOKUP, &remotehost_is_ip);
-	 apr_sha1_update(&sha1, value, _oidc_strlen(value));
-	 */
-
-	/* concat the nonce parameter to the hash input */
-	apr_sha1_update(&sha1, nonce, _oidc_strlen(nonce));
-
-	/* finalize the hash input and calculate the resulting hash output */
-	unsigned char hash[OIDC_SHA1_LEN];
-	apr_sha1_final(hash, &sha1);
-
-	/* base64url-encode the resulting hash and return it */
-	char *result = NULL;
-	oidc_util_base64url_encode(r, &result, (const char *)hash, OIDC_SHA1_LEN, TRUE);
-	return result;
-}
-
-/*
- * return the name for the state cookie
- */
-char *oidc_get_state_cookie_name(request_rec *r, const char *state) {
-	return apr_psprintf(r->pool, "%s%s", oidc_cfg_dir_state_cookie_prefix_get(r), state);
-}
-
 /*
  * check if s_json is valid provider metadata
  */
@@ -398,121 +336,6 @@ const char *oidc_original_request_method(request_rec *r, oidc_cfg_t *cfg, apr_by
 	return method;
 }
 
-// element in a list of state cookies
-typedef struct oidc_state_cookies_t {
-	char *name;
-	apr_time_t timestamp;
-	struct oidc_state_cookies_t *next;
-} oidc_state_cookies_t;
-
-/*
- * delete superfluous state cookies i.e. exceeding the maximum, starting with the oldest ones
- */
-static int oidc_delete_oldest_state_cookies(request_rec *r, oidc_cfg_t *c, int number_of_valid_state_cookies,
-					    int max_number_of_state_cookies, oidc_state_cookies_t *first) {
-	oidc_state_cookies_t *cur = NULL, *prev = NULL, *prev_oldest = NULL, *oldest = NULL;
-	// loop over the list of state cookies, deleting the oldest one until we reach an acceptable number
-	while (number_of_valid_state_cookies >= max_number_of_state_cookies) {
-		oldest = first;
-		prev_oldest = NULL;
-		prev = first;
-		cur = first ? first->next : NULL;
-		// find the oldest state cookie in the list (stored in "oldest")
-		while (cur) {
-			if ((cur->timestamp < oldest->timestamp)) {
-				oldest = cur;
-				prev_oldest = prev;
-			}
-			prev = cur;
-			cur = cur->next;
-		}
-		oidc_warn(r, "deleting oldest state cookie: %s (time until expiry %" APR_TIME_T_FMT " seconds)",
-			  oldest->name, apr_time_sec(oldest->timestamp - apr_time_now()));
-		oidc_http_set_cookie(r, oldest->name, "", 0, OIDC_COOKIE_EXT_SAME_SITE_NONE(c, r));
-		if (prev_oldest)
-			prev_oldest->next = oldest->next;
-		else
-			first = first ? first->next : NULL;
-		number_of_valid_state_cookies--;
-	}
-	return number_of_valid_state_cookies;
-}
-
-/*
- * clean state cookies that have expired i.e. for outstanding requests that will never return
- * successfully and return the number of remaining valid cookies/outstanding-requests while
- * doing so
- */
-int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg_t *c, const char *currentCookieName, int delete_oldest) {
-	int number_of_valid_state_cookies = 0;
-	oidc_state_cookies_t *first = NULL, *last = NULL;
-	char *cookie, *tokenizerCtx = NULL;
-	char *cookies = apr_pstrdup(r->pool, oidc_http_hdr_in_cookie_get(r));
-	if (cookies != NULL) {
-		cookie = apr_strtok(cookies, OIDC_STR_SEMI_COLON, &tokenizerCtx);
-		while (cookie != NULL) {
-			while (*cookie == OIDC_CHAR_SPACE)
-				cookie++;
-			if (_oidc_strstr(cookie, oidc_cfg_dir_state_cookie_prefix_get(r)) == cookie) {
-				char *cookieName = cookie;
-				while (cookie != NULL && *cookie != OIDC_CHAR_EQUAL)
-					cookie++;
-				if (*cookie == OIDC_CHAR_EQUAL) {
-					*cookie = '\0';
-					cookie++;
-					if ((currentCookieName == NULL) ||
-					    (_oidc_strcmp(cookieName, currentCookieName) != 0)) {
-						oidc_proto_state_t *proto_state =
-						    oidc_proto_state_from_cookie(r, c, cookie);
-						if (proto_state != NULL) {
-							json_int_t ts = oidc_proto_state_get_timestamp(proto_state);
-							if (apr_time_now() >
-							    ts + apr_time_from_sec(oidc_cfg_state_timeout_get(c))) {
-								oidc_warn(
-								    r, "state (%s) has expired (original_url=%s)",
-								    cookieName,
-								    oidc_proto_state_get_original_url(proto_state));
-								oidc_http_set_cookie(
-								    r, cookieName, "", 0,
-								    OIDC_COOKIE_EXT_SAME_SITE_NONE(c, r));
-							} else {
-								if (first == NULL) {
-									first = apr_pcalloc(
-									    r->pool, sizeof(oidc_state_cookies_t));
-									last = first;
-								} else {
-									last->next = apr_pcalloc(
-									    r->pool, sizeof(oidc_state_cookies_t));
-									last = last->next;
-								}
-								last->name = cookieName;
-								last->timestamp = ts;
-								last->next = NULL;
-								number_of_valid_state_cookies++;
-							}
-							oidc_proto_state_destroy(proto_state);
-						} else {
-							oidc_warn(
-							    r,
-							    "state cookie could not be retrieved/decoded, deleting: %s",
-							    cookieName);
-							oidc_http_set_cookie(r, cookieName, "", 0,
-									     OIDC_COOKIE_EXT_SAME_SITE_NONE(c, r));
-						}
-					}
-				}
-			}
-			cookie = apr_strtok(NULL, OIDC_STR_SEMI_COLON, &tokenizerCtx);
-		}
-	}
-
-	if (delete_oldest > 0)
-		number_of_valid_state_cookies = oidc_delete_oldest_state_cookies(
-		    r, c, number_of_valid_state_cookies, oidc_cfg_max_number_of_state_cookies_get(c), first);
-
-	return number_of_valid_state_cookies;
-}
-
 /*
  * get the mod_auth_openidc related context from the (userdata in the) request
  * (used for passing state between various Apache request processing stages and hook callbacks)

src/mod_auth_openidc.h

@@ -151,9 +151,7 @@ apr_byte_t oidc_provider_static_config(request_rec *r, oidc_cfg_t *c, oidc_provi
 const char *oidc_original_request_method(request_rec *r, oidc_cfg_t *cfg, apr_byte_t handle_discovery_response);
 oidc_provider_t *oidc_get_provider_for_issuer(request_rec *r, oidc_cfg_t *c, const char *issuer,
 					      apr_byte_t allow_discovery);
-char *oidc_get_state_cookie_name(request_rec *r, const char *state);
 int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg_t *c, const char *currentCookieName, int delete_oldest);
-char *oidc_get_browser_state_hash(request_rec *r, oidc_cfg_t *c, const char *nonce);
 apr_byte_t oidc_is_auth_capable_request(request_rec *r);
 apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg_t *c, const char *redirect_to_url,
 				      apr_byte_t restrict_to_host, char **err_str, char **err_desc);