compare hostnames and domains in a case insensitive way

cookie: fix oidc_util_cookie_domain_valid to check the actual request
bump to 2.4.16.10dev

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

Author: zandbelt

ChangeLog

@@ -1,8 +1,19 @@
+03/20/2025
+- core: compare hostnames and domains in a case insensitive way in:
+    oidc_request_check_cookie_domain
+    oidc_util_cookie_domain_valid
+    oidc_validate_redirect_url
+    oidc_cfg_parse_is_valid_url_scheme
+    oidc_discovery_target_link_uri_match
+- cookie: fix oidc_util_cookie_domain_valid so that it checks the incoming request against OIDCCookieDomain
+  rather than the OIDCRedirectURI and displays the correct error message if they don't match
+- bump to 2.4.16.10dev
+
 03/19/2025
 - release 2.4.16.9
 
 03/18/2025
-- core: use case insensitive hostname/domain comparison in oidc_check_cookie_domain
+- cookie: use case insensitive hostname/domain comparison in oidc_check_cookie_domain
 - authz: remove the Location header from HTML based step up authentication redirects
   as it may conflict with its HTTP 200 status code and confuse middle boxes
 - metrics: avoid double-free on shutdown by not calling pthread_exit; fixes #1207; thanks @studersi

configure.ac

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

src/cfg/parse.c

