From a1b8e7aa92e5e624a5f90bb736c307dae22230a1 Mon Sep 17 00:00:00 2001

From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>

Date: Mon, 27 Jul 2020 19:35:29 +0200

Subject: [PATCH 2/4] prevent XSS and open redirect on OIDC session

 managemement OP iframe

- apply OIDCRedirectURLsAllowed on the login_uri parameter

- thanks Andrew Brady

- bump to 2.4.4rc3

Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>

(cherry picked from commit 51d997899afea6ea454abda49bd4cd41aa7c0cdc)

---

 ChangeLog              | 12 ++++++------

 auth_openidc.conf      |  3 ++-

 configure.ac           |  2 +-

 src/mod_auth_openidc.c | 21 +++++++++++++++++----

 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog

index eba2ebc..075f98d 100644

--- a/ChangeLog

+++ b/ChangeLog

@@ -86,7 +86,7 @@

 - bump to 2.3.5rc0

 04/27/2018

-- avoid crash when a relative logout URL parameter is passed in; thanks Vivien Delenne 

+- avoid crash when a relative logout URL parameter is passed in; thanks Vivien Delenne

 - release 2.3.4

 03/22/2018

@@ -258,7 +258,7 @@

 - bump to 2.2.1rc6

 05/18/2017

-- fix parse function of OIDCRequestObject configuration option; thanks @suttod 

+- fix parse function of OIDCRequestObject configuration option; thanks @suttod

 05/17/2017

 - avoid crash when the X-Forwarded-Proto header is not correctly set by a reverse proxy in front of mod_auth_openidc

@@ -325,7 +325,7 @@

 02/20/2017

 - security fix: scrub headers for "AuthType oauth20"

-- release 2.1.6 

+- release 2.1.6

 02/15/2017

 - improve logging of session max duration and session inactivity timeout

@@ -534,7 +534,7 @@

 - bump to 1.9.0rc3

 7/19/2016

-- add support for chunked session cookies; closes #153; thanks @glatzert 

+- add support for chunked session cookies; closes #153; thanks @glatzert

 - bump to 1.9.0rc2

 7/9/2016

@@ -911,7 +911,7 @@

 1/1/2015

 - update copyright to 2015

-- use json_int_t (seconds) for "exp" and "iat" fields, instead of apr_time_t (microseconds) 

+- use json_int_t (seconds) for "exp" and "iat" fields, instead of apr_time_t (microseconds)

 - correct expiry debug printout

 - bump to 1.7.2rc1

@@ -1191,7 +1191,7 @@

 - support using a Bearer token on client registration calls

 4/22/2014

-- match request and response type 

+- match request and response type

 - check at_hash value on "token id_token" implicit flow

 - use shared memory caching by default

 - release 1.2

diff --git a/auth_openidc.conf b/auth_openidc.conf

index 87685f6..75cdb8e 100644

--- a/auth_openidc.conf

+++ b/auth_openidc.conf

@@ -786,7 +786,8 @@

 #OIDCStateInputHeaders [none|user-agent|x-forwarded-for|both]

 # Define one or more regular expressions that specify URLs (or domains) allowed for post logout and

-# other redirects such as the "return_to" value on refresh token requests, e.g.:

+# other redirects such as the "return_to" value on refresh token requests, and the "login_uri" value

+# on session management based logins through the OP iframe, e.g.:

 #   OIDCRedirectURLsAllowed ^https://www.example.com ^https://(\w+).example.org ^https://example.net/app

 # or:

 #   OIDCRedirectURLsAllowed ^https://www.example.com/logout$ ^https://www.example.com/app/return_to$ 

diff --git a/configure.ac b/configure.ac

index ad5ba0e..c61d117 100644

--- a/configure.ac

+++ b/configure.ac

@@ -91,7 +91,7 @@ HAVE_LIBJQ=0

 AC_ARG_WITH(jq,

         [  --with-jq=PATH          location of your libjq installation])

-    

+

 if test -n "$with_jq"

 then

     JQ_CFLAGS="-I$with_jq/include"

diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c

index 215ed5e..68fbca5 100644

--- a/src/mod_auth_openidc.c

+++ b/src/mod_auth_openidc.c

@@ -2688,7 +2688,8 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c,

 }

 static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c,

-		const char *url, char **err_str, char **err_desc) {

+		const char *url, apr_byte_t restrict_to_host, char **err_str,

+		char **err_desc) {

 	apr_uri_t uri;

 	const char *c_host = NULL;

 	apr_hash_index_t *hi = NULL;

@@ -2717,7 +2718,7 @@ static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c,

 			oidc_error(r, "%s: %s", *err_str, *err_desc);

 			return FALSE;

 		}

-	} else if (uri.hostname != NULL) {

+	} else if ((uri.hostname != NULL) && (restrict_to_host == TRUE)) {

 		c_host = oidc_get_current_url_host(r);

 		if ((strstr(c_host, uri.hostname) == NULL)

 				|| (strstr(uri.hostname, c_host) == NULL)) {

@@ -2792,7 +2793,7 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c,

 	} else {

 		/* do input validation on the logout parameter value */

-		if (oidc_validate_redirect_url(r, c, url, &error_str,

+		if (oidc_validate_redirect_url(r, c, url, TRUE, &error_str,

 				&error_description) == FALSE) {

 			return oidc_util_html_send_error(r, c->error_template, error_str,

 					error_description,

@@ -2948,6 +2949,18 @@ static int oidc_handle_session_management_iframe_rp(request_rec *r, oidc_cfg *c,

 	if (s_poll_interval == NULL)

 		s_poll_interval = "3000";

+	int poll_interval = s_poll_interval ? strtol(s_poll_interval, NULL, 10) : 0;

+	if ((poll_interval <= 0) || (poll_interval > 3600 * 24))

+		poll_interval = 3000;

+

+	char *login_uri = NULL, *error_str = NULL, *error_description = NULL;

+	oidc_util_get_request_parameter(r, "login_uri", &login_uri);

+	if ((login_uri != NULL)

+			&& (oidc_validate_redirect_url(r, c, login_uri, FALSE, &error_str,

+					&error_description) == FALSE)) {

+		return HTTP_BAD_REQUEST;

+	}

+

 	const char *redirect_uri = oidc_get_redirect_uri(r, c);

 	java_script = apr_psprintf(r->pool, java_script, origin, client_id,

 			session_state, op_iframe_id, s_poll_interval, redirect_uri,

@@ -3061,7 +3074,7 @@ static int oidc_handle_refresh_token_request(request_rec *r, oidc_cfg *c,

 	}

 	/* do input validation on the return to parameter value */

-	if (oidc_validate_redirect_url(r, c, return_to, &error_str,

+	if (oidc_validate_redirect_url(r, c, return_to, TRUE, &error_str,

 			&error_description) == FALSE) {

 		oidc_error(r, "return_to URL validation failed: %s: %s", error_str,

 				error_description);

-- 

2.31.1