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