@@ -213,8 +213,8 @@ static const char *oidc_cfg_parse_is_valid_url_scheme(apr_pool_t *pool, const ch
 	if (uri.scheme == NULL)
 		return apr_psprintf(pool, "'%s' cannot be parsed as a URL (no scheme set)", arg);
 
-	if ((scheme1 != NULL) && (_oidc_strcmp(uri.scheme, scheme1) != 0)) {
-		if ((scheme2 != NULL) && (_oidc_strcmp(uri.scheme, scheme2) != 0)) {
+	if ((scheme1 != NULL) && (_oidc_strnatcasecmp(uri.scheme, scheme1) != 0)) {
+		if ((scheme2 != NULL) && (_oidc_strnatcasecmp(uri.scheme, scheme2) != 0)) {
 			return apr_psprintf(pool, "'%s' cannot be parsed as a \"%s\" or \"%s\" URL (scheme == %s)!",
 					    arg, scheme1, scheme2, uri.scheme);
 		} else if (scheme2 == NULL) {

src/handle/discovery.c

@@ -247,9 +247,9 @@ static int oidc_discovery_target_link_uri_match(request_rec *r, oidc_cfg_t *cfg,
 	if (oidc_cfg_cookie_domain_get(cfg) == NULL) {
 		/* cookie_domain set: see if the target_link_uri matches the redirect_uri host (because the session
 		 * cookie will be set host-wide) */
-		if (_oidc_strcmp(o_uri.hostname, r_uri.hostname) != 0) {
-			char *p = _oidc_strstr(o_uri.hostname, r_uri.hostname);
-			if ((p == NULL) || (_oidc_strcmp(r_uri.hostname, p) != 0)) {
+		if (_oidc_strnatcasecmp(o_uri.hostname, r_uri.hostname) != 0) {
+			const char *p = oidc_util_strcasestr(o_uri.hostname, r_uri.hostname);
+			if ((p == NULL) || (_oidc_strnatcasecmp(r_uri.hostname, p) != 0)) {
 				oidc_error(r,
 					   "the URL hostname (%s) of the configured " OIDCRedirectURI
 					   " does not match the URL hostname of the \"target_link_uri\" (%s): aborting "
@@ -260,8 +260,8 @@ static int oidc_discovery_target_link_uri_match(request_rec *r, oidc_cfg_t *cfg,
 		}
 	} else {
 		/* cookie_domain set: see if the target_link_uri is within the cookie_domain */
-		char *p = _oidc_strstr(o_uri.hostname, oidc_cfg_cookie_domain_get(cfg));
-		if ((p == NULL) || (_oidc_strcmp(oidc_cfg_cookie_domain_get(cfg), p) != 0)) {
+		const char *p = oidc_util_strcasestr(o_uri.hostname, oidc_cfg_cookie_domain_get(cfg));
+		if ((p == NULL) || (_oidc_strnatcasecmp(oidc_cfg_cookie_domain_get(cfg), p) != 0)) {
 			oidc_error(r,
 				   "the domain (%s) configured in " OIDCCookieDomain
 				   " does not match the URL hostname (%s) of the \"target_link_uri\" (%s): aborting to "

src/handle/handle.h

@@ -108,6 +108,7 @@ int oidc_request_uri(request_rec *r, oidc_cfg_t *c);
 int oidc_request_authenticate_user(request_rec *r, oidc_cfg_t *c, oidc_provider_t *provider, const char *original_url,
 				   const char *login_hint, const char *id_token_hint, const char *prompt,
 				   const char *auth_request_params, const char *path_scope);
+apr_byte_t oidc_request_check_cookie_domain(request_rec *r, oidc_cfg_t *c, const char *original_url);
 
 // response.c
 apr_byte_t oidc_response_post_preserve_javascript(request_rec *r, const char *location, char **javascript,

src/handle/request.c

@@ -47,8 +47,7 @@
 #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,
-					    const char *original_url) {
+apr_byte_t oidc_request_check_cookie_domain(request_rec *r, oidc_cfg_t *c, const char *original_url) {
 	/*
 	 * printout errors if Cookie settings are not going to work
 	 */
@@ -58,44 +57,41 @@ static int oidc_request_check_cookie_domain(request_rec *r, oidc_cfg_t *c, oidc_
 	_oidc_memset(&r_uri, 0, sizeof(apr_uri_t));
 	apr_uri_parse(r->pool, original_url, &o_uri);
 	apr_uri_parse(r->pool, oidc_util_redirect_uri(r, c), &r_uri);
-	if ((_oidc_strcmp(o_uri.scheme, r_uri.scheme) != 0) && (_oidc_strcmp(r_uri.scheme, "https") == 0)) {
+	if ((_oidc_strnatcasecmp(o_uri.scheme, r_uri.scheme) != 0) && (_oidc_strcmp(r_uri.scheme, "https") == 0)) {
 		oidc_error(r,
 			   "the URL scheme (%s) of the configured " OIDCRedirectURI
 			   " does not match the URL scheme of the URL being accessed (%s): the \"state\" and "
 			   "\"session\" cookies will not be shared between the two!",
 			   r_uri.scheme, o_uri.scheme);
-		oidc_proto_state_destroy(proto_state);
-		return HTTP_INTERNAL_SERVER_ERROR;
+		return FALSE;
 	}
 
 	if (oidc_cfg_cookie_domain_get(c) == NULL) {
-		if (_oidc_strcmp(o_uri.hostname, r_uri.hostname) != 0) {
+		if (_oidc_strnatcasecmp(o_uri.hostname, r_uri.hostname) != 0) {
 			char *p = _oidc_strstr(o_uri.hostname, r_uri.hostname);
 			if ((p == NULL) || (_oidc_strcmp(r_uri.hostname, p) != 0)) {
 				oidc_error(r,
 					   "the URL hostname (%s) of the configured " OIDCRedirectURI
 					   " does not match the URL hostname of the URL being accessed (%s): the "
 					   "\"state\" and \"session\" cookies will not be shared between the two!",
 					   r_uri.hostname, o_uri.hostname);
-				oidc_proto_state_destroy(proto_state);
 				OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
-				return HTTP_INTERNAL_SERVER_ERROR;
+				return FALSE;
 			}
 		}
 	} else {
-		if (!oidc_util_cookie_domain_valid(r_uri.hostname, oidc_cfg_cookie_domain_get(c))) {
+		if (!oidc_util_cookie_domain_valid(o_uri.hostname, oidc_cfg_cookie_domain_get(c))) {
 			oidc_error(r,
 				   "the domain (%s) configured in " OIDCCookieDomain
 				   " does not match the URL hostname (%s) of the URL being accessed (%s): setting "
 				   "\"state\" and \"session\" cookies will not work!!",
 				   oidc_cfg_cookie_domain_get(c), o_uri.hostname, original_url);
-			oidc_proto_state_destroy(proto_state);
 			OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHN_REQUEST_ERROR_URL);
-			return HTTP_INTERNAL_SERVER_ERROR;
+			return FALSE;
 		}
 	}
 
-	return OK;
+	return TRUE;
 }
 
 static const char *oidc_request_samesite_cookie(request_rec *r, struct oidc_cfg_t *c) {
@@ -248,10 +244,9 @@ int oidc_request_authenticate_user(request_rec *r, oidc_cfg_t *c, oidc_provider_
 		return rc;
 	}
 
-	rc = oidc_request_check_cookie_domain(r, c, proto_state, original_url);
-	if (rc != OK) {
+	if (oidc_request_check_cookie_domain(r, c, original_url) == FALSE) {
 		oidc_proto_state_destroy(proto_state);
-		return rc;
+		return HTTP_INTERNAL_SERVER_ERROR;
 	}
 
 	/* send off to the OpenID Connect Provider */

src/mod_auth_openidc.c

@@ -888,7 +888,8 @@ apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg_t *c, const char
 			url_ipv6_aware = uri.hostname;
 		}
 
-		if ((_oidc_strstr(c_host, url_ipv6_aware) == NULL) || (_oidc_strstr(url_ipv6_aware, c_host) == NULL)) {
+		if ((oidc_util_strcasestr(c_host, url_ipv6_aware) == NULL) ||
+		    (oidc_util_strcasestr(url_ipv6_aware, c_host) == NULL)) {
 			*err_str = apr_pstrdup(r->pool, "Invalid Request");
 			*err_desc = apr_psprintf(
 			    r->pool, "URL value \"%s\" does not match the hostname of the current request \"%s\"",
@@ -898,19 +899,19 @@ apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg_t *c, const char
 		}
 	}
 
-	if ((uri.hostname == NULL) && (_oidc_strstr(url, "/") != url)) {
+	if ((uri.hostname == NULL) && (oidc_util_strcasestr(url, "/") != url)) {
 		*err_str = apr_pstrdup(r->pool, "Malformed URL");
 		*err_desc = apr_psprintf(
 		    r->pool, "No hostname was parsed and it does not seem to be relative, i.e starting with '/': %s",
 		    url);
 		oidc_error(r, "%s: %s", *err_str, *err_desc);
 		return FALSE;
-	} else if ((uri.hostname == NULL) && (_oidc_strstr(url, "//") == url)) {
+	} else if ((uri.hostname == NULL) && (oidc_util_strcasestr(url, "//") == url)) {
 		*err_str = apr_pstrdup(r->pool, "Malformed URL");
 		*err_desc = apr_psprintf(r->pool, "No hostname was parsed and starting with '//': %s", url);
 		oidc_error(r, "%s: %s", *err_str, *err_desc);
 		return FALSE;
-	} else if ((uri.hostname == NULL) && (_oidc_strstr(url, "/\\") == url)) {
+	} else if ((uri.hostname == NULL) && (oidc_util_strcasestr(url, "/\\") == url)) {
 		*err_str = apr_pstrdup(r->pool, "Malformed URL");
 		*err_desc = apr_psprintf(r->pool, "No hostname was parsed and starting with '/\\': %s", url);
 		oidc_error(r, "%s: %s", *err_str, *err_desc);

src/util.c

@@ -1989,15 +1989,15 @@ apr_byte_t oidc_util_regexp_first_match(apr_pool_t *pool, const char *input, con
  * check if the provided cookie domain value is valid
  */
 apr_byte_t oidc_util_cookie_domain_valid(const char *hostname, const char *cookie_domain) {
-	char *p = NULL;
+	const char *p = NULL;
 	const char *check_cookie = cookie_domain;
 	// Skip past the first char of a cookie_domain that starts
 	// with a ".", ASCII 46
 	if (check_cookie[0] == 46)
 		check_cookie++;
-	p = _oidc_strstr(hostname, check_cookie);
+	p = oidc_util_strcasestr(hostname, check_cookie);
 
-	if ((p == NULL) || (_oidc_strcmp(check_cookie, p) != 0)) {
+	if ((p == NULL) || (_oidc_strnatcasecmp(check_cookie, p) != 0)) {
 		return FALSE;
 	}
 	return TRUE;

test/test.c

@@ -1782,9 +1782,20 @@ static char *test_check_cookie_domain(request_rec *r) {
 	apr_table_set(r->headers_in, "Host", "ab001SB161djbn.xyz.com");
 
 	rv = oidc_check_cookie_domain(r, c, session);
-
 	TST_ASSERT_BYTE("oidc_check_cookie_domain", rv, TRUE);
 
+	rv = oidc_request_check_cookie_domain(r, c, "https://WWW.example.com/protected/index.html");
+	TST_ASSERT_BYTE("oidc_request_check_cookie_domain", rv, TRUE);
+
+	c->cookie_domain = ".XYZ.com";
+	rv = oidc_request_check_cookie_domain(r, c, "https://ab001sb161djbn.xyz.com/protected/index.html");
+	TST_ASSERT_BYTE("oidc_request_check_cookie_domain", rv, TRUE);
+
+	c->cookie_domain = "ab001SB161djbn.xyz.com";
+	rv = oidc_request_check_cookie_domain(r, c, "https://ab001sb161djbn.xyz.com/protected/index.html");
+	TST_ASSERT_BYTE("oidc_request_check_cookie_domain", rv, TRUE);
+
+	c->cookie_domain = NULL;
 	oidc_session_free(r, session);
 
 	return 0;